omalley commented on a change in pull request #647: URL: https://github.com/apache/orc/pull/647#discussion_r585069591
########## File path: java/core/src/java/org/apache/orc/OrcFilterContext.java ########## @@ -0,0 +1,88 @@ +/* + * 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.orc; + +import org.apache.hadoop.hive.ql.exec.vector.ColumnVector; +import org.apache.hadoop.hive.ql.exec.vector.ListColumnVector; +import org.apache.hadoop.hive.ql.exec.vector.MapColumnVector; +import org.apache.hadoop.hive.ql.exec.vector.VectorizedRowBatch; +import org.apache.hadoop.hive.ql.io.filter.MutableFilterContext; + +/** + * This defines the input for any filter operation. This is an extension of + * [[{@link VectorizedRowBatch}]] with schema. + * <p> + * This offers a convenience method of finding the column vector from a given column name + * that the filters can invoke to get access to the column vector. + */ +public interface OrcFilterContext extends MutableFilterContext { + /** + * Retrieves the column vector that matches the specified name. Allows support for nested struct + * references e.g. order.date where data is a field in a struct called order. + * + * @param name The column name whose vector should be retrieved + * @return The column vector + * @throws IllegalArgumentException if the field is not found or if the nested field is not part + * of a struct + */ + ColumnVector[] findColumnVector(String name); + + /** + * Utility method for determining if the leaf vector of the branch can be treated as having + * noNulls. + * This method navigates from the top to the leaf and checks if we have nulls anywhere in the + * branch as compared to checking just the leaf vector. + * + * @param vectorBranch The input vector branch from the root to the leaf + * @return true if the entire branch satisfies noNull else false + */ + static boolean noNulls(ColumnVector[] vectorBranch) { + for (ColumnVector v : vectorBranch) { + if (!v.noNulls) { + return false; + } + } + return true; + } + + /** + * Utility method for determining if a particular row element in the vector branch is null. + * This method navigates from the top to the leaf and checks if we have nulls anywhere in the + * branch as compared to checking just the leaf vector. + * + * @param vectorBranch The input vector branch from the root to the leaf + * @param idx The row index being tested + * @return true if the entire branch is not null for the idx otherwise false + * @throws IllegalArgumentException If a multivalued vector such as List or Map is encountered in + * the branch. + */ + static boolean isNull(ColumnVector[] vectorBranch, int idx) throws IllegalArgumentException { + for (ColumnVector v : vectorBranch) { + if (v instanceof ListColumnVector || v instanceof MapColumnVector) { + throw new IllegalArgumentException(String.format( + "Found vector: %s in branch. List and Map vectors are not supported in isNull " + + "determination", v)); + } + if (!v.noNulls && v.isNull[idx]) { Review comment: This should also honor v.isRepeating. ########## File path: java/core/src/test/org/apache/orc/TestOrcFilterContext.java ########## @@ -0,0 +1,202 @@ +/* + * 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.orc; + +import org.apache.hadoop.hive.ql.exec.vector.BytesColumnVector; +import org.apache.hadoop.hive.ql.exec.vector.ColumnVector; +import org.apache.hadoop.hive.ql.exec.vector.ListColumnVector; +import org.apache.hadoop.hive.ql.exec.vector.LongColumnVector; +import org.apache.hadoop.hive.ql.exec.vector.MapColumnVector; +import org.apache.hadoop.hive.ql.exec.vector.StructColumnVector; +import org.apache.hadoop.hive.ql.exec.vector.UnionColumnVector; +import org.apache.orc.impl.OrcFilterContextImpl; +import org.junit.Before; +import org.junit.Test; + +import static org.hamcrest.CoreMatchers.containsString; +import static org.hamcrest.CoreMatchers.instanceOf; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertThrows; +import static org.junit.Assert.assertTrue; +import static org.junit.jupiter.api.Assertions.assertEquals; + +public class TestOrcFilterContext { + private final TypeDescription schema = TypeDescription.createStruct() Review comment: It would probably be easier to use TypeDescription.fromString(...). ########## File path: java/core/src/java/org/apache/orc/impl/ParserUtils.java ########## @@ -244,80 +250,172 @@ public static TypeDescription parseType(ParserUtils.StringPosition source) { private static final Pattern INTEGER_PATTERN = Pattern.compile("^[0-9]+$"); + public static ColumnVector[] findBranchVectors(TypeDescription schema, Review comment: I think this would be better named findColumnVectors. ########## File path: java/core/src/java/org/apache/orc/impl/ParserUtils.java ########## @@ -244,80 +250,172 @@ public static TypeDescription parseType(ParserUtils.StringPosition source) { private static final Pattern INTEGER_PATTERN = Pattern.compile("^[0-9]+$"); + public static ColumnVector[] findBranchVectors(TypeDescription schema, + ParserUtils.StringPosition source, + VectorizedRowBatch batch) { + List<String> names = ParserUtils.splitName(source); + return vectorWalker.get().find(schema, names, true, batch).vectorBranch; + } + public static TypeDescription findSubtype(TypeDescription schema, ParserUtils.StringPosition source) { return findSubtype(schema, source, true); } - public static TypeDescription findSubtype(TypeDescription schema, - ParserUtils.StringPosition source, - boolean isSchemaEvolutionCaseAware) { - List<String> names = ParserUtils.splitName(source); - if (names.size() == 1 && INTEGER_PATTERN.matcher(names.get(0)).matches()) { - return schema.findSubtype(Integer.parseInt(names.get(0))); + private static final ThreadLocal<TypeWalker> typeWalker = ThreadLocal.withInitial(TypeWalker::new); + private static class TypeWalker { + private List<String> names; + private boolean caseAware; + + // Walk specific + TypeDescription current = null; + + private TypeWalker() { } - TypeDescription current = SchemaEvolution.checkAcidSchema(schema) - ? SchemaEvolution.getBaseRow(schema) : schema; - while (names.size() > 0) { - String first = names.remove(0); - switch (current.getCategory()) { - case STRUCT: { - int posn = -1; - if (isSchemaEvolutionCaseAware) { - posn = current.getFieldNames().indexOf(first); - } else { - // Case-insensitive search like ORC 1.5 - for (int i = 0; i < current.getFieldNames().size(); i++) { - if (current.getFieldNames().get(i).equalsIgnoreCase(first)) { - posn = i; - break; + + protected void handleLevel(int posn) { + this.current = current.getChildren().get(posn); + } + + private void findInternal() { Review comment: I think I'd prefer to leave the findInternal as a static method that takes a visitor. I've posted a branch with such a change on my github. https://github.com/omalley/orc/tree/orc-755 ########## File path: java/core/src/java/org/apache/orc/impl/ParserUtils.java ########## @@ -244,80 +250,172 @@ public static TypeDescription parseType(ParserUtils.StringPosition source) { private static final Pattern INTEGER_PATTERN = Pattern.compile("^[0-9]+$"); + public static ColumnVector[] findBranchVectors(TypeDescription schema, + ParserUtils.StringPosition source, + VectorizedRowBatch batch) { + List<String> names = ParserUtils.splitName(source); + return vectorWalker.get().find(schema, names, true, batch).vectorBranch; + } + public static TypeDescription findSubtype(TypeDescription schema, ParserUtils.StringPosition source) { return findSubtype(schema, source, true); } - public static TypeDescription findSubtype(TypeDescription schema, - ParserUtils.StringPosition source, - boolean isSchemaEvolutionCaseAware) { - List<String> names = ParserUtils.splitName(source); - if (names.size() == 1 && INTEGER_PATTERN.matcher(names.get(0)).matches()) { - return schema.findSubtype(Integer.parseInt(names.get(0))); + private static final ThreadLocal<TypeWalker> typeWalker = ThreadLocal.withInitial(TypeWalker::new); Review comment: I don't think we need the ThreadLocals. This code is only called at setup. ---------------------------------------------------------------- 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. For queries about this service, please contact Infrastructure at: [email protected]
