amogh-jahagirdar commented on code in PR #6575:
URL: https://github.com/apache/iceberg/pull/6575#discussion_r1073797662
##########
spark/v3.3/spark/src/main/java/org/apache/iceberg/spark/SparkCatalog.java:
##########
@@ -159,7 +160,15 @@ public Table loadTable(Identifier ident, String version)
throws NoSuchTableExcep
sparkTable.snapshotId() == null,
"Cannot do time-travel based on both table identifier and AS OF");
- return sparkTable.copyWithSnapshotId(Long.parseLong(version));
+ try {
Review Comment:
Currently there's no restrictions on what references can be named. For the
lookup code, I think we should always be able to differentiate between snapshot
ID and ref since for refs it will be in a quoted identifier, and should always
fail the Long.parseLong() with a NumberParseException.
But that's just me reading the code :), I think it's worth having a unit
test just for this case to give us that confidence that it works as expected in
this scenario. cc @jackye1995 let me know your thoughts
--
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-unsubscr...@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org
For additional commands, e-mail: issues-h...@iceberg.apache.org
