This change is 

All committers have signed the CLA.

Hi @ctkou! Thank you for your help and your interest in this.

Also your code looks very good. Thank you for this.

Since you now understand this code well, I have a pleading request -- could you take this opportunity to help us and remove an annoying thorn in our code?

1. the thorn: the Postgres name for the session variable is "timezone" without a space. It's bad for compatibility that CockroachDB names this "time zone" with a space. Hence my request: instead of piling up more code around this compatibility mismatch, can you modify sql/vars.go and rename the variable?

2. then modify sql.y so that SET TIME ZONE produces a SetVar on the "timezone" variable, not "time zone". Then modify the SET TIME ZONE tests in parse_test.go accordingly and all the logic tests that print out the list of variable names (I think there are some in pkg/sql/logictest/testdata/logic_test/pg_catalog, information_schema and perhaps elsewhere).

3. then you can remove the new rule for SET TIMEZONE that your patch was originally added, because it will now be handled automatically by the default code path.

4. you then don't need new tests for SET TIMEZONE in parse_test.go, however I would encourage you to add a few SET TIMEZONE tests in pkg/sql/logictest/testdata/logic_test/datetime to check that the behavior is indeed equivalent.

Reviewed 5 of 5 files at r1.
Review status: all files reviewed at latest revision, all discussions resolved, all commit checks successful.

Comments from Reviewable

@knz

Okay, thanks for reviewing my code. I will get that "thorn" out like you said. :)

Hi @knz ,

I am not familiar with parsing, but it seems to me that for Set, the TIMEZONE token is still needed after changing 'time zone' to 'timezone'. For generic_set, var_name and var_list are separated by either TO or '=', so "SET TIMEZONE = 'UTC'" and "SET TIMEZONE TO 'UTC'" works but "SET TIMEZONE 'UTC'" doesn't unless a TIMEZONE token is defined.

You are correct that supporting SET TIMEZONE 'UTC' would require a separate token and grammar rule. However, this syntax is not needed to solve #19440, and I believe is not supported by Postgres (at least I do not see it here: https://www.postgresql.org/docs/current/static/sql-set.html and psql returns an error if I use it)

This patch is very good. Thank you.

Can you please squash the two commits together before we can merge this? Thanks!

Reviewed 5 of 5 files at r2, 9 of 9 files at r3.
Review status: all files reviewed at latest revision, all discussions resolved, all commit checks successful.

Comments from Reviewable

Reviewed 3 of 3 files at r4.
Review status: all files reviewed at latest revision, all discussions resolved, all commit checks successful.

Comments from Reviewable

Thank you again!