Re: Add more information_schema columns

[Peter Eisentraut]

On 2/13/18 18:39, Tom Lane wrote:
> Andres Freund  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

Re: Add more information_schema columns

[Tom Lane]

Andres Freund  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

Re: Add more information_schema columns

[Andres Freund]

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

Re: Add more information_schema columns

[Michael Paquier]

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


signature.asc
Description: PGP signature

Re: Add more information_schema columns

[Michael Paquier]

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


signature.asc
Description: PGP signature

Re: Add more information_schema columns

[Peter Eisentraut]

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

Re: Add more information_schema columns

[Michael Paquier]

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


signature.asc
Description: PGP signature

Re: Add more information_schema columns

[Robert Haas]

On Tue, Feb 6, 2018 at 2:15 AM, Michael Paquier
 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

Re: Add more information_schema columns

[Michael Paquier]

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


signature.asc
Description: PGP signature

Add more information_schema columns

[Peter Eisentraut]

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
From c8e1585951859b1248f02c070929e9f83534092a Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <pete...@gmx.net>
Date: Mon, 5 Feb 2018 20:22:16 -0500
Subject: [PATCH] Add more information_schema columns

- table_constraints.enforced
- triggers.action_order
- triggers.action_reference_old_table
- triggers.action_reference_new_table
---
 doc/src/sgml/information_schema.sgml   | 20 +--
 src/backend/catalog/information_schema.sql | 12 ---
 src/test/regress/expected/triggers.out | 54 ++
 src/test/regress/sql/triggers.sql  |  5 +++
 4 files changed, 83 insertions(+), 8 deletions(-)

diff --git a/doc/src/sgml/information_schema.sgml 
b/doc/src/sgml/information_schema.sgml
index 0faa72f1d3..09ef2827f2 100644
--- a/doc/src/sgml/information_schema.sgml
+++ b/doc/src/sgml/information_schema.sgml
@@ -5317,6 +5317,13 @@ table_constraints 
Columns
   yes_or_no
   YES if the constraint is deferrable and 
initially deferred, NO if not
  
+ 
+  enforced
+  yes_or_no
+  Applies to a feature not available in
+  PostgreSQL (currently always
+  YES)
+ 
 

   
@@ -5761,7 +5768,14 @@ triggers Columns
  
   action_order
   cardinal_number
-  Not yet implemented
+  
+   Firing order among triggers on the same table having the same
+   event_manipulation,
+   action_timing, and
+   action_orientation.  In
+   PostgreSQL, triggers are fired in name
+   order, so this column reflects that.
+  
  
 
  
@@ -5806,13 +5820,13 @@ triggers Columns
  
   action_reference_old_table
   sql_identifier
-  Applies to a feature not available in 
PostgreSQL
+  Name of the old transition table, or null if 
none
  
 
  
   action_reference_new_table
   sql_identifier
-  Applies to a feature not available in 
PostgreSQL
+  Name of the new transition table, or null if 
none
  
 
  
diff --git a/src/backend/catalog/information_schema.sql 
b/src/backend/catalog/information_schema.sql
index 6fb1a1bc1c..6066597648 100644
--- a/src/backend/catalog/information_schema.sql
+++ b/src/backend/catalog/information_schema.sql
@@ -1783,7 +1783,8 @@ CREATE VIEW table_constraints AS
CAST(CASE WHEN c.condeferrable THEN 'YES' ELSE 'NO' END AS 
yes_or_no)
  AS is_deferrable,
CAST(CASE WHEN c.condeferred THEN 'YES' ELSE 'NO' END AS yes_or_no)
- AS initially_deferred
+ AS initially_deferred,
+   CAST('YES' AS yes_or_no) AS enforced
 
 FROM pg_namespace nc,
  pg_namespace nr,
@@ -1812,7 +1813,8 @@ CREATE VIEW table_constraints AS
CAST(r.relname AS sql_identifier) AS table_name,
CAST('CHECK' AS character_data) AS constraint_type,
CAST('NO' AS yes_or_no) AS is_deferrable,
-   CAST('NO' AS yes_or_no) AS initially_deferred
+   CAST('NO' AS yes_or_no) AS initially_deferred,
+   CAST('YES' AS yes_or_no) AS enforced
 
 FROM pg_namespace nr,
  pg_class r,
@@ -2084,8 +2086,8 @@ CREATE VIEW triggers AS
CAST(current_database() AS sql_identifier) AS event_object_catalog,
CAST(n.nspname AS sql_identifier) AS event_object_schema,
CAST(c.relname AS sql_identifier) AS event_object_table,
-   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,
CAST(
  CASE WHEN pg_has_role(c.relowner, 'USAGE')
THEN (regexp_match(pg_get_triggerdef(t.oid), E'.{35,} WHEN 
\\((.+)\\) EXECUTE PROCEDURE'))[1]
@@ -2103,8 +2105,8 @@ CREATE VIEW triggers AS
  -- hard-wired refs to TRIGGER_TYPE_BEFORE, TRIGGER_TYPE_INSTEAD
  CASE t.tgtype & 66 WHEN 2 THEN 'BEFORE' WHEN 64 THEN 'INSTEAD OF' 
ELSE 'AFTER' END
  AS character_data) AS action_timing,
-   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,
CAST(null AS sql_identifier) AS action_reference_old_row,
CAST(null AS sql_identifier) AS action_reference_new_row,
CAST(null AS time_stamp) AS created
diff --git a/src/test/regress/expected/triggers.out 
b/src/test/regress/expected/triggers.out
index 9a7aafcc96..7d60b4164f 100644
--- a/src/test/regress/expec