anoopj commented on code in PR #17735:
URL: https://github.com/apache/iceberg/pull/17735#discussion_r3824081782
##########
core/src/main/java/org/apache/iceberg/SnapshotScan.java:
##########
@@ -83,10 +83,12 @@ protected ScanMetrics scanMetrics() {
protected Map<Integer, PartitionSpec> specs() {
Map<Integer, PartitionSpec> specs = table().specs();
// requires latest schema
- if (!useSnapshotSchema()
- || snapshotId() == null
- || table().currentSnapshot() == null
- || snapshotId().equals(table().currentSnapshot().snapshotId())) {
+ if (!useSnapshotSchema() || snapshotId() == null) {
+ return specs;
+ }
+
+ Snapshot currentSnapshot = table().currentSnapshot();
+ if (currentSnapshot != null &&
snapshotId().equals(currentSnapshot.snapshotId())) {
return specs;
Review Comment:
We are still short circuiting on snapshot identity, but the thing that
actually determines whether we need to rebind is schema identity. There is a
separate bug that we could fix. The scenario is pure schema changes that don't
produce snapshot. In that case, the current snapshot can outlive its schema,
and the same bug reappears. e.g.
1. append -> snapshot S1 under schema A
2. Schema change to deletes a column "data". Current schema is now B, but S1
is still the current snapshot
3. `useSnapshot(S1_id).filter(equal("data", ...))` returns B-bound specs,
and residual binding throws Cannot find field 'data'
Comparing schema ids instead covers both this scenario and the empty-main
case you are trying to fix. So you would replace line 90-96 with:
```
// table specs are bound to the current schema; rebind only when
scanning a different schema
Schema snapshotSchema = tableSchema();
if (snapshotSchema.schemaId() == table().schema().schemaId()) {
return specs;
}
```
I'm OK if if you want to do it as a separate PR, but this seem simple enough
to just do it in one go.
--
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]
