LuciferYang commented on code in PR #60025:
URL: https://github.com/apache/spark/pull/60025#discussion_r4464271876
##########
sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/statsEstimation/FilterEstimationSuite.scala:
##########
@@ -963,6 +965,31 @@ class FilterEstimationSuite extends
StatsEstimationTestBase {
expectedRowCount = 10)
}
+ test("SPARK-60810: predicates on a read-side padded CHAR column use the
column's stats") {
+ val padded = CharVarcharUtils.addPaddingForScan(
+
DataTypeUtils.toAttribute(CharVarcharUtils.replaceCharVarcharWithStringInSchema(
+ StructType(Seq(StructField("cstring",
CharType(2))))).head).withExprId(attrString.exprId))
+ assert(padded match {
+ case CharVarcharUtils.ReadSidePadding(a: Attribute, _) => a.exprId ==
attrString.exprId
+ case _ => false
+ }, s"expected read-side padding over cstring, got $padded")
+ val child = childStatsTestPlan(Seq(attrString), tableRowCount = 10L)
+ // colStatString has 10 distinct values and no nulls over 10 rows.
+ Seq(
+ EqualTo(padded, Literal("A0")) -> 1,
+ EqualTo(Literal("A0"), padded) -> 1,
+ In(padded, Seq(Literal("A0"), Literal("A1"))) -> 2,
+ InSet(padded, Set(UTF8String.fromString("A0"),
UTF8String.fromString("A1"))) -> 2,
+ IsNull(padded) -> 0).foreach { case (condition, expectedRows) =>
+ val filter = Filter(condition, child)
+ assert(filter.stats.rowCount.contains(BigInt(expectedRows)), condition)
Review Comment:
Done in 1a264a7dee5. The equality case cannot tell the update from plain
scaling, since both leave a distinct count of 1, so the test checks the output
stat of `IS NOT NULL` over a child with 4 nulls instead: the update sets
`nullCount` to 0, where scaling alone would give 3. I also added the
`Not(EqualTo(...))` case.
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/util/CharVarcharUtils.scala:
##########
@@ -391,6 +391,22 @@ object CharVarcharUtils extends Logging with
SparkCharVarcharUtils {
}.getOrElse(attr)
}
+ /**
+ * Matches the read-side CHAR padding that [[addPaddingForScan]] wraps a
column in (when
+ * standard semantics are off) and extracts the padded expression and the
length argument.
+ */
+ private[sql] object ReadSidePadding {
+ def unapply(e: Expression): Option[(Expression, Expression)] = e match {
+ case s: StaticInvoke
+ if s.scalarFunction.isEmpty &&
+ s.staticObject == classOf[CharVarcharCodegenUtils] &&
+ s.functionName == READ_SIDE_PADDING &&
Review Comment:
I'll take this into SPARK-60830.
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/statsEstimation/FilterEstimation.scala:
##########
@@ -170,7 +173,11 @@ case class FilterEstimation(
* It returns None if the condition is not supported.
*/
def calculateSingleCondition(condition: Expression, update: Boolean):
Option[Double] = {
- condition match {
+ val unpadded = condition.mapChildren {
Review Comment:
I'd leave this as is. The rebuilt predicate is dropped right away, and
matching an "attribute or its padding" extractor would touch every branch of
the match.
--
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]
