-
Notifications
You must be signed in to change notification settings - Fork 3.1k
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
Add support for CORRESPONDING option in set operations #25260
base: master
Are you sure you want to change the base?
Conversation
3bb218f
to
46271ec
Compare
core/trino-main/src/main/java/io/trino/sql/analyzer/StatementAnalyzer.java
Outdated
Show resolved
Hide resolved
core/trino-main/src/main/java/io/trino/sql/planner/RelationPlanner.java
Outdated
Show resolved
Hide resolved
46271ec
to
2e81b33
Compare
Changing to draft as I missed logic to handle different number of columns between left and right relations. |
@@ -28,9 +28,9 @@ public class Except | |||
private final Relation left; | |||
private final Relation right; | |||
|
|||
public Except(NodeLocation location, Relation left, Relation right, boolean distinct) | |||
public Except(NodeLocation location, Relation left, Relation right, boolean distinct, boolean corresponding) |
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.
Using a boolean
is not going to be sufficient for modeling the list of columns in the CORRESPONDING clause. It will require a bigger change later.
Add support for passing in the list of columns in the grammar and AST, even if, for the first version, the analyzer rejects any queries where the list is provided.
RelationPlan plan = process(child, null); | ||
|
||
if (node.isCorresponding() && child.equals(rightRelation)) { |
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.
The planner should not be doing any mappings based on names. That should have been established during analysis, and any necessary supporting data structure recorded in the Analysis object. E.g., a list of fields to select in the order in which they should be considered + the corresponding relation types.
/** | ||
* Gets the field at the specified name. | ||
*/ | ||
public Optional<Field> getFieldByName(String name) |
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.
How does this handle standard SQL identifier canonicalization and matching?
Description
Add support for CORRESPONDING option in set operations.
The option matches columns by name instead of by position:
The option is defined in SQL spec
Release notes