Akash3121 commented on code in PR #942:
URL: https://github.com/apache/incubator-xtable/pull/942#discussion_r4211653135
##########
xtable-core/src/main/java/org/apache/xtable/kernel/DeltaKernelConversionTarget.java:
##########
@@ -317,24 +314,26 @@ public String getTableFormat() {
@Override
public Optional<String> getTargetCommitIdentifier(String sourceIdentifier) {
- // Delta Kernel 4.0.0 does not support commit tags in commitInfo, which
are required for
- // source-to-target commit identifier mapping. This limitation is
documented in:
- // https://github.com/delta-io/delta/issues/6167
- // XTable tracking issue:
https://github.com/apache/incubator-xtable/issues/819
- //
- // Unlike DeltaConversionTarget (which uses Delta Standalone with commit
tag support),
- // DeltaKernelConversionTarget cannot retrieve commit tags from Delta
Kernel's API.
- // Rather than silently scanning all commits (O(n) performance cost) and
always returning
- // empty, we explicitly throw an exception to indicate this feature is
unsupported.
- //
- // When Delta Kernel adds commit tag support, this method can be
reimplemented to:
- // 1. Scan commit history using tableImpl.getChanges(engine, 0,
currentVersion, actionSet)
- // 2. Extract tags from CommitInfo.tags MapValue
- // 3. Parse XTABLE_METADATA from tags and match sourceIdentifier
- throw new NotSupportedException(
- "Source-to-target commit identifier mapping is not supported in
DeltaKernelConversionTarget. "
- + "Delta Kernel 4.0.0 does not support commit tags in commitInfo. "
- + "See: https://github.com/delta-io/delta/issues/6167";);
+ if (sourceIdentifier == null || sourceIdentifier.trim().isEmpty()) {
+ return Optional.empty();
+ }
+
+ String sourceIdentifierDomain =
getSourceIdentifierDomainName(sourceIdentifier);
+ Table table = Table.forPath(engine, basePath);
+ Snapshot latestSnapshot;
+ try {
+ latestSnapshot = table.getLatestSnapshot(engine);
+ } catch (TableNotFoundException e) {
+ return Optional.empty();
+ }
+
+ return ((SnapshotImpl) latestSnapshot)
+ .getLatestTransactionVersion(engine, sourceIdentifierDomain)
Review Comment:
You are correct, Good Catch. I removed the mismatched
DomainMetadata/SetTransaction approach and now scan Delta Kernel’s retained
metadata history for the matching XTABLE_METADATA, returning the commit
version where it was originally recorded. I also added coverage for current and
historical mappings, carried-forward and malformed metadata, unknown IDs, and
invalid inputs.
The updated tests and CI build pass pls review when you have time
@vinishjail97. Thank you
--
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]
