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.
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
Avoid alias duplicates in PASTE JOIN and fix aliases requirement #58654
Avoid alias duplicates in PASTE JOIN and fix aliases requirement #58654
Conversation
This is an automatic comment. The PR descriptions does not match the template. Please, edit it accordingly. The error is: More than one changelog category specified: 'New Feature', 'Improvement' |
This is an automated comment for commit e28fd94 with description of existing statuses. It's updated for the latest CI running ❌ Click here to open a full report in a separate page Successful checks
|
@@ -2249,7 +2251,13 @@ void QueryAnalyzer::validateJoinTableExpressionWithoutAlias(const QueryTreeNodeP | |||
return; | |||
|
|||
auto table_expression_node_type = table_expression_node->getNodeType(); | |||
if (table_expression_node_type == QueryTreeNodeType::TABLE_FUNCTION || | |||
auto * join_typed = join_node->as<JoinNode>(); | |||
if (table_expression_node_type == QueryTreeNodeType::QUERY && join_typed->getKind() == JoinKind::Paste) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, I already forgot what table_expression_node
corresponds to. Why do we check only one node while checkDuplicateTableNamesOrAlias
expects left and right expression to be query ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We check only one node each time because we pass into validateJoinTableExpressionWithoutAlias
only one argument (either left or right), so we call this function two times for each of the values.
@@ -1451,6 +1451,7 @@ const char * ParserAlias::restricted_keywords[] = | |||
"ASOF", | |||
"BETWEEN", | |||
"CROSS", | |||
"PASTE", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Btw, I recently though about it and found out that it can be breaking change and we need to mention it properly in a changelog. Because paste
is parsed as a keyword instead of alias name. wee need to consider which queries may stop work if someone uses paste
as subquery name: (SELECT ... ) paste ...
, (SELECT ... ) AS paste ...
and so on, and explain it in a changelog. Maybe even something already changed when we introduced PASTE join, but for me it became clear only after we found this thing with restricted_keywords
Changelog category (leave one):
Changelog entry (a user-readable short description of the changes that goes to CHANGELOG.md):
Allow queries without aliases for subqueries for
PASTE JOIN
.The
PASTE
keyword is now reserved to improve the parsing ofPASTE JOIN
operations. This change ensures that the parser correctly identifiesPASTE JOIN
operations and prevents conflicts with identifiers namedpaste
. If your queries containpaste
as an identifier, you should update them to avoid syntax errors.For example, this query will be no longer executed:
You may want to update your queries that contains
paste
identifier as following:or quote the
paste
identifier:Documentation entry for user-facing changes