Skip to content

Commit b90b38e

Browse files
committed
Address comments
1 parent 3250ba3 commit b90b38e

File tree

4 files changed

+39
-14
lines changed

4 files changed

+39
-14
lines changed

pom.xml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,7 @@
8787
<spotless.version>2.43.0</spotless.version>
8888
<apache.rat.version>0.16.1</apache.rat.version>
8989
<google.java.format.version>1.8</google.java.format.version>
90-
<delta.standalone.version>0.5.0</delta.standalone.version>
90+
<delta.standalone.version>3.3.0</delta.standalone.version>
9191
<delta.hive.version>3.0.0</delta.hive.version>
9292
<project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
9393
<target.dir.pattern>**/target/**</target.dir.pattern>

xtable-core/src/main/java/org/apache/xtable/avro/AvroSchemaConverter.java

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -160,12 +160,14 @@ private InternalSchema toInternalSchema(
160160
metadata.put(
161161
InternalSchema.MetadataKey.TIMESTAMP_PRECISION, InternalSchema.MetadataValue.MICROS);
162162
} else if (logicalType instanceof LogicalTypes.LocalTimestampMillis) {
163-
// TODO: Hudi 0.x writes INT64 in parquet, TimestampNTZType support added in 1.x
163+
// TODO: https://github.com/apache/incubator-xtable/issues/672
164+
// Hudi 0.x writes INT64 in parquet, TimestampNTZType support added in 1.x
164165
newDataType = InternalType.LONG;
165166
metadata.put(
166167
InternalSchema.MetadataKey.TIMESTAMP_PRECISION, InternalSchema.MetadataValue.MILLIS);
167168
} else if (logicalType instanceof LogicalTypes.LocalTimestampMicros) {
168-
// TODO: Hudi 0.x writes INT64 in parquet, TimestampNTZType support added in 1.x
169+
// TODO: https://github.com/apache/incubator-xtable/issues/672
170+
// Hudi 0.x writes INT64 in parquet, TimestampNTZType support added in 1.x
169171
newDataType = InternalType.LONG;
170172
metadata.put(
171173
InternalSchema.MetadataKey.TIMESTAMP_PRECISION, InternalSchema.MetadataValue.MICROS);

xtable-core/src/test/java/org/apache/xtable/delta/TestDeltaSchemaExtractor.java

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -322,13 +322,36 @@ public void testTimestamps() {
322322
.metadata(metadata)
323323
.build())
324324
.defaultValue(InternalField.Constants.NULL_DEFAULT_VALUE)
325+
.build(),
326+
InternalField.builder()
327+
.name("requiredTimestampNtz")
328+
.schema(
329+
InternalSchema.builder()
330+
.name("timestamp_ntz")
331+
.dataType(InternalType.TIMESTAMP_NTZ)
332+
.isNullable(false)
333+
.metadata(metadata)
334+
.build())
335+
.build(),
336+
InternalField.builder()
337+
.name("optionalTimestampNtz")
338+
.schema(
339+
InternalSchema.builder()
340+
.name("timestamp_ntz")
341+
.dataType(InternalType.TIMESTAMP_NTZ)
342+
.isNullable(true)
343+
.metadata(metadata)
344+
.build())
345+
.defaultValue(InternalField.Constants.NULL_DEFAULT_VALUE)
325346
.build()))
326347
.build();
327348

328349
StructType structRepresentationTimestamp =
329350
new StructType()
330351
.add("requiredTimestamp", DataTypes.TimestampType, false)
331-
.add("optionalTimestamp", DataTypes.TimestampType, true);
352+
.add("optionalTimestamp", DataTypes.TimestampType, true)
353+
.add("requiredTimestampNtz", DataTypes.TimestampNTZType, false)
354+
.add("optionalTimestampNtz", DataTypes.TimestampNTZType, true);
332355

333356
Assertions.assertEquals(
334357
internalSchemaTimestamp,

xtable-core/src/test/java/org/apache/xtable/delta/TestDeltaSync.java

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,6 @@
7878
import io.delta.standalone.expressions.EqualTo;
7979
import io.delta.standalone.expressions.Expression;
8080
import io.delta.standalone.expressions.Literal;
81-
import io.delta.standalone.internal.exception.DeltaErrors;
8281
import io.delta.standalone.types.IntegerType;
8382
import io.delta.standalone.types.StringType;
8483

@@ -96,6 +95,7 @@
9695
import org.apache.xtable.model.storage.DataLayoutStrategy;
9796
import org.apache.xtable.model.storage.FileFormat;
9897
import org.apache.xtable.model.storage.InternalDataFile;
98+
import org.apache.xtable.model.storage.InternalFile;
9999
import org.apache.xtable.model.storage.PartitionFileGroup;
100100
import org.apache.xtable.model.storage.TableFormat;
101101
import org.apache.xtable.schema.SchemaFieldFinder;
@@ -460,17 +460,10 @@ public void testTimestampNtz() {
460460

461461
TableFormatSync.getInstance()
462462
.syncSnapshot(Collections.singletonList(conversionTarget), snapshot1);
463-
// Delta standalone library can't read versions (3,7) and needs delta kernel dependency.
464-
Assertions.assertThrows(
465-
DeltaErrors.InvalidProtocolVersionException.class,
466-
() ->
467-
validateDeltaTable(basePath, new HashSet<>(Arrays.asList(dataFile1, dataFile2)), null));
463+
validateDeltaTableUsingSpark(basePath, new HashSet<>(Arrays.asList(dataFile1, dataFile2)));
468464
TableFormatSync.getInstance()
469465
.syncSnapshot(Collections.singletonList(conversionTarget), snapshot2);
470-
Assertions.assertThrows(
471-
DeltaErrors.InvalidProtocolVersionException.class,
472-
() ->
473-
validateDeltaTable(basePath, new HashSet<>(Arrays.asList(dataFile1, dataFile2)), null));
466+
validateDeltaTableUsingSpark(basePath, new HashSet<>(Arrays.asList(dataFile2, dataFile3)));
474467
}
475468

476469
private static Stream<Arguments> timestampPartitionTestingArgs() {
@@ -514,6 +507,13 @@ private void validateDeltaTable(
514507
internalDataFiles.size(), count, "Number of files from DeltaScan don't match expectation");
515508
}
516509

510+
private void validateDeltaTableUsingSpark(
511+
Path basePath, Set<InternalDataFile> internalDataFiles) {
512+
Dataset<Row> dataset = sparkSession.read().format("delta").load(basePath.toString());
513+
long countFromFiles = internalDataFiles.stream().mapToLong(InternalFile::getRecordCount).sum();
514+
Assertions.assertEquals(countFromFiles, dataset.count());
515+
}
516+
517517
private InternalSnapshot buildSnapshot(
518518
InternalTable table, String sourceIdentifier, InternalDataFile... dataFiles) {
519519
return InternalSnapshot.builder()

0 commit comments

Comments
 (0)