Copilot commented on code in PR #4992: URL: https://github.com/apache/calcite/pull/4992#discussion_r3356834379
########## arrow/src/main/java/org/apache/calcite/adapter/arrow/ArrowJavaFilterEnumerator.java: ########## @@ -0,0 +1,238 @@ +/* + * 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.adapter.arrow; + +import org.apache.calcite.util.ImmutableIntList; +import org.apache.calcite.util.Util; + +import org.apache.arrow.vector.ValueVector; +import org.apache.arrow.vector.VectorSchemaRoot; +import org.apache.arrow.vector.ipc.ArrowFileReader; +import org.apache.arrow.vector.ipc.message.ArrowRecordBatch; +import org.apache.arrow.vector.types.pojo.Field; +import org.apache.arrow.vector.types.pojo.Schema; + +import java.io.IOException; +import java.math.BigDecimal; +import java.util.ArrayList; +import java.util.List; +import java.util.regex.Pattern; + +import static java.util.Objects.requireNonNull; + +/** + * Enumerator that evaluates Arrow filter tokens in Java. + */ +class ArrowJavaFilterEnumerator extends AbstractArrowEnumerator { + private final List<List<ConditionToken>> conditions; + private final Schema schema; + private final Runnable onClose; + private final List<ValueVector> filterVectors; + + ArrowJavaFilterEnumerator(ArrowFileReader arrowFileReader, + ImmutableIntList fields, List<List<List<String>>> conditions, + Schema schema, Runnable onClose) { + super(arrowFileReader, fields); + this.conditions = toConditionTokens(conditions); + this.schema = schema; + this.onClose = onClose; + this.filterVectors = new ArrayList<>(schema.getFields().size()); + } + + @Override protected void evaluateOperator(ArrowRecordBatch arrowRecordBatch) { + } + + @Override protected void loadNextArrowBatch() { + super.loadNextArrowBatch(); + final VectorSchemaRoot root; + try { + root = arrowFileReader.getVectorSchemaRoot(); + } catch (IOException e) { + throw Util.toUnchecked(e); + } + filterVectors.clear(); + for (int i = 0; i < schema.getFields().size(); i++) { + filterVectors.add(root.getVector(i)); + } + } + + @Override public boolean moveNext() { + while (true) { + if (currRowIndex >= rowCount - 1) { + final boolean hasNextBatch; + try { + hasNextBatch = arrowFileReader.loadNextBatch(); + } catch (IOException e) { + throw Util.toUnchecked(e); + } + if (!hasNextBatch) { + return false; + } + currRowIndex = -1; + this.valueVectors.clear(); + loadNextArrowBatch(); + } + currRowIndex++; + if (matches(currRowIndex)) { + return true; + } + } + } + + private boolean matches(int rowIndex) { + for (List<ConditionToken> orGroup : conditions) { + boolean any = false; + for (ConditionToken token : orGroup) { + if (matches(token, rowIndex)) { + any = true; + break; + } + } + if (!any) { + return false; + } + } + return true; + } + + private boolean matches(ConditionToken token, int rowIndex) { + final Object value = getValue(fieldVector(token.fieldName), rowIndex); + switch (token.operator) { + case IS_NULL: + return value == null; + case IS_NOT_NULL: + return value != null; + case IS_TRUE: + return Boolean.TRUE.equals(value); + case IS_FALSE: + return Boolean.FALSE.equals(value); + case IS_NOT_TRUE: + return !Boolean.TRUE.equals(value); + case IS_NOT_FALSE: + return !Boolean.FALSE.equals(value); + case EQUAL: + return value != null && compare(value, literal(token)) == 0; + case NOT_EQUAL: + return value != null && compare(value, literal(token)) != 0; + case LESS_THAN: + return value != null && compare(value, literal(token)) < 0; + case LESS_THAN_OR_EQUAL: + return value != null && compare(value, literal(token)) <= 0; + case GREATER_THAN: + return value != null && compare(value, literal(token)) > 0; + case GREATER_THAN_OR_EQUAL: + return value != null && compare(value, literal(token)) >= 0; + case LIKE: + return value != null + && like(value.toString(), requireNonNull(token.value, "value")); + default: + throw new AssertionError("Unhandled Arrow filter operator: " + token.operator); + } + } + + private ValueVector fieldVector(String fieldName) { + final Field field = schema.findField(fieldName); + final int index = schema.getFields().indexOf(field); + if (index < 0) { + throw new IllegalArgumentException("Unknown Arrow field: " + fieldName); + } + return filterVectors.get(index); + } + + private static Object literal(ConditionToken token) { + final String type = requireNonNull(token.valueType, "valueType"); + final String value = requireNonNull(token.value, "value"); + if (type.startsWith("decimal")) { + return new BigDecimal(value); + } else if (type.equals("integer")) { + return Integer.valueOf(value); + } else if (type.equals("long")) { + return Long.valueOf(value); + } else if (type.equals("float")) { + return Float.valueOf(value); + } else if (type.equals("double")) { + return Double.valueOf(value); + } else if (type.equals("string")) { + return unquote(value); + } + throw new UnsupportedOperationException("Unsupported literal type: " + type); + } + + private static int compare(Object left, Object right) { + if (left instanceof BigDecimal || right instanceof BigDecimal) { + return toBigDecimal(left).compareTo(toBigDecimal(right)); + } + if (left instanceof Number && right instanceof Number) { + return Double.compare(((Number) left).doubleValue(), + ((Number) right).doubleValue()); + } + return left.toString().compareTo(right.toString()); + } + + private static BigDecimal toBigDecimal(Object value) { + if (value instanceof BigDecimal) { + return (BigDecimal) value; + } + return new BigDecimal(value.toString()); + } + + private static boolean like(String value, String pattern) { + return Pattern.compile(toRegex(unquote(pattern)), Pattern.DOTALL) + .matcher(value).matches(); + } Review Comment: `like(...)` compiles a new regex `Pattern` for every row evaluation. For LIKE-heavy queries this will be a major hotspot; caching compiled patterns (keyed by the unquoted LIKE pattern) avoids repeated compilation and reduces GC pressure. ########## arrow/src/main/java/org/apache/calcite/adapter/arrow/ArrowJavaFilterEnumerator.java: ########## @@ -0,0 +1,238 @@ +/* + * 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.adapter.arrow; + +import org.apache.calcite.util.ImmutableIntList; +import org.apache.calcite.util.Util; + +import org.apache.arrow.vector.ValueVector; +import org.apache.arrow.vector.VectorSchemaRoot; +import org.apache.arrow.vector.ipc.ArrowFileReader; +import org.apache.arrow.vector.ipc.message.ArrowRecordBatch; +import org.apache.arrow.vector.types.pojo.Field; +import org.apache.arrow.vector.types.pojo.Schema; + +import java.io.IOException; +import java.math.BigDecimal; +import java.util.ArrayList; +import java.util.List; +import java.util.regex.Pattern; + +import static java.util.Objects.requireNonNull; + +/** + * Enumerator that evaluates Arrow filter tokens in Java. + */ +class ArrowJavaFilterEnumerator extends AbstractArrowEnumerator { + private final List<List<ConditionToken>> conditions; + private final Schema schema; + private final Runnable onClose; + private final List<ValueVector> filterVectors; + + ArrowJavaFilterEnumerator(ArrowFileReader arrowFileReader, + ImmutableIntList fields, List<List<List<String>>> conditions, + Schema schema, Runnable onClose) { + super(arrowFileReader, fields); + this.conditions = toConditionTokens(conditions); + this.schema = schema; + this.onClose = onClose; + this.filterVectors = new ArrayList<>(schema.getFields().size()); + } + + @Override protected void evaluateOperator(ArrowRecordBatch arrowRecordBatch) { + } + + @Override protected void loadNextArrowBatch() { + super.loadNextArrowBatch(); + final VectorSchemaRoot root; + try { + root = arrowFileReader.getVectorSchemaRoot(); + } catch (IOException e) { + throw Util.toUnchecked(e); + } + filterVectors.clear(); + for (int i = 0; i < schema.getFields().size(); i++) { + filterVectors.add(root.getVector(i)); + } + } Review Comment: `loadNextArrowBatch` currently delegates to `super.loadNextArrowBatch()`, which always constructs an `ArrowRecordBatch` via `VectorUnloader` and calls `evaluateOperator(...)`. Since `evaluateOperator` is empty here, that record-batch work is unnecessary overhead on every batch. Consider inlining the minimal logic needed (populate projection vectors, set `rowCount`, populate `filterVectors`) without creating an `ArrowRecordBatch`. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
