Re: delta relations in AFTER triggers

Thomas Munro <thomas.munro@enterprisedb.com>

From: Thomas Munro <thomas.munro@enterprisedb.com>
To: Kevin Grittner <kgrittn@gmail.com>
Cc: Haribabu Kommi <kommi.haribabu@gmail.com>, Jim Nasby <Jim.Nasby@bluetreble.com>, Craig Ringer <craig@2ndquadrant.com>, Michael Paquier <michael.paquier@gmail.com>, Robert Haas <robertmhaas@gmail.com>, David Fetter <david@fetter.org>, Kevin Grittner <kgrittn@ymail.com>, Tom Lane <tgl@sss.pgh.pa.us>, Amit Khandekar <amit.khandekar@enterprisedb.com>, Alvaro Herrera <alvherre@2ndquadrant.com>, Pg Hackers <pgsql-hackers@postgresql.org>
Date: 2017-03-12T21:08:12Z
Lists: pgsql-hackers

Attachments

On Fri, Mar 10, 2017 at 11:48 AM, Kevin Grittner <kgrittn@gmail.com> wrote:
> On Tue, Mar 7, 2017 at 6:28 PM, Kevin Grittner <kgrittn@gmail.com> wrote:
>
>> New patch attached.
>
> And bit-rotted less than 24 hours later by fcec6caa.
>
> New patch attached just to fix bit-rot.
>
> That conflicting patch might be a candidate to merge into the new
> Ephemeral Named Relation provided by my patch, for more flexibility
> and extensibility...

Thanks.  I found a new way to break it: run the trigger function so
that the plan is cached by plpgsql, then ALTER TABLE incompatibly,
then run the trigger function again.  See attached.

On Wed, Mar 8, 2017 at 1:28 PM, Kevin Grittner <kgrittn@gmail.com> wrote:
>>> I was looking for omissions that would cause some kind of statements
>>> to miss out on ENRs arbitrarily.  It seemed to me that
>>> parse_analyze_varparams should take a QueryEnvironment, mirroring
>>> parse_analyze, so that PrepareQuery could pass it in.  Otherwise,
>>> PREPARE wouldn't see ENRs.  Is there any reason why SPI_prepare should
>>> see them but PREPARE not?
>>
>> Any thoughts about that?
>
> Do you see any way to test that code, or would it be dead code there
> "just in case" we later decided to do something that needed it?  I'm
> not a big fan of the latter.  I've had to spend too much time
> maintaining and/or ripping out code that fits that description.

I guess you could test it by reaching PREPARE and EXECUTE via dynamic
SQL inside a plpgsql function (ie EXECUTE 'EXECUTE ...').

Really I was just trying to be thorough and examine every path into
the parser and analyser to make sure they all supported the new
QueryEnvironment argument.  When I found that the PREPARE path didn't,
my first thought was that there may be PLs that wouldn't be able to
take advantage of plan reuse any other way, but I see that all the
built-in PLs expose SPI_prepare, so that isn't a problem for them.

You're probably right that it's not actually very useful.  We've
recorded this obscure omission in the archives.

> Miscellanea:
>
> Do you suppose we should have all PLs that are part of the base
> distro covered?

I vote for doing that in Postgres 11.  My pl/python patch[1] may be a
useful starting point, but I haven't submitted it in this CF and
nobody has shown up with pl/tcl or pl/perl versions.

> What is necessary to indicate an additional SQL feature covered?

I assume you're talking about information_schema.sql_features, and I
see you've created a new thread to talk about that.  I'm not sure
about that, but a couple of thoughts occurred to me when looking for
references to transition tables in an old draft standard I have.
These are both cases where properties of the subject table should
probably also affect access to the derived transition tables:

* What privileges implications are there for transition tables?  I'm
wondering about column and row level privileges; for example, if you
can't see a column in the subject table, I'm guessing you shouldn't be
allowed to see it in the transition table either, but I'm not sure.

* In future we could consider teaching it about functional
dependencies as required by the spec; if you can SELECT id, name FROM
<subject table> GROUP BY id, I believe you should be able to SELECT
id, name FROM <transition table> GROUP BY id, but currently you can't.

[1] https://www.postgresql.org/message-id/CAEepm=3WVmPmz3bkFtK2kcnD9Kr7hxPZ2SKJ8SfZX_VSuTeh2A@mail.gmail.com

-- 
Thomas Munro
http://www.enterprisedb.com

Commits

  1. Don't permit transition tables with TRUNCATE triggers.

  2. Pass EXEC_FLAG_REWIND when initializing a tuplestore scan.

  3. Faster expression evaluation and targetlist projection.

  4. Remove obsoleted code relating to targetlist SRF evaluation.

  5. Implement syntax for transition tables in AFTER triggers.