Support '::' casts in autocomplete grammar #3
Conversation
jkillian
force-pushed
the
jk_support_casts
branch
from
e4555fa to
f1d2757
Compare
| it.skip('testing', () => { | ||
| assertLocations({ | ||
| beforeCursor: 'select boo::INT, bar::int from customers; ', | ||
| expectedLocations: [] | ||
| }); | ||
| }); |
There was a problem hiding this comment.
Just a nice test to quickly see what the parser is returning, obviously can be deleted before merging
jkillian
force-pushed
the
jk_support_casts
branch
from
24c8514 to
d2871cd
Compare
| }, | ||
| { | ||
| "namePrefix": "should suggest columns for order by after a cast in a join", | ||
| "beforeCursor": "select * from a join b on a.a = b.b::date order by ", |
There was a problem hiding this comment.
Added this test as a direct test of issues users were reporting, it failed before this PR and passes now which is great.
One downside still though, if your casted type doesn't match one of the below types defined in PrimitiveType, then things still don't work. I wish something like the below would still work correctly also, not sure the right way to accomplish it though::
select * from a join b on a.a = b.b::dte order by
| @@ -36,16 +36,20 @@ | |||
| 'ASC' { return 'ASC'; } | |||
| 'BETWEEN' { this.begin('between'); return 'BETWEEN'; } | |||
| 'BIGINT' { return 'BIGINT'; } | |||
| 'BINARY' { return 'BINARY'; } | |||
There was a problem hiding this comment.
Had to manually specify all the possible types here otherwise they weren't treated in a case-sensitive manner. (This file starts with %options case-insensitive flex so I think that's what makes it work if listed here)
| it.skip('useful for testing', () => { | ||
| assertLocations({ | ||
| beforeCursor: 'select * from a join b on a.a = b.b::date order by ', | ||
| expectedLocations: [] | ||
| }); | ||
| }); |
There was a problem hiding this comment.
Changing the skip to only is a really easy way to test out changes to the parser while you're working on things so going to leave this in as a utility if it isn't too irksome
| @@ -3927,6 +3927,70 @@ | |||
| "lowerCase": false | |||
| } | |||
| }, | |||
| { | |||
There was a problem hiding this comment.
Next three tests are similar to the ones above for CAST(foo AS type) expressions
| : ValueExpression '::' AnyCursor | ||
| { | ||
| parser.suggestKeywords(parser.getTypeKeywords()); | ||
| $$ = { types: [ 'T' ] }; | ||
| } |
There was a problem hiding this comment.
AnyCursor
: 'CURSOR'
| 'PARTIAL_CURSOR'
;
The above is the definition of AnyCursor, I'm not really sure when to use it vs regular 'CURSOR' vs 'PARTIAL_CURSOR'.
| | ValueExpression_EDIT '::' PrimitiveTypeOrError | ||
| { | ||
| parser.addColRefIfExists($1); | ||
| $$ = { types: [ $3.toUpperCase() ] } | ||
| } |
There was a problem hiding this comment.
Was hoping this clause would fix the issue with invalid types like my_col::dte, but it didn't 😢
Updates the autocomplete grammar so that it understands cast expressions of the type
some_col::type. This prevents autocomplete from occasionally breaking as reported by users when using the above syntax.Left some comments inline about the change to clarify the intentions.
cc @chrisvlopez