Обсуждение: pg_constraint.conincluding is useless
Hi Already mentioned this in https://postgr.es/m/20180831205020.nxhw6ypysgshjtnl@alvherre.pgsql While trying to add support for foreign keys to partitioned tables, I noticed that commit 8224de4f42cc ("Indexes with INCLUDE columns and their support in B-tree") added a column to pg_constraint that appears to be there only to enable ruleutils.c to print out the list of columns in a PRIMARY KEY or UNIQUE constraint that uses included columns. However, this is pretty easy to obtain from pg_index.conkey instead, so I claim that that column is useless. In fact, here's a patch to remove it. This requires a catversion bump, for which it may seem a bit late; however I think it's better to release pg11 without a useless catalog column only to remove it in pg12 ... Thoughts? -- Álvaro Herrera
Вложения
Alvaro Herrera <alvherre@2ndquadrant.com> writes: > This requires a catversion bump, for which it may seem a bit late; > however I think it's better to release pg11 without a useless catalog > column only to remove it in pg12 ... Catversion bumps during beta are routine. If we had put out rc1 I'd say it was too late, but we have not. If we do do a bump for beta4, I'd be strongly tempted to address the lack of a unique index for pg_constraint as well, cf https://www.postgresql.org/message-id/10110.1535907645@sss.pgh.pa.us regards, tom lane
On Sun, Sep 02, 2018 at 01:27:25PM -0400, Tom Lane wrote: > Alvaro Herrera <alvherre@2ndquadrant.com> writes: > > This requires a catversion bump, for which it may seem a bit late; > > however I think it's better to release pg11 without a useless catalog > > column only to remove it in pg12 ... > > Catversion bumps during beta are routine. If we had put out rc1 > I'd say it was too late, but we have not. At the same time Covering indexes are a new thing, so if the timing allows, let's move on with having a cleaner catalog layer from the start, that's less compatibility breakages to justify later on. Hopefully. > If we do do a bump for beta4, I'd be strongly tempted to address the > lack of a unique index for pg_constraint as well, cf > https://www.postgresql.org/message-id/10110.1535907645@sss.pgh.pa.us Yeah... I looked at the thread. -- Michael
Вложения
Greetings, * Tom Lane (tgl@sss.pgh.pa.us) wrote: > Alvaro Herrera <alvherre@2ndquadrant.com> writes: > > This requires a catversion bump, for which it may seem a bit late; > > however I think it's better to release pg11 without a useless catalog > > column only to remove it in pg12 ... Yeah, I really don't think we want to have another catalog column that we end up wanting to remove later on, if we can avoid it.. > Catversion bumps during beta are routine. If we had put out rc1 > I'd say it was too late, but we have not. I agree that rc1 would be too late. On the flip side, I don't think we should really consider catversion bumps during beta to be 'routine'. > If we do do a bump for beta4, I'd be strongly tempted to address the > lack of a unique index for pg_constraint as well, cf > https://www.postgresql.org/message-id/10110.1535907645@sss.pgh.pa.us All that said, given that we've got two pretty good reasons for a catversion bump, and one is to remove a useless column before it ever gets in a release, I'm +1 for making both of these changes. Thanks! Stephen
Вложения
On Sun, Sep 2, 2018 at 01:27:25PM -0400, Tom Lane wrote: > Alvaro Herrera <alvherre@2ndquadrant.com> writes: > > This requires a catversion bump, for which it may seem a bit late; > > however I think it's better to release pg11 without a useless catalog > > column only to remove it in pg12 ... > > Catversion bumps during beta are routine. If we had put out rc1 > I'd say it was too late, but we have not. > > If we do do a bump for beta4, I'd be strongly tempted to address the > lack of a unique index for pg_constraint as well, cf > https://www.postgresql.org/message-id/10110.1535907645@sss.pgh.pa.us Uh, if we add a unique index later, wouldn't that potentially cause future restores to fail? Seems we better add it now. -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com + As you are, so once was I. As I am, so you will be. + + Ancient Roman grave inscription +
On 2018-Sep-07, Bruce Momjian wrote: > On Sun, Sep 2, 2018 at 01:27:25PM -0400, Tom Lane wrote: > > Alvaro Herrera <alvherre@2ndquadrant.com> writes: > > > This requires a catversion bump, for which it may seem a bit late; > > > however I think it's better to release pg11 without a useless catalog > > > column only to remove it in pg12 ... > > > > Catversion bumps during beta are routine. If we had put out rc1 > > I'd say it was too late, but we have not. > > > > If we do do a bump for beta4, I'd be strongly tempted to address the > > lack of a unique index for pg_constraint as well, cf > > https://www.postgresql.org/message-id/10110.1535907645@sss.pgh.pa.us > > Uh, if we add a unique index later, wouldn't that potentially cause > future restores to fail? Seems we better add it now. Committed on Sep 4th as 17b7c302b5fc. -- Álvaro Herrera https://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Bruce Momjian <bruce@momjian.us> writes: > On Sun, Sep 2, 2018 at 01:27:25PM -0400, Tom Lane wrote: >> If we do do a bump for beta4, I'd be strongly tempted to address the >> lack of a unique index for pg_constraint as well, cf >> https://www.postgresql.org/message-id/10110.1535907645@sss.pgh.pa.us > Uh, if we add a unique index later, wouldn't that potentially cause > future restores to fail? Seems we better add it now. Yup, done already. regards, tom lane