[CALCITE-7121] Allow to use hyphens in unquoted Table Names in dialects with backticks quoting #4487
Conversation
…ts with backticks quoting
| }; | ||
|
|
||
| sql("select * from foo-bar.baz cross join (select alpha-omega from t) as t") | ||
| .withDialect(MYSQL) |
There was a problem hiding this comment.
mysql was picked here since it has backticks
There was a problem hiding this comment.
This test and the one following it look identical. Yet one is supposed to succeed and the other to fail. How does that work?
There was a problem hiding this comment.
I updated tests to have same as for BigQuery
| CharLiteralStyle.BQ_DOUBLE))) { | ||
| CharLiteralStyle.BQ_DOUBLE)) | ||
| || config.charLiteralStyles().equals( | ||
| EnumSet.of(CharLiteralStyle.STANDARD)))) { |
There was a problem hiding this comment.
With this change, any dialect whose allowHyphenInUnquotedTableName returns true will allow identifiers with back-ticks. I don't think anyone will expect that.
There was a problem hiding this comment.
then it is not clear what is the purpose of allowHyphenInUnquotedTableName returning true then?
Isn't this what is meant in javadoc
There was a problem hiding this comment.
Well, suppose someone has asked for Oracle-style quoted identifiers, "my table". If they also make allowHyphenInUnquotedTableName return true then they aren't going to get the parser behavior that they asked for.
There was a problem hiding this comment.
yes, same happens without this PR changes, and makes me thinking that javadoc doesn't reflect current behavior...
Initially (also put in jira description) I was thinking to make it working for backticks quoting only.
Do you think it would make sense to make it working for all (asking since so far I don't know other well known DB vendors supporting this)
There was a problem hiding this comment.
You should take the time to understand how it currently works. The implementation is subtle.
Lexical states are a very clever tool (I introduced them myself, to allow switching between quoting styles) but they are costly. Each lexical state creates a whole new copy of the parser's transition table and therefore makes the parser much larger.
Therefore we should only support the combinations that people actually need.
Relying on the sets of literal styles to deduce that we are dealing with BigQuery was a hack on my part. (See https://issues.apache.org/jira/browse/CALCITE-4247.) But that doesn't give you license to make the hack worse. In fact, if you want to change things, you have to figure out a way to make things better.
|
|
This pull request has been marked as stale due to 30 days of inactivity. It will be closed in 90 days if no further activity occurs. If you think that’s incorrect or this pull request requires a review, please simply write any comment. If closed, you can revive the PR at any time and @mention a reviewer or discuss it on the dev@calcite.apache.org list. Thank you for your contributions. |
|
This pull request has been marked as stale due to 30 days of inactivity. It will be closed in 90 days if no further activity occurs. If you think that’s incorrect or this pull request requires a review, please simply write any comment. If closed, you can revive the PR at any time and @mention a reviewer or discuss it on the dev@calcite.apache.org list. Thank you for your contributions. |
2 participants
The PR to allow usage of hyphens in unquoted table names for dialects with backticks (not only BigQuery)