This is an automated email from the ASF dual-hosted git repository.
vinishjail97 pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/incubator-xtable.git
The following commit(s) were added to refs/heads/main by this push:
new 3353cb55 fix: Handle absolute paths in Delta log file listing (#907)
(#932)
3353cb55 is described below
commit 3353cb558620b5d8bf23d289d5ca7c5cd7040bfc
Author: Arvin <[email protected]>
AuthorDate: Sat Sep 26 08:11:20 2026 +0800
fix: Handle absolute paths in Delta log file listing (#907) (#932)
* [Bug] Handle absolute paths in Delta log file listing
* Share Delta path resolution in DeltaPathUtils and cover the Kernel
converter
---------
Co-authored-by: Vinish Reddy <[email protected]>
---
.../apache/xtable/delta/DeltaActionsConverter.java | 17 +---
.../org/apache/xtable/delta/DeltaPathUtils.java | 58 ++++++++++++++
.../xtable/kernel/DeltaKernelActionsConverter.java | 27 +------
.../apache/xtable/delta/TestDeltaPathUtils.java | 61 ++++++++++++++
.../kernel/TestDeltaKernelActionsConverter.java | 93 ++++++++++++++++++++++
5 files changed, 220 insertions(+), 36 deletions(-)
diff --git
a/xtable-core/src/main/java/org/apache/xtable/delta/DeltaActionsConverter.java
b/xtable-core/src/main/java/org/apache/xtable/delta/DeltaActionsConverter.java
index c7d27488..e4725542 100644
---
a/xtable-core/src/main/java/org/apache/xtable/delta/DeltaActionsConverter.java
+++
b/xtable-core/src/main/java/org/apache/xtable/delta/DeltaActionsConverter.java
@@ -24,8 +24,6 @@ import java.util.List;
import lombok.AccessLevel;
import lombok.NoArgsConstructor;
-import org.apache.hadoop.fs.Path;
-
import org.apache.spark.sql.delta.Snapshot;
import org.apache.spark.sql.delta.actions.AddFile;
import org.apache.spark.sql.delta.actions.DeletionVectorDescriptor;
@@ -82,7 +80,7 @@ public class DeltaActionsConverter {
includeColumnStats ? fileStats.getColumnStats() :
Collections.emptyList();
long recordCount = fileStats.getNumRecords();
return InternalDataFile.builder()
- .physicalPath(getFullPathToFile(tableBasePath, addFile.path()))
+ .physicalPath(DeltaPathUtils.getFullPathToFile(tableBasePath,
addFile.path()))
.fileFormat(fileFormat)
.fileSizeBytes(addFile.size())
.lastModified(addFile.modificationTime())
@@ -110,7 +108,7 @@ public class DeltaActionsConverter {
List<InternalPartitionField> partitionFields,
DeltaPartitionExtractor partitionExtractor) {
return InternalDataFile.builder()
- .physicalPath(getFullPathToFile(tableBasePath, removeFile.path()))
+ .physicalPath(DeltaPathUtils.getFullPathToFile(tableBasePath,
removeFile.path()))
.fileFormat(fileFormat)
.partitionValues(
partitionExtractor.partitionValueExtraction(
@@ -129,14 +127,7 @@ public class DeltaActionsConverter {
}
static String getFullPathToFile(Snapshot snapshot, String dataFilePath) {
- return getFullPathToFile(tableBasePath(snapshot), dataFilePath);
- }
-
- static String getFullPathToFile(String tableBasePath, String dataFilePath) {
- if (dataFilePath.startsWith(tableBasePath)) {
- return dataFilePath;
- }
- return tableBasePath + Path.SEPARATOR + dataFilePath;
+ return DeltaPathUtils.getFullPathToFile(tableBasePath(snapshot),
dataFilePath);
}
private static String tableBasePath(Snapshot snapshot) {
@@ -164,6 +155,6 @@ public class DeltaActionsConverter {
}
String dataFilePath = addFile.path();
- return getFullPathToFile(tableBasePath, dataFilePath);
+ return DeltaPathUtils.getFullPathToFile(tableBasePath, dataFilePath);
}
}
diff --git
a/xtable-core/src/main/java/org/apache/xtable/delta/DeltaPathUtils.java
b/xtable-core/src/main/java/org/apache/xtable/delta/DeltaPathUtils.java
new file mode 100644
index 00000000..141aac33
--- /dev/null
+++ b/xtable-core/src/main/java/org/apache/xtable/delta/DeltaPathUtils.java
@@ -0,0 +1,58 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.xtable.delta;
+
+import lombok.AccessLevel;
+import lombok.NoArgsConstructor;
+
+import org.apache.hadoop.fs.Path;
+
+/**
+ * Shared path resolution for Delta log actions. Used by both Delta Standalone
and Delta Kernel
+ * implementations.
+ */
+@NoArgsConstructor(access = AccessLevel.PRIVATE)
+public class DeltaPathUtils {
+
+ /**
+ * Resolves the path of an AddFile or RemoveFile action against the table
base path.
+ *
+ * <p>Delta allows the path to be relative to the table root or absolute
(see PROTOCOL.md). An
+ * absolute path (one with a URI scheme, e.g. s3://, hdfs://, file://, or
starting with the
+ * separator) must be used as-is: concatenating it onto the table base path
would point at a
+ * non-existent location and silently drop records from the converted table.
+ *
+ * @param tableBasePath the table base path
+ * @param dataFilePath the path recorded in the Delta log (relative or
absolute)
+ * @return the full absolute path to the file
+ */
+ public static String getFullPathToFile(String tableBasePath, String
dataFilePath) {
+ if (isAbsolutePath(dataFilePath)) {
+ return dataFilePath;
+ }
+ return tableBasePath.endsWith(Path.SEPARATOR)
+ ? tableBasePath + dataFilePath
+ : tableBasePath + Path.SEPARATOR + dataFilePath;
+ }
+
+ private static boolean isAbsolutePath(String dataFilePath) {
+ return dataFilePath.startsWith(Path.SEPARATOR)
+ || new Path(dataFilePath).toUri().getScheme() != null;
+ }
+}
diff --git
a/xtable-core/src/main/java/org/apache/xtable/kernel/DeltaKernelActionsConverter.java
b/xtable-core/src/main/java/org/apache/xtable/kernel/DeltaKernelActionsConverter.java
index 1346a517..bf51c7c7 100644
---
a/xtable-core/src/main/java/org/apache/xtable/kernel/DeltaKernelActionsConverter.java
+++
b/xtable-core/src/main/java/org/apache/xtable/kernel/DeltaKernelActionsConverter.java
@@ -25,13 +25,12 @@ import java.util.Map;
import lombok.AccessLevel;
import lombok.NoArgsConstructor;
-import org.apache.hadoop.fs.Path;
-
import io.delta.kernel.Table;
import io.delta.kernel.engine.Engine;
import io.delta.kernel.internal.actions.AddFile;
import io.delta.kernel.internal.actions.RemoveFile;
+import org.apache.xtable.delta.DeltaPathUtils;
import org.apache.xtable.exception.NotSupportedException;
import org.apache.xtable.model.schema.InternalField;
import org.apache.xtable.model.schema.InternalPartitionField;
@@ -69,7 +68,7 @@ public class DeltaKernelActionsConverter {
long recordCount = fileStats.getNumRecords();
return InternalDataFile.builder()
- .physicalPath(getFullPathToFile(addFile.getPath(), tableBasePath))
+ .physicalPath(DeltaPathUtils.getFullPathToFile(tableBasePath,
addFile.getPath()))
.fileFormat(fileFormat)
.fileSizeBytes(addFile.getSize())
.lastModified(addFile.getModificationTime())
@@ -93,7 +92,7 @@ public class DeltaKernelActionsConverter {
DeltaKernelPartitionExtractor partitionExtractor,
Map<String, String> partitionValues) {
return InternalDataFile.builder()
- .physicalPath(getFullPathToFile(removeFile.getPath(), tableBasePath))
+ .physicalPath(DeltaPathUtils.getFullPathToFile(tableBasePath,
removeFile.getPath()))
.fileFormat(fileFormat)
.partitionValues(
partitionExtractor.partitionValueExtraction(partitionValues,
partitionFields))
@@ -120,24 +119,6 @@ public class DeltaKernelActionsConverter {
*/
static String getFullPathToFile(String dataFilePath, Engine engine, Table
table) {
String tableBasePath = table.getPath(engine);
- return getFullPathToFile(dataFilePath, tableBasePath);
- }
-
- /**
- * Constructs the full path to a file using a provided base path (most
efficient).
- *
- * @param dataFilePath the data file path (relative or absolute)
- * @param tableBasePath the table base path
- * @return the full absolute path to the file
- */
- static String getFullPathToFile(String dataFilePath, String tableBasePath) {
- // Check if the file path is already absolute and under the table base path
- // Use separator check to avoid false positives (e.g., "/foo" matching
"/foobar/x.parquet")
- String basePathWithSeparator =
- tableBasePath.endsWith(Path.SEPARATOR) ? tableBasePath : tableBasePath
+ Path.SEPARATOR;
- if (dataFilePath.startsWith(basePathWithSeparator)) {
- return dataFilePath;
- }
- return basePathWithSeparator + dataFilePath;
+ return DeltaPathUtils.getFullPathToFile(tableBasePath, dataFilePath);
}
}
diff --git
a/xtable-core/src/test/java/org/apache/xtable/delta/TestDeltaPathUtils.java
b/xtable-core/src/test/java/org/apache/xtable/delta/TestDeltaPathUtils.java
new file mode 100644
index 00000000..3defd993
--- /dev/null
+++ b/xtable-core/src/test/java/org/apache/xtable/delta/TestDeltaPathUtils.java
@@ -0,0 +1,61 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.xtable.delta;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+
+import java.util.stream.Stream;
+
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.Arguments;
+import org.junit.jupiter.params.provider.MethodSource;
+
+class TestDeltaPathUtils {
+
+ private static Stream<Arguments> fullPathCases() {
+ return Stream.of(
+ // relative paths are resolved against the table base path
+ Arguments.of("s3a://bucket/tab", "part-0.parquet",
"s3a://bucket/tab/part-0.parquet"),
+ Arguments.of("s3a://bucket/tab/", "part-0.parquet",
"s3a://bucket/tab/part-0.parquet"),
+ Arguments.of(
+ "s3a://bucket/tab",
+ "year=2024/part-0.parquet",
+ "s3a://bucket/tab/year=2024/part-0.parquet"),
+ // absolute paths are used as-is
+ Arguments.of(
+ "s3a://bucket/tab",
+ "s3a://bucket/tab/part-0.parquet",
+ "s3a://bucket/tab/part-0.parquet"),
+ Arguments.of(
+ "s3a://bucket/tab", "s3://bucket/tab/part-0.parquet",
"s3://bucket/tab/part-0.parquet"),
+ Arguments.of(
+ "s3a://bucket/tab",
+ "s3://other-bucket/x/part-0.parquet",
+ "s3://other-bucket/x/part-0.parquet"),
+ Arguments.of(
+ "s3a://bucket/tab", "file:///local/t/part-0.parquet",
"file:///local/t/part-0.parquet"),
+ Arguments.of("/tmp/tab", "/tmp/other/part-0.parquet",
"/tmp/other/part-0.parquet"));
+ }
+
+ @ParameterizedTest
+ @MethodSource("fullPathCases")
+ void getFullPathToFile(String tableBasePath, String dataFilePath, String
expected) {
+ assertEquals(expected, DeltaPathUtils.getFullPathToFile(tableBasePath,
dataFilePath));
+ }
+}
diff --git
a/xtable-core/src/test/java/org/apache/xtable/kernel/TestDeltaKernelActionsConverter.java
b/xtable-core/src/test/java/org/apache/xtable/kernel/TestDeltaKernelActionsConverter.java
new file mode 100644
index 00000000..e6df0f38
--- /dev/null
+++
b/xtable-core/src/test/java/org/apache/xtable/kernel/TestDeltaKernelActionsConverter.java
@@ -0,0 +1,93 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.xtable.kernel;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+import java.util.Collections;
+
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.CsvSource;
+
+import io.delta.kernel.internal.actions.AddFile;
+import io.delta.kernel.internal.actions.RemoveFile;
+
+import org.apache.xtable.model.stat.FileStats;
+import org.apache.xtable.model.storage.FileFormat;
+import org.apache.xtable.model.storage.InternalDataFile;
+
+class TestDeltaKernelActionsConverter {
+ private static final String TABLE_BASE_PATH = "s3a://bucket/tab";
+
+ private final DeltaKernelActionsConverter actionsConverter =
+ DeltaKernelActionsConverter.getInstance();
+
+ @ParameterizedTest
+ @CsvSource({
+ "part-0.parquet, s3a://bucket/tab/part-0.parquet",
+ "s3://bucket/tab/part-0.parquet, s3://bucket/tab/part-0.parquet",
+ "/tmp/other/part-0.parquet, /tmp/other/part-0.parquet"
+ })
+ void convertAddActionResolvesPath(String loggedPath, String expectedPath) {
+ AddFile addFile = mock(AddFile.class);
+ when(addFile.getPath()).thenReturn(loggedPath);
+ DeltaKernelStatsExtractor statsExtractor =
mock(DeltaKernelStatsExtractor.class);
+ when(statsExtractor.getColumnStatsForFile(any(), any()))
+
.thenReturn(FileStats.builder().columnStats(Collections.emptyList()).build());
+
+ InternalDataFile dataFile =
+ actionsConverter.convertAddActionToInternalDataFile(
+ addFile,
+ TABLE_BASE_PATH,
+ FileFormat.APACHE_PARQUET,
+ Collections.emptyList(),
+ Collections.emptyList(),
+ false,
+ DeltaKernelPartitionExtractor.getInstance(),
+ statsExtractor,
+ Collections.emptyMap());
+
+ assertEquals(expectedPath, dataFile.getPhysicalPath());
+ }
+
+ @ParameterizedTest
+ @CsvSource({
+ "part-0.parquet, s3a://bucket/tab/part-0.parquet",
+ "s3://bucket/tab/part-0.parquet, s3://bucket/tab/part-0.parquet",
+ "/tmp/other/part-0.parquet, /tmp/other/part-0.parquet"
+ })
+ void convertRemoveActionResolvesPath(String loggedPath, String expectedPath)
{
+ RemoveFile removeFile = mock(RemoveFile.class);
+ when(removeFile.getPath()).thenReturn(loggedPath);
+
+ InternalDataFile dataFile =
+ actionsConverter.convertRemoveActionToInternalDataFile(
+ removeFile,
+ TABLE_BASE_PATH,
+ FileFormat.APACHE_PARQUET,
+ Collections.emptyList(),
+ DeltaKernelPartitionExtractor.getInstance(),
+ Collections.emptyMap());
+
+ assertEquals(expectedPath, dataFile.getPhysicalPath());
+ }
+}
