pg_constraint.conincluding is useless
Hi
Already mentioned this in
/messages/by-id/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
Attachments:
0001-remove-conincluding.patchtext/plain; charset=us-asciiDownload+70-68
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
/messages/by-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
/messages/by-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
/messages/by-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
/messages/by-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
/messages/by-id/10110.1535907645@sss.pgh.pa.usUh, 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
/messages/by-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