feat: support byte string literal in bq#802
Conversation
Pull Request Test Coverage Report for Build 4214280739
💛 - Coveralls |
AugustoFKL
left a comment
There was a problem hiding this comment.
Left a comment.
One thing that bothers me is that you didn't test the 'b' literal. Please add some tests for it, if possible.
| } | ||
| Ok(Some(Token::Whitespace(Whitespace::Newline))) | ||
| } | ||
| // BigQuery uses b or B for byte string literal |
There was a problem hiding this comment.
This is something related to BigQuery, as I couldn't find this on MySQL. Maybe we could make this dialect-specific so we avoid permissive syntax on other dialects...?
There was a problem hiding this comment.
How about using dialect_of! like this.
b @ 'B' | b @ 'b' if dialect_of!(self is BigQueryDialect | GenericDialect) => { ..|
We can only choose between 'b' or 'B' when converting AST into string result for test. So, I didn't test it. Is there any good approach?? |
|
@togami2864 wym? I'd test it by using 'verified statement' usually... Considering the select, you put as an example: SELECT B'abc', B"abc"The resulting query is equals to the input? If so, use the |
alamb
left a comment
There was a problem hiding this comment.
This PR is looking good to me - thank you for the work @togami2864 and your help getting this ready for merge @AugustoFKL
It looks like CI is failing (though that may be resolved with a merge from main to get the clippy fixes). Is there anything else we are waiting on?
c01a6bb to
41e78a0
Compare
|
All tasks are done. There is nothing else we waiting on. |
alamb
left a comment
There was a problem hiding this comment.
Looks good to me -- thank you @togami2864 and @AugustoFKL
https://cloud.google.com/bigquery/docs/reference/standard-sql/lexical#string_and_bytes_literals