-
Notifications
You must be signed in to change notification settings - Fork 245
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
Correctly parse named columns for joins #240
Conversation
What is your take on the “no column references” limitation? |
First of all, it is in line withe the standard and pgSQL, which we try to mostly comply to. Second, it also makes sense IMO. |
Instead of parsing the list of columns as conjunctions of equality predicates and adding an extra boolean flag to struct JoinDefinition {
JoinDefinition();
virtual ~JoinDefinition();
TableRef* left;
TableRef* right;
Expr* condition; // set for `foo JOIN bar ON foo.x = bar.x`
std::vector<char*>* namedColumns; // set for `foo JOIN bar USING (x)`
// both unset for `foo NATURAL JOIN bar`
JoinType type;
}; This would be a bit more explicit and leaves resolving the column names to a predicate to the DBMS. However, it differs from the way we handled |
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.
Not much to say.
Feel free to zünd. |
This PR allows a list of identifiers (a.k.a column names) when joins name columns with
USING (...)
. This behavior is in line with the SQL standard and PostgreSQL. Furthermore, we add a flag if join conditions come from specifying them directly (foo JOIN bar ON (...)
) or from named columns (foo JOIN bar USING (...)
) so the database can adjust the emitted columns addording to the SQL standard (emit both columns for each predicate inON
, emit a single column for each column inUSING
).For details, see #239.
Fixes #239.