[CALCITE-7585] SqlMerge unparse EXISTS predicates in ON clause without parentheses#5017
Conversation
|
@dssysolyatin I have added you are a reviewer since you raised this problem originally |
|
|
||
| /** | ||
| * Unparses a WHERE clause. | ||
| * Unparses a condition clause, such as WHERE or ON. |
There was a problem hiding this comment.
this deserves a bit more elaboration, since both WHERE and ON can be used in several places in a SQL statement.
There was a problem hiding this comment.
Thanks, I clarified the Javadoc to describe that the helper uses the existing WHERE_LIST frame for predicate-list formatting.
| public static void unparseWhereClause(SqlWriter writer, SqlNode where, | ||
| int leftPrec, int rightPrec) { | ||
| writer.sep("WHERE"); | ||
| public static void unparseConditionClause(SqlWriter writer, |
There was a problem hiding this comment.
I would suggest to drop the clauseKeyword parameter from this function and just write
writer.sep("ON");
SqlUtil.unparseConditionClause()
and
write.sep("WHERE")
SqlUtil.unparseConditionClause()
There was a problem hiding this comment.
Thanks, I removed the clauseKeyword parameter and the unparseWhereClause wrapper.
mihaibudiu
left a comment
mihaibudiu
left a comment
There was a problem hiding this comment.
Let's see if @dssysolyatin is happy too, but I think this looks good.
mihaibudiu
added
the
LGTM-will-merge-soon
dssysolyatin
left a comment
dssysolyatin
left a comment
There was a problem hiding this comment.
Looks good, thanks @zzwqqq
|
You already have two approvals, so you can squash your commits. |
zzwqqq
force-pushed
the
fix_unparse_on_sqlmerge
branch
from
d455572 to
f2d5e46
Compare
Thanks @xiedeyantu @mihaibudiu @dssysolyatin. I have updated the PR description and squashed the commits. |
|
I will merge once CI completes |
|
4 participants
Jira Link
CALCITE-7585
Changes Proposed
Fixes unparsing of
EXISTSpredicates in theONclause ofMERGE.SqlMergewas unparsing theONcondition directly. When the condition contains anEXISTSsubquery, this can produce invalid SQL such asON EXISTS SELECT ....This change adds
SqlUtil.unparseConditionClausefor unparsing conditions with the existingWHERE_LISTframe.SELECT,UPDATE,DELETE, andMERGEnow emit their clause keyword explicitly and then use the shared helper for the condition.Adds a regression test for
MERGE ... ON EXISTS (...).