Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

sql: fix confupdtype, confdeltype, confmatchtype in pg_constraint #35052

Merged
merged 1 commit into from
Feb 20, 2019

Conversation

knz
Copy link
Contributor

@knz knz commented Feb 19, 2019

Fixes #34862. For TypeORM compat. cc @awoods187

@knz knz requested review from bobvawter and BramGruneir February 19, 2019 13:53
@knz knz requested review from a team February 19, 2019 13:54
@cockroach-teamcity
Copy link
Member

This change is Reviewable

Copy link
Member

@BramGruneir BramGruneir left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:lgtm: with minor fixes

Reviewed 4 of 4 files at r1.
Reviewable status: :shipit: complete! 1 of 0 LGTMs obtained (waiting on @bobvawter and @knz)


pkg/sql/pg_catalog.go, line 646 at r1 (raw file):

		sqlbase.ForeignKeyReference_FULL:   fkMatchTypeFull,
		// TODO(knz,bram): add when partial is supported.
		// sqlbase.ForeignKeyReferenc_PARTIAL: fkMatchTypeSimple,

might as well put in it even if it's not supported and just leave a note that it's not


pkg/sql/pg_catalog.go, line 649 at r1 (raw file):

	}
	// Avoid unused warning for constants.
	_ = fkMatchTypePartial

remove this.


pkg/sql/logictest/testdata/logic_test/pg_catalog, line 1720 at r1 (raw file):

    i INT DEFAULT 123 REFERENCES t(x) ON UPDATE SET DEFAULT,
    j INT REFERENCES t(x) ON UPDATE CASCADE
  );

Please add one tests case that has both a delete and updated to ensure that both paths are always followed.


pkg/sql/sqlbase/structured.proto, line 219 at r1 (raw file):

    SIMPLE = 0;
    FULL = 1;
    // TODO(knz/bram): Update pg_constraint when PARTIAL is supported.

Just a note that the chance of us supporting it is pretty low.

@knz knz force-pushed the 20190219-fk-metadata branch from 1ea5603 to a6c85a3 Compare February 20, 2019 06:22
Release note (bug fix): The columns `confupdtype`, `confdeltype` and
`confmatchtype` in `pg_constraint` now report the FK constraint
parameters properly, for compatibility with PostgreSQL clients that
use them.
@knz knz force-pushed the 20190219-fk-metadata branch from a6c85a3 to 0f16a6f Compare February 20, 2019 06:24
Copy link
Contributor Author

@knz knz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (and 1 stale) (waiting on @bobvawter and @BramGruneir)


pkg/sql/pg_catalog.go, line 646 at r1 (raw file):

Previously, BramGruneir (Bram Gruneir) wrote…

might as well put in it even if it's not supported and just leave a note that it's not

Done.


pkg/sql/pg_catalog.go, line 649 at r1 (raw file):

Previously, BramGruneir (Bram Gruneir) wrote…

remove this.

Done.


pkg/sql/logictest/testdata/logic_test/pg_catalog, line 1720 at r1 (raw file):

Previously, BramGruneir (Bram Gruneir) wrote…

Please add one tests case that has both a delete and updated to ensure that both paths are always followed.

Done.


pkg/sql/sqlbase/structured.proto, line 219 at r1 (raw file):

Previously, BramGruneir (Bram Gruneir) wrote…

Just a note that the chance of us supporting it is pretty low.

Done.

@knz
Copy link
Contributor Author

knz commented Feb 20, 2019

TFYR!

bors r+

@knz
Copy link
Contributor Author

knz commented Feb 20, 2019

bors r-

@craig
Copy link
Contributor

craig bot commented Feb 20, 2019

Canceled

@knz
Copy link
Contributor Author

knz commented Feb 20, 2019

bors r+

@knz knz changed the title sql: fix conupdtype, confdeltype, conmatchtype in pg_constraint sql: fix confupdtype, confdeltype, confmatchtype in pg_constraint Feb 20, 2019
craig bot pushed a commit that referenced this pull request Feb 20, 2019
35052: sql: fix conupdtype, confdeltype, conmatchtype in pg_constraint r=knz a=knz

Fixes #34862. For TypeORM compat. cc @awoods187 


Co-authored-by: Raphael 'kena' Poss <[email protected]>
@craig
Copy link
Contributor

craig bot commented Feb 20, 2019

Build succeeded

@craig craig bot merged commit 0f16a6f into cockroachdb:master Feb 20, 2019
@knz knz deleted the 20190219-fk-metadata branch February 20, 2019 07:15
knz added a commit to knz/cockroach that referenced this pull request Mar 9, 2019
…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`.
craig bot pushed a commit that referenced this pull request Mar 9, 2019
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]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants