Repository navigation
Make IDSET and IN_ID_SET work in the multi-stage engine #19757
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,169 @@ | ||
| /** | ||
| * 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. | ||
| /// | ||
| /// The flag does not stop the multi-stage planner from pushing `IN_ID_SET` below a join into a leaf stage, because | ||
| /// the planner uses the operator of the `IN_ID_SET` transform function, which is deterministic. The operator table | ||
| /// registers transform functions before scalar functions, and skips this function because its name is taken. If | ||
| /// the transform function's operator is removed, this function's volatile operator takes its place, and | ||
| /// `PinotRuleUtils.isRelocatable` then keeps `IN_ID_SET` above joins. Tests in `JoinPlans.json` and | ||
| /// `PhysicalOptimizerPlans.json` pin the push-down. | ||
| @Nullable | ||
| @ScalarFunction(nullableParameters = true, isDeterministic = false) | ||
| 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 |
|---|---|---|
|
|
@@ -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: | ||
|
|
@@ -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); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This changes existing results. With null handling on, The PR description explains it, but I think it needs the backward-incompat label and a release note, plus a line in the
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done in 021374b. I added the One note on |
||
| } | ||
|
|
||
| @Override | ||
| public int[] transformToIntValuesSV(ValueBlock valueBlock) { | ||
| int length = valueBlock.getNumDocs(); | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
isDeterministic = falseworks around literal typing (BigDecimal for an INT literal), but it also marks the function volatile. That is harmless for planning today only becausePinotOperatorTableregistersTransformFunctionType.IN_ID_SETbefore 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
PinotSqlFunctioninstead. ThenPinotRuleUtils.isRelocatable()returns false andPinotFilterJoinRulestops pushingIN_ID_SETbelow 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
getStoredTypeand keep the function deterministic.There was a problem hiding this comment.
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
IdSetFunctionsand atTransformFunctionType.IN_ID_SET, and new cases inJoinPlans.jsonandPhysicalOptimizerPlans.jsonpin the push-down. When I made theIN_ID_SEToperator volatile, both cases failed, with the filter above the join.I kept
isDeterministic = falseinstead 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 thatPinotEvaluateLiteralRulepasses raw Calcite values (BigDecimal for every exact number) toObjectparameters, where the runtime passes Integer or Long. From the code, this also changes fold-time results of otherObject-parameter functions, such astoThetaSketch. So I think the fix belongs in the rule, in a separate PR.