boneanxs commented on code in PR #8452:
URL: https://github.com/apache/hudi/pull/8452#discussion_r1184492603


##########
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:
   If `enableMetadataTable` was true, it will throw `TableNotFoundException` 
when creating `HoodieTableMetaClient` in `HoodieBackedTableMetadata`, now since 
we will also create `HoodieTableMetaClient` in `FileSystemBackedTableMetadata`, 
so now the same exception will be throw.



##########
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:
   Now we get field value using case sensitive resolution, I'm thinking maybe 
we also need to support case insensitive.
   
   Or maybe I should use case insensitive by default, since 
`spark.sql.caseSensitive` is default to 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:
   These expressions were first introduced in pr 
https://github.com/apache/hudi/pull/6725 to allow filter partitions when sync 
to HMS, so they were in `hudi-hive-sync` before, and now these expressions 
actually can be reused, so I moved them to `hudi-common` package



-- 
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]

Reply via email to