Re: dropping column prevented due to inherited index - Mailing list pgsql-hackers

From Amit Langote
Subject Re: dropping column prevented due to inherited index
Date
Msg-id CA+HiwqGGHdrvWND4-HD9anwYDFdf+GJ4H2hiOCo0tzu9nd0euw@mail.gmail.com
Whole thread Raw
In response to Re: dropping column prevented due to inherited index  (Michael Paquier <michael@paquier.xyz>)
Responses Re: dropping column prevented due to inherited index
List pgsql-hackers
On Thu, Oct 10, 2019 at 4:53 PM Michael Paquier <michael@paquier.xyz> wrote:
> On Thu, Oct 10, 2019 at 02:56:32PM +0900, Amit Langote wrote:
> > /* Initialize addrs on the first invocation. */
>
> I would add "recursive" here, to give:
> /* Initialize addrs on the first recursive invocation. */

Actually, the code initializes it on the first call (recursing is
false) and asserts that it must have been already initialized in a
recursive (recursing is true) call.

> > +    /*
> > +     * The recursion is ending, hence perform the actual column deletions.
> > +     */
> >
> > Maybe:
> >
> > /* All columns to be dropped must now be in addrs, so drop. */
>
> I think that it would be better to clarify as well that this stands
> after all the child relations have been checked, so what about that?
> "The resursive lookup for inherited child relations is done.  All the
> child relations have been scanned and the object addresses of all the
> columns to-be-dropped are registered in addrs, so drop."

Okay, sure.  Maybe it's better to write the comment inside the if
block, because if recursing is true, we don't drop yet.

Thoughts on suggestion to expand the test case?

Thanks,
Amit



pgsql-hackers by date:

Previous
From: Amit Langote
Date:
Subject: Re: [PATCH] Do not use StdRdOptions in Access Methods
Next
From: John Naylor
Date:
Subject: generating catcache control data