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

REFCURSOR[] columns can be uniquely indexed and FK'd but cause unexpected errors #115701

Closed
chrisseto opened this issue Dec 6, 2023 · 4 comments · Fixed by #115707
Closed

REFCURSOR[] columns can be uniquely indexed and FK'd but cause unexpected errors #115701

chrisseto opened this issue Dec 6, 2023 · 4 comments · Fixed by #115707
Assignees
Labels
branch-release-23.2 Used to mark GA and release blockers and technical advisories for 23.2 C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. O-schema-testing Originated from SQL Schema testing strategies T-sql-foundations SQL Foundations Team (formerly SQL Schema + SQL Sessions)

Comments

@chrisseto
Copy link
Contributor

chrisseto commented Dec 6, 2023

Describe the problem

Currently, it's not possible to create a unique refcusor column.

demo@127.0.0.1:26257/demoapp/movr> CREATE TABLE t(a refcursor UNIQUE);
ERROR: unimplemented: column a is of type refcursor and thus is not indexable
SQLSTATE: 0A000
HINT: You have attempted to use a feature that is not yet implemented.
See: https://go.crdb.dev/issue-v/35730/v24.1

Therefore it's not possible to make a foreign key between refcursors as well.

demo@127.0.0.1:26257/demoapp/movr> CREATE TABLE t(a refcursor);
CREATE TABLE

Time: 11ms total (execution 11ms / network 0ms)

demo@127.0.0.1:26257/demoapp/movr> alter table t add constraint test foreign key (a) references t(a);
ERROR: there is no unique constraint matching given keys for referenced table t
SQLSTATE: 23503

However, recursor[] columns can be uniquely index and foreign key'd. Though attempting to add a foreign key constraint will fail with the following error:

demo@127.0.0.1:26257/demoapp/movr> CREATE TABLE t(a refcursor[] unique);
CREATE TABLE

Time: 10ms total (execution 10ms / network 0ms)

demo@127.0.0.1:26257/demoapp/movr> alter table t add constraint test foreign key (a) references t(a);
ERROR: validate fk constraint: could not identify a comparison function for type refcursor
SQLSTATE: 42883

Expected behavior
I would expect that creating a unique refcursor[] column would fail with the same error as a unique refcursor column.

Jira issue: CRDB-34159

@chrisseto chrisseto added C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. O-schema-testing Originated from SQL Schema testing strategies T-sql-foundations SQL Foundations Team (formerly SQL Schema + SQL Sessions) labels Dec 6, 2023
@blathers-crl blathers-crl bot added this to Triage in SQL Foundations Dec 6, 2023
@mgartner
Copy link
Collaborator

mgartner commented Dec 6, 2023

An index on a REFCURSOR[] column should also be disallowed:

CREATE TABLE t(a refcursor[], index (a));

It currently succeeds during creation.

@mgartner mgartner removed this from Triage in SQL Foundations Dec 6, 2023
Copy link

blathers-crl bot commented Dec 6, 2023

Hi @mgartner, please add branch-* labels to identify which branch(es) this release-blocker affects.

🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf.

@mgartner
Copy link
Collaborator

mgartner commented Dec 6, 2023

Marking as a GA-blocker for now, but this is probably unlikely to be hit. I think the fix should be easy though.

@rafiss rafiss added the branch-release-23.2 Used to mark GA and release blockers and technical advisories for 23.2 label Dec 6, 2023
@mgartner
Copy link
Collaborator

mgartner commented Dec 6, 2023

Doesn't look like this will cause many problems besides unexpected errors:

defaultdb> CREATE TABLE t(a refcursor[], index (a));

defaultdb> select * from t where a = ARRAY['foo'];
ERROR: could not identify a comparison function for type refcursor

Some of our randomized tests will probably hit this at some point, so we should fix it, but I don't think it's a blocker.

chrisseto added a commit to chrisseto/cockroach that referenced this issue Dec 6, 2023
Previously, it was possible to circumvent the check performed by
`ColumnTypeIsIndexable` by wrapping a type in an array. Doing so would
violate some invariants across the system and lead to undefined
behavior.

This commit updates `ColumnTypeIsIndexable` to unwrap array types, if
necessary, to prevent invalid indexes from being created.

Fixes: cockroachdb#115701
Epic: none
Release note (bug fix): Indexes may no longer be created on REFCURSOR[]s
columns. REFCURSOR columns themselves are not indexable.
craig bot pushed a commit that referenced this issue Dec 6, 2023
115671: storage: populate OmitInRangefeeds in MVCC code r=RaduBerinde a=RaduBerinde

This change populates the `MVCCValueHEader.OmitInRangeFeeds` flag based on the corresponding flag in the transaction.

Fixes: #113634
Release note: None

115707: sql: prevent creating indexes on arrays of non-indexable types r=DrewKimball,fqazi,rafiss a=chrisseto

#### ae7fbfb sql: prevent creating indexes on arrays of non-indexable types

Previously, it was possible to circumvent the check performed by
`ColumnTypeIsIndexable` and `ColumnTypeIsInvertedIndexable` by wrapping
a type in an array. Doing so would violate some invariants across the
system and lead to undefined behavior.

This commit updates both `ColumnTypeIsIndexable` and
`ColumnTypeIsInvertedIndexable` to unwrap array types, as necessary, to
prevent invalid indexes from being created.

Fixes: #115701
Epic: none
Release note (bug fix): Standard and inverted indexes may no longer be
created on REFCURSOR[]s columns. REFCURSOR columns themselves are not
indexable.

115713: Revert "s3: disable content pre-upload checksums" r=stevendanna a=adityamaru

This reverts commit 50c453a.

Release note (bug fix): fixes a regression that causes uploads to object locked buckets to fail because of the absence of an MD5 hash

Informs: #115602

Co-authored-by: Radu Berinde <radu@cockroachlabs.com>
Co-authored-by: Chris Seto <chriskseto@gmail.com>
Co-authored-by: adityamaru <adityamaru@gmail.com>
@craig craig bot closed this as completed in ae7fbfb Dec 6, 2023
chrisseto added a commit to chrisseto/cockroach that referenced this issue Dec 11, 2023
Previously, it was possible to circumvent the check performed by
`ColumnTypeIsIndexable` and `ColumnTypeIsInvertedIndexable` by wrapping
a type in an array. Doing so would violate some invariants across the
system and lead to undefined behavior.

This commit updates both `ColumnTypeIsIndexable` and
`ColumnTypeIsInvertedIndexable` to unwrap array types, as necessary, to
prevent invalid indexes from being created.

Fixes: cockroachdb#115701
Epic: none
Release note (bug fix): Standard and inverted indexes may no longer be
created on REFCURSOR[]s columns. REFCURSOR columns themselves are not
indexable.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
branch-release-23.2 Used to mark GA and release blockers and technical advisories for 23.2 C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. O-schema-testing Originated from SQL Schema testing strategies T-sql-foundations SQL Foundations Team (formerly SQL Schema + SQL Sessions)
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

3 participants