Thread: information_schema and not-null constraints
In reference to [1], 0001 attached to this email contains the updated
view definitions that I propose.
In 0002, I took the tests added by Peter's proposed patch and put them
in a separate test file that runs at the end. There are some issues,
however. One is that the ORDER BY clause in the check_constraints view
is not fully deterministic, because the table name is not part of the
view definition, so we cannot sort by table name. In the current
regression database there is only one case[2] where two constraints have
the same name and different definition:
inh_check_constraint │ 2 │ ((f1 > 0)) NOT VALID ↵
│ │ ((f1 > 0))
(on tables invalid_check_con and invalid_check_con_child). I assume
this is going to bite us at some point. We could just add a WHERE
clause to omit that one constraint.
Another issue I notice eyeballing at the results is that foreign keys on
partitioned tables are listing the rows used to implement the
constraints on partitions, which are sort-of "internal" constraints (and
are not displayed by psql's \d). I hope this is a relatively simple fix
that we could extract from the code used by psql.
Anyway, I think I'm going to get 0001 committed sometime tomorrow, and
then play a bit more with 0002 to try and get it pushed soon also.
Thanks
[1] https://postgr.es/m/81b461c4-edab-5d8c-2f88-203108425340@enterprisedb.com
[2]
select constraint_name, count(*),
string_agg(distinct check_clause, E'\n')
from information_schema.check_constraints
group by constraint_name
having count(*) > 1;
--
Álvaro Herrera 48°01'N 7°57'E — https://www.EnterpriseDB.com/
"You don't solve a bad join with SELECT DISTINCT" #CupsOfFail
https://twitter.com/connor_mc_d/status/1431240081726115845
Attachment
Alvaro Herrera <alvherre@alvh.no-ip.org> writes:
> In 0002, I took the tests added by Peter's proposed patch and put them
> in a separate test file that runs at the end. There are some issues,
> however. One is that the ORDER BY clause in the check_constraints view
> is not fully deterministic, because the table name is not part of the
> view definition, so we cannot sort by table name.
I object very very strongly to this proposed test method. It
completely undoes the work I did in v15 (cc50080a8 and related)
to make the core regression test scripts mostly independent of each
other. Even without considering the use-case of running a subset of
the tests, the new test's expected output will constantly be needing
updates as side effects of unrelated changes.
regards, tom lane
On 2023-Sep-04, Alvaro Herrera wrote: > In reference to [1], 0001 attached to this email contains the updated > view definitions that I propose. Given the downthread discussion, I propose the attached. There are no changes to v2, other than dropping the test part. We can improve the situation for domains separately and likewise for testing. -- Álvaro Herrera 48°01'N 7°57'E — https://www.EnterpriseDB.com/
Attachment
On 2023-Sep-06, Alvaro Herrera wrote: > On 2023-Sep-04, Alvaro Herrera wrote: > > > In reference to [1], 0001 attached to this email contains the updated > > view definitions that I propose. > > Given the downthread discussion, I propose the attached. There are no > changes to v2, other than dropping the test part. Pushed. -- Álvaro Herrera PostgreSQL Developer — https://www.EnterpriseDB.com/
On 06.09.23 19:52, Alvaro Herrera wrote:
> + SELECT current_database()::information_schema.sql_identifier AS constraint_catalog,
> + rs.nspname::information_schema.sql_identifier AS constraint_schema,
> + con.conname::information_schema.sql_identifier AS constraint_name,
> + format('CHECK (%s IS NOT NULL)', at.attname)::information_schema.character_data AS check_clause
Small correction here: This should be
pg_catalog.format('%s IS NOT NULL', at.attname)::information_schema.character_data AS check_clause
That is, the word "CHECK" and the parentheses should not be part of the
produced value.
On 14.09.23 10:20, Peter Eisentraut wrote:
> On 06.09.23 19:52, Alvaro Herrera wrote:
>> + SELECT current_database()::information_schema.sql_identifier AS
>> constraint_catalog,
>> + rs.nspname::information_schema.sql_identifier AS
>> constraint_schema,
>> + con.conname::information_schema.sql_identifier AS
>> constraint_name,
>> + format('CHECK (%s IS NOT NULL)',
>> at.attname)::information_schema.character_data AS check_clause
>
> Small correction here: This should be
>
> pg_catalog.format('%s IS NOT NULL',
> at.attname)::information_schema.character_data AS check_clause
>
> That is, the word "CHECK" and the parentheses should not be part of the
> produced value.
I have committed this fix.
On 2023-Sep-18, Peter Eisentraut wrote:
> On 14.09.23 10:20, Peter Eisentraut wrote:
> > Small correction here: This should be
> >
> > pg_catalog.format('%s IS NOT NULL',
> > at.attname)::information_schema.character_data AS check_clause
> >
> > That is, the word "CHECK" and the parentheses should not be part of the
> > produced value.
>
> I have committed this fix.
Thanks.
--
Álvaro Herrera 48°01'N 7°57'E — https://www.EnterpriseDB.com/
On 14.09.23 10:20, Peter Eisentraut wrote:
> On 06.09.23 19:52, Alvaro Herrera wrote:
>> + SELECT current_database()::information_schema.sql_identifier AS
>> constraint_catalog,
>> + rs.nspname::information_schema.sql_identifier AS
>> constraint_schema,
>> + con.conname::information_schema.sql_identifier AS
>> constraint_name,
>> + format('CHECK (%s IS NOT NULL)',
>> at.attname)::information_schema.character_data AS check_clause
>
> Small correction here: This should be
>
> pg_catalog.format('%s IS NOT NULL',
> at.attname)::information_schema.character_data AS check_clause
>
> That is, the word "CHECK" and the parentheses should not be part of the
> produced value.
Slightly related, so let's just tack it on here:
While testing this, I noticed that the way the check_clause of regular
check constraints is computed appears to be suboptimal. It currently does
CAST(substring(pg_get_constraintdef(con.oid) from 7) AS character_data)
which ends up with an extra set of parentheses, which is ignorable, but
it also leaves in suffixes like "NOT VALID", which don't belong into
that column. Earlier in this thread I had contemplated a fix for the
first issue, but that wouldn't address the second issue. I think we can
fix this quite simply by using pg_get_expr() instead. I don't know why
it wasn't done like that to begin with, maybe it was just a (my?)
mistake. See attached patch.
Attachment
On 19.09.23 09:01, Peter Eisentraut wrote: > While testing this, I noticed that the way the check_clause of regular > check constraints is computed appears to be suboptimal. It currently does > > CAST(substring(pg_get_constraintdef(con.oid) from 7) AS character_data) > > which ends up with an extra set of parentheses, which is ignorable, but > it also leaves in suffixes like "NOT VALID", which don't belong into > that column. Earlier in this thread I had contemplated a fix for the > first issue, but that wouldn't address the second issue. I think we can > fix this quite simply by using pg_get_expr() instead. I don't know why > it wasn't done like that to begin with, maybe it was just a (my?) > mistake. See attached patch. committed