szehon-ho commented on code in PR #58702:
URL: https://github.com/apache/spark/pull/58702#discussion_r3993906764
##########
sql/catalyst/src/test/scala/org/apache/spark/sql/connector/catalog/InMemoryTableWithV2Filter.scala:
##########
@@ -203,14 +203,23 @@ object InMemoryTableWithV2Filter {
}
}
+ /**
+ * Whether every predicate has a shape [[evalPredicate]] can evaluate: a
plain column, or a
+ * column and a literal. A predicate over an expression, e.g. a cast, is not
supported and
+ * returned to Spark, as a real connector without expression support would
do.
+ */
def supportsPredicates(predicates: Array[Predicate]): Boolean = {
- predicates.flatMap(splitAnd).forall {
- case p: Predicate if p.name().equals("=") => true
- case p: Predicate if p.name().equals("<=>") => true
- case p: Predicate if p.name().equals("IS_NULL") => true
- case p: Predicate if p.name().equals("IS_NOT_NULL") => true
- case p: Predicate if p.name().equals("ALWAYS_TRUE") => true
- case _ => false
+ predicates.flatMap(splitAnd).forall { p =>
Review Comment:
suggestion:
```
def supportsPredicates(predicates: Array[Predicate]): Boolean = {
predicates.flatMap(splitAnd).forall { p =>
(p.name(), p.children().toSeq) match {
case ("=" | "<=>", Seq(_: NamedReference, _: LiteralValue[_])) => true
case ("IS_NULL" | "IS_NOT_NULL", Seq(_: NamedReference)) => true
case ("ALWAYS_TRUE", _) => true
case _ => false
}
}
}
```
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/internal/connector/PartitionPredicateField.scala:
##########
@@ -20,14 +20,19 @@ package org.apache.spark.sql.internal.connector
import org.apache.spark.sql.catalyst.expressions.AttributeReference
/**
- * Metadata for one partition field.
+ * Metadata for one field of `Table.partitioning()`. A partition predicate is
built over the
+ * fields in partitioning order, so their ordinals match the partition key a
connector passes to
+ * `PartitionPredicate.eval`.
*
* @param fieldNames the multi-part field name from the table's partitioning
- * (e.g. `Seq("s", "tz")`).
- * @param attrRef the [[AttributeReference]] for the partition field.
- * Created from the resolved partition field so it carries the
- * flattened dotted name (e.g. `"s.tz"`) for nested fields.
+ * (e.g. `Seq("s", "tz")`) for an identity transform, or
the transform's
+ * description (e.g. `Seq("bucket(4, id)")`) otherwise.
+ * @param attrRef the [[AttributeReference]] a filter can reference, for an
identity transform.
Review Comment:
nit: we can say 'for now Spark doesnt support'. it was in the plan but
never implemented yet
--
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]
