suibianwanwank commented on code in PR #3883:
URL: https://github.com/apache/calcite/pull/3883#discussion_r1716252471
##########
core/src/main/java/org/apache/calcite/sql/validate/SqlValidatorImpl.java:
##########
@@ -3774,11 +3780,136 @@ protected void validateJoin(SqlJoin join,
SqlValidatorScope scope) {
RESOURCE.crossJoinDisallowsCondition());
}
break;
+ case LEFT_ASOF:
+ case ASOF: {
+ // In addition to the standard join checks, the ASOF join requires the
+ // ON conditions to be a conjunction of simple equalities from both
relations.
+ SqlAsofJoin asof = (SqlAsofJoin) join;
+ SqlNode matchCondition = getMatchCondition(asof);
+ matchCondition = expand(matchCondition, joinScope);
+ join.setOperand(6, matchCondition);
+ validateWhereOrOn(joinScope, matchCondition, "MATCH_CONDITION");
+ SqlNode condition = join.getCondition();
+ if (condition == null) {
+ throw newValidationError(join, RESOURCE.joinRequiresCondition());
+ }
+ ConjunctionOfEqualities conj = new ConjunctionOfEqualities();
+ condition.accept(conj);
+ if (conj.illegal) {
+ throw newValidationError(condition,
RESOURCE.asofConditionMustBeComparison());
+ }
+
+ CompareFromBothSides validateCompare =
+ new CompareFromBothSides(joinScope,
+ catalogReader, RESOURCE.asofConditionMustBeComparison());
+ condition.accept(validateCompare);
+
+ // It also requires the MATCH condition to be a comparison.
+ if (!(matchCondition instanceof SqlCall)) {
+ throw newValidationError(matchCondition,
RESOURCE.asofMatchMustBeComparison());
+ }
+ SqlCall matchCall = (SqlCall) matchCondition;
+ SqlOperator operator = matchCall.getOperator();
+ if (!SqlKind.ORDER_COMPARISON.contains(operator.kind)) {
+ throw newValidationError(matchCondition,
RESOURCE.asofMatchMustBeComparison());
+ }
+
+ // Change the exception in validateCompare when we validate the match
condition
+ validateCompare =
+ new CompareFromBothSides(joinScope,
+ catalogReader, RESOURCE.asofMatchMustBeComparison());
+ matchCondition.accept(validateCompare);
+ break;
+ }
default:
throw Util.unexpected(joinType);
}
}
+ /**
+ * Shuttle which determines whether all SqlCalls that are
+ * comparisons are comparing columns from both namespaces.
+ * The shuttle will throw an exception if that happens.
+ * If it returns all SqlCalls have the expected shape.
+ */
+ private class CompareFromBothSides extends SqlShuttle {
+ final SqlValidatorScope scope;
+ final SqlValidatorCatalogReader catalogReader;
+ final Resources.ExInst<SqlValidatorException> exception;
+
+ private CompareFromBothSides(
+ SqlValidatorScope scope,
+ SqlValidatorCatalogReader catalogReader,
+ Resources.ExInst<SqlValidatorException> exception) {
+ this.scope = scope;
+ this.catalogReader = catalogReader;
+ this.exception = exception;
+ }
+
+ @Override public @Nullable SqlNode visit(final SqlCall call) {
+ SqlKind kind = call.getKind();
+ if (SqlKind.COMPARISON.contains(kind)) {
+ assert call.getOperandList().size() == 2;
+
+ boolean leftFound = false;
+ boolean rightFound = false;
+ // The two sides of the comparison must be from different tables
+ for (SqlNode operand : call.getOperandList()) {
+ if (!(operand instanceof SqlIdentifier)) {
+ throw newValidationError(call, this.exception);
+ }
+ // We know that all identifiers have been expanded by the caller,
+ // so they have the shape namespace.field
+ SqlIdentifier id = (SqlIdentifier) operand;
+ final SqlNameMatcher nameMatcher = catalogReader.nameMatcher();
+ final SqlValidatorScope.ResolvedImpl resolved = new
SqlValidatorScope.ResolvedImpl();
+ // Lookup just the first component of the name
+ scope.resolve(id.names.subList(0, id.names.size() - 1), nameMatcher,
false, resolved);
+ SqlValidatorScope.Resolve resolve = resolved.only();
+ int index = resolve.path.steps().get(0).i;
+ if (index == 0) {
+ leftFound = true;
+ }
+ if (index == 1) {
+ rightFound = true;
+ }
+
+ if (!leftFound && !rightFound) {
+ throw newValidationError(call, this.exception);
+ }
+ }
+ if (!leftFound || !rightFound) {
+ // The comparison does not look at both tables
+ throw newValidationError(call, this.exception);
+ }
+ }
+ return super.visit(call);
+ }
+ }
+
+ /**
+ * Shuttle which determines whether an expression is a simple conjunction
+ * of equalities. */
+ private static class ConjunctionOfEqualities extends SqlShuttle {
+ boolean illegal = false;
+
+ @Override public @Nullable SqlNode visit(final
org.apache.calcite.sql.SqlCall call) {
+ SqlKind kind = call.getKind();
+ if (kind != SqlKind.AND && kind != SqlKind.EQUALS) {
+ illegal = true;
+ }
+ if (kind == SqlKind.AND) {
+ List<SqlNode> operands = call.getOperandList();
+ for (SqlNode operand : operands) {
Review Comment:
It seems that there might still be an issue here. If the condition is a.id =
b.id AND a.id2 = b.id2 AND a.id3 = b.id3, this might not pass as expected
because the conditions in SqlNode are not in the same operandList.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
