sql: remove last 2 blockers for pgcli support#40949
sql: remove last 2 blockers for pgcli support#40949craig[bot] merged 2 commits intocockroachdb:masterfrom
Conversation
Previously, syntax for the special oid wrapper types (like REGCLASS, REGTYPE, and so on) was not available, despite the backend being able to handle these types just fine. This commit adds syntax for this (e.g. `select t::regclass[]`) to the parser and hooks it up to the type system. Release note (sql change): support the syntax for oid wrapper arrays, like REGCLASS[]. Release justification: low risk, cosmetic compatibility improvement.
Release note: None Release justification: low-risk, cosmetic compatibility improvement
|
This change is |
rafiss
left a comment
There was a problem hiding this comment.
lgtm! i understood everything except for one of the parts you removed
Reviewed 3 of 3 files at r1.
Reviewable status:complete! 0 of 0 LGTMs obtained (waiting on @jordanlewis, @RaduBerinde, @rafiss, and @spaskob)
pkg/sql/parser/sql.y, line 6756 at r1 (raw file):
| postgres_oid { $$.val = $1.colType()
what was this doing? like assigning a variable as part of parsing somehow?
jordanlewis
left a comment
There was a problem hiding this comment.
Reviewable status:
pkg/sql/parser/sql.y, line 6756 at r1 (raw file):
Previously, rafiss (Rafi Shamim) wrote…
what was this doing? like assigning a variable as part of parsing somehow?
So this functionality is actually preserved via the movement that I did - it used to be that postgres_oid was a typename but not a simple_typename. All simple_typenames are typenames, so what this change does is move postgres_oid to be a simple_typename. This has the ancillary benefit of automatically adding the array forms to the grammar, which was the point of this change.
The rule here I think was actually unnecessary. The $$.val = blah thing is used to set the "result" of the parsing rule. I think it's unnecessary if you're just exactly using the output of the rule that's matching.
jordanlewis
left a comment
There was a problem hiding this comment.
TFTR! Tried to explain below. We can talk more about grammar stuff offline if you want to learn more about how it works.
bors r+
Reviewable status:
rafiss
left a comment
There was a problem hiding this comment.
Reviewable status:
pkg/sql/parser/sql.y, line 6756 at r1 (raw file):
Previously, jordanlewis (Jordan Lewis) wrote…
So this functionality is actually preserved via the movement that I did - it used to be that
postgres_oidwas atypenamebut not asimple_typename. Allsimple_typenamesaretypenames, so what this change does is movepostgres_oidto be asimple_typename. This has the ancillary benefit of automatically adding the array forms to the grammar, which was the point of this change.The rule here I think was actually unnecessary. The
$$.val = blahthing is used to set the "result" of the parsing rule. I think it's unnecessary if you're just exactly using the output of the rule that's matching.
thanks! i understood the typename / simple_typename rules part of the change
40949: sql: remove last 2 blockers for pgcli support r=jordanlewis a=jordanlewis ``` sql: add syntax for arrays of oid wrapper types Previously, syntax for the special oid wrapper types (like REGCLASS, REGTYPE, and so on) was not available, despite the backend being able to handle these types just fine. This commit adds syntax for this (e.g. `select t::regclass[]`) to the parser and hooks it up to the type system. ``` ``` tree: add support for REGTYPE casts of the trigger type ``` Release note (sql change): support the syntax for oid wrapper arrays, like REGCLASS[], and REGTYPE casts for the unsupported `trigger` type. Release justification: low risk, cosmetic compatibility improvement. Co-authored-by: Jordan Lewis <jordanthelewis@gmail.com>
Build succeeded |
|
excited about this! |
Release note (sql change): support the syntax for oid wrapper arrays, like REGCLASS[], and REGTYPE casts for the unsupported
triggertype.Release justification: low risk, cosmetic compatibility improvement.