Обсуждение: Add more information_schema columns
Here is a patch that fills in a few more information schema columns, in particular those related to the trigger transition tables feature. -- Peter Eisentraut http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Вложения
On Mon, Feb 05, 2018 at 08:59:31PM -0500, Peter Eisentraut wrote: > Here is a patch that fills in a few more information schema columns, in > particular those related to the trigger transition tables feature. It is unfortunate that this cannot be backpatched. Here are few comments, the logic and theh definitions look correct to me. > - CAST(null AS cardinal_number) AS action_order, > -- XXX strange hacks follow > + CAST(rank() OVER (PARTITION BY n.oid, c.oid, em.num, (t.tgtype & 1 & 66) ORDER BY t.tgname) AS cardinal_number)AS action_order, Better to use parenthesis for (t.tgtype & 1 & 66) perhaps? You may want to comment that this is to filter per row-statement first, and then with after/before/instead of, which are what the 1 and the 66 are for. > - CAST(null AS sql_identifier) AS action_reference_old_table, > - CAST(null AS sql_identifier) AS action_reference_new_table, > + CAST(tgoldtable AS sql_identifier) AS action_reference_old_table, > + CAST(tgnewtable AS sql_identifier) AS action_reference_new_table, > +SELECT trigger_name, event_manipulation, event_object_schema, > event_object_table, action_order, action_condition, > action_orientation, action_timing, action_reference_old_table, > action_reference_new_table FROM information_schema.triggers ORDER BY > 1, 2; Writing those SQL queries across multiple lines would make them easier to read... -- Michael
Вложения
On 2/6/18 02:15, Michael Paquier wrote: >> - CAST(null AS cardinal_number) AS action_order, >> -- XXX strange hacks follow >> + CAST(rank() OVER (PARTITION BY n.oid, c.oid, em.num, (t.tgtype & 1 & 66) ORDER BY t.tgname) AS cardinal_number)AS action_order, > > Better to use parenthesis for (t.tgtype & 1 & 66) perhaps? You may want > to comment that this is to filter per row-statement first, and then with > after/before/instead of, which are what the 1 and the 66 are for. Added more comments. >> - CAST(null AS sql_identifier) AS action_reference_old_table, >> - CAST(null AS sql_identifier) AS action_reference_new_table, >> + CAST(tgoldtable AS sql_identifier) AS action_reference_old_table, >> + CAST(tgnewtable AS sql_identifier) AS action_reference_new_table, > >> +SELECT trigger_name, event_manipulation, event_object_schema, >> event_object_table, action_order, action_condition, >> action_orientation, action_timing, action_reference_old_table, >> action_reference_new_table FROM information_schema.triggers ORDER BY >> 1, 2; > > Writing those SQL queries across multiple lines would make them easier > to read... done How about the attached version? -- Peter Eisentraut http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Вложения
On Tue, Feb 6, 2018 at 2:15 AM, Michael Paquier
<michael.paquier@gmail.com> wrote:
> Better to use parenthesis for (t.tgtype & 1 & 66) perhaps? You may want
> to comment that this is to filter per row-statement first, and then with
> after/before/instead of, which are what the 1 and the 66 are for.
What possible point can there be to such an expression? It's always 0.
rhaas=# select distinct tgtype::smallint & 1 & 66 from
generate_series(-32768,32767) tgtype;
?column?
----------
0
(1 row)
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
On Tue, Feb 06, 2018 at 03:16:59PM -0500, Robert Haas wrote: > What possible point can there be to such an expression? It's always 0. > > rhaas=# select distinct tgtype::smallint & 1 & 66 from > generate_series(-32768,32767) tgtype; > ?column? > ---------- > 0 > (1 row) Of course you are right here. I just had a look at the patch again after waking up and the current patch builds action_order based only on action_timing and action_orientation, but it forgets event_manipulation. For example row and statement triggers are correctly filtered, but the ordering number includes both insert, update and delete types. So what you need to use instead is (t.tgtype & 1), (t.tgtype & 66) as filter. -- Michael
Вложения
On 2/6/18 17:15, Michael Paquier wrote:
> On Tue, Feb 06, 2018 at 03:16:59PM -0500, Robert Haas wrote:
>> What possible point can there be to such an expression? It's always 0.
>>
>> rhaas=# select distinct tgtype::smallint & 1 & 66 from
>> generate_series(-32768,32767) tgtype;
>> ?column?
>> ----------
>> 0
>> (1 row)
>
> Of course you are right here. I just had a look at the patch again
> after waking up and the current patch builds action_order based only on
> action_timing and action_orientation, but it forgets event_manipulation.
> For example row and statement triggers are correctly filtered, but the
> ordering number includes both insert, update and delete types. So what
> you need to use instead is (t.tgtype & 1), (t.tgtype & 66) as filter.
I think what I had meant to write was something like
(t.tgtype & (1 | 66))
but maybe it's clearer to write it all out as you did.
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On Tue, Feb 06, 2018 at 10:45:52PM -0500, Peter Eisentraut wrote: > I think what I had meant to write was something like > > (t.tgtype & (1 | 66)) > > but maybe it's clearer to write it all out as you did. If you prefer that, that's fine for me as well. I tend to prefer the formulation where both expressions are separated to make clearer that ordering needs to be split for all three characteristics. -- Michael
Вложения
On 2/7/18 00:14, Michael Paquier wrote: > On Tue, Feb 06, 2018 at 10:45:52PM -0500, Peter Eisentraut wrote: >> I think what I had meant to write was something like >> >> (t.tgtype & (1 | 66)) >> >> but maybe it's clearer to write it all out as you did. > > If you prefer that, that's fine for me as well. I tend to prefer the > formulation where both expressions are separated to make clearer that > ordering needs to be split for all three characteristics. Committed with the separate entries. -- Peter Eisentraut http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On Wed, Feb 07, 2018 at 10:50:12AM -0500, Peter Eisentraut wrote: > Committed with the separate entries. Thanks. The result looks fine to me. -- Michael
Вложения
On 2018-02-07 10:50:12 -0500, Peter Eisentraut wrote: > On 2/7/18 00:14, Michael Paquier wrote: > > On Tue, Feb 06, 2018 at 10:45:52PM -0500, Peter Eisentraut wrote: > >> I think what I had meant to write was something like > >> > >> (t.tgtype & (1 | 66)) > >> > >> but maybe it's clearer to write it all out as you did. > > > > If you prefer that, that's fine for me as well. I tend to prefer the > > formulation where both expressions are separated to make clearer that > > ordering needs to be split for all three characteristics. > > Committed with the separate entries. Do we have a policy about catversion bumps for information schema changes? A cluster from before this commit fails the regression tests after the change, but still mostly works... Greetings, Andres Freund
Andres Freund <andres@anarazel.de> writes:
> Do we have a policy about catversion bumps for information schema
> changes? A cluster from before this commit fails the regression tests
> after the change, but still mostly works...
I think historically we've not bumped catversion, on the grounds that
there's no incompatibility with the backend as such. However, it is
kind of annoying that not updating means the regression tests fail.
Informally, I'm sure most developers take "catversion bump" to mean
"you need to initdb". So I'd support saying that an information_schema
change should include a catversion bump if it involves any changes in
regression test results.
regards, tom lane
On 2/13/18 18:39, Tom Lane wrote: > Andres Freund <andres@anarazel.de> writes: >> Do we have a policy about catversion bumps for information schema >> changes? A cluster from before this commit fails the regression tests >> after the change, but still mostly works... > > I think historically we've not bumped catversion, on the grounds that > there's no incompatibility with the backend as such. However, it is > kind of annoying that not updating means the regression tests fail. > Informally, I'm sure most developers take "catversion bump" to mean > "you need to initdb". So I'd support saying that an information_schema > change should include a catversion bump if it involves any changes in > regression test results. I will do that in the future if that is the preference. -- Peter Eisentraut http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services