yyanyy commented on code in PR #57487:
URL: https://github.com/apache/spark/pull/57487#discussion_r3687048878
##########
sql/catalyst/src/main/java/org/apache/spark/sql/connector/read/SupportsReportStatistics.java:
##########
@@ -36,4 +38,50 @@ public interface SupportsReportStatistics extends Scan {
* Returns the estimated statistics of this data source scan.
*/
Statistics estimateStatistics();
+
+ /**
+ * Returns the estimated size in bytes of this scan without computing full
statistics.
+ * <p>
+ * When cost-based optimization or plan statistics are disabled, Spark
primarily needs the scan's
+ * size in bytes (for example, for broadcast-join thresholding). This method
lets connectors that
+ * can produce a size estimate cheaply serve it directly and avoid computing
the full statistics.
+ * <p>
+ * The default implementation returns {@code OptionalLong.empty()},
signalling that the connector
+ * does not offer a cheap size estimate. In that case Spark falls back to
+ * {@link #estimateStatistics()}, so a connector that only implements {@link
#estimateStatistics()}
+ * keeps the same size-estimation behavior it had before this method
existed. Connectors override
+ * this method only when they have a genuinely cheaper size estimate than
+ * {@link #estimateStatistics()}.
+ *
+ * @since 4.3.0
+ */
+ default OptionalLong estimateSizeInBytes() {
Review Comment:
This is actually to address the comment in
https://github.com/apache/spark/pull/57487#discussion_r3670460884 ; with the
old code, the size-only path (computeStats when both CBO and plan-stats are
disabled) goes through V2StatisticsUtils.computeSizeInBytes, which is
structured as: (A) try the cheap estimateSizeInBytes(), else (B) fall back to
reading full estimateStatistics() once and deriving the size from sizeInBytes,
or from numRows * avgRowSize.
Then for a connector reporting `numRows` but no `sizeInBytes` and do not do
override `estimateSizeInBytes`, this will result in
1. (A): `estimateSizeInBytes` -> `estimateStatistics` then return empty
2. fall back to (B): `estimateStatistics` again to recover `numRows`
An alternative could be to restructure the fallback to avoid the second
call, but I feel that the current approach is cleaner.
--
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]
