This is an automated email from the ASF dual-hosted git repository. rubenql pushed a commit to branch main in repository https://gitbox.apache.org/repos/asf/calcite.git
The following commit(s) were added to refs/heads/main by this push: new 295df907d1 [CALCITE-5730] Initial null values can be dropped by EnumerableLimitSort with offset 295df907d1 is described below commit 295df907d126ffa059756e2e0cc8b69051ff6da7 Author: guofeng.my <guofeng...@bytedance.com> AuthorDate: Thu Jun 1 17:06:50 2023 +0800 [CALCITE-5730] Initial null values can be dropped by EnumerableLimitSort with offset --- .../test/enumerable/EnumerableLimitSortTest.java | 172 +++++++++++++++++++++ .../apache/calcite/linq4j/EnumerableDefaults.java | 4 +- .../apache/calcite/linq4j/test/LimitSortTest.java | 23 +-- 3 files changed, 187 insertions(+), 12 deletions(-) diff --git a/core/src/test/java/org/apache/calcite/test/enumerable/EnumerableLimitSortTest.java b/core/src/test/java/org/apache/calcite/test/enumerable/EnumerableLimitSortTest.java new file mode 100644 index 0000000000..5c0d35ca4b --- /dev/null +++ b/core/src/test/java/org/apache/calcite/test/enumerable/EnumerableLimitSortTest.java @@ -0,0 +1,172 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to you under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.calcite.test.enumerable; + +import org.apache.calcite.adapter.enumerable.EnumerableRules; +import org.apache.calcite.adapter.java.ReflectiveSchema; +import org.apache.calcite.config.CalciteConnectionProperty; +import org.apache.calcite.config.Lex; +import org.apache.calcite.plan.RelOptPlanner; +import org.apache.calcite.runtime.Hook; +import org.apache.calcite.test.CalciteAssert; +import org.apache.calcite.test.schemata.hr.HrSchemaBig; + +import org.junit.jupiter.api.Test; + +import java.util.function.Consumer; + +/** Tests for + * {@link org.apache.calcite.adapter.enumerable.EnumerableLimitSort}. */ +public class EnumerableLimitSortTest { + + /** Test case for + * <a href="https://issues.apache.org/jira/browse/CALCITE-5730">[CALCITE-5730] + * First nulls can be dropped by EnumerableLimitSort with offset</a>. */ + @Test void nullsFirstWithLimitAndOffset() { + tester("select commission from emps order by commission nulls first limit 1 offset 1 ") + .explainContains("EnumerableCalc(expr#0..4=[{inputs}], commission=[$t4])\n" + + " EnumerableLimitSort(sort0=[$4], dir0=[ASC-nulls-first], offset=[1], fetch=[1])\n" + + " EnumerableTableScan(table=[[s, emps]])") + .returnsOrdered("commission=null"); + } + + @Test void nullsLastWithLimitAndOffset() { + tester("select commission from emps order by commission desc nulls last limit 8 offset 10 ") + .explainContains("EnumerableCalc(expr#0..4=[{inputs}], commission=[$t4])\n" + + " EnumerableLimitSort(sort0=[$4], dir0=[DESC-nulls-last], offset=[10], fetch=[8])\n" + + " EnumerableTableScan(table=[[s, emps]])") + .returnsOrdered( + "commission=1000", + "commission=1000", + "commission=500", + "commission=500", + "commission=500", + "commission=500", + "commission=500", + "commission=500"); + } + + @Test void nullsFirstWithLimit() { + tester("select commission from emps order by commission nulls first limit 13 ") + .explainContains("EnumerableCalc(expr#0..4=[{inputs}], commission=[$t4])\n" + + " EnumerableLimitSort(sort0=[$4], dir0=[ASC-nulls-first], fetch=[13])\n" + + " EnumerableTableScan(table=[[s, emps]])") + .returnsOrdered( + "commission=null", + "commission=null", + "commission=null", + "commission=null", + "commission=null", + "commission=null", + "commission=null", + "commission=null", + "commission=null", + "commission=null", + "commission=null", + "commission=null", + "commission=250"); + } + + @Test void nullsLastWithLimit() { + tester("select commission from emps order by commission nulls last limit 5 ") + .explainContains("EnumerableCalc(expr#0..4=[{inputs}], commission=[$t4])\n" + + " EnumerableLimitSort(sort0=[$4], dir0=[ASC], fetch=[5])\n" + + " EnumerableTableScan(table=[[s, emps]])") + .returnsOrdered( + "commission=250", + "commission=250", + "commission=250", + "commission=250", + "commission=250"); + } + + @Test void multiOrderByColumnsWithLimitAndOffset() { + tester("select commission, salary, empid from emps" + + " order by commission nulls first, salary asc, empid desc limit 4 offset 6 ") + .explainContains( + "EnumerableCalc(expr#0..4=[{inputs}], commission=[$t4], salary=[$t3], empid=[$t0])\n" + + " EnumerableLimitSort(sort0=[$4], sort1=[$3], sort2=[$0], dir0=[ASC-nulls-first], dir1=[ASC], dir2=[DESC], offset=[6], fetch=[4])\n" + + " EnumerableTableScan(table=[[s, emps]])") + .returnsOrdered( + "commission=null; salary=7000.0; empid=23", + "commission=null; salary=7000.0; empid=19", + "commission=null; salary=7000.0; empid=15", + "commission=null; salary=7000.0; empid=11"); + } + + @Test void multiOrderByColumnsWithLimit() { + tester("select commission, deptno from emps" + + " order by commission desc nulls first, deptno asc limit 13 ") + .explainContains("EnumerableCalc(expr#0..4=[{inputs}], commission=[$t4], deptno=[$t1])\n" + + " EnumerableLimitSort(sort0=[$4], sort1=[$1], dir0=[DESC], dir1=[ASC], fetch=[13])\n" + + " EnumerableTableScan(table=[[s, emps]])\n") + .returnsOrdered( + "commission=null; deptno=8", + "commission=null; deptno=10", + "commission=null; deptno=10", + "commission=null; deptno=10", + "commission=null; deptno=10", + "commission=null; deptno=10", + "commission=null; deptno=10", + "commission=null; deptno=10", + "commission=null; deptno=10", + "commission=null; deptno=10", + "commission=null; deptno=60", + "commission=null; deptno=80", + "commission=1000; deptno=10"); + } + + @Test void multiOrderByColumnsNullsLastWithLimitAndOffset() { + tester("select commission, salary from emps" + + " order by commission desc nulls last, salary limit 6 offset 12 ") + .explainContains("EnumerableCalc(expr#0..4=[{inputs}], commission=[$t4], salary=[$t3])\n" + + " EnumerableLimitSort(sort0=[$4], sort1=[$3], dir0=[DESC-nulls-last], dir1=[ASC], offset=[12], fetch=[6])\n" + + " EnumerableTableScan(table=[[s, emps]])") + .returnsOrdered( + "commission=500; salary=8000.0", + "commission=500; salary=8000.0", + "commission=500; salary=8000.0", + "commission=500; salary=8000.0", + "commission=500; salary=8000.0", + "commission=500; salary=8000.0"); + } + + @Test void multiOrderByColumnsNullsLastWithLimit() { + tester("select commission, empid from emps" + + " order by commission nulls last, empid desc limit 4 ") + .explainContains("EnumerableCalc(expr#0..4=[{inputs}], commission=[$t4], empid=[$t0])\n" + + " EnumerableLimitSort(sort0=[$4], sort1=[$0], dir0=[ASC], dir1=[DESC], fetch=[4])\n" + + " EnumerableTableScan(table=[[s, emps]])") + .returnsOrdered( + "commission=250; empid=48", + "commission=250; empid=44", + "commission=250; empid=40", + "commission=250; empid=36"); + } + + private CalciteAssert.AssertQuery tester(String sqlQuery) { + return CalciteAssert.that() + .with(CalciteConnectionProperty.LEX, Lex.JAVA) + .with(CalciteConnectionProperty.FORCE_DECORRELATE, false) + .withSchema("s", new ReflectiveSchema(new HrSchemaBig())) + .query(sqlQuery) + .withHook(Hook.PLANNER, (Consumer<RelOptPlanner>) planner -> { + planner.removeRule(EnumerableRules.ENUMERABLE_SORT_RULE); + planner.addRule(EnumerableRules.ENUMERABLE_LIMIT_SORT_RULE); + }); + } +} diff --git a/linq4j/src/main/java/org/apache/calcite/linq4j/EnumerableDefaults.java b/linq4j/src/main/java/org/apache/calcite/linq4j/EnumerableDefaults.java index 6a1a21766f..9bf171e5a4 100644 --- a/linq4j/src/main/java/org/apache/calcite/linq4j/EnumerableDefaults.java +++ b/linq4j/src/main/java/org/apache/calcite/linq4j/EnumerableDefaults.java @@ -2735,7 +2735,7 @@ public abstract class EnumerableDefaults { if (offset > 0) { // search the key up to (but excluding) which we have to remove entries from the map int skipped = 0; - TKey until = null; + TKey until = (TKey) DUMMY; for (Map.Entry<TKey, List<TSource>> e : map.entrySet()) { skipped += e.getValue().size(); @@ -2751,7 +2751,7 @@ public abstract class EnumerableDefaults { break; } } - if (until == null) { + if (until == DUMMY) { // the offset is bigger than the number of rows in the map return Linq4j.emptyEnumerator(); } diff --git a/linq4j/src/test/java/org/apache/calcite/linq4j/test/LimitSortTest.java b/linq4j/src/test/java/org/apache/calcite/linq4j/test/LimitSortTest.java index ac8d92cb21..aff085576f 100644 --- a/linq4j/src/test/java/org/apache/calcite/linq4j/test/LimitSortTest.java +++ b/linq4j/src/test/java/org/apache/calcite/linq4j/test/LimitSortTest.java @@ -54,7 +54,7 @@ class LimitSortTest { int a = n < 2 ? 0 : rnd.nextInt(n / 2); String k = Integer.toString(a, Character.MAX_RADIX); Row r = new Row(); - r.key = "" + k; + r.key = rnd.nextBoolean() ? null : ("" + k); r.index = i; return r; }); @@ -82,7 +82,10 @@ class LimitSortTest { int tmp = rnd.nextInt(10_000); int offset = Math.max(0, (int) (tmp - .1 * tmp)); - Comparator<String> cmp = Comparator.<String>naturalOrder()::compare; + Comparator<String> natural = Comparator.naturalOrder(); + Comparator<String> cmp + = rnd.nextBoolean() ? Comparator.nullsFirst(natural) : Comparator.nullsLast(natural); + Enumerable<Row> ordered = EnumerableDefaults.orderBy(this.enumerable(seed), s -> s.key, @@ -100,7 +103,7 @@ class LimitSortTest { Row left = result.get(i - 1); Row right = result.get(i); // use left < right instead of <=, as rows might not appear twice - assertTrue(isSmaller(left, right), + assertTrue(isSmaller(left, right, cmp), "The following elements have not been ordered correctly: " + left + " " + right); } @@ -121,9 +124,9 @@ class LimitSortTest { int actFetch = 0; for (Row r : (Iterable<Row>) this.rowStream(seed)::iterator) { totalItems++; - if (isSmaller(r, first)) { + if (isSmaller(r, first, cmp)) { actOffset++; - } else if (isSmallerEq(r, last)) { + } else if (isSmallerEq(r, last, cmp)) { actFetch++; } } @@ -137,25 +140,25 @@ class LimitSortTest { } /** A comparison function that takes the order of creation into account. */ - private static boolean isSmaller(Row left, Row right) { + private static boolean isSmaller(Row left, Row right, Comparator<String> cmp) { if (right == null) { return true; } - int c = left.key.compareTo(right.key); + int c = cmp.compare(left.key, right.key); if (c != 0) { return c < 0; } return left.index < right.index; } - /** See {@link #isSmaller(Row, Row)}. */ - private static boolean isSmallerEq(Row left, Row right) { + /** See {@link #isSmaller(Row, Row, Comparator)}. */ + private static boolean isSmallerEq(Row left, Row right, Comparator<String> cmp) { if (right == null) { return true; } - int c = left.key.compareTo(right.key); + int c = cmp.compare(left.key, right.key); if (c != 0) { return c < 0; }