cloud-fan commented on code in PR #58145:
URL: https://github.com/apache/spark/pull/58145#discussion_r3882909925


##########
sql/catalyst/src/test/scala/org/apache/spark/sql/connector/catalog/InMemoryTableWithTableSample.scala:
##########
@@ -367,3 +371,69 @@ class InMemoryTableWithLegacyTableSample(
         seed: Long): Boolean = true
   }
 }
+
+/**
+ * Sample table plus Catalyst advisory filters. V1 `SupportsPushDownFilters` 
cannot
+ * mix with `SupportsPushDownCatalystFilters` (`pushedFilters` return types 
clash),
+ * so this wraps the sample builder. The advisory SQL is specified by the 
`advisory-filter`
+ * property.
+ */
+class InMemoryTableWithTableSampleAndAdvisoryFilters(
+    name: String,
+    columns: Array[Column],
+    partitioning: Array[Transform],
+    properties: util.Map[String, String])
+  extends InMemoryTableWithTableSample(name, columns, partitioning, 
properties) {
+
+  override def newScanBuilder(options: CaseInsensitiveStringMap): ScanBuilder 
= {
+    new SampleAndAdvisoryScanBuilder(new 
InMemoryTableSampleScanBuilder(schema, options), schema)
+  }
+
+  private class SampleAndAdvisoryScanBuilder(
+      inner: InMemoryTableSampleScanBuilder,
+      tableSchema: StructType)
+    extends ScanBuilder
+      with SupportsPushDownTableSample
+      with SupportsPushDownCatalystFilters {
+
+    override def pushTableSample(
+        lowerBound: Double,
+        upperBound: Double,
+        withReplacement: Boolean,
+        seed: Long): Boolean =
+      inner.pushTableSample(lowerBound, upperBound, withReplacement, seed)
+
+    override def pushTableSample(
+        lowerBound: Double,
+        upperBound: Double,
+        withReplacement: Boolean,
+        seed: Long,
+        sampleMethod: SampleMethod): Boolean =
+      inner.pushTableSample(lowerBound, upperBound, withReplacement, seed, 
sampleMethod)
+
+    override def pushFilters(filters: Seq[Expression]): Seq[Expression] = Nil

Review Comment:
   **Non-blocking:** `pushFilters` returns `Nil`, which declares every 
predicate fully handled, but this wrapper neither evaluates the predicates nor 
forwards them to `inner`. With `WHERE id >= 3`, the scan still returns rows 1 
and 2 and Spark adds no `FilterExec`; the current `id >= 1` test masks this 
because all fixture rows pass. Please return the filters (or actually 
enforce/forward them), and use a predicate that rejects at least one fixture 
row.



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