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]