bvaradar commented on code in PR #8452: URL: https://github.com/apache/hudi/pull/8452#discussion_r1183257664
########## hudi-common/src/main/java/org/apache/hudi/expression/Predicates.java: ########## @@ -0,0 +1,400 @@ +/* + * 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.hudi.expression; + +import org.apache.hudi.internal.schema.Type; + +import java.util.ArrayList; +import java.util.Collections; +import java.util.Comparator; +import java.util.List; +import java.util.Set; +import java.util.stream.Collectors; + +public class Predicates { + + public static True alwaysTrue() { + return True.get(); + } + + public static False alwaysFalse() { + return False.get(); + } + + public static And and(Expression left, Expression right) { + return new And(left, right); + } + + public static Or or(Expression left, Expression right) { + return new Or(left, right); + } + + public static BinaryComparison gt(Expression left, Expression right) { + return new BinaryComparison(left, Expression.Operator.GT, right); + } + + public static BinaryComparison lt(Expression left, Expression right) { + return new BinaryComparison(left, Expression.Operator.LT, right); + } + + public static BinaryComparison eq(Expression left, Expression right) { + return new BinaryComparison(left, Expression.Operator.EQ, right); + } + + public static BinaryComparison gteq(Expression left, Expression right) { + return new BinaryComparison(left, Expression.Operator.GT_EQ, right); + } + + public static BinaryComparison lteq(Expression left, Expression right) { + return new BinaryComparison(left, Expression.Operator.LT_EQ, right); + } + + public static BinaryComparison startsWith(Expression left, Expression right) { + return new BinaryComparison(left, Expression.Operator.STARTS_WITH, right); + } + + public static StringContains contains(Expression left, Expression right) { + return new StringContains(left, right); + } + + public static In in(Expression left, List<Expression> validExpressions) { + return new In(left, validExpressions); + } + + public static IsNull isNull(Expression child) { + return new IsNull(child); + } + + public static IsNotNull isNotNull(Expression child) { + return new IsNotNull(child); + } + + public static Not not(Expression expr) { + return new Not(expr); + } + + public static class True extends LeafExpression implements Predicate { Review Comment: Rename True to TrueExpression as it is very similar to Boolean type. Same for False ########## hudi-common/src/main/java/org/apache/hudi/expression/ArrayData.java: ########## @@ -16,22 +16,25 @@ * limitations under the License. */ -package org.apache.hudi.hive.expression; Review Comment: Any specific reason why we are renaming packages ? ########## hudi-common/src/main/java/org/apache/hudi/expression/BindVisitor.java: ########## @@ -0,0 +1,179 @@ +/* + * 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.hudi.expression; + +import org.apache.hudi.internal.schema.Types; + +import java.util.List; +import java.util.stream.Collectors; + +public class BindVisitor implements ExpressionVisitor<Expression> { + + protected final Types.RecordType recordType; + protected final boolean caseSensitive; + + public BindVisitor(Types.RecordType recordType, boolean caseSensitive) { + this.recordType = recordType; + this.caseSensitive = caseSensitive; + } + + @Override + public Expression alwaysTrue() { + return Predicates.True.get(); + } + + @Override + public Expression alwaysFalse() { + return Predicates.False.get(); + } + + @Override + public Expression visitAnd(Predicates.And and) { + if (and.getLeft() instanceof Predicates.False + || and.getRight() instanceof Predicates.False) { + return alwaysFalse(); + } + + Expression left = and.getLeft().accept(this); + Expression right = and.getRight().accept(this); + if (left instanceof Predicates.False + || right instanceof Predicates.False) { + return alwaysFalse(); + } + + if (left instanceof Predicates.True + && right instanceof Predicates.True) { + return alwaysTrue(); + } + + if (left instanceof Predicates.True) { + return right; + } + + if (right instanceof Predicates.True) { + return left; + } + + return Predicates.and(left, right); + } + + @Override + public Expression visitOr(Predicates.Or or) { + if (or.getLeft() instanceof Predicates.True + || or.getRight() instanceof Predicates.True) { + return alwaysTrue(); + } + + Expression left = or.getLeft().accept(this); + Expression right = or.getRight().accept(this); + if (left instanceof Predicates.True + || right instanceof Predicates.True) { + return alwaysTrue(); + } + + if (left instanceof Predicates.False + && right instanceof Predicates.False) { + return alwaysFalse(); + } + + if (left instanceof Predicates.False) { + return right; + } + + if (right instanceof Predicates.False) { + return left; + } + + return Predicates.or(left, right); + } + + @Override + public Expression visitLiteral(Literal literal) { + return literal; + } + + @Override + public Expression visitNameReference(NameReference attribute) { + // TODO Should consider caseSensitive? Review Comment: Related point: What is the general case sensitivity approach we have taken here for partition push down filters ? ########## hudi-common/src/main/java/org/apache/hudi/expression/BinaryLike.java: ########## @@ -16,46 +16,26 @@ * limitations under the License. */ -package org.apache.hudi.hive.expression; +package org.apache.hudi.expression; import java.util.Arrays; +import java.util.List; /** * The expression that accept two child expressions. */ -public class BinaryOperator extends Expression { +public abstract class BinaryLike implements Expression { Review Comment: BinaryExpression sounds more appropriate ? ########## hudi-client/hudi-spark-client/src/test/java/org/apache/hudi/io/storage/row/TestHoodieRowCreateHandle.java: ########## @@ -190,16 +189,8 @@ public void testInstantiationFailure(boolean enableMetadataTable) { HoodieTable table = HoodieSparkTable.create(cfg, context, metaClient); new HoodieRowCreateHandle(table, cfg, " def", UUID.randomUUID().toString(), "001", RANDOM.nextInt(100000), RANDOM.nextLong(), RANDOM.nextLong(), SparkDatasetTestUtils.STRUCT_TYPE); fail("Should have thrown exception"); - } catch (HoodieInsertException ioe) { Review Comment: why are we changing this testcase ? -- 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]
