sunchao commented on a change in pull request #33803:
URL: https://github.com/apache/spark/pull/33803#discussion_r696115604



##########
File path: 
sql/catalyst/src/main/java/org/apache/spark/sql/connector/expressions/filter/AlwaysTrue.java
##########
@@ -0,0 +1,36 @@
+/*
+ * 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.spark.sql.connector.expressions.filter;
+
+import org.apache.spark.annotation.Evolving;
+import org.apache.spark.sql.connector.expressions.NamedReference;
+
+/**
+ * A filter that always evaluates to `true`.
+ *
+ * @since 3.3.0
+ */
+@Evolving
+public final class AlwaysTrue extends Filter {
+
+  @Override
+  public String toString() { return "AlwaysTrue"; }

Review comment:
       nit: should we just name this 'TRUE'? 

##########
File path: 
sql/catalyst/src/main/java/org/apache/spark/sql/connector/expressions/filter/AlwaysFalse.java
##########
@@ -0,0 +1,36 @@
+/*
+ * 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.spark.sql.connector.expressions.filter;
+
+import org.apache.spark.annotation.Evolving;
+import org.apache.spark.sql.connector.expressions.NamedReference;
+
+/**
+ * A filter that always evaluates to `false`.
+ *
+ * @since 3.3.0
+ */
+@Evolving
+public final class AlwaysFalse extends Filter {
+
+  @Override
+  public String toString() { return "AlwaysFalse"; }

Review comment:
       nit: should we just name this 'FALSE'?

##########
File path: 
sql/catalyst/src/main/java/org/apache/spark/sql/connector/expressions/filter/EqualTo.java
##########
@@ -0,0 +1,53 @@
+/*
+ * 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.spark.sql.connector.expressions.filter;
+
+import org.apache.spark.annotation.Evolving;
+import org.apache.spark.sql.connector.expressions.FieldReference;
+import org.apache.spark.sql.connector.expressions.Literal;
+import org.apache.spark.sql.connector.expressions.NamedReference;
+
+/**
+ * A filter that evaluates to `true` iff the field evaluates to a value
+ * equal to `value`.
+ *
+ * @since 3.3.0
+ */
+@Evolving
+public final class EqualTo<T> extends Filter {
+  private final FieldReference column;
+  private final Literal<T> value;
+
+  public EqualTo(FieldReference column, Literal<T> value) {
+    this.column = column;
+    this.value = value;
+  }
+
+  public FieldReference column() { return column; }
+  public Literal<T> value() { return value; }
+
+  @Override
+  public String toString() { return column.describe() + " = " + 
value.describe(); }
+
+  @Override
+  public NamedReference[] references() {
+    NamedReference[] arr = new NamedReference[1];

Review comment:
       nit: this can just be one-line: `return new NamedReference[] { column };`

##########
File path: 
sql/catalyst/src/main/java/org/apache/spark/sql/connector/expressions/filter/In.java
##########
@@ -0,0 +1,83 @@
+/*
+ * 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.spark.sql.connector.expressions.filter;
+
+import java.util.Arrays;
+
+import org.apache.spark.annotation.Evolving;
+import org.apache.spark.sql.connector.expressions.FieldReference;
+import org.apache.spark.sql.connector.expressions.Literal;
+import org.apache.spark.sql.connector.expressions.NamedReference;
+
+/**
+ * A filter that evaluates to `true` iff the field evaluates to one of the 
values in the array.
+ *
+ * @since 3.3.0
+ */
+@Evolving
+public final class In<T> extends Filter {
+  private final FieldReference column;
+  private final Literal<T>[] values;
+
+  public In(FieldReference column, Literal<T>[] values) {
+    this.column = column;
+    this.values = values;
+  }
+
+  public FieldReference column() { return column; }
+  public Literal<T>[] values() { return values; }
+
+  @Override
+  public int hashCode() {
+    int h = column.hashCode();
+    for (Literal v : values) {

Review comment:
       can we auto-generate these and use `Arrays.hashCode`? what I got is:
   ```java
     @Override
     public boolean equals(Object o) {
       if (this == o) return true;
       if (o == null || getClass() != o.getClass()) return false;
       In<?> in = (In<?>) o;
       return Objects.equals(column, in.column) && Arrays.equals(values, 
in.values);
     }
   
     @Override
     public int hashCode() {
       int result = Objects.hash(column);
       result = 31 * result + Arrays.hashCode(values);
       return result;
     }
   ```

##########
File path: 
sql/catalyst/src/main/java/org/apache/spark/sql/connector/expressions/filter/AlwaysFalse.java
##########
@@ -0,0 +1,36 @@
+/*
+ * 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.spark.sql.connector.expressions.filter;
+
+import org.apache.spark.annotation.Evolving;
+import org.apache.spark.sql.connector.expressions.NamedReference;
+
+/**
+ * A filter that always evaluates to `false`.
+ *
+ * @since 3.3.0
+ */
+@Evolving
+public final class AlwaysFalse extends Filter {
+
+  @Override
+  public String toString() { return "AlwaysFalse"; }
+
+  @Override
+  public NamedReference[] references() { return new NamedReference[0]; }

Review comment:
       nit: we can just return `EMPTY_REFERNCE` from the parent class?

##########
File path: 
sql/catalyst/src/main/java/org/apache/spark/sql/connector/expressions/filter/StringContains.java
##########
@@ -0,0 +1,52 @@
+/*
+ * 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.spark.sql.connector.expressions.filter;
+
+import org.apache.spark.annotation.Evolving;
+import org.apache.spark.sql.connector.expressions.FieldReference;
+import org.apache.spark.sql.connector.expressions.NamedReference;
+
+/**
+ * A filter that evaluates to `true` iff the field evaluates to
+ * a string that contains `value`.
+ *
+ * @since 3.3.0
+ */
+@Evolving
+public final class StringContains extends Filter {
+  private final FieldReference column;
+  private final String value;
+
+  public StringContains(FieldReference column, String value) {
+    this.column = column;
+    this.value = value;
+  }
+
+  public FieldReference column() { return column; }
+  public String value() { return value; }
+
+  @Override
+  public String toString() { return column.describe() + " StringContains " + 
value; }

Review comment:
       nit: maybe we can use format like `STRING_CONTAINS(column.describe(), 
value)`

##########
File path: 
sql/catalyst/src/main/java/org/apache/spark/sql/connector/expressions/filter/IsNull.java
##########
@@ -0,0 +1,48 @@
+/*
+ * 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.spark.sql.connector.expressions.filter;
+
+import org.apache.spark.annotation.Evolving;
+import org.apache.spark.sql.connector.expressions.FieldReference;
+import org.apache.spark.sql.connector.expressions.NamedReference;
+
+/**
+ * A filter that evaluates to `true` iff the field evaluates to null.
+ *
+ * @since 3.3.0
+ */
+@Evolving
+public final class IsNull extends Filter {
+  private final FieldReference column;
+
+  public IsNull(FieldReference column) {
+    this.column = column;
+  }
+
+  public FieldReference column() { return column; }
+
+  @Override
+  public String toString() { return column.describe() + " IsNull"; }

Review comment:
       nit: maybe `IS NULL`?

##########
File path: 
sql/catalyst/src/main/java/org/apache/spark/sql/connector/expressions/filter/And.java
##########
@@ -0,0 +1,53 @@
+/*
+ * 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.spark.sql.connector.expressions.filter;
+
+import org.apache.spark.annotation.Evolving;
+import org.apache.spark.sql.connector.expressions.NamedReference;
+
+/**
+ * A filter that evaluates to `true` iff both `left` and `right` evaluate to 
`true`.
+ *
+ * @since 3.3.0
+ */
+@Evolving
+public final class And extends Filter {
+  private final Filter left;
+  private final Filter right;
+
+  public And(Filter left, Filter right) {
+    this.left = left;
+    this.right = right;
+  }
+
+  public Filter left() { return left; }
+  public Filter right() { return right; }
+
+  @Override
+  public String toString() { return left.describe() + " and " + 
right.describe(); }

Review comment:
       nit: maybe capital `and` -> `AND`.

##########
File path: 
sql/catalyst/src/main/java/org/apache/spark/sql/connector/expressions/filter/IsNotNull.java
##########
@@ -0,0 +1,48 @@
+/*
+ * 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.spark.sql.connector.expressions.filter;
+
+import org.apache.spark.annotation.Evolving;
+import org.apache.spark.sql.connector.expressions.FieldReference;
+import org.apache.spark.sql.connector.expressions.NamedReference;
+
+/**
+ * A filter that evaluates to `true` iff the field evaluates to a non-null 
value.
+ *
+ * @since 3.3.0
+ */
+@Evolving
+public final class IsNotNull extends Filter {
+  private final FieldReference column;
+
+  public IsNotNull(FieldReference column) {
+    this.column = column;
+  }
+
+  public FieldReference column() { return column; }
+
+  @Override
+  public String toString() { return column.describe() + " IsNotNull"; }

Review comment:
       nit: maybe `IS NOT NULL`? 

##########
File path: 
sql/catalyst/src/main/java/org/apache/spark/sql/connector/expressions/filter/In.java
##########
@@ -0,0 +1,83 @@
+/*
+ * 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.spark.sql.connector.expressions.filter;
+
+import java.util.Arrays;
+
+import org.apache.spark.annotation.Evolving;
+import org.apache.spark.sql.connector.expressions.FieldReference;
+import org.apache.spark.sql.connector.expressions.Literal;
+import org.apache.spark.sql.connector.expressions.NamedReference;
+
+/**
+ * A filter that evaluates to `true` iff the field evaluates to one of the 
values in the array.
+ *
+ * @since 3.3.0
+ */
+@Evolving
+public final class In<T> extends Filter {
+  private final FieldReference column;
+  private final Literal<T>[] values;
+
+  public In(FieldReference column, Literal<T>[] values) {
+    this.column = column;
+    this.values = values;
+  }
+
+  public FieldReference column() { return column; }
+  public Literal<T>[] values() { return values; }
+
+  @Override
+  public int hashCode() {
+    int h = column.hashCode();
+    for (Literal v : values) {
+      h *= 41;
+      h += v.hashCode();
+    }
+    return h;
+  }
+
+  @Override
+  public boolean equals(Object obj) {
+    if (obj instanceof In) {
+      return (((In) obj).column.equals(column) && Arrays.equals(((In) 
obj).values, values));
+    } else {
+      return false;
+    }
+  }
+
+  @Override
+  public String toString() {
+    StringBuilder str = new StringBuilder();

Review comment:
       can just be one-liner?
   ```java
   
Arrays.stream(values).map(Expression::describe).collect(Collectors.joining(", 
"));
   ```

##########
File path: 
sql/catalyst/src/main/java/org/apache/spark/sql/connector/expressions/filter/EqualTo.java
##########
@@ -0,0 +1,53 @@
+/*
+ * 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.spark.sql.connector.expressions.filter;
+
+import org.apache.spark.annotation.Evolving;
+import org.apache.spark.sql.connector.expressions.FieldReference;
+import org.apache.spark.sql.connector.expressions.Literal;
+import org.apache.spark.sql.connector.expressions.NamedReference;
+
+/**
+ * A filter that evaluates to `true` iff the field evaluates to a value
+ * equal to `value`.
+ *
+ * @since 3.3.0
+ */
+@Evolving
+public final class EqualTo<T> extends Filter {
+  private final FieldReference column;

Review comment:
       can this just be an `Expression`? I'm thinking whether this can allow us 
to pushdown transforms & functions in future, such as: `bucket(col) == 4`. 
Depending on the storage layout, data sources can optimize this.
   
   
   

##########
File path: 
sql/catalyst/src/main/java/org/apache/spark/sql/connector/expressions/filter/Filter.java
##########
@@ -0,0 +1,48 @@
+/*
+ * 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.spark.sql.connector.expressions.filter;
+
+import org.apache.spark.annotation.Evolving;
+import org.apache.spark.sql.connector.expressions.Expression;
+import org.apache.spark.sql.connector.expressions.NamedReference;
+
+/**
+ * Filter base class
+ *
+ * @since 3.3.0
+ */
+@Evolving
+public abstract class Filter implements Expression {
+
+  private static final NamedReference[] EMPTY_REFERENCE = new 
NamedReference[0];

Review comment:
       nit: space after




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



---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to