deniskuzZ commented on code in PR #6789:
URL: https://github.com/apache/hive/pull/6789#discussion_r4375378312
##########
iceberg/iceberg-handler/src/main/java/org/apache/iceberg/mr/mapreduce/IcebergInputFormat.java:
##########
@@ -72,31 +72,36 @@
*/
public class IcebergInputFormat<T> extends InputFormat<Void, T> {
- private static TableScan createTableScan(Table table, Configuration conf) {
+ private static TableScan createTableScan(Table table, Configuration conf,
Table serverSidePinTable) {
TableScan scan = table.newScan();
- long snapshotId = -1;
- try {
- snapshotId = conf.getLong(InputFormatConfig.SNAPSHOT_ID, -1);
- } catch (NumberFormatException e) {
- String version = conf.get(InputFormatConfig.SNAPSHOT_ID);
- SnapshotRef ref = table.refs().get(version);
- if (ref == null) {
- throw new RuntimeException("Cannot find matching snapshot ID or
reference name for version " + version);
+ if (HiveTableUtil.hasExplicitScanSnapshotBinding(conf)) {
+ long snapshotId = -1;
+ try {
+ snapshotId = conf.getLong(InputFormatConfig.SNAPSHOT_ID, -1);
+ } catch (NumberFormatException e) {
+ String version = conf.get(InputFormatConfig.SNAPSHOT_ID);
+ Table refsTable = serverSidePinTable != null ? serverSidePinTable :
table;
+ SnapshotRef ref = refsTable.refs().get(version);
+ if (ref == null) {
+ throw new RuntimeException("Cannot find matching snapshot ID or
reference name for version " + version);
+ }
+ snapshotId = ref.snapshotId();
+ }
+ String refName = conf.get(InputFormatConfig.OUTPUT_TABLE_SNAPSHOT_REF);
+ if (StringUtils.isNotEmpty(refName)) {
Review Comment:
the `useRef` exception for branches with an older schema makes sense since
there's no public API to pin a snapshot while keeping the table schema. But in
that case a commit to the branch between compile and split time still changes
what we read. Could we fail fast there instead of silently reading the new
head, e.g.
````
TableScan pinned = scan.useRef(refName);
Preconditions.checkState(pinned.snapshot().snapshotId() == ref.snapshotId(),
"Branch %s moved since query compilation", refName);
````
Also, when the ref is missing from the serialized table we still fall back
to the live `useRef`. Could that throw, as in the original suggestion?
--
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]
