Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,8 @@ public enum TransformFunctionType {
MAP_VALUE("mapValue",
ReturnTypes.cascade(opBinding -> positionalComponentType(opBinding, 2), SqlTypeTransforms.FORCE_NULLABLE),
OperandTypes.family(List.of(SqlTypeFamily.ARRAY, SqlTypeFamily.ANY, SqlTypeFamily.ARRAY))),
IN_ID_SET("inIdSet", ReturnTypes.BOOLEAN, OperandTypes.family(SqlTypeFamily.ANY, SqlTypeFamily.CHARACTER)),
// Nullable: both the transform and the scalar function return NULL for a NULL value
IN_ID_SET("inIdSet", ReturnTypes.BOOLEAN_NULLABLE, OperandTypes.family(SqlTypeFamily.ANY, SqlTypeFamily.CHARACTER)),
LOOKUP("lookUp"),
GROOVY("groovy"),
SCALAR("scalar"),
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,162 @@
/**
* 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.pinot.core.function.scalar;

import com.google.common.annotations.VisibleForTesting;
import java.io.IOException;
import java.math.BigDecimal;
import java.sql.Timestamp;
import java.util.UUID;
import javax.annotation.Nullable;
import org.apache.pinot.core.query.utils.idset.IdSet;
import org.apache.pinot.core.query.utils.idset.IdSets;
import org.apache.pinot.spi.annotations.ScalarFunction;
import org.apache.pinot.spi.data.FieldSpec.DataType;
import org.apache.pinot.spi.utils.ByteArray;
import org.apache.pinot.spi.utils.UuidUtils;


/// Scalar form of the `IN_ID_SET` transform function, for queries that cannot run the transform function:
/// - In the multi-stage engine, wherever the predicate is not pushed into a leaf stage, e.g. a filter over a join,
/// `HAVING` or a join condition. Leaf stages keep using the `InIdSetTransformFunction`, because transform functions
/// take precedence over scalar functions.
/// - In the single-stage engine, after aggregation, e.g. in `HAVING`. Aggregations such as `MAX` return DOUBLE there,
/// so cast them to the type the IdSet was built from.
///
/// The method is not static, so each call site gets its own instance (see `FunctionInvoker`), which caches the last
/// deserialized IdSet:
/// - A literal IdSet reaches every row as the same String instance, so it is deserialized once per call site.
/// - An IdSet read from rows (e.g. a joined `IDSET` subquery result) arrives as a new but equal String instance with
/// every data block, which costs one String comparison per block.
/// - A call site whose IdSet changes from row to row (e.g. a correlated `IDSET` subquery, with one IdSet per group)
/// deserializes the IdSet on every change, so it only suits small IdSets.
///
/// Not thread-safe: an instance serves one call site, which one thread evaluates at a time.
public class IdSetFunctions {
private String _serializedIdSet;
private IdSet _idSet;
// The IdSet and the value class whose combination passed the last value type validation
private IdSet _validatedIdSet;
private Class<?> _validatedValueClass;

/// Returns whether the IdSet contains the value. As with an `IN` subquery, the result is `false` for an empty or
/// `null` IdSet, even when the value is `null`, and `null` for a `null` value otherwise. `IDSET` returns a `null`
/// IdSet over zero rows when null handling is enabled.
///
/// Values arrive as their external Java types. BOOLEAN, TIMESTAMP and UUID values are looked up through their
/// stored INT, LONG and BYTES values, as the transform function does.
///
/// `isDeterministic = false` keeps all-literal calls out of compile-time evaluation, which passes literals as Java
/// types the lookup cannot map to a value type (e.g. BigDecimal for an INT literal). The runtime evaluates them
/// instead, as before this function existed. The flag also marks the function VOLATILE, so ingestion transforms
/// cannot use it.
@Nullable
@ScalarFunction(nullableParameters = true, isDeterministic = false)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

isDeterministic = false works around literal typing (BigDecimal for an INT literal), but it also marks the function volatile. That is harmless for planning today only because PinotOperatorTable registers TransformFunctionType.IN_ID_SET before the scalar functions, so Calcite sees the transform's operator, which is deterministic.

If that operator ever goes away, the planner picks this scalar's PinotSqlFunction instead. Then PinotRuleUtils.isRelocatable() returns false and PinotFilterJoinRule stops pushing IN_ID_SET below joins. Results stay correct, but the leaf transform path is lost silently.

Could we add a comment here that names this coupling, and a plan test (EXPLAIN) that pins a filter over a join down to the leaf? Another option is to map BigDecimal literals to the set's type in getStoredType and keep the function deterministic.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in 021374b. The comment is in IdSetFunctions and at TransformFunctionType.IN_ID_SET, and new cases in JoinPlans.json and PhysicalOptimizerPlans.json pin the push-down. When I made the IN_ID_SET operator volatile, both cases failed, with the filter above the join.

I kept isDeterministic = false instead of mapping BigDecimal to the set's type. A Bloom set does not record whether it was built from INT or FLOAT values (or LONG or DOUBLE), so the mapping could look up the wrong bits. The root cause is that PinotEvaluateLiteralRule passes raw Calcite values (BigDecimal for every exact number) to Object parameters, where the runtime passes Integer or Long. From the code, this also changes fold-time results of other Object-parameter functions, such as toThetaSketch. So I think the fix belongs in the rule, in a separate PR.

public Boolean inIdSet(@Nullable Object value, @Nullable String serializedIdSet) {
if (serializedIdSet == null) {
return false;
}
IdSet idSet = getIdSet(serializedIdSet);
if (value == null) {
return idSet.getType() == IdSet.Type.EMPTY ? false : null;
}
if (idSet != _validatedIdSet || value.getClass() != _validatedValueClass) {
IdSets.validateValueType(idSet, getStoredType(value));
_validatedIdSet = idSet;
_validatedValueClass = value.getClass();
}
if (value instanceof Integer) {
return idSet.contains(((Integer) value).intValue());
}
if (value instanceof Long) {
return idSet.contains(((Long) value).longValue());
}
if (value instanceof Float) {
return idSet.contains(((Float) value).floatValue());
}
if (value instanceof Double) {
return idSet.contains(((Double) value).doubleValue());
}
if (value instanceof String) {
return idSet.contains((String) value);
}
if (value instanceof byte[]) {
return idSet.contains((byte[]) value);
}
if (value instanceof Boolean) {
return idSet.contains((Boolean) value ? 1 : 0);
}
if (value instanceof Timestamp) {
return idSet.contains(((Timestamp) value).getTime());
}
if (value instanceof UUID) {
return idSet.contains(UuidUtils.toBytes((UUID) value));
}
if (value instanceof ByteArray) {
return idSet.contains(((ByteArray) value).getBytes());
}
throw new IllegalStateException("Unsupported value class: " + value.getClass().getName());
}

@VisibleForTesting
IdSet getIdSet(String serializedIdSet) {
if (serializedIdSet != _serializedIdSet) {
if (!serializedIdSet.equals(_serializedIdSet)) {
_idSet = deserialize(serializedIdSet);
}
// Keep the latest instance: the following rows of a data block share it, so they match by reference
_serializedIdSet = serializedIdSet;
}
return _idSet;
}

private static IdSet deserialize(String serializedIdSet) {
try {
return IdSets.fromBase64String(serializedIdSet);
} catch (IOException | RuntimeException e) {
throw new IllegalArgumentException("Caught exception while deserializing IdSet: " + e, e);
}
}

private static DataType getStoredType(Object value) {
if (value instanceof Integer || value instanceof Boolean) {
return DataType.INT;
}
if (value instanceof Long || value instanceof Timestamp) {
return DataType.LONG;
}
if (value instanceof Float) {
return DataType.FLOAT;
}
if (value instanceof Double) {
return DataType.DOUBLE;
}
if (value instanceof String) {
return DataType.STRING;
}
if (value instanceof byte[] || value instanceof UUID || value instanceof ByteArray) {
return DataType.BYTES;
}
if (value instanceof BigDecimal) {
return DataType.BIG_DECIMAL;
}
throw new IllegalArgumentException(
"Cannot look up values of class " + value.getClass().getName() + " in an IdSet");
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -22,13 +22,15 @@
import java.io.IOException;
import java.util.List;
import java.util.Map;
import javax.annotation.Nullable;
import org.apache.pinot.common.function.TransformFunctionType;
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.core.query.utils.idset.IdSet;
import org.apache.pinot.core.query.utils.idset.IdSets;
import org.apache.pinot.spi.data.FieldSpec.DataType;
import org.roaringbitmap.RoaringBitmap;


/// The IN_ID_SET transform function takes 2 arguments:
Expand Down Expand Up @@ -61,16 +63,24 @@ public void init(List<TransformFunction> arguments, Map<String, ColumnContext> c
_transformFunction = arguments.get(0);
try {
_idSet = IdSets.fromBase64String(((LiteralTransformFunction) arguments.get(1)).getStringLiteral());
} catch (IOException e) {
throw new IllegalArgumentException("Caught exception while deserializing IdSet", e);
} catch (IOException | RuntimeException e) {
throw new IllegalArgumentException("Caught exception while deserializing IdSet: " + e, e);
}
IdSets.validateValueType(_idSet, _transformFunction.getResultMetadata().getDataType().getStoredType());
}

@Override
public TransformResultMetadata getResultMetadata() {
return BOOLEAN_SV_NO_DICTIONARY_METADATA;
}

/// As with an `IN` subquery, a NULL value is in no empty IdSet, so the result is never NULL for an empty IdSet.
@Nullable
@Override
public RoaringBitmap getNullBitmap(ValueBlock valueBlock) {
return _idSet.getType() == IdSet.Type.EMPTY ? null : super.getNullBitmap(valueBlock);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This changes existing results. With null handling on, IN_ID_SET(nullableCol, <empty set>) used to be NULL for NULL rows and is now FALSE, so = 0 or NOT IN_ID_SET(...) now keeps those rows. The single-stage IN_SUBQUERY changes too when the subquery matches no rows.

The PR description explains it, but I think it needs the backward-incompat label and a release note, plus a line in the IN_ID_SET / IN_SUBQUERY docs. Also, only the multi-stage leaf case in IdSet.json covers this today. A single-stage case in IdSetQueriesTest (empty set, nullable column, null handling on) would lock it in for that engine as well.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in 021374b. I added the backward-incompat label, a release note in the description, and IdSetQueriesTest.testInIdSetNullHandling (single-stage, nullable column, null handling on, empty and non-empty sets). The docs line needs a separate PR in the docs repo.

One note on IN_SUBQUERY: its subquery takes null handling from the broker config or the request options, not from the outer query's SET. If the subquery runs with null handling on and matches no rows, IDSET returns NULL, and the rewrite already fails with a NullPointerException today. I listed that under Known limits.

}

@Override
public int[] transformToIntValuesSV(ValueBlock valueBlock) {
int length = valueBlock.getNumDocs();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,14 +18,13 @@
*/
package org.apache.pinot.core.query.utils.idset;

import com.google.common.base.Preconditions;
import com.google.common.hash.BloomFilter;
import com.google.common.hash.Funnel;
import com.google.common.hash.Funnels;
import java.io.ByteArrayInputStream;
import java.io.ByteArrayOutputStream;
import java.io.IOException;
import java.nio.ByteBuffer;
import java.util.List;
import org.apache.pinot.spi.data.FieldSpec.DataType;


Expand All @@ -36,12 +35,16 @@ public class BloomFilterIdSet implements IdSet {

private enum FunnelType {
// DO NOT change the ids as the ser/de relies on them
INT((byte) 0), LONG((byte) 1), STRING((byte) 2), BYTES((byte) 3);
INT((byte) 0, DataType.INT, DataType.FLOAT), LONG((byte) 1, DataType.LONG, DataType.DOUBLE),
STRING((byte) 2, DataType.STRING), BYTES((byte) 3, DataType.BYTES);

private final byte _id;
// Stored types whose values hash through this funnel: FLOAT and DOUBLE values hash as their raw bits
private final List<DataType> _valueTypes;

FunnelType(byte id) {
FunnelType(byte id, DataType... valueTypes) {
_id = id;
_valueTypes = List.of(valueTypes);
}

public byte getId() {
Expand Down Expand Up @@ -93,6 +96,12 @@ BloomFilter getBloomFilter() {
return _bloomFilter;
}

/// Returns the stored types whose values can be looked up in this IdSet. The serialized form records only the
/// funnel, so an IdSet built from INT values also accepts FLOAT values (and LONG also accepts DOUBLE).
List<DataType> getValueTypes() {
return _funnelType._valueTypes;
}

@Override
public Type getType() {
return Type.BLOOM_FILTER;
Expand Down Expand Up @@ -174,13 +183,11 @@ public byte[] toBytes()
return byteArrayOutputStream.toByteArray();
}

/// Deserializes the BloomFilterIdSet from a ByteBuffer.
/// Deserializes the BloomFilterIdSet from a ByteBuffer, which may be read-only or direct.
///
/// NOTE: The ByteBuffer does not include the IdSet.Type byte.
static BloomFilterIdSet fromByteBuffer(ByteBuffer byteBuffer)
throws IOException {
Preconditions.checkArgument(byteBuffer.hasArray(),
"Cannot deserialize BloomFilter from ByteBuffer not backed by an accessible byte array");
// Count the IdSet.Type byte
int serializedSizeInBytes = 1 + byteBuffer.remaining();
byte funnelTypeId = byteBuffer.get();
Expand All @@ -204,12 +211,10 @@ static BloomFilterIdSet fromByteBuffer(ByteBuffer byteBuffer)
funnel = Funnels.byteArrayFunnel();
break;
default:
throw new IllegalStateException();
throw new IllegalStateException("Unsupported BloomFilter funnel type id: " + funnelTypeId);
}
// NOTE: No need to close the stream.
BloomFilter bloomFilter = BloomFilter.readFrom(
new ByteArrayInputStream(byteBuffer.array(), byteBuffer.arrayOffset() + byteBuffer.position(),
byteBuffer.remaining()), funnel);
BloomFilter bloomFilter = BloomFilter.readFrom(IdSets.toInputStream(byteBuffer), funnel);
return new BloomFilterIdSet(funnelType, bloomFilter, serializedSizeInBytes);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,14 @@
*/
package org.apache.pinot.core.query.utils.idset;

import com.fasterxml.jackson.databind.util.ByteBufferBackedInputStream;
import java.io.ByteArrayInputStream;
import java.io.IOException;
import java.io.InputStream;
import java.nio.ByteBuffer;
import java.util.Base64;
import java.util.List;
import java.util.stream.Collectors;
import org.apache.pinot.spi.data.FieldSpec.DataType;
import org.roaringbitmap.PeekableIntIterator;
import org.roaringbitmap.longlong.LongIterator;
Expand All @@ -38,6 +43,10 @@ private IdSets() {
public static final int DEFAULT_EXPECTED_INSERTIONS = 5_000_000;
public static final double DEFAULT_FPP = 0.03;

// Stored types whose values can be looked up in an IdSet
private static final List<DataType> LOOKUP_VALUE_TYPES =
List.of(DataType.INT, DataType.LONG, DataType.FLOAT, DataType.DOUBLE, DataType.STRING, DataType.BYTES);

/// Creates an IdSet.
///
/// @param dataType Data type of the ids
Expand Down Expand Up @@ -78,6 +87,40 @@ public static EmptyIdSet emptyIdSet() {
return EmptyIdSet.INSTANCE;
}

/// Validates that values of the given stored type can be looked up in the given IdSet. Rejects e.g. LONG values
/// in an IdSet built from INT values, where every lookup would otherwise throw or hash to the wrong bits. An empty
/// IdSet accepts every lookup type, as it records no value type.
///
/// @throws IllegalArgumentException if values of the stored type cannot be looked up in the IdSet
public static void validateValueType(IdSet idSet, DataType storedType) {
if (!LOOKUP_VALUE_TYPES.contains(storedType)) {
throw new IllegalArgumentException(
String.format("Cannot look up %s values in an IdSet, supported value types: %s", storedType,
LOOKUP_VALUE_TYPES));
}
List<DataType> valueTypes;
switch (idSet.getType()) {
case EMPTY:
return;
case ROARING_BITMAP:
valueTypes = List.of(DataType.INT);
break;
case ROARING_64_NAVIGABLE_MAP:
valueTypes = List.of(DataType.LONG);
break;
case BLOOM_FILTER:
valueTypes = ((BloomFilterIdSet) idSet).getValueTypes();
break;
default:
throw new IllegalStateException("Unsupported IdSet type: " + idSet.getType());
}
if (!valueTypes.contains(storedType)) {
throw new IllegalArgumentException(
String.format("Cannot look up %s values in an IdSet built from %s values", storedType,
valueTypes.stream().map(DataType::name).collect(Collectors.joining(" or "))));
}
}

/// Merges 2 IdSets, converts to BloomFilterIdSet if the size exceeds the size threshold.
///
/// @param idSet1 First IdSet to merge
Expand Down Expand Up @@ -187,7 +230,7 @@ public static IdSet fromBytes(byte[] bytes)
return fromByteBuffer(ByteBuffer.wrap(bytes));
}

/// Deserializes the IdSet from a ByteBuffer.
/// Deserializes the IdSet from a ByteBuffer, which may be read-only or direct (e.g. a multi-stage mailbox payload).
public static IdSet fromByteBuffer(ByteBuffer byteBuffer)
throws IOException {
byte typeId = byteBuffer.get();
Expand All @@ -201,7 +244,7 @@ public static IdSet fromByteBuffer(ByteBuffer byteBuffer)
case 3:
return BloomFilterIdSet.fromByteBuffer(byteBuffer);
default:
throw new IllegalStateException();
throw new IllegalStateException("Unsupported IdSet type id: " + typeId);
}
}

Expand All @@ -212,4 +255,15 @@ public static IdSet fromBase64String(String base64String)
throws IOException {
return fromBytes(Base64.getDecoder().decode(base64String));
}

/// Returns an [InputStream] over the remaining bytes of the given buffer, without moving its position. Reads the
/// backing array when the buffer exposes one; read-only and direct buffers do not, so they are read through a
/// duplicate.
static InputStream toInputStream(ByteBuffer byteBuffer) {
if (byteBuffer.hasArray()) {
return new ByteArrayInputStream(byteBuffer.array(), byteBuffer.arrayOffset() + byteBuffer.position(),
byteBuffer.remaining());
}
return new ByteBufferBackedInputStream(byteBuffer.duplicate());
}
}
Loading
Loading