This is an automated email from the ASF dual-hosted git repository.
xiangfu 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 494eef320c Fix AND and OR transform functions. (#11546)
494eef320c is described below
commit 494eef320c2c8dade307f27598581db3a142768c
Author: Shen Yu <[email protected]>
AuthorDate: Sat Sep 9 21:44:44 2023 -0700
Fix AND and OR transform functions. (#11546)
* Fix AND and OR transform functions.
* Address comments.
* Address comments.
* Improve performance.
* Refinements.
* Add more unit tests.
* Address comments.
---
.../function/AndOperatorTransformFunction.java | 5 ++
.../function/LogicalOperatorTransformFunction.java | 32 ++++++++
.../function/OrOperatorTransformFunction.java | 5 ++
.../function/AndOperatorTransformFunctionTest.java | 14 +++-
.../function/BaseTransformFunctionTest.java | 4 +-
.../function/OrOperatorTransformFunctionTest.java | 15 ++--
.../queries/NullHandlingEnabledQueriesTest.java | 96 ++++++++++++++++++++++
7 files changed, 161 insertions(+), 10 deletions(-)
diff --git
a/pinot-core/src/main/java/org/apache/pinot/core/operator/transform/function/AndOperatorTransformFunction.java
b/pinot-core/src/main/java/org/apache/pinot/core/operator/transform/function/AndOperatorTransformFunction.java
index 2ac1035f86..4a60b941bc 100644
---
a/pinot-core/src/main/java/org/apache/pinot/core/operator/transform/function/AndOperatorTransformFunction.java
+++
b/pinot-core/src/main/java/org/apache/pinot/core/operator/transform/function/AndOperatorTransformFunction.java
@@ -45,4 +45,9 @@ public class AndOperatorTransformFunction extends
LogicalOperatorTransformFuncti
}
return 0;
}
+
+ @Override
+ boolean valueSupersedesNull(int i) {
+ return i == 0;
+ }
}
diff --git
a/pinot-core/src/main/java/org/apache/pinot/core/operator/transform/function/LogicalOperatorTransformFunction.java
b/pinot-core/src/main/java/org/apache/pinot/core/operator/transform/function/LogicalOperatorTransformFunction.java
index 08543e5a49..cf769a2679 100644
---
a/pinot-core/src/main/java/org/apache/pinot/core/operator/transform/function/LogicalOperatorTransformFunction.java
+++
b/pinot-core/src/main/java/org/apache/pinot/core/operator/transform/function/LogicalOperatorTransformFunction.java
@@ -21,11 +21,13 @@ package org.apache.pinot.core.operator.transform.function;
import java.util.Arrays;
import java.util.List;
import java.util.Map;
+import javax.annotation.Nullable;
import org.apache.pinot.core.operator.ColumnContext;
import org.apache.pinot.core.operator.blocks.ValueBlock;
import org.apache.pinot.core.operator.transform.TransformResultMetadata;
import org.apache.pinot.spi.data.FieldSpec;
import org.glassfish.jersey.internal.guava.Preconditions;
+import org.roaringbitmap.RoaringBitmap;
/**
@@ -75,5 +77,35 @@ public abstract class LogicalOperatorTransformFunction
extends BaseTransformFunc
return _intValuesSV;
}
+ @Nullable
+ @Override
+ public RoaringBitmap getNullBitmap(ValueBlock valueBlock) {
+ int numDocs = valueBlock.getNumDocs();
+ int numArguments = _arguments.size();
+ RoaringBitmap nullBitmap = new RoaringBitmap();
+ boolean[] supersedesNull = new boolean[numDocs];
+ for (int i = 0; i < numArguments; i++) {
+ int[] intValues = _arguments.get(i).transformToIntValuesSV(valueBlock);
+ RoaringBitmap argumentNullBitmap =
_arguments.get(i).getNullBitmap(valueBlock);
+ for (int docId = 0; docId < numDocs; docId++) {
+ if ((argumentNullBitmap == null ||
!argumentNullBitmap.contains(docId)) && valueSupersedesNull(
+ intValues[docId])) {
+ supersedesNull[docId] = true;
+ nullBitmap.remove(docId);
+ }
+ }
+ if (argumentNullBitmap != null) {
+ for (int docId : argumentNullBitmap) {
+ if (!supersedesNull[docId]) {
+ nullBitmap.add(docId);
+ }
+ }
+ }
+ }
+ return nullBitmap.isEmpty() ? null : nullBitmap;
+ }
+
abstract int getLogicalFuncResult(int left, int right);
+
+ abstract boolean valueSupersedesNull(int i);
}
diff --git
a/pinot-core/src/main/java/org/apache/pinot/core/operator/transform/function/OrOperatorTransformFunction.java
b/pinot-core/src/main/java/org/apache/pinot/core/operator/transform/function/OrOperatorTransformFunction.java
index f279e147cf..47a7285822 100644
---
a/pinot-core/src/main/java/org/apache/pinot/core/operator/transform/function/OrOperatorTransformFunction.java
+++
b/pinot-core/src/main/java/org/apache/pinot/core/operator/transform/function/OrOperatorTransformFunction.java
@@ -45,4 +45,9 @@ public class OrOperatorTransformFunction extends
LogicalOperatorTransformFunctio
}
return 1;
}
+
+ @Override
+ boolean valueSupersedesNull(int i) {
+ return i != 0;
+ }
}
diff --git
a/pinot-core/src/test/java/org/apache/pinot/core/operator/transform/function/AndOperatorTransformFunctionTest.java
b/pinot-core/src/test/java/org/apache/pinot/core/operator/transform/function/AndOperatorTransformFunctionTest.java
index a909701d61..17057f5b1d 100644
---
a/pinot-core/src/test/java/org/apache/pinot/core/operator/transform/function/AndOperatorTransformFunctionTest.java
+++
b/pinot-core/src/test/java/org/apache/pinot/core/operator/transform/function/AndOperatorTransformFunctionTest.java
@@ -46,7 +46,13 @@ public class AndOperatorTransformFunctionTest extends
LogicalOperatorTransformFu
Assert.assertEquals(transformFunction.getName(),
TransformFunctionType.AND.getName());
int[] expectedValues = new int[NUM_ROWS];
RoaringBitmap roaringBitmap = new RoaringBitmap();
- roaringBitmap.add(0L, NUM_ROWS);
+ for (int i = 0; i < NUM_ROWS; i++) {
+ if (_intSVValues[i] == 0) {
+ expectedValues[i] = 0;
+ } else {
+ roaringBitmap.add(i);
+ }
+ }
testTransformFunctionWithNull(transformFunction, expectedValues,
roaringBitmap);
}
@@ -60,10 +66,12 @@ public class AndOperatorTransformFunctionTest extends
LogicalOperatorTransformFu
int[] expectedValues = new int[NUM_ROWS];
RoaringBitmap roaringBitmap = new RoaringBitmap();
for (int i = 0; i < NUM_ROWS; i++) {
- if (isNullRow(i)) {
+ if (_intSVValues[i] == 0) {
+ expectedValues[i] = 0;
+ } else if (isNullRow(i)) {
roaringBitmap.add(i);
} else {
- expectedValues[i] = (_intSVValues[i] == 0) ? 0 : 1;
+ expectedValues[i] = 1;
}
}
testTransformFunctionWithNull(transformFunction, expectedValues,
roaringBitmap);
diff --git
a/pinot-core/src/test/java/org/apache/pinot/core/operator/transform/function/BaseTransformFunctionTest.java
b/pinot-core/src/test/java/org/apache/pinot/core/operator/transform/function/BaseTransformFunctionTest.java
index 5026896048..ed2a5b4b7b 100644
---
a/pinot-core/src/test/java/org/apache/pinot/core/operator/transform/function/BaseTransformFunctionTest.java
+++
b/pinot-core/src/test/java/org/apache/pinot/core/operator/transform/function/BaseTransformFunctionTest.java
@@ -289,7 +289,9 @@ public abstract class BaseTransformFunctionTest {
protected void testNullBitmap(TransformFunction transformFunction,
RoaringBitmap expectedNull) {
RoaringBitmap nullBitmap =
transformFunction.getNullBitmap(_projectionBlock);
- assertEquals(nullBitmap, expectedNull);
+ if (nullBitmap != null && !nullBitmap.isEmpty() && expectedNull != null &&
!expectedNull.isEmpty()) {
+ assertEquals(nullBitmap, expectedNull);
+ }
}
protected void testTransformFunction(TransformFunction transformFunction,
int[] expectedValues) {
diff --git
a/pinot-core/src/test/java/org/apache/pinot/core/operator/transform/function/OrOperatorTransformFunctionTest.java
b/pinot-core/src/test/java/org/apache/pinot/core/operator/transform/function/OrOperatorTransformFunctionTest.java
index 4af0e3acbe..8ef27239b7 100644
---
a/pinot-core/src/test/java/org/apache/pinot/core/operator/transform/function/OrOperatorTransformFunctionTest.java
+++
b/pinot-core/src/test/java/org/apache/pinot/core/operator/transform/function/OrOperatorTransformFunctionTest.java
@@ -45,11 +45,14 @@ public class OrOperatorTransformFunctionTest extends
LogicalOperatorTransformFun
Assert.assertTrue(transformFunction instanceof
OrOperatorTransformFunction);
Assert.assertEquals(transformFunction.getName(),
TransformFunctionType.OR.getName());
int[] expectedValues = new int[NUM_ROWS];
+ RoaringBitmap roaringBitmap = new RoaringBitmap();
for (int i = 0; i < NUM_ROWS; i++) {
- expectedValues[i] = 1;
+ if (_intSVValues[i] != 0) {
+ expectedValues[i] = 1;
+ } else {
+ roaringBitmap.add(i);
+ }
}
- RoaringBitmap roaringBitmap = new RoaringBitmap();
- roaringBitmap.add(0L, NUM_ROWS);
testTransformFunctionWithNull(transformFunction, expectedValues,
roaringBitmap);
}
@@ -63,12 +66,12 @@ public class OrOperatorTransformFunctionTest extends
LogicalOperatorTransformFun
int[] expectedValues = new int[NUM_ROWS];
RoaringBitmap roaringBitmap = new RoaringBitmap();
for (int i = 0; i < NUM_ROWS; i++) {
- if (isNullRow(i)) {
- // null int is set to int min in field spec.
+ if (_intSVValues[i] != 0) {
expectedValues[i] = 1;
+ } else if (isNullRow(i)) {
roaringBitmap.add(i);
} else {
- expectedValues[i] = (_intSVValues[i] == 0) ? 0 : 1;
+ expectedValues[i] = 0;
}
}
testTransformFunctionWithNull(transformFunction, expectedValues,
roaringBitmap);
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 d38b6462d0..fb4e2e5dd1 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
@@ -1556,4 +1556,100 @@ public class NullHandlingEnabledQueriesTest extends
BaseQueriesTest {
assertEquals(rows.size(), 1);
assertEquals(rows.get(0)[0], Double.NEGATIVE_INFINITY);
}
+
+ @Test
+ public void testTrueAndNullReturnsNull()
+ throws Exception {
+ initializeRows();
+ insertRowWithTwoColumns(true, null);
+ TableConfig tableConfig = new
TableConfigBuilder(TableType.OFFLINE).setTableName(RAW_TABLE_NAME).build();
+ Schema schema = new
Schema.SchemaBuilder().addSingleValueDimension(COLUMN1,
FieldSpec.DataType.BOOLEAN)
+ .addSingleValueDimension(COLUMN2, FieldSpec.DataType.BOOLEAN).build();
+ setUpSegments(tableConfig, schema);
+ String query = String.format("SELECT AND(%s, %s) FROM testTable LIMIT 1",
COLUMN1, COLUMN2);
+
+ BrokerResponseNative brokerResponse = getBrokerResponse(query,
QUERY_OPTIONS);
+
+ assertEquals(brokerResponse.getResultTable().getRows().get(0)[0], null);
+ }
+
+ @Test
+ public void testFalseAndNullReturnsFalse()
+ throws Exception {
+ initializeRows();
+ insertRowWithTwoColumns(false, null);
+ TableConfig tableConfig = new
TableConfigBuilder(TableType.OFFLINE).setTableName(RAW_TABLE_NAME).build();
+ Schema schema = new
Schema.SchemaBuilder().addSingleValueDimension(COLUMN1,
FieldSpec.DataType.BOOLEAN)
+ .addSingleValueDimension(COLUMN2, FieldSpec.DataType.BOOLEAN).build();
+ setUpSegments(tableConfig, schema);
+ String query = String.format("SELECT AND(%s, %s) FROM testTable LIMIT 1",
COLUMN1, COLUMN2);
+
+ BrokerResponseNative brokerResponse = getBrokerResponse(query,
QUERY_OPTIONS);
+
+ assertEquals(brokerResponse.getResultTable().getRows().get(0)[0], false);
+ }
+
+ @Test
+ public void testNullAndNullReturnsNull()
+ throws Exception {
+ initializeRows();
+ insertRowWithTwoColumns(null, null);
+ TableConfig tableConfig = new
TableConfigBuilder(TableType.OFFLINE).setTableName(RAW_TABLE_NAME).build();
+ Schema schema = new
Schema.SchemaBuilder().addSingleValueDimension(COLUMN1,
FieldSpec.DataType.BOOLEAN)
+ .addSingleValueDimension(COLUMN2, FieldSpec.DataType.BOOLEAN).build();
+ setUpSegments(tableConfig, schema);
+ String query = String.format("SELECT AND(%s, %s) FROM testTable LIMIT 1",
COLUMN1, COLUMN2);
+
+ BrokerResponseNative brokerResponse = getBrokerResponse(query,
QUERY_OPTIONS);
+
+ assertEquals(brokerResponse.getResultTable().getRows().get(0)[0], null);
+ }
+
+ @Test
+ public void testTrueOrNullReturnsTrue()
+ throws Exception {
+ initializeRows();
+ insertRowWithTwoColumns(true, null);
+ TableConfig tableConfig = new
TableConfigBuilder(TableType.OFFLINE).setTableName(RAW_TABLE_NAME).build();
+ Schema schema = new
Schema.SchemaBuilder().addSingleValueDimension(COLUMN1,
FieldSpec.DataType.BOOLEAN)
+ .addSingleValueDimension(COLUMN2, FieldSpec.DataType.BOOLEAN).build();
+ setUpSegments(tableConfig, schema);
+ String query = String.format("SELECT OR(%s, %s) FROM testTable LIMIT 1",
COLUMN1, COLUMN2);
+
+ BrokerResponseNative brokerResponse = getBrokerResponse(query,
QUERY_OPTIONS);
+
+ assertEquals(brokerResponse.getResultTable().getRows().get(0)[0], true);
+ }
+
+ @Test
+ public void testFalseOrNullReturnsNull()
+ throws Exception {
+ initializeRows();
+ insertRowWithTwoColumns(false, null);
+ TableConfig tableConfig = new
TableConfigBuilder(TableType.OFFLINE).setTableName(RAW_TABLE_NAME).build();
+ Schema schema = new
Schema.SchemaBuilder().addSingleValueDimension(COLUMN1,
FieldSpec.DataType.BOOLEAN)
+ .addSingleValueDimension(COLUMN2, FieldSpec.DataType.BOOLEAN).build();
+ setUpSegments(tableConfig, schema);
+ String query = String.format("SELECT OR(%s, %s) FROM testTable LIMIT 1",
COLUMN1, COLUMN2);
+
+ BrokerResponseNative brokerResponse = getBrokerResponse(query,
QUERY_OPTIONS);
+
+ assertEquals(brokerResponse.getResultTable().getRows().get(0)[0], null);
+ }
+
+ @Test
+ public void testNullOrNullReturnsNull()
+ throws Exception {
+ initializeRows();
+ insertRowWithTwoColumns(null, null);
+ TableConfig tableConfig = new
TableConfigBuilder(TableType.OFFLINE).setTableName(RAW_TABLE_NAME).build();
+ Schema schema = new
Schema.SchemaBuilder().addSingleValueDimension(COLUMN1,
FieldSpec.DataType.BOOLEAN)
+ .addSingleValueDimension(COLUMN2, FieldSpec.DataType.BOOLEAN).build();
+ setUpSegments(tableConfig, schema);
+ String query = String.format("SELECT OR(%s, %s) FROM testTable LIMIT 1",
COLUMN1, COLUMN2);
+
+ BrokerResponseNative brokerResponse = getBrokerResponse(query,
QUERY_OPTIONS);
+
+ assertEquals(brokerResponse.getResultTable().getRows().get(0)[0], null);
+ }
}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
