LuciferYang commented on code in PR #60025:
URL: https://github.com/apache/spark/pull/60025#discussion_r4464270762
##########
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 {
+ case CharVarcharUtils.ReadSidePadding(attr: Attribute, _) => attr
+ case other => other
+ }
+ unpadded match {
Review Comment:
Done in 1a264a7dee5. `evaluateEquality` now ignores a histogram for string
and binary columns and uses the distinct count, and `JoinEstimation` takes the
histogram path only when the intersected bounds exist, which they never do for
string and binary keys. This fixes STRING columns as well. Since it is a few
lines in the same estimators and this PR is what sends CHAR columns there, I
kept it here rather than in a separate JIRA. Without it, the new tests fail
with the `MatchError` you describe and with a `NoSuchElementException` from the
join path.
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/statsEstimation/FilterEstimation.scala:
##########
@@ -161,7 +162,9 @@ case class FilterEstimation(
/**
* Returns a percentage of rows meeting a single condition in Filter node.
* Currently we only support binary predicates where one side is a column,
- * and the other is a literal.
+ * and the other is a literal. A side may also be the read-side CHAR padding
of a column, which
+ * is what a predicate pushed below the padding Project compares. It is
estimated as the column
+ * itself, as `EstimationUtils.getAliasStats` does above the padding Project.
Review Comment:
Added to the description: with CBO or plan statistics enabled, the pruned
relation's size now comes from the estimated row count, which can be too small
for a skewed partition, as it already is for STRING partition columns.
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/statsEstimation/EstimationUtils.scala:
##########
@@ -87,6 +88,15 @@ object EstimationUtils {
expressions.collect {
case alias @ Alias(attr: Attribute, _) if attributeStats.contains(attr)
=>
alias.toAttribute -> attributeStats(attr)
+ // Read-side CHAR padding wraps each CHAR column of a scan. With padding
on, ANALYZE TABLE
+ // reads the table through it as well, so the statistics already
describe padded values.
+ // Statistics collected otherwise (with padding off, or reported by a
connector) can
+ // describe shorter values, and every non-null padded value has at least
N characters.
+ case alias @ Alias(CharVarcharUtils.ReadSidePadding(attr: Attribute,
Literal(n: Int, _)), _)
+ if attributeStats.contains(attr) =>
+ val stat = attributeStats(attr)
+ alias.toAttribute -> stat.copy(
+ avgLen = stat.avgLen.map(_.max(n)), maxLen =
stat.maxLen.map(_.max(n)))
Review Comment:
Done in 1a264a7dee5. The floor is skipped when the distinct count is 0, and
`ProjectEstimationSuite` covers an all-null `CHAR(30)` column, whose lengths
stay at 20. A stat without a distinct count still gets the floor.
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/statsEstimation/EstimationUtils.scala:
##########
@@ -87,6 +88,15 @@ object EstimationUtils {
expressions.collect {
case alias @ Alias(attr: Attribute, _) if attributeStats.contains(attr)
=>
alias.toAttribute -> attributeStats(attr)
+ // Read-side CHAR padding wraps each CHAR column of a scan. With padding
on, ANALYZE TABLE
+ // reads the table through it as well, so the statistics already
describe padded values.
+ // Statistics collected otherwise (with padding off, or reported by a
connector) can
+ // describe shorter values, and every non-null padded value has at least
N characters.
Review Comment:
Done in 1a264a7dee5, with your wording plus a sentence on the distinct count
of 0 from comment 9.
--
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]
