dajiaohuang commented on code in PR #984:
URL: https://github.com/apache/incubator-xtable/pull/984#discussion_r4382719653
##########
xtable-core/src/main/java/org/apache/xtable/kernel/DeltaKernelSchemaExtractor.java:
##########
@@ -381,10 +380,9 @@ private DataType convertFieldType(InternalField field) {
private FieldMetadata getFieldMetadata(InternalField field) {
FieldMetadata.Builder metadataBuilder = FieldMetadata.builder();
- // Propagate column mapping ID to maintain Delta column mapping
- if (field.getFieldId() != null) {
- metadataBuilder.putLong(DELTA_COLUMN_MAPPING_ID,
field.getFieldId().longValue());
- }
+ // Column-mapping IDs are only valid when the target enables Delta column
mapping. This
Review Comment:
That is a reasonable alternative if the Delta Kernel target is intended to
enable Delta column mapping whenever source field IDs are present. This PR
follows #936’s stated expected behavior: when the target does not enable column
mapping, omit the annotations and keep the reader 1 / writer 2 protocol. At
this PR head, getConfigurationsForDeltaSync() configures XTable metadata and
log retention but does not enable column mapping. Enabling it would change the
target’s table configuration/protocol behavior, rather than just remove
metadata that is invalid under the current configuration. I kept this fix
scoped to the reported failure; enabling column mapping would need a deliberate
target-compatibility change.
--
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]
