You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by ja...@apache.org on 2023/06/29 02:23:50 UTC
[pinot] branch master updated: Fix the NULL ordering in TableResizer for GROUP BY. (#10939)
This is an automated email from the ASF dual-hosted git repository.
jackie pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/pinot.git
The following commit(s) were added to refs/heads/master by this push:
new fc26d6d897 Fix the NULL ordering in TableResizer for GROUP BY. (#10939)
fc26d6d897 is described below
commit fc26d6d8975b4cd46e26e460236a30e8b1eb2cde
Author: Shen Yu <sh...@startree.ai>
AuthorDate: Wed Jun 28 19:23:44 2023 -0700
Fix the NULL ordering in TableResizer for GROUP BY. (#10939)
---
.../apache/pinot/core/data/table/TableResizer.java | 7 +--
.../pinot/queries/BigDecimalQueriesTest.java | 2 +-
.../pinot/queries/NullEnabledQueriesTest.java | 2 +-
.../queries/NullHandlingEnabledQueriesTest.java | 54 ++++++++++++++++++++++
4 files changed, 60 insertions(+), 5 deletions(-)
diff --git a/pinot-core/src/main/java/org/apache/pinot/core/data/table/TableResizer.java b/pinot-core/src/main/java/org/apache/pinot/core/data/table/TableResizer.java
index 464b33350d..4299e5665e 100644
--- a/pinot-core/src/main/java/org/apache/pinot/core/data/table/TableResizer.java
+++ b/pinot-core/src/main/java/org/apache/pinot/core/data/table/TableResizer.java
@@ -86,10 +86,12 @@ public class TableResizer {
_numOrderByExpressions = orderByExpressions.size();
_orderByValueExtractors = new OrderByValueExtractor[_numOrderByExpressions];
Comparator[] comparators = new Comparator[_numOrderByExpressions];
+ int[] nullComparisonResults = new int[_numOrderByExpressions];
for (int i = 0; i < _numOrderByExpressions; i++) {
OrderByExpressionContext orderByExpression = orderByExpressions.get(i);
_orderByValueExtractors[i] = getOrderByValueExtractor(orderByExpression.getExpression());
comparators[i] = orderByExpression.isAsc() ? Comparator.naturalOrder() : Comparator.reverseOrder();
+ nullComparisonResults[i] = orderByExpression.isNullsLast() ? -1 : 1;
}
boolean nullHandlingEnabled = queryContext.isNullHandlingEnabled();
if (nullHandlingEnabled) {
@@ -101,10 +103,9 @@ public class TableResizer {
if (v2 == null) {
continue;
}
- // The default null ordering is NULLS LAST, regardless of the ordering direction.
- return 1;
+ return -nullComparisonResults[i];
} else if (v2 == null) {
- return -1;
+ return nullComparisonResults[i];
}
int result = comparators[i].compare(v1, v2);
if (result != 0) {
diff --git a/pinot-core/src/test/java/org/apache/pinot/queries/BigDecimalQueriesTest.java b/pinot-core/src/test/java/org/apache/pinot/queries/BigDecimalQueriesTest.java
index 8be7131e5d..a712208ff6 100644
--- a/pinot-core/src/test/java/org/apache/pinot/queries/BigDecimalQueriesTest.java
+++ b/pinot-core/src/test/java/org/apache/pinot/queries/BigDecimalQueriesTest.java
@@ -311,7 +311,7 @@ public class BigDecimalQueriesTest extends BaseQueriesTest {
}
{
String query = String.format(
- "SELECT COUNT(*) AS count, %s FROM testTable GROUP BY %s ORDER BY %s DESC LIMIT 1000",
+ "SELECT COUNT(*) AS count, %s FROM testTable GROUP BY %s ORDER BY %s DESC NULLS LAST LIMIT 1000",
BIG_DECIMAL_COLUMN, BIG_DECIMAL_COLUMN, BIG_DECIMAL_COLUMN);
BrokerResponseNative brokerResponse = getBrokerResponse(query, queryOptions);
ResultTable resultTable = brokerResponse.getResultTable();
diff --git a/pinot-core/src/test/java/org/apache/pinot/queries/NullEnabledQueriesTest.java b/pinot-core/src/test/java/org/apache/pinot/queries/NullEnabledQueriesTest.java
index afc7f1f61b..5a57945ca7 100644
--- a/pinot-core/src/test/java/org/apache/pinot/queries/NullEnabledQueriesTest.java
+++ b/pinot-core/src/test/java/org/apache/pinot/queries/NullEnabledQueriesTest.java
@@ -523,7 +523,7 @@ public class NullEnabledQueriesTest extends BaseQueriesTest {
}
{
String query = String.format(
- "SELECT COUNT(*) AS count, %s FROM testTable GROUP BY %s ORDER BY %s DESC LIMIT 1000", COLUMN_NAME,
+ "SELECT COUNT(*) AS count, %s FROM testTable GROUP BY %s ORDER BY %s DESC NULLS LAST LIMIT 1000", COLUMN_NAME,
COLUMN_NAME, COLUMN_NAME);
BrokerResponseNative brokerResponse = getBrokerResponse(query, queryOptions);
ResultTable resultTable = brokerResponse.getResultTable();
diff --git a/pinot-core/src/test/java/org/apache/pinot/queries/NullHandlingEnabledQueriesTest.java b/pinot-core/src/test/java/org/apache/pinot/queries/NullHandlingEnabledQueriesTest.java
index 804635031b..ad5ba94b39 100644
--- a/pinot-core/src/test/java/org/apache/pinot/queries/NullHandlingEnabledQueriesTest.java
+++ b/pinot-core/src/test/java/org/apache/pinot/queries/NullHandlingEnabledQueriesTest.java
@@ -45,6 +45,7 @@ import org.testng.annotations.Test;
import static org.testng.Assert.assertEquals;
import static org.testng.Assert.assertNotNull;
import static org.testng.Assert.assertNull;
+import static org.testng.AssertJUnit.assertArrayEquals;
import static org.testng.AssertJUnit.assertTrue;
@@ -392,4 +393,57 @@ public class NullHandlingEnabledQueriesTest extends BaseQueriesTest {
List<Object[]> rows = resultTable.getRows();
assertEquals(rows.size(), 3);
}
+
+ @Test
+ public void testGroupByOrderBy()
+ throws Exception {
+ initializeRows();
+ insertRow(null);
+ insertRow(1);
+ insertRow(1);
+ insertRow(2);
+ insertRow(3);
+ TableConfig tableConfig = new TableConfigBuilder(TableType.OFFLINE).setTableName(RAW_TABLE_NAME).build();
+ Schema schema = new Schema.SchemaBuilder().addSingleValueDimension(COLUMN1, FieldSpec.DataType.INT).build();
+ setUpSegments(tableConfig, schema);
+ String query =
+ String.format("SELECT count(*), %s FROM testTable GROUP BY %s ORDER BY %s ASC NULLS LAST", COLUMN1, COLUMN1,
+ COLUMN1);
+
+ BrokerResponseNative brokerResponse = getBrokerResponse(query, QUERY_OPTIONS);
+
+ ResultTable resultTable = brokerResponse.getResultTable();
+ List<Object[]> rows = resultTable.getRows();
+ assertEquals(rows.size(), 4);
+ assertArrayEquals(rows.get(0), new Object[]{(long) 2 * NUM_OF_SEGMENT_COPIES, 1});
+ assertArrayEquals(rows.get(1), new Object[]{(long) NUM_OF_SEGMENT_COPIES, 2});
+ assertArrayEquals(rows.get(2), new Object[]{(long) NUM_OF_SEGMENT_COPIES, 3});
+ assertArrayEquals(rows.get(3), new Object[]{(long) NUM_OF_SEGMENT_COPIES, null});
+ }
+
+ @Test
+ public void testGroupByOrderByWithLimit()
+ throws Exception {
+ initializeRows();
+ insertRow(null);
+ insertRow(1);
+ insertRow(1);
+ insertRow(2);
+ insertRow(3);
+ TableConfig tableConfig = new TableConfigBuilder(TableType.OFFLINE).setTableName(RAW_TABLE_NAME).build();
+ Schema schema = new Schema.SchemaBuilder().addSingleValueDimension(COLUMN1, FieldSpec.DataType.INT).build();
+ setUpSegments(tableConfig, schema);
+ String query =
+ String.format("SELECT count(*), %s FROM testTable GROUP BY %s ORDER BY %s DESC NULLS FIRST LIMIT 3", COLUMN1,
+ COLUMN1, COLUMN1);
+
+ BrokerResponseNative brokerResponse = getBrokerResponse(query, QUERY_OPTIONS);
+
+ ResultTable resultTable = brokerResponse.getResultTable();
+ List<Object[]> rows = resultTable.getRows();
+ assertEquals(rows.size(), 3);
+ assertArrayEquals(rows.get(0), new Object[]{(long) NUM_OF_SEGMENT_COPIES, null});
+ assertArrayEquals(rows.get(1), new Object[]{(long) NUM_OF_SEGMENT_COPIES, 3});
+ assertArrayEquals(rows.get(2), new Object[]{(long) NUM_OF_SEGMENT_COPIES, 2});
+ }
}
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org