sql: fix the match type reporting in information_schema.referential_constraints #35575
Conversation
|
This change is |
…onstraints This is the info_schema complement to cockroachdb#35052. Release note (bug fix): CockroachDB now properly reports the composite foreign key match type in `information_schema.referential_constraints`.
knz
force-pushed
the
20190309-fix-match-type
branch
from
727d745 to
b7fe419
Compare
There was a problem hiding this comment.
Reviewed 1 of 2 files at r1.
Reviewable status:complete! 0 of 0 LGTMs obtained (waiting on @bobvawter, @BramGruneir, and @knz)
pkg/sql/information_schema.go, line 632 at r1 (raw file):
return err } var matchType tree.Datum = tree.DNull
Should the default be null and not none? Still strange that none is simple.
I used the same values as in postgres. To my eyes the pg compatibility is most important. WDYT? |
There was a problem hiding this comment.
Oh, I agree, in PG it should be none for simple right?
But when should it ever be null? Every match needs a type.
We have the legacy of null for match type from way back before cascading was added, but those should be set to simple now.
Reviewable status:
Yes correct.
The value NULL will never occur in practice; it is used as fall-back when the Does this clarify? |
|
(we did the same in |
There was a problem hiding this comment.
What if fk.Match is empty?
Reviewable status:
There was a problem hiding this comment.
Looking at pg_catalog.pg_constraint, I think this needs to be fixed there too. If there is no match value, we must now assume that it is simple.
Reviewable status:
|
Can you double check? When the descriptor does not have the This is also the value you have chosen for What is the case that's missing? |
There was a problem hiding this comment.
Ah, you're right. Carry on!
LGTM
Reviewed 1 of 2 files at r1.
Reviewable status:
|
queue after #35583 merges |
|
bors r+ |
35573: sql/parser: add a missing contextual help annotation r=knz a=knz Release note: None 35575: sql: fix the match type reporting in information_schema.referential_constraints r=knz a=knz This is the info_schema complement to #35052. Release note (bug fix): CockroachDB now properly reports the composite foreign key match type in `information_schema.referential_constraints`. 35577: sql: properly mark sequences as `bigint` in information_schema.sequences r=knz a=knz Release note (bug fix): CockroachDB now properly reports `bigint` in information_schema.sequences.type, for compatibility with PostgreSQL. 35578: sql: populate information_schema.table_privileges.with_hierarchy r=knz a=knz Pg doc says: *In the SQL standard, WITH HIERARCHY OPTION is a separate (sub-)privilege allowing certain operations on table inheritance hierarchies. In PostgreSQL, this is included in the SELECT privilege, so this column shows YES if the privilege is SELECT, else NO.* https://www.postgresql.org/docs/9.5/infoschema-table-privileges.html Release note: None Co-authored-by: Raphael 'kena' Poss <[email protected]>
Build succeeded |
This is the info_schema complement to #35052.
Release note (bug fix): CockroachDB now properly reports the composite
foreign key match type in
information_schema.referential_constraints.