the-other-tim-brown commented on code in PR #428:
URL: https://github.com/apache/incubator-xtable/pull/428#discussion_r1587049624
##########
core/src/main/java/org/apache/xtable/delta/DeltaSchemaExtractor.java:
##########
@@ -183,10 +189,11 @@ private InternalSchema toInternalSchema(
case "timestamp":
type = InternalType.TIMESTAMP;
// Timestamps in Delta are microsecond precision by default
- metadata =
- Collections.singletonMap(
- InternalSchema.MetadataKey.TIMESTAMP_PRECISION,
- InternalSchema.MetadataValue.MICROS);
+ metadata = DEFAULT_TIMESTAMP_PRECISION_METADATA;
+ break;
+ case "timestamp_ntz":
Review Comment:
The test class should also get a small update
https://github.com/apache/incubator-xtable/blob/main/core/src/test/java/org/apache/xtable/delta/TestDeltaSchemaExtractor.java
##########
.vscode/settings.json:
##########
@@ -0,0 +1,3 @@
+{
Review Comment:
Can you update the `.gitignore` with `.vscode/*` as well?
##########
core/src/main/java/org/apache/xtable/delta/DeltaConversionTarget.java:
##########
@@ -268,9 +281,23 @@ private void commitTransaction() {
}
private Map<String, String> getConfigurationsForDeltaSync() {
+
+ // The output of detail() method has only one row with the following
schema.
+ // deltaTable.detail() is added to the constructor for this class, and
sets
+ // a private variable to of deltaTable
+
+ // limit the results to the attributes needed
+ Dataset<Row> record = deltaTable.detail().select("minWriterVersion",
"minReaderVersion");
Review Comment:
Can you get this information from `deltaLog.snapshot().metadata()`?
##########
core/src/main/java/org/apache/xtable/delta/DeltaConversionTarget.java:
##########
@@ -268,9 +281,23 @@ private void commitTransaction() {
}
private Map<String, String> getConfigurationsForDeltaSync() {
+
Review Comment:
We can test this
[here](https://github.com/apache/incubator-xtable/blob/main/core/src/test/java/org/apache/xtable/delta/ITDeltaConversionTargetSource.java)
I would expect we can add the timestamp_ntz and see that the delta table has
the correct versions set for min reader/writer?
--
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: commits-unsubscr...@xtable.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
