Horton

Thread

Commits

  1. Remove pg_constraint.conincluding

  2. Indexes with INCLUDE columns and their support in B-tree

  1. pg_constraint.conincluding is useless

    Alvaro Herrera <alvherre@2ndquadrant.com> — 2018-09-02T16:50:18Z

    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

  2. Re: pg_constraint.conincluding is useless

    Tom Lane <tgl@sss.pgh.pa.us> — 2018-09-02T17:27:25Z

    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

  3. Re: pg_constraint.conincluding is useless

    Michael Paquier <michael@paquier.xyz> — 2018-09-02T18:35:41Z

    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

  4. Re: pg_constraint.conincluding is useless

    Stephen Frost <sfrost@snowman.net> — 2018-09-02T19:14:11Z

    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

  5. Re: pg_constraint.conincluding is useless

    Bruce Momjian <bruce@momjian.us> — 2018-09-07T19:48:29Z

    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 +

  6. Re: pg_constraint.conincluding is useless

    Alvaro Herrera <alvherre@2ndquadrant.com> — 2018-09-07T20:02:57Z

    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

  7. Re: pg_constraint.conincluding is useless

    Tom Lane <tgl@sss.pgh.pa.us> — 2018-09-07T20:12:59Z

    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