amogh-jahagirdar commented on code in PR #6575:
URL: https://github.com/apache/iceberg/pull/6575#discussion_r1073797220
##########
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, since the ref is in 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().
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: [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]
