github-actions[bot] commented on code in PR #69461:
URL: https://github.com/apache/doris/pull/69461#discussion_r4355706224
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/PushDownScoreTopNIntoOlapScan.java:
##########
@@ -295,20 +298,79 @@ private Optional<ScoreRangeInfo>
extractScoreRangeInfo(Expression predicate) {
}
}
+ // A cast of score() changes the comparison semantics: CAST(score() AS
DECIMALV3(p,s))
+ // rounds before comparing. Convert the rounded threshold back to an
equivalent raw
+ // score threshold instead of dropping the cast, which would drop rows
whose raw score
+ // rounds up to the requested value (DORIS-28583).
+ if (left instanceof Cast && isNumericLiteral(right)) {
+ Optional<ScoreRangeInfo> adjusted =
+ extractCastScoreRangeInfo(comp, (Cast) left, right, false);
+ if (adjusted.isPresent()) {
+ return adjusted;
+ }
+ }
+
+ if (right instanceof Cast && isNumericLiteral(left)) {
+ Optional<ScoreRangeInfo> adjusted =
+ extractCastScoreRangeInfo(comp, (Cast) right, left, true);
+ if (adjusted.isPresent()) {
+ return adjusted;
+ }
+ }
+
return Optional.empty();
}
/**
- * Check if the expression is a Score function, possibly wrapped in Cast
expressions.
- * The optimizer may wrap score() in Cast for type coercion (e.g., score()
>= 4.0 may become
- * CAST(score() AS DECIMAL) >= 4.0).
+ * Convert a rounded score comparison, such as
+ * CAST(score() AS DECIMALV3(2,1)) >= 0.5, into an equivalent raw score
range.
+ *
+ * The cast rounds the score half away from zero, so the rounded value
reaches the threshold
+ * for raw scores up to half of the last kept digit below it. Dropping the
cast but keeping
+ * the literal threshold compares the raw score instead and loses those
rows.
+ */
+ private Optional<ScoreRangeInfo>
extractCastScoreRangeInfo(ComparisonPredicate comp, Cast cast,
+ Expression literal, boolean reversed) {
+ if (!isScoreExpression(cast.child())) {
+ return Optional.empty();
+ }
+ int scale;
+ if (cast.getDataType() instanceof DecimalV3Type) {
+ scale = ((DecimalV3Type) cast.getDataType()).getScale();
+ } else if (cast.getDataType().isIntegerLikeType()) {
Review Comment:
[P1] Use truncation semantics for integer score casts
Doris FLOAT-to-integer casts truncate (`CastToInt::from_float` uses
`std::trunc`), so they cannot use the decimal half-step inverse. A concrete
reduced input plan is:
```text
TopN(s DESC, 10)
Project(id, score() AS s)
Filter(title MATCH_ANY 'apple' AND CAST(score() AS INT) >= 1)
Scan(t)
```
With equal-length rows `(1,'apple'), (2,'banana')`, the matching score is
about 0.693 and its INT cast is 0. This branch generates raw `score >= 0.5`,
and `pushDown` deletes the original cast predicate, so row 1 is now returned
incorrectly. Previously raw `score >= 1` rejected it. TRY_CAST and the reversed
comparison are affected too. Derive integer bounds from truncation separately
from decimal rounding, or retain the exact predicate after materializing score;
add integer-cast oracle coverage.
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/PushDownScoreTopNIntoOlapScan.java:
##########
@@ -295,20 +298,79 @@ private Optional<ScoreRangeInfo>
extractScoreRangeInfo(Expression predicate) {
}
}
+ // A cast of score() changes the comparison semantics: CAST(score() AS
DECIMALV3(p,s))
+ // rounds before comparing. Convert the rounded threshold back to an
equivalent raw
+ // score threshold instead of dropping the cast, which would drop rows
whose raw score
+ // rounds up to the requested value (DORIS-28583).
+ if (left instanceof Cast && isNumericLiteral(right)) {
+ Optional<ScoreRangeInfo> adjusted =
+ extractCastScoreRangeInfo(comp, (Cast) left, right, false);
+ if (adjusted.isPresent()) {
+ return adjusted;
+ }
+ }
+
+ if (right instanceof Cast && isNumericLiteral(left)) {
+ Optional<ScoreRangeInfo> adjusted =
+ extractCastScoreRangeInfo(comp, (Cast) right, left, true);
+ if (adjusted.isPresent()) {
+ return adjusted;
+ }
+ }
+
return Optional.empty();
}
/**
- * Check if the expression is a Score function, possibly wrapped in Cast
expressions.
- * The optimizer may wrap score() in Cast for type coercion (e.g., score()
>= 4.0 may become
- * CAST(score() AS DECIMAL) >= 4.0).
+ * Convert a rounded score comparison, such as
+ * CAST(score() AS DECIMALV3(2,1)) >= 0.5, into an equivalent raw score
range.
+ *
+ * The cast rounds the score half away from zero, so the rounded value
reaches the threshold
+ * for raw scores up to half of the last kept digit below it. Dropping the
cast but keeping
+ * the literal threshold compares the raw score instead and loses those
rows.
+ */
+ private Optional<ScoreRangeInfo>
extractCastScoreRangeInfo(ComparisonPredicate comp, Cast cast,
+ Expression literal, boolean reversed) {
+ if (!isScoreExpression(cast.child())) {
+ return Optional.empty();
+ }
+ int scale;
+ if (cast.getDataType() instanceof DecimalV3Type) {
+ scale = ((DecimalV3Type) cast.getDataType()).getScale();
+ } else if (cast.getDataType().isIntegerLikeType()) {
+ scale = 0;
+ } else {
+ return Optional.empty();
+ }
+ TExprOpcode op = reversed ? getReversedMinScoreOpcode(comp) :
getMinScoreOpcode(comp);
+ if (op == null) {
+ return Optional.empty();
+ }
+ double halfStep = 0.5 * Math.pow(10.0, -scale);
+ double threshold = extractNumericValue(literal);
+ // round_s(score) >= t <=> score >= t - halfStep
+ // round_s(score) > t <=> score >= t + halfStep
+ double rawThreshold = op == TExprOpcode.GE ? threshold - halfStep :
threshold + halfStep;
+ return Optional.of(new ScoreRangeInfo(TExprOpcode.GE, rawThreshold));
+ }
+
+ /**
+ * Check if the expression is a Score function, allowing only a value
preserving
+ * float -> double widening cast (which the analyzer may insert for type
coercion).
+ * Rounding casts are handled by extractCastScoreRangeInfo.
*/
private boolean isScoreExpression(Expression expr) {
if (expr instanceof Score) {
return true;
}
if (expr instanceof Cast) {
- return isScoreExpression(((Cast) expr).child());
+ Cast cast = (Cast) expr;
+ // Only the value preserving float -> double widening may be
dropped. Casts such as
+ // CAST(score() AS DECIMALV3(2,1)) round the score, so using the
raw score with the
+ // rounded threshold is not equivalent to the original predicate.
+ return cast.child().getDataType() instanceof FloatType
Review Comment:
[P2] Keep coerced numeric cast predicates evaluable
Comparing the decimal-cast score with a DOUBLE literal adds another cast
during analysis:
```text
TopN(s DESC, 10)
Project(id, score() AS s)
Filter(title MATCH_ANY 'apple' AND
CAST(CAST(score() AS DECIMALV3(2,1)) AS DOUBLE) >= DOUBLE(0.5))
Scan(t)
```
For example, the SQL predicate `CAST(score() AS DECIMALV3(2,1)) >= CAST(0.5
AS DOUBLE)` produces this tree in both comparison-coercion modes. The wrapper
survives simplification, but the new recognizer rejects its DECIMAL child,
`extractCastScoreRangeInfo` also returns empty, and `pushDown` throws the
score-range usage error. This previously accepted query now fails instead of
filtering rounded scores. The narrowed target set also rejects direct LARGEINT
casts because `isIntegerLikeType()` excludes LARGEINT. Recognize supported
coercion chains while preserving the inner cast semantics, or evaluate the
original predicate above materialized score instead of failing analysis.
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/PushDownScoreTopNIntoOlapScan.java:
##########
@@ -295,20 +298,79 @@ private Optional<ScoreRangeInfo>
extractScoreRangeInfo(Expression predicate) {
}
}
+ // A cast of score() changes the comparison semantics: CAST(score() AS
DECIMALV3(p,s))
+ // rounds before comparing. Convert the rounded threshold back to an
equivalent raw
+ // score threshold instead of dropping the cast, which would drop rows
whose raw score
+ // rounds up to the requested value (DORIS-28583).
+ if (left instanceof Cast && isNumericLiteral(right)) {
+ Optional<ScoreRangeInfo> adjusted =
+ extractCastScoreRangeInfo(comp, (Cast) left, right, false);
+ if (adjusted.isPresent()) {
+ return adjusted;
+ }
+ }
+
+ if (right instanceof Cast && isNumericLiteral(left)) {
+ Optional<ScoreRangeInfo> adjusted =
+ extractCastScoreRangeInfo(comp, (Cast) right, left, true);
+ if (adjusted.isPresent()) {
+ return adjusted;
+ }
+ }
+
return Optional.empty();
}
/**
- * Check if the expression is a Score function, possibly wrapped in Cast
expressions.
- * The optimizer may wrap score() in Cast for type coercion (e.g., score()
>= 4.0 may become
- * CAST(score() AS DECIMAL) >= 4.0).
+ * Convert a rounded score comparison, such as
+ * CAST(score() AS DECIMALV3(2,1)) >= 0.5, into an equivalent raw score
range.
+ *
+ * The cast rounds the score half away from zero, so the rounded value
reaches the threshold
+ * for raw scores up to half of the last kept digit below it. Dropping the
cast but keeping
+ * the literal threshold compares the raw score instead and loses those
rows.
+ */
+ private Optional<ScoreRangeInfo>
extractCastScoreRangeInfo(ComparisonPredicate comp, Cast cast,
+ Expression literal, boolean reversed) {
+ if (!isScoreExpression(cast.child())) {
+ return Optional.empty();
+ }
+ int scale;
+ if (cast.getDataType() instanceof DecimalV3Type) {
+ scale = ((DecimalV3Type) cast.getDataType()).getScale();
+ } else if (cast.getDataType().isIntegerLikeType()) {
+ scale = 0;
+ } else {
+ return Optional.empty();
+ }
+ TExprOpcode op = reversed ? getReversedMinScoreOpcode(comp) :
getMinScoreOpcode(comp);
+ if (op == null) {
+ return Optional.empty();
+ }
+ double halfStep = 0.5 * Math.pow(10.0, -scale);
+ double threshold = extractNumericValue(literal);
+ // round_s(score) >= t <=> score >= t - halfStep
+ // round_s(score) > t <=> score >= t + halfStep
+ double rawThreshold = op == TExprOpcode.GE ? threshold - halfStep :
threshold + halfStep;
Review Comment:
[P1] Preserve strictness when the decimal half-step rounds away
For `CAST(score() AS DECIMALV3(21,19)) > 1.0000000000000000000`, `halfStep`
is `5e-20`; double addition makes `rawThreshold` exactly 1.0, and the
unconditional GE admits equality. This is reachable with the existing
indexed-table shape:
```text
TopN(s DESC, 100)
Project(id, score() AS s)
Filter(SEARCH('*', '{"default_field":"title","mode":"lucene"}') AND
CAST(score() AS DECIMALV3(21,19)) > 1.0000000000000000000)
Scan(t)
```
SEARCH match-all uses `AllScorer`, which returns exactly `1.0F`. Its decimal
cast equals 1, so the input predicate rejects every row; the rewritten scan
uses GE 1 and returns them after the cast predicate is removed. The previous GT
1 rejected them correctly. Derive the bound in the actual score domain with
strictness preserved, or keep the exact residual predicate when a double bound
cannot encode it. Cover high-scale > and reversed < at equality.
--
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]
