rdblue commented on code in PR #6651:
URL: https://github.com/apache/iceberg/pull/6651#discussion_r1101931339
##########
spark/v3.3/spark/src/main/java/org/apache/iceberg/spark/SparkReadConf.java:
##########
@@ -83,7 +83,11 @@ public Long endSnapshotId() {
}
public String branch() {
- return
confParser.stringConf().option(SparkReadOptions.BRANCH).parseOptional();
+ return confParser
+ .stringConf()
+ .sessionConf(SparkReadOptions.BRANCH)
Review Comment:
I don't think this should be the Spark read option. Session configs are
defined in `SparkSQLProperties` and use a form like `spark.sql.iceberg._____`.
In this case, how about `spark.sql.iceberg.read-branch`?
I think we also need to be careful about the behavior of this, and possibly
use a different `SparkReadConf` option for it. When the SQL read branch is set,
we want to use that if it is present and otherwise fall back to the main
branch. That may make this complicated enough that we should leave the SQL
session read branch out of this commit and add it in a follow-up.
##########
spark/v3.3/spark/src/main/java/org/apache/iceberg/spark/SparkUtil.java:
##########
@@ -295,4 +296,13 @@ public static List<Expression> partitionMapToExpression(
public static String toColumnName(NamedReference ref) {
return DOT.join(ref.fieldNames());
}
+
+ /** * method to get the latest snapshot using a branch */
Review Comment:
This has an extra `*` in the comment.
--
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
