kbendick commented on a change in pull request #2757:
URL: https://github.com/apache/iceberg/pull/2757#discussion_r670214102
##########
File path: spark/src/test/java/org/apache/iceberg/spark/data/AvroDataTest.java
##########
@@ -185,4 +190,51 @@ public void testMixedTypes() throws IOException {
writeAndValidate(schema);
}
+
+ @Test
+ public void testTimestampWithoutZone() throws IOException {
+ withSQLConf(ImmutableMap.of(SparkUtil.HANDLE_TIMESTAMP_WITHOUT_TIMEZONE,
"true"), () -> {
+ Schema schema = TypeUtil.assignIncreasingFreshIds(new Schema(
+ required(0, "id", LongType.get()),
+ optional(1, "ts_without_zone",
Types.TimestampType.withoutZone())));
+
+ writeAndValidate(schema);
+ });
+ }
+
+ protected void withSQLConf(Map<String, String> conf, Action action) throws
IOException {
+ SQLConf sqlConf = SQLConf.get();
+
+ Map<String, String> currentConfValues = Maps.newHashMap();
+ conf.keySet().forEach(confKey -> {
+ if (sqlConf.contains(confKey)) {
+ String currentConfValue = sqlConf.getConfString(confKey);
+ currentConfValues.put(confKey, currentConfValue);
+ }
+ });
+
+ conf.forEach((confKey, confValue) -> {
+ if (SQLConf.staticConfKeys().contains(confKey)) {
+ throw new RuntimeException("Cannot modify the value of a static
config: " + confKey);
+ }
+ sqlConf.setConfString(confKey, confValue);
+ });
+
+ try {
+ action.invoke();
+ } finally {
+ conf.forEach((confKey, confValue) -> {
+ if (currentConfValues.containsKey(confKey)) {
+ sqlConf.setConfString(confKey, currentConfValues.get(confKey));
+ } else {
+ sqlConf.unsetConf(confKey);
+ }
+ });
+ }
+ }
+
+ @FunctionalInterface
+ protected interface Action {
+ void invoke() throws IOException;
+ }
Review comment:
As a follow up PR (in a separate PR either before or after this one is
merged), particularly if you're looking for some more work to do to contribute
to the project, you _might_ explore if this combination of `withSQLConf` and
the corresponding `@FunctionalInterface` can be abstracted into their own
interface that one could mix into tests.
I'm not 100% sure how that would look, maybe an interface like
`ConfigurableTestSQLConf` or something?
Again, just copying it for now is fine, but it would be nice to reduce the
code duplication and make this easier for others to use in the future. Your
exploration might find that it’s better to not do that (I’m more of a Scala
developer myself and so to me it feels like a mixin). Something to think about
for later though!
--
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]
