From: Tom Lane <tgl@sss.pgh.pa.us>
To: Julien Rouhaud <julien.rouhaud@dalibo.com>
Cc: Philippe BEAUDOIN <phb.emaj@free.fr>
Cc: pgsql-hackers <pgsql-hackers@postgresql.org>
Subject: Re: [GENERAL] Column rename in an extension update script
Date: Tue, 02 May 2017 17:23:17 -0400
Message-ID: <20109.1493760197@sss.pgh.pa.us> (raw)
In-Reply-To: <24fbcf75-04d2-9d10-ac12-c99fe4ca96a4@dalibo.com>
References: <2cf7941e-4e41-7714-3de8-37b1a8f74dff@free.fr>
<24fbcf75-04d2-9d10-ac12-c99fe4ca96a4@dalibo.com>
List-Unsubscribe: <mailto:majordomo@postgresql.org?body=unsub%20pgsql-hackers>
Julien Rouhaud <julien.rouhaud@dalibo.com> writes:
> moving this thread to -hackers, since this looks like a bug.
> On 01/05/2017 08:54, Philippe BEAUDOIN wrote:
>> I am coding an update script for an extension. And I am in trouble when
>> trying to rename a column of an existing table.
>>
>> Just after the ALTER TABLE statement, I want to access this table. But
>> at this time, the altered column is not visible with its new name.
> I can reproduce this issue.
> It looks like this is due to missing cache invalidation between the
> ALTER TABLE execution and next query planning (failure happens during
> pg_analyze_and_rewrite()).
Yes. Kind of surprising nobody noticed this before --- you'd certainly
think that extension scripts would have lots of cases of statements
depending on DDL done just before them. I think it must have been masked
by the fact that many DDL commands *end* with CommandCounterIncrement,
or at least have one pretty close to the end. But not renameatt().
> AFAICT, CommandCounterIncrement() is responsible for handling
> invalidation messages, but in execute_sql_string() this function is
> called before executing a Stmt instead of after. Moving the
> CommandCounterIncrement() just before the PopActiveSnapshot() call (and
> removing the final one) fixes this issue for me, but I have no idea if
> this is the right fix.
I'm inclined to add one before the parse step, rather than removing any.
The sequence of bump-the-command-counter-then-capture-a-snapshot is
pretty well established in places like spi.c, so I don't want to change
that usage. Also, I think part of the point here was to ensure that
any DDL done *before* reaching execute_sql_string() is visible to the
first command. Also note the assumption in ApplyExtensionUpdates that
execute_sql_string will do at least one CommandCounterIncrement, even
if the script is empty. A CCI that has nothing to do is quite cheap,
and we're not that worried about performance here anyway IMO, so I'd
just as soon err on the side of having more than enough CCIs.
regards, tom lane
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-general@postgresql.org
Cc: tgl@sss.pgh.pa.us, julien.rouhaud@dalibo.com, phb.emaj@free.fr, pgsql-hackers@postgresql.org
Subject: Re: [GENERAL] Column rename in an extension update script
In-Reply-To: <20109.1493760197@sss.pgh.pa.us>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox