On Fri, Mar 10, 2017 at 11:48 AM, Kevin Grittner <kgri...@gmail.com> wrote:
> On Tue, Mar 7, 2017 at 6:28 PM, Kevin Grittner <kgri...@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 <kgri...@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_vsute...@mail.gmail.com

-- 
Thomas Munro
http://www.enterprisedb.com
DROP TABLE IF EXISTS hoge;

CREATE TABLE hoge
(
  id int primary key,
  name text
);

CREATE OR REPLACE FUNCTION hoge_upd_func()
  RETURNS TRIGGER
  LANGUAGE plpgsql
AS $$
BEGIN
  RAISE WARNING 'old table = %, new table = %', (SELECT string_agg(id || '=' || 
name, ',') FROM d), (SELECT string_agg(id || '=' || name, ',') FROM i);
  RETURN NULL;
END;
$$;

CREATE TRIGGER hoge_upd_trigger
  AFTER UPDATE ON hoge
  REFERENCING OLD TABLE AS d NEW TABLE AS i
  FOR EACH STATEMENT EXECUTE PROCEDURE hoge_upd_func();

insert into hoge values (1, '1'), (2, '2'), (3, '3');
update hoge set name = name || name;

-- now change 'name' to an integer to see what happens...
alter table hoge alter column name type int using name::integer;
update hoge set name = (name::text || name::text)::integer;

-- at this point we get an error message:
-- ERROR:  attribute 2 has wrong type
-- DETAIL:  Table has type integer, but query expects text.

-- That error ^ can be cleared by recreating the function hoge_upd_func.

-- now drop column 'name' 
alter table hoge drop column name;
update hoge set id = id;

-- segfault!
-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to