rdblue commented on a change in pull request #3722:
URL: https://github.com/apache/iceberg/pull/3722#discussion_r770139467
##########
File path:
spark/v3.2/spark/src/main/java/org/apache/iceberg/spark/source/SparkTable.java
##########
@@ -186,22 +184,37 @@ public ScanBuilder
newScanBuilder(CaseInsensitiveStringMap options) {
icebergTable.refresh();
}
- SparkScanBuilder scanBuilder = new SparkScanBuilder(sparkSession(),
icebergTable, options);
-
- if (requestedSchema != null) {
- scanBuilder.pruneColumns(requestedSchema);
- }
-
- return scanBuilder;
+ // If the table is loaded using the Spark DataFrame API, and
option("snapshot-id", <snapshot_id>)
+ // or option("as-of-timestamp", <timestamp>) is applied to the
DataFrameReader, SparkTable will be
+ // constructed with a non-null snapshotId. Subsequently
SparkTable#newScanBuilder will be called
+ // with the options, which will include "snapshot-id" or "as-of-timestamp".
+ // On the other hand, if the table is loaded using SQL, with the table
suffixed with a snapshot
+ // or timestamp selector, then SparkTable will be constructed with a
non-null snapshotId, but
+ // SparkTable#newScanBuilder will be called without the "snapshot-id" or
"as-of-timestamp" option.
+ // We therefore add a "snapshot-id" option here in this latter case.
+ CaseInsensitiveStringMap scanOptions =
Review comment:
I'm not sure this is worth the complexity. Why not just always add the
snapshot ID?
--
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]
