parser: add test to check that bare_label_keywords is updated #97878
Conversation
|
It looks like your PR touches SQL parser code but doesn't add or edit parser tests. Please make sure you add or edit parser tests if you edit the parser. 🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf. |
|
This change is |
rafiss
force-pushed
the
test-column-bare-labels
branch
from
b510033 to
22891cf
Compare
| // Any new keyword should be added to this list. | ||
| bare_label_keywords: | ||
| AS_JSON | ||
| ABORT |
There was a problem hiding this comment.
It make sense that we want to match psql's grammar, but I don't think we have to, because at this moment user won't have valid SELECT 1 xxx queries against these keywords. I guess one counter point to my opinion is that users upgrading from much older version might run into problems because some keywords added before we have this list heh.
That's being said I'm ok with this.
There was a problem hiding this comment.
i think it's still worth it to match PG's grammar as close as we can. the reason is that we have potential future users who will be migrating their apps from PG to CRDB. for all these users, we want to make sure that queries that worked with PG also work with CRDB, to minimize the friction of migrating to CRDB.
Previously, it was very easy to add a new keyword, but forget to add it to bare_label_keywords. This could cause regressions as queries of the form `SELECT 1 new_label` would fail. The new test is a guardrail against that, and the grammar was updated to match Postgres bare label keywords as best we can. Release note: None
rafiss
force-pushed
the
test-column-bare-labels
branch
from
22891cf to
3c5c5da
Compare
|
tftr! bors r=chengxiong-ruan |
|
Build succeeded: |
fixes #84309
Previously, it was very easy to add a new keyword, but forget to add it to bare_label_keywords. This could cause regressions as queries of the form
SELECT 1 new_labelwould fail. The new test is a guardrail against that, and the grammar was updated to match Postgres bare label keywords as best we can.Release note: None