sql: currval('') produces bad error message

[jordanlewis]

[email protected]:62609/defaultdb> select currval('');
pq: currval(): syntax error at or near "to"
DETAIL: source SQL:
ALTER TABLE  RENAME TO x
                    ^
HINT: try \h ALTER TABLE

The problem here is insufficient checking of preconditions before string interpolation to create the alter table.

In fact, it seems that many of the commands like this one and the privilege checking builtins suffer from lack of string sanitization.

Epic CRDB-8948

[knz]

well spotted! thanks

[maddyblue]

This error has some trickyness to it too. This function calls our parser.ParseTableName function which takes a string, puts it into an ALTER TABLE in the parser to get our normal parsing rules to parse it. The above error is happening because we are doing simple sprintf-style string substitution, and thus the empty argument of curval gets inserted as a blank string.

Ok so first attempt at a fix is to use our double quote-style identifier thing. This however breaks the separator . as a name separator and correctly fails the tests. But since weirdo names are valid table/sequence names (single quote, double quote, period) as long as you double quote them, I think this leads to the conclusion that this function can not correctly support all possible sequence names until we have real OID support. That is, we cannot now differentiate between running currval on table t of database d and currval on table d.t since both would be currval('d.t').

[maddyblue]

I take it back: a user would have to correctly quote things in that case. currval('"d.t"')

[github-actions]

We have marked this issue as stale because it has been inactive for
18 months. If this issue is still relevant, removing the stale label
or adding a comment will keep it active. Otherwise, we'll close it in
5 days to keep the issue queue tidy. Thank you for your contribution
to CockroachDB!

[knz]

This bug still exists in v21.1

[otan]

We should do what Postgres does and make currval take in an oid instead of a string.

We should then use the same logic as ParseDOid to parse this into an appropriate stirng.