cockroachdb / cockroach

CockroachDB — the cloud native, distributed SQL database designed for high availability, effortless scale, and control over data placement.
https://www.cockroachlabs.com
Other
29.87k stars 3.77k forks source link

The sql.defaults.require_explicit_primary_keys.enabled cluster setting doesn't recognise explicit primary keys #128462

Closed smcvey closed 1 week ago

smcvey commented 1 month ago

Describe the problem

There appears to be a bug with the sql.defaults.require_explicit_primary_keys.enabled setting. When the setting is enabled, and a new table is created with an explicitly specified primary key, it still produces a warning that the primary key is missing.

To Reproduce

On a new cluster:

1) Set the require_explicit_primary_keys.enabled setting, either globally:

SET CLUSTER SETTING sql.defaults.require_explicit_primary_keys.enabled = true;

or

ALTER ROLE <role> SET require_explicit_primary_keys = 'true';

(and then reconnect to create a new session)

or locally:

SET require_explicit_primary_keys = 'true';

2) Attempt to create a table with an explicit primary key:

CREATE TABLE t (
    id UUID PRIMARY KEY -- <-- primary key right here
);

Despite having specified the primary key, it still produces this error and fails to create the table:

ERROR: no primary key specified for table t (require_explicit_primary_keys = true)

Another variant of the table where the constraint is explicitly written:

CREATE TABLE t (
    id UUID NOT NULL,
    CONSTRAINT t_pkey PRIMARY KEY (id ASC) -- <-- primary key right here
);

> ERROR: no primary key specified for table t (require_explicit_primary_keys = true)

Expected behavior

Tables should be created as long as a PRIMARY KEY is specified in the creation, as in the examples above.

Environment:

Additional context The setting is currently useless. The end-users cannot force tables to always have explicit primary keys because the setting always has to be disabled.

Jira issue: CRDB-41030

Epic CRDB-40419

blathers-crl[bot] commented 1 month ago

Hi @smcvey, please add branch-* labels to identify which branch(es) this C-bug affects.

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

rafiss commented 1 month ago

@dikshant can you help us determine the priority here? If this setting is not used at all by customers, then perhaps we could just remove it.

dikshant commented 3 weeks ago

I am still trying to find the telemetry for this, the old Looker views don't seem to be working.

rafiss commented 2 weeks ago

We'll go ahead and just attempt to fix this, since it is likely lower effort than figuring out the telemetry or going through the deprecation process.