Skip to content

[CALCITE-7121] Allow to use hyphens in unquoted Table Names in dialects with backticks quoting #4487

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
snuyanzin wants to merge 2 commits into
base: main
Choose a base branch
from
Open

[CALCITE-7121] Allow to use hyphens in unquoted Table Names in dialects with backticks quoting #4487

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4f42fb7
[CALCITE-7121] Allow to use hyphens in unquoted Table Names in dialec…
snuyanzin Aug 2, 2025
d105292
Add tests similar to the ones for BigQuery
snuyanzin Aug 3, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions core/src/main/java/org/apache/calcite/sql/parser/SqlAbstractParserImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -619,9 +619,11 @@ public static LexicalState forConfig(SqlParser.Config config) {
return BQID;
case BACK_TICK:
if (config.conformance().allowHyphenInUnquotedTableName()
&& config.charLiteralStyles().equals(
&& (config.charLiteralStyles().equals(
EnumSet.of(CharLiteralStyle.BQ_SINGLE,
CharLiteralStyle.BQ_DOUBLE))) {
CharLiteralStyle.BQ_DOUBLE))
|| config.charLiteralStyles().equals(
EnumSet.of(CharLiteralStyle.STANDARD)))) {
Copy link
Copy Markdown
Contributor

@julianhyde julianhyde Aug 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With this change, any dialect whose allowHyphenInUnquotedTableName returns true will allow identifiers with back-ticks. I don't think anyone will expect that.

Copy link
Copy Markdown
Contributor Author

@snuyanzin snuyanzin Aug 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

then it is not clear what is the purpose of allowHyphenInUnquotedTableName returning true then?
Isn't this what is meant in javadoc

calcite/core/src/main/java/org/apache/calcite/sql/validate/SqlConformance.java

Lines 247 to 250 in 2fafa52

* Whether to allow hyphens in an unquoted table name.
*
* <p>If true, {@code SELECT * FROM foo-bar.baz-buzz} is valid, and is parsed
* as if the user had written {@code SELECT * FROM `foo-bar`.`baz-buzz`}.

Copy link
Copy Markdown
Contributor

@julianhyde julianhyde Aug 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Contributor Author

@snuyanzin snuyanzin Aug 5, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)

Copy link
Copy Markdown
Contributor

@julianhyde julianhyde Aug 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

return BQID;
}
if (!config.conformance().allowHyphenInUnquotedTableName()
Expand Down
19 changes: 19 additions & 0 deletions testkit/src/main/java/org/apache/calcite/sql/parser/SqlParserTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -987,6 +987,25 @@ private void checkLarge(int n) {
.fails("(?s)Encountered \"-\" at .*")
.withDialect(BIG_QUERY)
.fails("(?s)Encountered \"-\" at .*");

final SqlConformance nonBigQueryConformance = new SqlAbstractConformance() {
@Override public boolean allowHyphenInUnquotedTableName() {
return true;
}
};

sql("select * from foo-bar.baz cross join (select alpha-omega from t) as t")
.withDialect(MYSQL)
Copy link
Copy Markdown
Contributor Author

@snuyanzin snuyanzin Aug 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

mysql was picked here since it has backticks

Copy link
Copy Markdown
Contributor

@julianhyde julianhyde Aug 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test and the one following it look identical. Yet one is supposed to succeed and the other to fail. How does that work?

Copy link
Copy Markdown
Contributor Author

@snuyanzin snuyanzin Aug 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I updated tests to have same as for BigQuery

.withConformance(nonBigQueryConformance)
.ok("SELECT *\n"
+ "FROM `foo-bar`.`baz`\n"
+ "CROSS JOIN (SELECT (`alpha` - `omega`)\n"
+ "FROM `t`) AS `t`");

sql("select * from foo ^-^ bar.baz cross join (select alpha-omega from t) as t")
.withDialect(MYSQL)
.withConformance(nonBigQueryConformance)
.fails("(?s)Encountered \"-\" at .*");
}

@Test void testHyphenatedColumnName() {
Expand Down
Loading