This is an automated email from the ASF dual-hosted git repository.
yiguolei pushed a commit to branch branch-4.2
in repository https://gitbox.apache.org/repos/asf/doris.git
The following commit(s) were added to refs/heads/branch-4.2 by this push:
new 301d2914708 branch-4.2: [fix](analyzer) Decide a GROUP BY / ORDER BY
ordinal on the value the user wrote #68096 (#70047)
301d2914708 is described below
commit 301d2914708ddf369f69a02b378a4d05e34540ae
Author: morrySnow <[email protected]>
AuthorDate: Wed Nov 4 08:08:59 2026 +0800
branch-4.2: [fix](analyzer) Decide a GROUP BY / ORDER BY ordinal on the
value the user wrote #68096 (#70047)
picked from master #68096
Co-authored-by: Ambuj Upadhyay <[email protected]>
---
.../doris/nereids/parser/LogicalPlanBuilder.java | 10 ++--
.../nereids/rules/analysis/BindExpression.java | 15 ++++--
.../nereids/parser/LogicalPlanBuilderTest.java | 24 ++++++++++
.../nereids/rules/analysis/BindExpressionTest.java | 53 ++++++++++++++++++++++
4 files changed, 96 insertions(+), 6 deletions(-)
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/nereids/parser/LogicalPlanBuilder.java
b/fe/fe-core/src/main/java/org/apache/doris/nereids/parser/LogicalPlanBuilder.java
index b9932c79f10..58741bb3ffe 100644
---
a/fe/fe-core/src/main/java/org/apache/doris/nereids/parser/LogicalPlanBuilder.java
+++
b/fe/fe-core/src/main/java/org/apache/doris/nereids/parser/LogicalPlanBuilder.java
@@ -4553,9 +4553,13 @@ public class LogicalPlanBuilder extends
DorisParserBaseVisitor<Object> {
List<OrderKey> newOrderKeys = sort.getOrderKeys().stream()
.map(key -> {
if (key.getExpr() instanceof IntegerLikeLiteral) {
- return key.withExpression(
- new UnboundSlot(String.valueOf(
- ((IntegerLikeLiteral)
key.getExpr()).getIntValue())));
+ // The slot is named after the literal, so
narrowing it renames it:
+ // getIntValue() truncates to the low 32 bits,
turning
+ // `order by 4294967297` into a reference to "1".
Keep the digits
+ // the user wrote.
+ return key.withExpression(new UnboundSlot(
+ ((IntegerLikeLiteral) key.getExpr())
+
.getBigDecimalValue().toBigIntegerExact().toString()));
}
return key;
})
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/analysis/BindExpression.java
b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/analysis/BindExpression.java
index b86fabbd9c1..fcad7c8bdca 100644
---
a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/analysis/BindExpression.java
+++
b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/analysis/BindExpression.java
@@ -139,6 +139,7 @@ import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
import org.jetbrains.annotations.NotNull;
+import java.math.BigDecimal;
import java.util.ArrayList;
import java.util.Collection;
import java.util.HashMap;
@@ -2026,9 +2027,17 @@ public class BindExpression implements
AnalysisRuleFactory {
private Expression bindWithOrdinal(
Expression unbound, SimpleExprAnalyzer analyzer, List<? extends
Expression> boundSelectOutput) {
if (unbound instanceof IntegerLikeLiteral) {
- int ordinal = ((IntegerLikeLiteral) unbound).getIntValue();
- if (ordinal >= 1 && ordinal <= boundSelectOutput.size()) {
- Expression boundSelectItem = boundSelectOutput.get(ordinal -
1);
+ // Decide positional-vs-constant on the value the user actually
wrote.
+ // getIntValue() is getNumber().intValue(), which truncates a
BIGINT or a
+ // LARGEINT to its low 32 bits, so an ordinal congruent to a valid
one
+ // modulo 2^32 passed this test and bound to a real select item:
+ // `GROUP BY 4294967297` silently became `GROUP BY <select item
1>`.
+ // LargeIntLiteral overrides getBigDecimalValue but not
getLongValue, so
+ // the BigDecimal form is the only one that is width independent.
+ BigDecimal ordinal = ((IntegerLikeLiteral)
unbound).getBigDecimalValue();
+ if (ordinal.compareTo(BigDecimal.ONE) >= 0
+ &&
ordinal.compareTo(BigDecimal.valueOf(boundSelectOutput.size())) <= 0) {
+ Expression boundSelectItem =
boundSelectOutput.get(ordinal.intValue() - 1);
return boundSelectItem instanceof Alias ?
boundSelectItem.child(0) : boundSelectItem;
} else {
return unbound; // bound literal
diff --git
a/fe/fe-core/src/test/java/org/apache/doris/nereids/parser/LogicalPlanBuilderTest.java
b/fe/fe-core/src/test/java/org/apache/doris/nereids/parser/LogicalPlanBuilderTest.java
index 9690e24ea85..59c413cf82c 100644
---
a/fe/fe-core/src/test/java/org/apache/doris/nereids/parser/LogicalPlanBuilderTest.java
+++
b/fe/fe-core/src/test/java/org/apache/doris/nereids/parser/LogicalPlanBuilderTest.java
@@ -18,6 +18,7 @@
package org.apache.doris.nereids.parser;
import org.apache.doris.nereids.analyzer.UnboundSlot;
+import org.apache.doris.nereids.trees.expressions.Expression;
import org.apache.doris.nereids.trees.expressions.literal.IntegerLikeLiteral;
import org.apache.doris.nereids.trees.plans.commands.DeleteFromCommand;
import org.apache.doris.nereids.trees.plans.commands.DeleteFromUsingCommand;
@@ -248,4 +249,27 @@ public class LogicalPlanBuilderTest {
// Second key: column name remains as UnboundSlot
Assertions.assertInstanceOf(UnboundSlot.class,
sort.getOrderKeys().get(1).getExpr());
}
+
+ @Test
+ public void testDeleteAndUpdateOrderByWideOrdinalKeepsItsDigits() {
+ // The ordinal becomes an UnboundSlot named after the literal, so
narrowing the literal
+ // renames the slot. 4294967297 is 2^32 + 1, which getIntValue()
turned into "1".
+ // 18446744073709551617 is 2^64 + 1, a LARGEINT, which getLongValue()
also narrows to 1,
+ // so it pins the width-independent accessor rather than merely a
wider one.
+ for (String ordinal : new String[] {"4294967297",
"18446744073709551617"}) {
+ assertFirstOrderKeyIsSlotNamed("DELETE FROM t ORDER BY " + ordinal
+ " LIMIT 10", ordinal);
+ assertFirstOrderKeyIsSlotNamed("UPDATE t SET c1 = 10 ORDER BY " +
ordinal + " LIMIT 10", ordinal);
+ }
+ }
+
+ private void assertFirstOrderKeyIsSlotNamed(String sql, String
expectedName) {
+ LogicalPlan plan = parser.parseSingle(sql);
+ LogicalPlan query = plan instanceof UpdateCommand
+ ? ((UpdateCommand) plan).getLogicalQuery()
+ : ((DeleteFromUsingCommand) plan).getLogicalQuery();
+ LogicalSort<?> sort = (LogicalSort<?>) ((LogicalLimit<?>)
query).child();
+ Expression key = sort.getOrderKeys().get(0).getExpr();
+ Assertions.assertInstanceOf(UnboundSlot.class, key, sql);
+ Assertions.assertEquals(expectedName, ((UnboundSlot) key).getName(),
sql);
+ }
}
diff --git
a/fe/fe-core/src/test/java/org/apache/doris/nereids/rules/analysis/BindExpressionTest.java
b/fe/fe-core/src/test/java/org/apache/doris/nereids/rules/analysis/BindExpressionTest.java
index 10360ce3128..00cbfb2232e 100644
---
a/fe/fe-core/src/test/java/org/apache/doris/nereids/rules/analysis/BindExpressionTest.java
+++
b/fe/fe-core/src/test/java/org/apache/doris/nereids/rules/analysis/BindExpressionTest.java
@@ -19,6 +19,7 @@ package org.apache.doris.nereids.rules.analysis;
import org.apache.doris.catalog.Column;
import org.apache.doris.catalog.PrimitiveType;
+import org.apache.doris.common.NereidsException;
import org.apache.doris.datasource.iceberg.IcebergMergeOperation;
import org.apache.doris.datasource.iceberg.IcebergUtils;
import org.apache.doris.nereids.exceptions.AnalysisException;
@@ -27,10 +28,12 @@ import org.apache.doris.nereids.rules.RulePromise;
import org.apache.doris.nereids.trees.expressions.Alias;
import org.apache.doris.nereids.trees.expressions.Expression;
import org.apache.doris.nereids.trees.expressions.NamedExpression;
+import org.apache.doris.nereids.trees.expressions.Slot;
import org.apache.doris.nereids.trees.expressions.SlotReference;
import org.apache.doris.nereids.trees.expressions.literal.IntegerLiteral;
import org.apache.doris.nereids.trees.expressions.literal.TinyIntLiteral;
import org.apache.doris.nereids.trees.plans.JoinType;
+import org.apache.doris.nereids.trees.plans.logical.LogicalSort;
import org.apache.doris.nereids.types.SmallIntType;
import org.apache.doris.nereids.types.StringType;
import org.apache.doris.nereids.types.VariantType;
@@ -57,6 +60,56 @@ class BindExpressionTest extends TestWithFeService
implements GeneratedPlanPatte
);
}
+ @Test
+ void testGroupByOrdinalIsNotNarrowed() {
+ // getIntValue() is getNumber().intValue(), so a BIGINT or LARGEINT
ordinal was truncated
+ // to its low 32 bits before the `>= 1 && <= selectItems` test.
4294967297 is 2^32 + 1 and
+ // 18446744073709551617 is 2^64 + 1: both truncated to 1 and silently
bound to the first
+ // select item, while the plainly out-of-range 3 did not. The LARGEINT
narrows to 1 through
+ // getLongValue() as well, so it pins the width-independent accessor,
not merely a wider one.
+ String outOfRange = "select col1, count(*) from t1 group by 3";
+
+ // As a constant, the literal leaves col1 ungrouped.
checkPlannerResult surfaces that as
+ // NereidsException, and every out-of-range ordinal must fail for that
same reason.
+ NereidsException expected =
Assertions.assertThrows(NereidsException.class,
+ () ->
PlanChecker.from(connectContext).checkPlannerResult(outOfRange),
+ "an ordinal past the end of the select list must not bind");
+ for (String ordinal : new String[] {"4294967297",
"18446744073709551617"}) {
+ String wrapsToOne = "select col1, count(*) from t1 group by " +
ordinal;
+ NereidsException actual =
Assertions.assertThrows(NereidsException.class,
+ () ->
PlanChecker.from(connectContext).checkPlannerResult(wrapsToOne),
+ ordinal + " must not be narrowed into range");
+ Assertions.assertEquals(expected.getMessage(), actual.getMessage(),
+ ordinal + " must be rejected for the same reason as the
out-of-range ordinal");
+ }
+ }
+
+ @Test
+ void testOrderByOrdinalIsNotNarrowed() {
+ // bindWithOrdinal also serves ORDER BY, on a plain select and on a
set operation. There an
+ // out-of-range ordinal does not throw -- it stays a constant sort key
-- so the check is on
+ // the analyzed plan: a wide ordinal must never turn into a sort on a
column.
+ String[] queries = {
+ "select col1, col2 from t1 order by %s",
+ "select col1, col2 from t1 union all select col1, col2 from t2
order by %s",
+ };
+ for (String query : queries) {
+ // The check can tell the two apart: a real ordinal does sort on a
column.
+ PlanChecker.from(connectContext)
+ .analyze(String.format(query, "1"))
+
.matches(logicalSort().when(BindExpressionTest::sortsOnASlot));
+ for (String ordinal : new String[] {"4294967297",
"18446744073709551617"}) {
+ PlanChecker.from(connectContext)
+ .analyze(String.format(query, ordinal))
+
.nonMatch(logicalSort().when(BindExpressionTest::sortsOnASlot));
+ }
+ }
+ }
+
+ private static boolean sortsOnASlot(LogicalSort<?> sort) {
+ return sort.getOrderKeys().stream().anyMatch(key -> key.getExpr()
instanceof Slot);
+ }
+
@Test
void testJoin() {
for (JoinType joinType : JoinType.values()) {
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
