szehon-ho commented on code in PR #7732: URL: https://github.com/apache/iceberg/pull/7732#discussion_r1847150692
########## spark/v3.4/spark/src/test/java/org/apache/iceberg/spark/source/TestIcebergSourceTablesBase.java: ########## @@ -1953,6 +1953,65 @@ public void testTableWithInt96Timestamp() throws IOException { } } + @Test + public void testSessionConfigSupport() { + PartitionSpec spec = PartitionSpec.builderFor(SCHEMA).identity("id").build(); + TableIdentifier tableIdentifier = TableIdentifier.of("db", "session_config_table"); + Table table = createTable(tableIdentifier, SCHEMA, spec); + + List<SimpleRecord> records = + Lists.newArrayList( + new SimpleRecord(1, "a"), new SimpleRecord(2, "b"), new SimpleRecord(3, "c")); + + Dataset<Row> df = spark.createDataFrame(records, SimpleRecord.class); + + df.select("id", "data") + .write() + .format("iceberg") + .mode(SaveMode.Append) + .save(loadLocation(tableIdentifier)); + + long snapshotId = table.currentSnapshot().snapshotId(); + + withSQLConf( + // set write option through session configuration + ImmutableMap.of("spark.datasource.iceberg.overwrite-mode", "dynamic"), Review Comment: Yea sorry, i was not clear. Its just a suggestion. I think, because the test is to test SessionConfigSupport functionality only, it may be more clear for the reader if the first check (on write part) is like: ``` 'spark.datasource.iceberg.snapshot-property.foo=bar' and then check if foo is set on latest snapshot summary? ``` Because i think the reader of the test need to know what is 'dynamic overwrite' mode to understand the assert (its not related to the feature), whereas the above is a bit more self-explanatory imo. I think the read part is decently understandable without additional context. -- 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