You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@paimon.apache.org by "ulysses-you (via GitHub)" <gi...@apache.org> on 2024/09/06 03:01:25 UTC

[PR] [spark] Fix reported statistics does not do column pruning [paimon]

ulysses-you opened a new pull request, #4137:
URL: https://github.com/apache/paimon/pull/4137

   <!-- Please specify the module before the PR name: [core] ... or [flink] ... -->
   
   ### Purpose
   
   <!-- Linking this pull request to the issue -->
   `SupportsReportStatistics#estimateStatistics` should report the statistics after do column pruning, filter push down. This pr fixes it does not do column pruning. In addition, we should consider the metadata column size.
   
   Introduce `defaultSize` method for `DataType`, so that we can get the size in bytes without column statistics.
   
   <!-- What is the purpose of the change -->
   
   ### Tests
   
   <!-- List UT and IT cases to verify this change -->
   add test
   
   ### API and Format
   
   <!-- Does this change affect API or storage format -->
   no
   
   ### Documentation
   
   <!-- Does this change introduce a new feature -->
   


-- 
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: issues-unsubscribe@paimon.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] [spark] Fix reported statistics does not do column pruning [paimon]

Posted by "ulysses-you (via GitHub)" <gi...@apache.org>.
ulysses-you commented on PR #4137:
URL: https://github.com/apache/paimon/pull/4137#issuecomment-2345084975

   thank you @Zouxxyy. I tested with 20 columns and using `explain cost` to see the sinzeInBytes after doing analyze, it does affect:
   
   without this pr:
   
   ```
   explain cost select c1, c2, c3 from w_t;
   
   == Optimized Logical Plan ==
   RelationV2[c1#163, c2#164, c3#165] default.w_t, Statistics(sizeInBytes=36.8 MiB, rowCount=1.00E+5)
   
   explain cost select * from w_t;
   
   == Optimized Logical Plan ==
   RelationV2[c1#194, c2#195, c3#196, c4#197, c5#198, c6#199, c7#200, c8#201, c9#202, c10#203, c11#204, c12#205, c13#206, c14#207, c15#208, c16#209, c17#210, c18#211, c19#212, c20#213] default.w_t, Statistics(sizeInBytes=36.8 MiB, rowCount=1.00E+5)
   ```
   
   with this pr:
   ```
   explain cost select c1, c2, c3 from w_t;
   
   == Optimized Logical Plan ==
   RelationV2[c1#5, c2#6, c3#7] default.w_t, Statistics(sizeInBytes=5.5 MiB, rowCount=1.00E+5)
   
   explain cost select * from w_t;
   
   == Optimized Logical Plan ==
   RelationV2[c1#36, c2#37, c3#38, c4#39, c5#40, c6#41, c7#42, c8#43, c9#44, c10#45, c11#46, c12#47, c13#48, c14#49, c15#50, c16#51, c17#52, c18#53, c19#54, c20#55] default.w_t, Statistics(sizeInBytes=36.8 MiB, rowCount=1.00E+5)
   ```
   
   In general, the `sinzeInBytes` would affect broadcast join, runtime filter, etc. So it can happen that, nothing changes with pr if the query does not hit the related case.


-- 
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: issues-unsubscribe@paimon.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] [spark] Fix reported statistics does not do column pruning [paimon]

Posted by "Zouxxyy (via GitHub)" <gi...@apache.org>.
Zouxxyy commented on PR #4137:
URL: https://github.com/apache/paimon/pull/4137#issuecomment-2343930064

   @ulysses-you Thanks for the contribution, have you tested the effect in production scenarios?
   We tested the old codes and it had the same effect as parquet's analyze.


-- 
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: issues-unsubscribe@paimon.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] [spark] Fix reported statistics does not do column pruning [paimon]

Posted by "JingsongLi (via GitHub)" <gi...@apache.org>.
JingsongLi merged PR #4137:
URL: https://github.com/apache/paimon/pull/4137


-- 
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: issues-unsubscribe@paimon.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] [spark] Fix reported statistics does not do column pruning [paimon]

Posted by "JingsongLi (via GitHub)" <gi...@apache.org>.
JingsongLi commented on PR #4137:
URL: https://github.com/apache/paimon/pull/4137#issuecomment-2347982397

   +1


-- 
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: issues-unsubscribe@paimon.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] [spark] Fix reported statistics does not do column pruning [paimon]

Posted by "Zouxxyy (via GitHub)" <gi...@apache.org>.
Zouxxyy commented on code in PR #4137:
URL: https://github.com/apache/paimon/pull/4137#discussion_r1756273512


##########
paimon-spark/paimon-spark-common/src/main/scala/org/apache/paimon/spark/PaimonStatistics.scala:
##########
@@ -40,10 +40,53 @@ case class PaimonStatistics[T <: PaimonBaseScan](scan: T) extends Statistics {
 
   private lazy val paimonStats = if (scan.statistics.isPresent) scan.statistics.get() else null
 
-  lazy val paimonStatsEnabled: Boolean = paimonStats != null
+  lazy val paimonStatsEnabled: Boolean = {
+    paimonStats != null &&
+    paimonStats.mergedRecordSize().isPresent &&
+    paimonStats.mergedRecordCount().isPresent
+  }
+
+  private def getSizeForField(field: DataField): Long = {
+    Option(paimonStats.colStats().get(field.name()))
+      .map(_.avgLen())
+      .filter(_.isPresent)
+      .map(_.getAsLong)
+      .getOrElse(field.`type`().defaultSize().toLong)

Review Comment:
   Should we use the default size of Spark's filed here? because the data will actually be converted to Spark 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: issues-unsubscribe@paimon.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] [spark] Fix reported statistics does not do column pruning [paimon]

Posted by "ulysses-you (via GitHub)" <gi...@apache.org>.
ulysses-you commented on PR #4137:
URL: https://github.com/apache/paimon/pull/4137#issuecomment-2333226675

   cc @JingsongLi @Zouxxyy thank you


-- 
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: issues-unsubscribe@paimon.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] [spark] Fix reported statistics does not do column pruning [paimon]

Posted by "ulysses-you (via GitHub)" <gi...@apache.org>.
ulysses-you commented on code in PR #4137:
URL: https://github.com/apache/paimon/pull/4137#discussion_r1756349196


##########
paimon-spark/paimon-spark-common/src/main/scala/org/apache/paimon/spark/PaimonStatistics.scala:
##########
@@ -40,10 +40,53 @@ case class PaimonStatistics[T <: PaimonBaseScan](scan: T) extends Statistics {
 
   private lazy val paimonStats = if (scan.statistics.isPresent) scan.statistics.get() else null
 
-  lazy val paimonStatsEnabled: Boolean = paimonStats != null
+  lazy val paimonStatsEnabled: Boolean = {
+    paimonStats != null &&
+    paimonStats.mergedRecordSize().isPresent &&
+    paimonStats.mergedRecordCount().isPresent
+  }
+
+  private def getSizeForField(field: DataField): Long = {
+    Option(paimonStats.colStats().get(field.name()))
+      .map(_.avgLen())
+      .filter(_.isPresent)
+      .map(_.getAsLong)
+      .getOrElse(field.`type`().defaultSize().toLong)

Review Comment:
   I prefer to use Paimon's data type default size. During scan we have not converted to Spark row yet. Paimon returns `SparkInternalRow` to Spark which is maintained using Paimon's memory structure. The row conversion happens when Spark would wrap a Project. Then if there is a Project, Spark would re-calculate the sizeInBytes using Spark's data type default size.



-- 
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: issues-unsubscribe@paimon.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] [spark] Fix reported statistics does not do column pruning [paimon]

Posted by "Zouxxyy (via GitHub)" <gi...@apache.org>.
Zouxxyy commented on code in PR #4137:
URL: https://github.com/apache/paimon/pull/4137#discussion_r1756440521


##########
paimon-spark/paimon-spark-common/src/main/scala/org/apache/paimon/spark/PaimonStatistics.scala:
##########
@@ -40,10 +40,53 @@ case class PaimonStatistics[T <: PaimonBaseScan](scan: T) extends Statistics {
 
   private lazy val paimonStats = if (scan.statistics.isPresent) scan.statistics.get() else null
 
-  lazy val paimonStatsEnabled: Boolean = paimonStats != null
+  lazy val paimonStatsEnabled: Boolean = {
+    paimonStats != null &&
+    paimonStats.mergedRecordSize().isPresent &&
+    paimonStats.mergedRecordCount().isPresent
+  }
+
+  private def getSizeForField(field: DataField): Long = {
+    Option(paimonStats.colStats().get(field.name()))
+      .map(_.avgLen())
+      .filter(_.isPresent)
+      .map(_.getAsLong)
+      .getOrElse(field.`type`().defaultSize().toLong)

Review Comment:
   Got it!



-- 
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: issues-unsubscribe@paimon.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] [spark] Fix reported statistics does not do column pruning [paimon]

Posted by "JingsongLi (via GitHub)" <gi...@apache.org>.
JingsongLi commented on PR #4137:
URL: https://github.com/apache/paimon/pull/4137#issuecomment-2339858690

   @Zouxxyy We may need to run a TPC-DS test to show the compact.


-- 
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: issues-unsubscribe@paimon.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org