sql: Drop and add a primary key constraint trick doesn't work with primary key named as sql keyword

[chengxiong-ruan]

Describe the problem
If a table has a primary key whose name is a sql key world, then the Drop and add a primary key constraint trick won't work. The workaround is to rename the primary key to something else >_>.

The reason is that to use sql keyword as name, we need to double quote the string, and this line of code doesn't work because name.String() return a name with the double quote in the string :(

To Reproduce

demo@127.0.0.1:26257/movr> create table stats ( id string, new_id string, CONSTRAINT "primary" PRIMARY KEY (id) );
demo@127.0.0.1:26257/movr> ALTER TABLE stats ALTER COLUMN new_id SET NOT NULL;
demo@127.0.0.1:26257/movr> BEGIN; ALTER TABLE stats DROP CONSTRAINT "primary"; ALTER TABLE stats ADD CONSTRAINT "primary" PRIMARY KEY (new_id); COMMIT;

BEGIN
ALTER TABLE
ERROR: constraint with name "primary" already exists
SQLSTATE: 42710

If you rename the primary key to something else, then it works....

demo@127.0.0.1:26257/movr> ALTER TABLE stats RENAME CONSTRAINT "primary" TO p_key;
demo@127.0.0.1:26257/movr> BEGIN; ALTER TABLE stats DROP CONSTRAINT p_key; ALTER TABLE stats ADD CONSTRAINT p_key PRIMARY KEY (new_id); COMMIT;

Note: timings for multiple statements on a single line are not supported. See https://go.crdb.dev/issue-v/48180/v22.2.

demo@127.0.0.1:26257/movr> show create table stats;
  table_name |               create_statement
-------------+------------------------------------------------
  stats      | CREATE TABLE public.stats (
             |     id STRING NOT NULL,
             |     new_id STRING NOT NULL,
             |     CONSTRAINT p_key PRIMARY KEY (new_id ASC)
             | )

Expected behavior
The trick should work with any primary key names?

Environment:
This can be repro on v22.1, v22.2 and master

Jira issue: CRDB-20965

[ajwerner]

Can you explain what's special about the name primary in the code?

[chengxiong-ruan]

Can you explain what's special about the name primary in the code?

Sorry, my goland went crazy today...just had a chance to debug. It turned out that it's not about the name primary, it's actually about all sql keywords (double quoted). I updated the issue title and description.

[chengxiong-ruan]

Looks like an easy fix... just use string(name) instead of name.String() which escapes keywords at this line.

[theodore-hyman]

Hello lovely engineering folks! Customer is looking for a timeframe on the fix, if anyone can give any insight into what dot release the fix will land on, please let me know.

If you all need more time to investigate and work on it and you don't know right now, that's ok too, just checking in. Thank you

[ajwerner]

@chengxiong-ruan want to backport to 22.1?

[ajwerner]

It has been fixed on master and will be in 22.2.1 (note not 22.2.0). We'll get it into the next 22.1 point release

[chengxiong-ruan]

Oh, oops, I'll backport to 22.1.

[chengxiong-ruan]

@theodore-hyman I'll manage to merge the backport to 22.1 tomorrow, and the next patch release 22.1.11 will be published on Nov 14th given the release timeline.