wangyum commented on code in PR #59258:
URL: https://github.com/apache/spark/pull/59258#discussion_r4236597845
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/QueryPlanConstraints.scala:
##########
@@ -126,6 +125,92 @@ trait ConstraintHelper {
inferred -- constraints
}
+ /**
+ * Infers the full additional constraint set for a given set of constraints
by combining the
+ * equality-based inferences above ([[inferAdditionalConstraints]],
+ * [[inferConstraintsFromLiteralBindings]]) with transitive inequality
chaining
+ * ([[inferInequalityConstraints]]). The inequality step is iterated to a
fixed point because a
+ * single pass only chains pairs present in the input set: e.g. `a < b && b
< c && c < 5`
+ * requires one pass to derive `b < 5` and a second to derive `a < 5` from
it.
+ */
+ def inferAllAdditionalConstraints(constraints: ExpressionSet): ExpressionSet
= {
+ @tailrec
+ def chainInequalities(inferred: ExpressionSet): ExpressionSet = {
+ val newlyInferred = inferInequalityConstraints(constraints ++ inferred)
+ if (newlyInferred.isEmpty) inferred else chainInequalities(inferred ++
newlyInferred)
+ }
+
+ chainInequalities(
+ inferAdditionalConstraints(constraints) ++
+ inferConstraintsFromLiteralBindings(constraints))
+ }
+
+ /**
+ * Infers an additional set of constraints from a given set of inequality
constraints.
+ * For e.g., if an operator has constraints of the form (`a > b`, `b > 5`),
this returns an
+ * additional constraint of the form `a > 5`.
+ */
+ private def inferInequalityConstraints(constraints: ExpressionSet):
ExpressionSet = {
+ // Substituting through a comparison is only safe under binary-stable
collations, for the
+ // same reason as in inferConstraintsFromLiteralBindings:
non-binary-stable collations may
+ // order values differently than their binary representation would
suggest. Attribute-to-
+ // attribute equality is excluded here too: its transitivity is already
handled by
+ // inferAdditionalConstraints via substitution, so chaining it here would
only re-derive the
+ // same fact as a redundant <= / >= pair instead of the clean substituted
equality that path
+ // already produces.
+ val validComparisons = constraints.filter {
+ case b: BinaryComparison =>
+ b.deterministic && !b.isInstanceOf[EqualNullSafe] &&
Review Comment:
good catch
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
