From 622cf149eef769964ad9a58e0ea762538c4e50b0 Mon Sep 17 00:00:00 2001 From: TimothyW553 Date: Mon, 10 Nov 2025 15:45:45 -0800 Subject: [PATCH 01/42] initial commit Signed-off-by: TimothyW553 --- .../kernel/spark/utils/CatalogTableUtils.java | 89 +++++++++++++++++++ 1 file changed, 89 insertions(+) create mode 100644 kernel-spark/src/main/java/io/delta/kernel/spark/utils/CatalogTableUtils.java diff --git a/kernel-spark/src/main/java/io/delta/kernel/spark/utils/CatalogTableUtils.java b/kernel-spark/src/main/java/io/delta/kernel/spark/utils/CatalogTableUtils.java new file mode 100644 index 00000000000..d61f54a24c0 --- /dev/null +++ b/kernel-spark/src/main/java/io/delta/kernel/spark/utils/CatalogTableUtils.java @@ -0,0 +1,89 @@ +package io.delta.kernel.spark.utils; + +import static java.util.Objects.requireNonNull; +import java.util.Map; +import java.util.Objects; +import org.apache.spark.sql.connector.catalog.Table; + +public class CatalogTableUtils { + + // UC persists markers and feature flags into Spark Table properties during resolution. + // - Keys under delta.unityCatalog.* (e.g. tableId) identify the table as Unity Catalog managed, + // which tells the connector to use UC-aware code paths instead of path-only logic. + // - delta.feature.catalogOwned[-preview] advertise CCv2 support; when their value is + // "supported" the table should opt into catalog-owned commit coordination. + static final String UNITY_CATALOG_PROPERTY_PREFIX = "delta.unityCatalog."; + static final String UNITY_CATALOG_TABLE_ID_PROP = UNITY_CATALOG_PROPERTY_PREFIX + "tableId"; + static final String FEATURE_CATALOG_OWNED = "delta.feature.catalogOwned"; + static final String FEATURE_CATALOG_OWNED_PREVIEW = "delta.feature.catalogOwned-preview"; + private static final String SUPPORTED = "supported"; + + private CatalogTableUtils() {} + + /** + * A table is a CCv2 table if it is catalog managed and has catalogOwned feature enabled. + * Catalog managed tables are identified by the presence of delta.unityCatalog.* properties. + * CatalogOwned feature is identified by the presence of delta.feature.catalogOwned[-preview] properties. + * + * @param table the table + * @return true if the table is a CCv2 table, false otherwise + */ + public static boolean isCCv2Table(Table table) { + requireNonNull(table, "table is null"); + Map tableProperties = table.properties(); + if (!isCatalogManagedTable(tableProperties)) { + return false; + } + + return isCatalogOwnedFeatureSupported(tableProperties, FEATURE_CATALOG_OWNED) + || isCatalogOwnedFeatureSupported(tableProperties, FEATURE_CATALOG_OWNED_PREVIEW); + } + + public static boolean isCatalogManagedTable(Table table) { + requireNonNull(table, "table is null"); + return isCatalogManagedTable(table.properties()); + } + + /** + * A table is catalog managed if it has a non-empty tableId under delta.unityCatalog.* properties. + * Refer to connectors/spark/src/main/scala/io/unitycatalog/spark/UCSingleCatalog.scala for more details. + * + * @param tableProperties the table properties + * @return true if the table is catalog managed, false otherwise + */ + static boolean isCatalogManagedTable(Map tableProperties) { + if (tableProperties == null) { + return false; + } + + String tableId = tableProperties.get(UNITY_CATALOG_TABLE_ID_PROP); + if (tableId != null && !tableId.trim().isEmpty()) { + return true; + } + + return tableProperties.keySet().stream() + .filter(Objects::nonNull) + .anyMatch(key -> key.startsWith(UNITY_CATALOG_PROPERTY_PREFIX)); + } + + /** + * A table has catalogOwned feature enabled if table's + * properties["delta.feature.catalogOwned"] is "supported" + * We currently use both preview and stable version of the feature but may only support stable version in the future. + * + * @param tableProperties the table properties + * @param featureKey the feature key + * @return true if the feature is supported, false otherwise + */ + private static boolean isCatalogOwnedFeatureSupported( + Map tableProperties, String featureKey) { + if (tableProperties == null) { + return false; + } + String value = tableProperties.get(featureKey); + if (value == null) { + return false; + } + return SUPPORTED.equalsIgnoreCase(value.trim()); + } +} From 857aa90a04371f3e7f2a3f4032698c7a416fe9ef Mon Sep 17 00:00:00 2001 From: TimothyW553 Date: Mon, 10 Nov 2025 16:50:28 -0800 Subject: [PATCH 02/42] unify comments Signed-off-by: TimothyW553 --- .../kernel/spark/utils/CatalogTableUtils.java | 49 +++++++------------ 1 file changed, 18 insertions(+), 31 deletions(-) diff --git a/kernel-spark/src/main/java/io/delta/kernel/spark/utils/CatalogTableUtils.java b/kernel-spark/src/main/java/io/delta/kernel/spark/utils/CatalogTableUtils.java index d61f54a24c0..8d128d9e9d3 100644 --- a/kernel-spark/src/main/java/io/delta/kernel/spark/utils/CatalogTableUtils.java +++ b/kernel-spark/src/main/java/io/delta/kernel/spark/utils/CatalogTableUtils.java @@ -5,13 +5,24 @@ import java.util.Objects; import org.apache.spark.sql.connector.catalog.Table; -public class CatalogTableUtils { - - // UC persists markers and feature flags into Spark Table properties during resolution. - // - Keys under delta.unityCatalog.* (e.g. tableId) identify the table as Unity Catalog managed, - // which tells the connector to use UC-aware code paths instead of path-only logic. - // - delta.feature.catalogOwned[-preview] advertise CCv2 support; when their value is - // "supported" the table should opt into catalog-owned commit coordination. +/** + * Unity Catalog persists Unity-specific metadata onto Spark {@link Table} instances when they are + * resolved through `UCSingleCatalog`. This helper centralises the logic for interpreting those + * properties so the Kernel connector can decide when to use catalog-owned (CCv2) behaviour. + * + *

The constants below mirror the property keys written by the UC → Spark connector: + * + *

    + *
  • `delta.unityCatalog.tableId` (and other `delta.unityCatalog.*` keys) flag a table as + * catalog-managed. + *
  • `delta.feature.catalogOwned` (plus its `-preview` variant) signal that catalog-owned commit + * coordination is enabled; both map to the value {@code supported} when active. + *
+ * + *

See {@code connectors/spark/.../UCSingleCatalog.scala} for the producer side of these + * properties. + */ +public final class CatalogTableUtils { static final String UNITY_CATALOG_PROPERTY_PREFIX = "delta.unityCatalog."; static final String UNITY_CATALOG_TABLE_ID_PROP = UNITY_CATALOG_PROPERTY_PREFIX + "tableId"; static final String FEATURE_CATALOG_OWNED = "delta.feature.catalogOwned"; @@ -20,14 +31,6 @@ public class CatalogTableUtils { private CatalogTableUtils() {} - /** - * A table is a CCv2 table if it is catalog managed and has catalogOwned feature enabled. - * Catalog managed tables are identified by the presence of delta.unityCatalog.* properties. - * CatalogOwned feature is identified by the presence of delta.feature.catalogOwned[-preview] properties. - * - * @param table the table - * @return true if the table is a CCv2 table, false otherwise - */ public static boolean isCCv2Table(Table table) { requireNonNull(table, "table is null"); Map tableProperties = table.properties(); @@ -44,13 +47,6 @@ public static boolean isCatalogManagedTable(Table table) { return isCatalogManagedTable(table.properties()); } - /** - * A table is catalog managed if it has a non-empty tableId under delta.unityCatalog.* properties. - * Refer to connectors/spark/src/main/scala/io/unitycatalog/spark/UCSingleCatalog.scala for more details. - * - * @param tableProperties the table properties - * @return true if the table is catalog managed, false otherwise - */ static boolean isCatalogManagedTable(Map tableProperties) { if (tableProperties == null) { return false; @@ -66,15 +62,6 @@ static boolean isCatalogManagedTable(Map tableProperties) { .anyMatch(key -> key.startsWith(UNITY_CATALOG_PROPERTY_PREFIX)); } - /** - * A table has catalogOwned feature enabled if table's - * properties["delta.feature.catalogOwned"] is "supported" - * We currently use both preview and stable version of the feature but may only support stable version in the future. - * - * @param tableProperties the table properties - * @param featureKey the feature key - * @return true if the feature is supported, false otherwise - */ private static boolean isCatalogOwnedFeatureSupported( Map tableProperties, String featureKey) { if (tableProperties == null) { From 7690d847380e280cdb7fab8856b58316d6fcbfb4 Mon Sep 17 00:00:00 2001 From: TimothyW553 Date: Mon, 10 Nov 2025 17:09:06 -0800 Subject: [PATCH 03/42] edit comment Signed-off-by: TimothyW553 --- .../kernel/spark/utils/CatalogTableUtils.java | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/kernel-spark/src/main/java/io/delta/kernel/spark/utils/CatalogTableUtils.java b/kernel-spark/src/main/java/io/delta/kernel/spark/utils/CatalogTableUtils.java index 8d128d9e9d3..d8422c731ca 100644 --- a/kernel-spark/src/main/java/io/delta/kernel/spark/utils/CatalogTableUtils.java +++ b/kernel-spark/src/main/java/io/delta/kernel/spark/utils/CatalogTableUtils.java @@ -7,20 +7,17 @@ /** * Unity Catalog persists Unity-specific metadata onto Spark {@link Table} instances when they are - * resolved through `UCSingleCatalog`. This helper centralises the logic for interpreting those + * resolved through UCSingleCatalog. This helper centralises the logic for interpreting those * properties so the Kernel connector can decide when to use catalog-owned (CCv2) behaviour. * - *

The constants below mirror the property keys written by the UC → Spark connector: - * + *

These constants mirror the property keys by the UC <> Spark connector *

    - *
  • `delta.unityCatalog.tableId` (and other `delta.unityCatalog.*` keys) flag a table as - * catalog-managed. - *
  • `delta.feature.catalogOwned` (plus its `-preview` variant) signal that catalog-owned commit - * coordination is enabled; both map to the value {@code supported} when active. + *
  • delta.unityCatalog.* (e.g., tableId) flags a table as catalog-managed + *
  • delta.feature.catalogOwned[-preview] signals that CCv2 (catalog-owned commit coordination) + * is enabled. Both map to the value "supported" when active *
* - *

See {@code connectors/spark/.../UCSingleCatalog.scala} for the producer side of these - * properties. + *

See {@code connectors/spark/.../UCSingleCatalog.scala} for the producer side of these props */ public final class CatalogTableUtils { static final String UNITY_CATALOG_PROPERTY_PREFIX = "delta.unityCatalog."; From 6a1d2a5c1b5b346699245ec5967e3dd2ff1107ee Mon Sep 17 00:00:00 2001 From: TimothyW553 Date: Tue, 11 Nov 2025 00:47:12 -0800 Subject: [PATCH 04/42] change to catalogtable Signed-off-by: TimothyW553 --- .../kernel/spark/utils/CatalogTableUtils.java | 48 +++++++++++-------- 1 file changed, 29 insertions(+), 19 deletions(-) diff --git a/kernel-spark/src/main/java/io/delta/kernel/spark/utils/CatalogTableUtils.java b/kernel-spark/src/main/java/io/delta/kernel/spark/utils/CatalogTableUtils.java index d8422c731ca..fd1177432c9 100644 --- a/kernel-spark/src/main/java/io/delta/kernel/spark/utils/CatalogTableUtils.java +++ b/kernel-spark/src/main/java/io/delta/kernel/spark/utils/CatalogTableUtils.java @@ -1,23 +1,28 @@ package io.delta.kernel.spark.utils; import static java.util.Objects.requireNonNull; +import java.util.Collections; import java.util.Map; import java.util.Objects; -import org.apache.spark.sql.connector.catalog.Table; +import org.apache.spark.sql.catalyst.catalog.CatalogTable; +import scala.jdk.javaapi.CollectionConverters; /** - * Unity Catalog persists Unity-specific metadata onto Spark {@link Table} instances when they are - * resolved through UCSingleCatalog. This helper centralises the logic for interpreting those - * properties so the Kernel connector can decide when to use catalog-owned (CCv2) behaviour. + * Unity Catalog persists Unity-specific metadata onto Spark {@link CatalogTable} instances when + * they are resolved through UCSingleCatalog. This helper centralises the logic for interpreting + * those properties so the Kernel connector can decide when to use catalog-owned (CCv2) behaviour. + * + *

The constants below mirror the property keys written by the UC <-> Spark connector: * - *

These constants mirror the property keys by the UC <> Spark connector *

    - *
  • delta.unityCatalog.* (e.g., tableId) flags a table as catalog-managed - *
  • delta.feature.catalogOwned[-preview] signals that CCv2 (catalog-owned commit coordination) - * is enabled. Both map to the value "supported" when active + *
  • {@code delta.unityCatalog.*} (for example {@code tableId}) flags a table as + * catalog-managed. + *
  • {@code delta.feature.catalogOwned[-preview]} signals that CCv2 (catalog-owned commit + * coordination) is enabled. Both variants use the value {@code supported} when active. *
* - *

See {@code connectors/spark/.../UCSingleCatalog.scala} for the producer side of these props + *

See {@code connectors/spark/.../UCSingleCatalog.scala} for the producer side of these + * properties. */ public final class CatalogTableUtils { static final String UNITY_CATALOG_PROPERTY_PREFIX = "delta.unityCatalog."; @@ -28,20 +33,18 @@ public final class CatalogTableUtils { private CatalogTableUtils() {} - public static boolean isCCv2Table(Table table) { + public static boolean isCCv2Table(CatalogTable table) { requireNonNull(table, "table is null"); - Map tableProperties = table.properties(); - if (!isCatalogManagedTable(tableProperties)) { - return false; - } - - return isCatalogOwnedFeatureSupported(tableProperties, FEATURE_CATALOG_OWNED) - || isCatalogOwnedFeatureSupported(tableProperties, FEATURE_CATALOG_OWNED_PREVIEW); + Map tableProperties = toJavaMap(table.properties()); + + return isCatalogManagedTable(tableProperties) + && (isCatalogOwnedFeatureSupported(tableProperties, FEATURE_CATALOG_OWNED) + || isCatalogOwnedFeatureSupported(tableProperties, FEATURE_CATALOG_OWNED_PREVIEW)); } - public static boolean isCatalogManagedTable(Table table) { + public static boolean isCatalogManagedTable(CatalogTable table) { requireNonNull(table, "table is null"); - return isCatalogManagedTable(table.properties()); + return isCatalogManagedTable(toJavaMap(table.properties())); } static boolean isCatalogManagedTable(Map tableProperties) { @@ -70,4 +73,11 @@ private static boolean isCatalogOwnedFeatureSupported( } return SUPPORTED.equalsIgnoreCase(value.trim()); } + + private static Map toJavaMap(scala.collection.immutable.Map scalaMap) { + if (scalaMap == null || scalaMap.isEmpty()) { + return Collections.emptyMap(); + } + return CollectionConverters.asJava(scalaMap); + } } From a57a34234125b67475a36e3c3b96f07a486d083a Mon Sep 17 00:00:00 2001 From: Timothy Wang Date: Wed, 12 Nov 2025 10:09:10 -0800 Subject: [PATCH 05/42] add tests Signed-off-by: Timothy Wang --- .../kernel/spark/utils/CatalogTableUtils.java | 8 +- .../spark/utils/CatalogTableUtilsTest.java | 174 ++++++++++++++++++ 2 files changed, 179 insertions(+), 3 deletions(-) create mode 100644 kernel-spark/src/test/java/io/delta/kernel/spark/utils/CatalogTableUtilsTest.java diff --git a/kernel-spark/src/main/java/io/delta/kernel/spark/utils/CatalogTableUtils.java b/kernel-spark/src/main/java/io/delta/kernel/spark/utils/CatalogTableUtils.java index fd1177432c9..6322a8c76be 100644 --- a/kernel-spark/src/main/java/io/delta/kernel/spark/utils/CatalogTableUtils.java +++ b/kernel-spark/src/main/java/io/delta/kernel/spark/utils/CatalogTableUtils.java @@ -1,6 +1,7 @@ package io.delta.kernel.spark.utils; import static java.util.Objects.requireNonNull; + import java.util.Collections; import java.util.Map; import java.util.Objects; @@ -36,7 +37,7 @@ private CatalogTableUtils() {} public static boolean isCCv2Table(CatalogTable table) { requireNonNull(table, "table is null"); Map tableProperties = toJavaMap(table.properties()); - + return isCatalogManagedTable(tableProperties) && (isCatalogOwnedFeatureSupported(tableProperties, FEATURE_CATALOG_OWNED) || isCatalogOwnedFeatureSupported(tableProperties, FEATURE_CATALOG_OWNED_PREVIEW)); @@ -51,7 +52,6 @@ static boolean isCatalogManagedTable(Map tableProperties) { if (tableProperties == null) { return false; } - String tableId = tableProperties.get(UNITY_CATALOG_TABLE_ID_PROP); if (tableId != null && !tableId.trim().isEmpty()) { return true; @@ -71,10 +71,12 @@ private static boolean isCatalogOwnedFeatureSupported( if (value == null) { return false; } + return SUPPORTED.equalsIgnoreCase(value.trim()); } - private static Map toJavaMap(scala.collection.immutable.Map scalaMap) { + private static Map toJavaMap( + scala.collection.immutable.Map scalaMap) { if (scalaMap == null || scalaMap.isEmpty()) { return Collections.emptyMap(); } diff --git a/kernel-spark/src/test/java/io/delta/kernel/spark/utils/CatalogTableUtilsTest.java b/kernel-spark/src/test/java/io/delta/kernel/spark/utils/CatalogTableUtilsTest.java new file mode 100644 index 00000000000..f160f65c8c3 --- /dev/null +++ b/kernel-spark/src/test/java/io/delta/kernel/spark/utils/CatalogTableUtilsTest.java @@ -0,0 +1,174 @@ +/* + * Copyright (2025) The Delta Lake Project Authors. + * + * Licensed 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 io.delta.kernel.spark.utils; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.util.Collections; +import java.util.HashMap; +import java.util.Map; +import java.util.stream.Stream; +import org.apache.spark.sql.catalyst.TableIdentifier; +import org.apache.spark.sql.catalyst.catalog.CatalogStorageFormat; +import org.apache.spark.sql.catalyst.catalog.CatalogStorageFormat$; +import org.apache.spark.sql.catalyst.catalog.CatalogTable; +import org.apache.spark.sql.catalyst.catalog.CatalogTable$; +import org.apache.spark.sql.catalyst.catalog.CatalogTableType$; +import org.apache.spark.sql.types.StructType; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; +import org.junit.jupiter.params.provider.ValueSource; +import scala.Option; +import scala.Option$; +import scala.collection.immutable.Map$; +import scala.jdk.javaapi.CollectionConverters; + +/** Tests for {@link CatalogTableUtils}. */ +public class CatalogTableUtilsTest { + + @ParameterizedTest(name = "{0}") + @MethodSource("catalogManagedMapCases") + void mapBasedDetection(String name, Map properties, boolean expected) { + assertEquals(expected, CatalogTableUtils.isCatalogManagedTable(properties)); + } + + static Stream catalogManagedMapCases() { + return Stream.of( + Arguments.of("null map", null, false), + Arguments.of("empty map", Collections.emptyMap(), false), + Arguments.of( + "table id present", + javaMap(CatalogTableUtils.UNITY_CATALOG_TABLE_ID_PROP, "123"), + true), + Arguments.of( + "unity catalog prefix fallback", javaMap("delta.unityCatalog.random", "enabled"), true), + Arguments.of( + "blank table id still catalog managed", + javaMap(CatalogTableUtils.UNITY_CATALOG_TABLE_ID_PROP, " "), + true), + Arguments.of("irrelevant properties only", javaMap("delta.feature.other", "true"), false)); + } + + @Test + void catalogTableOverloadMatchesMapDetection() { + Map managed = javaMap(CatalogTableUtils.UNITY_CATALOG_TABLE_ID_PROP, "abc-123"); + Map unmanaged = Collections.emptyMap(); + + assertTrue(CatalogTableUtils.isCatalogManagedTable(catalogTableWithProperties(managed))); + assertFalse(CatalogTableUtils.isCatalogManagedTable(catalogTableWithProperties(unmanaged))); + } + + @ParameterizedTest(name = "feature {0} enables CCv2 when catalog managed") + @ValueSource( + strings = { + CatalogTableUtils.FEATURE_CATALOG_OWNED, + CatalogTableUtils.FEATURE_CATALOG_OWNED_PREVIEW + }) + void isCCv2TableRecognisesSupportedFeatures(String featureKey) { + CatalogTable table = + catalogTableWithProperties( + javaMap( + CatalogTableUtils.UNITY_CATALOG_TABLE_ID_PROP, + "table-id", + featureKey, + " supported ")); + + assertTrue(CatalogTableUtils.isCatalogManagedTable(table)); + assertTrue(CatalogTableUtils.isCCv2Table(table)); + } + + @Test + void isCCv2TableRequiresFeatureFlag() { + CatalogTable table = + catalogTableWithProperties( + javaMap(CatalogTableUtils.UNITY_CATALOG_TABLE_ID_PROP, "table-id")); + + assertTrue(CatalogTableUtils.isCatalogManagedTable(table)); + assertFalse(CatalogTableUtils.isCCv2Table(table)); + } + + @Test + void isCCv2TableRejectsUnsupportedValues() { + CatalogTable table = + catalogTableWithProperties( + javaMap( + CatalogTableUtils.UNITY_CATALOG_TABLE_ID_PROP, + "table-id", + CatalogTableUtils.FEATURE_CATALOG_OWNED, + "disabled")); + + assertTrue(CatalogTableUtils.isCatalogManagedTable(table)); + assertFalse(CatalogTableUtils.isCCv2Table(table)); + } + + @Test + void isCCv2TableIgnoresFeatureWithoutCatalogManagement() { + CatalogTable table = + catalogTableWithProperties(javaMap(CatalogTableUtils.FEATURE_CATALOG_OWNED, "supported")); + + assertFalse(CatalogTableUtils.isCatalogManagedTable(table)); + assertFalse(CatalogTableUtils.isCCv2Table(table)); + } + + private static Map javaMap(String... keyValues) { + if (keyValues.length % 2 != 0) { + throw new IllegalArgumentException("keyValues length must be even"); + } + Map map = new HashMap<>(); + for (int i = 0; i < keyValues.length; i += 2) { + map.put(keyValues[i], keyValues[i + 1]); + } + return map; + } + + private static CatalogTable catalogTableWithProperties(Map properties) { + Option none = Option$.MODULE$.empty(); + TableIdentifier identifier = new TableIdentifier("test_table", none, none); + CatalogStorageFormat storage = CatalogStorageFormat$.MODULE$.empty(); + + scala.collection.immutable.Map scalaProps = + properties == null || properties.isEmpty() + ? Map$.MODULE$.empty() + : scala.collection.immutable.Map$.MODULE$.from( + CollectionConverters.asScala(properties).toSeq()); + + return CatalogTable$.MODULE$.apply( + identifier, + CatalogTableType$.MODULE$.MANAGED(), + storage, + new StructType(), + CatalogTable$.MODULE$.apply$default$5(), + CatalogTable$.MODULE$.apply$default$6(), + CatalogTable$.MODULE$.apply$default$7(), + CatalogTable$.MODULE$.apply$default$8(), + CatalogTable$.MODULE$.apply$default$9(), + CatalogTable$.MODULE$.apply$default$10(), + CatalogTable$.MODULE$.apply$default$11(), + scalaProps, + CatalogTable$.MODULE$.apply$default$13(), + CatalogTable$.MODULE$.apply$default$14(), + CatalogTable$.MODULE$.apply$default$15(), + CatalogTable$.MODULE$.apply$default$16(), + CatalogTable$.MODULE$.apply$default$17(), + CatalogTable$.MODULE$.apply$default$18(), + CatalogTable$.MODULE$.apply$default$19(), + CatalogTable$.MODULE$.apply$default$20()); + } +} From eeadbdb470cdb6749844bb95ead4fe8d02037b30 Mon Sep 17 00:00:00 2001 From: Timothy Wang Date: Wed, 12 Nov 2025 12:15:38 -0800 Subject: [PATCH 06/42] update methods and testS Signed-off-by: Timothy Wang --- .../kernel/spark/utils/CatalogTableUtils.java | 74 +++---- .../spark/utils/delta-uc.code-workspace | 15 ++ .../spark/utils/CatalogTableUtilsTest.java | 189 ++++++++---------- 3 files changed, 124 insertions(+), 154 deletions(-) create mode 100644 kernel-spark/src/main/java/io/delta/kernel/spark/utils/delta-uc.code-workspace diff --git a/kernel-spark/src/main/java/io/delta/kernel/spark/utils/CatalogTableUtils.java b/kernel-spark/src/main/java/io/delta/kernel/spark/utils/CatalogTableUtils.java index 6322a8c76be..d6c95fae40b 100644 --- a/kernel-spark/src/main/java/io/delta/kernel/spark/utils/CatalogTableUtils.java +++ b/kernel-spark/src/main/java/io/delta/kernel/spark/utils/CatalogTableUtils.java @@ -2,77 +2,63 @@ import static java.util.Objects.requireNonNull; +import io.delta.storage.commit.uccommitcoordinator.UCCommitCoordinatorClient; import java.util.Collections; import java.util.Map; -import java.util.Objects; import org.apache.spark.sql.catalyst.catalog.CatalogTable; import scala.jdk.javaapi.CollectionConverters; /** - * Unity Catalog persists Unity-specific metadata onto Spark {@link CatalogTable} instances when - * they are resolved through UCSingleCatalog. This helper centralises the logic for interpreting - * those properties so the Kernel connector can decide when to use catalog-owned (CCv2) behaviour. + * Utility helpers for inspecting Delta-related metadata persisted on Spark {@link CatalogTable} + * instances by Unity Catalog. * - *

The constants below mirror the property keys written by the UC <-> Spark connector: + *

Unity Catalog marks catalog-managed tables via feature flags stored in table properties. This + * helper centralises the logic for interpreting those properties so the Kernel connector can decide + * when to use catalog-owned (CCv2) behaviour. * *

    - *
  • {@code delta.unityCatalog.*} (for example {@code tableId}) flags a table as - * catalog-managed. - *
  • {@code delta.feature.catalogOwned[-preview]} signals that CCv2 (catalog-owned commit - * coordination) is enabled. Both variants use the value {@code supported} when active. + *
  • {@link #isCatalogManaged(CatalogTable)} checks whether either {@code + * delta.feature.catalogManaged} or {@code delta.feature.catalogOwned-preview} is set to + * {@code supported}, signalling that a catalog manages the table. + *
  • {@link #isUnityCatalogManagedTable(CatalogTable)} additionally verifies the presence of the + * Unity Catalog table identifier ({@link UCCommitCoordinatorClient#UC_TABLE_ID_KEY}) to + * confirm that the table is backed by Unity Catalog. *
- * - *

See {@code connectors/spark/.../UCSingleCatalog.scala} for the producer side of these - * properties. */ public final class CatalogTableUtils { - static final String UNITY_CATALOG_PROPERTY_PREFIX = "delta.unityCatalog."; - static final String UNITY_CATALOG_TABLE_ID_PROP = UNITY_CATALOG_PROPERTY_PREFIX + "tableId"; - static final String FEATURE_CATALOG_OWNED = "delta.feature.catalogOwned"; + static final String FEATURE_CATALOG_MANAGED = "delta.feature.catalogManaged"; static final String FEATURE_CATALOG_OWNED_PREVIEW = "delta.feature.catalogOwned-preview"; private static final String SUPPORTED = "supported"; private CatalogTableUtils() {} - public static boolean isCCv2Table(CatalogTable table) { + // Checks whether *any* catalog manages this table via CCv2 semantics by checking + // if the catalogManaged/catalogOwned-preview flags are 'supported' + public static boolean isCatalogManaged(CatalogTable table) { requireNonNull(table, "table is null"); Map tableProperties = toJavaMap(table.properties()); - - return isCatalogManagedTable(tableProperties) - && (isCatalogOwnedFeatureSupported(tableProperties, FEATURE_CATALOG_OWNED) - || isCatalogOwnedFeatureSupported(tableProperties, FEATURE_CATALOG_OWNED_PREVIEW)); + return isCatalogManagedFeatureEnabled(tableProperties, FEATURE_CATALOG_MANAGED) + || isCatalogManagedFeatureEnabled(tableProperties, FEATURE_CATALOG_OWNED_PREVIEW); } - public static boolean isCatalogManagedTable(CatalogTable table) { + // Checks if table is *Unity Catalog* managed - meaning it isCatalogManaged and it contains + // a UC identifier (UC_TABLE_ID_KEY) + public static boolean isUnityCatalogManagedTable(CatalogTable table) { requireNonNull(table, "table is null"); - return isCatalogManagedTable(toJavaMap(table.properties())); - } - - static boolean isCatalogManagedTable(Map tableProperties) { - if (tableProperties == null) { - return false; - } - String tableId = tableProperties.get(UNITY_CATALOG_TABLE_ID_PROP); - if (tableId != null && !tableId.trim().isEmpty()) { - return true; - } - - return tableProperties.keySet().stream() - .filter(Objects::nonNull) - .anyMatch(key -> key.startsWith(UNITY_CATALOG_PROPERTY_PREFIX)); + Map tableProperties = toJavaMap(table.properties()); + boolean isUCBacked = tableProperties.containsKey(UCCommitCoordinatorClient.UC_TABLE_ID_KEY); + return isUCBacked && isCatalogManaged(table); } - private static boolean isCatalogOwnedFeatureSupported( + public static boolean isCatalogManagedFeatureEnabled( Map tableProperties, String featureKey) { - if (tableProperties == null) { + requireNonNull(tableProperties, "tableProperties is null"); + requireNonNull(featureKey, "featureKey is null"); + String featureValue = tableProperties.get(featureKey); + if (featureValue == null) { return false; } - String value = tableProperties.get(featureKey); - if (value == null) { - return false; - } - - return SUPPORTED.equalsIgnoreCase(value.trim()); + return featureValue.equalsIgnoreCase(SUPPORTED); } private static Map toJavaMap( diff --git a/kernel-spark/src/main/java/io/delta/kernel/spark/utils/delta-uc.code-workspace b/kernel-spark/src/main/java/io/delta/kernel/spark/utils/delta-uc.code-workspace new file mode 100644 index 00000000000..21426cccaaa --- /dev/null +++ b/kernel-spark/src/main/java/io/delta/kernel/spark/utils/delta-uc.code-workspace @@ -0,0 +1,15 @@ +{ + "folders": [ + { + "path": "../../../../../../../../.." + }, + { + "path": "../../../../../../../../../../unitycatalog" + } + ], + "settings": { + "files.watcherExclude": { + "**/target": true + } + } +} \ No newline at end of file diff --git a/kernel-spark/src/test/java/io/delta/kernel/spark/utils/CatalogTableUtilsTest.java b/kernel-spark/src/test/java/io/delta/kernel/spark/utils/CatalogTableUtilsTest.java index f160f65c8c3..1048ccf8503 100644 --- a/kernel-spark/src/test/java/io/delta/kernel/spark/utils/CatalogTableUtilsTest.java +++ b/kernel-spark/src/test/java/io/delta/kernel/spark/utils/CatalogTableUtilsTest.java @@ -15,160 +15,129 @@ */ package io.delta.kernel.spark.utils; -import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertTrue; import java.util.Collections; -import java.util.HashMap; import java.util.Map; -import java.util.stream.Stream; import org.apache.spark.sql.catalyst.TableIdentifier; -import org.apache.spark.sql.catalyst.catalog.CatalogStorageFormat; import org.apache.spark.sql.catalyst.catalog.CatalogStorageFormat$; import org.apache.spark.sql.catalyst.catalog.CatalogTable; import org.apache.spark.sql.catalyst.catalog.CatalogTable$; import org.apache.spark.sql.catalyst.catalog.CatalogTableType$; import org.apache.spark.sql.types.StructType; import org.junit.jupiter.api.Test; -import org.junit.jupiter.params.ParameterizedTest; -import org.junit.jupiter.params.provider.Arguments; -import org.junit.jupiter.params.provider.MethodSource; -import org.junit.jupiter.params.provider.ValueSource; -import scala.Option; import scala.Option$; import scala.collection.immutable.Map$; import scala.jdk.javaapi.CollectionConverters; /** Tests for {@link CatalogTableUtils}. */ -public class CatalogTableUtilsTest { - - @ParameterizedTest(name = "{0}") - @MethodSource("catalogManagedMapCases") - void mapBasedDetection(String name, Map properties, boolean expected) { - assertEquals(expected, CatalogTableUtils.isCatalogManagedTable(properties)); - } - - static Stream catalogManagedMapCases() { - return Stream.of( - Arguments.of("null map", null, false), - Arguments.of("empty map", Collections.emptyMap(), false), - Arguments.of( - "table id present", - javaMap(CatalogTableUtils.UNITY_CATALOG_TABLE_ID_PROP, "123"), - true), - Arguments.of( - "unity catalog prefix fallback", javaMap("delta.unityCatalog.random", "enabled"), true), - Arguments.of( - "blank table id still catalog managed", - javaMap(CatalogTableUtils.UNITY_CATALOG_TABLE_ID_PROP, " "), - true), - Arguments.of("irrelevant properties only", javaMap("delta.feature.other", "true"), false)); - } +class CatalogTableUtilsTest { @Test - void catalogTableOverloadMatchesMapDetection() { - Map managed = javaMap(CatalogTableUtils.UNITY_CATALOG_TABLE_ID_PROP, "abc-123"); - Map unmanaged = Collections.emptyMap(); + void catalogManagedFlagEnablesDetection() { + // Arrange: table with catalogManaged flag set to "supported" + CatalogTable table = + catalogTableWithProperties(Map.of(CatalogTableUtils.FEATURE_CATALOG_MANAGED, "supported")); - assertTrue(CatalogTableUtils.isCatalogManagedTable(catalogTableWithProperties(managed))); - assertFalse(CatalogTableUtils.isCatalogManagedTable(catalogTableWithProperties(unmanaged))); + // Act & Assert + assertTrue( + CatalogTableUtils.isCatalogManaged(table), "Should detect catalog management with flag"); + assertFalse( + CatalogTableUtils.isUnityCatalogManagedTable(table), "Should not detect Unity without ID"); } - @ParameterizedTest(name = "feature {0} enables CCv2 when catalog managed") - @ValueSource( - strings = { - CatalogTableUtils.FEATURE_CATALOG_OWNED, - CatalogTableUtils.FEATURE_CATALOG_OWNED_PREVIEW - }) - void isCCv2TableRecognisesSupportedFeatures(String featureKey) { + @Test + void previewFlagEnablesDetectionIgnoringCase() { + // Arrange: table with preview flag set to mixed case CatalogTable table = catalogTableWithProperties( - javaMap( - CatalogTableUtils.UNITY_CATALOG_TABLE_ID_PROP, - "table-id", - featureKey, - " supported ")); - - assertTrue(CatalogTableUtils.isCatalogManagedTable(table)); - assertTrue(CatalogTableUtils.isCCv2Table(table)); + Map.of(CatalogTableUtils.FEATURE_CATALOG_OWNED_PREVIEW, "SuPpOrTeD")); + + // Act & Assert + assertTrue( + CatalogTableUtils.isCatalogManaged(table), "Should detect via preview flag ignoring case"); + assertFalse( + CatalogTableUtils.isUnityCatalogManagedTable(table), "Should not detect Unity without ID"); } @Test - void isCCv2TableRequiresFeatureFlag() { - CatalogTable table = - catalogTableWithProperties( - javaMap(CatalogTableUtils.UNITY_CATALOG_TABLE_ID_PROP, "table-id")); - - assertTrue(CatalogTableUtils.isCatalogManagedTable(table)); - assertFalse(CatalogTableUtils.isCCv2Table(table)); + void noFlagsMeansNotManaged() { + // Arrange: empty properties + CatalogTable table = catalogTableWithProperties(Collections.emptyMap()); + + // Act & Assert + assertFalse( + CatalogTableUtils.isCatalogManaged(table), "Should not detect management without flags"); + assertFalse( + CatalogTableUtils.isUnityCatalogManagedTable(table), + "Should not detect Unity without ID or flags"); } @Test - void isCCv2TableRejectsUnsupportedValues() { + void unityManagementRequiresFlagAndId() { + // Arrange: table with both flag and UC ID CatalogTable table = catalogTableWithProperties( - javaMap( - CatalogTableUtils.UNITY_CATALOG_TABLE_ID_PROP, - "table-id", - CatalogTableUtils.FEATURE_CATALOG_OWNED, - "disabled")); - - assertTrue(CatalogTableUtils.isCatalogManagedTable(table)); - assertFalse(CatalogTableUtils.isCCv2Table(table)); + Map.of( + CatalogTableUtils.FEATURE_CATALOG_MANAGED, "supported", + io.delta.storage.commit.uccommitcoordinator.UCCommitCoordinatorClient + .UC_TABLE_ID_KEY, + "abc-123")); + + // Act & Assert + assertTrue( + CatalogTableUtils.isCatalogManaged(table), "Should detect general catalog management"); + assertTrue( + CatalogTableUtils.isUnityCatalogManagedTable(table), "Should detect Unity with ID present"); } @Test - void isCCv2TableIgnoresFeatureWithoutCatalogManagement() { + void unityManagementFailsWithoutId() { + // Arrange: table with flag but no UC ID CatalogTable table = - catalogTableWithProperties(javaMap(CatalogTableUtils.FEATURE_CATALOG_OWNED, "supported")); - - assertFalse(CatalogTableUtils.isCatalogManagedTable(table)); - assertFalse(CatalogTableUtils.isCCv2Table(table)); - } - - private static Map javaMap(String... keyValues) { - if (keyValues.length % 2 != 0) { - throw new IllegalArgumentException("keyValues length must be even"); - } - Map map = new HashMap<>(); - for (int i = 0; i < keyValues.length; i += 2) { - map.put(keyValues[i], keyValues[i + 1]); - } - return map; + catalogTableWithProperties(Map.of(CatalogTableUtils.FEATURE_CATALOG_MANAGED, "supported")); + + // Act & Assert + assertTrue( + CatalogTableUtils.isCatalogManaged(table), "Should detect general catalog management"); + assertFalse( + CatalogTableUtils.isUnityCatalogManagedTable(table), + "Should fail Unity detection without ID"); } private static CatalogTable catalogTableWithProperties(Map properties) { - Option none = Option$.MODULE$.empty(); - TableIdentifier identifier = new TableIdentifier("test_table", none, none); - CatalogStorageFormat storage = CatalogStorageFormat$.MODULE$.empty(); - scala.collection.immutable.Map scalaProps = - properties == null || properties.isEmpty() - ? Map$.MODULE$.empty() - : scala.collection.immutable.Map$.MODULE$.from( + properties.isEmpty() + ? Map$.MODULE$.empty() + : scala.collection.immutable.Map$.MODULE$.from( CollectionConverters.asScala(properties).toSeq()); + @SuppressWarnings("unchecked") + scala.collection.immutable.Seq emptySeq = + (scala.collection.immutable.Seq) scala.collection.immutable.Seq$.MODULE$.empty(); + return CatalogTable$.MODULE$.apply( - identifier, + new TableIdentifier("tbl", Option$.MODULE$.empty(), Option$.MODULE$.empty()), CatalogTableType$.MODULE$.MANAGED(), - storage, + CatalogStorageFormat$.MODULE$.empty(), new StructType(), - CatalogTable$.MODULE$.apply$default$5(), - CatalogTable$.MODULE$.apply$default$6(), - CatalogTable$.MODULE$.apply$default$7(), - CatalogTable$.MODULE$.apply$default$8(), - CatalogTable$.MODULE$.apply$default$9(), - CatalogTable$.MODULE$.apply$default$10(), - CatalogTable$.MODULE$.apply$default$11(), - scalaProps, - CatalogTable$.MODULE$.apply$default$13(), - CatalogTable$.MODULE$.apply$default$14(), - CatalogTable$.MODULE$.apply$default$15(), - CatalogTable$.MODULE$.apply$default$16(), - CatalogTable$.MODULE$.apply$default$17(), - CatalogTable$.MODULE$.apply$default$18(), - CatalogTable$.MODULE$.apply$default$19(), - CatalogTable$.MODULE$.apply$default$20()); + Option$.MODULE$.empty(), // provider: Option[String] + emptySeq, // partitionColumnNames: Seq[String] + Option$.MODULE$.empty(), // bucketSpec: Option[BucketSpec] + "", // owner: String + 0L, // createTime: Long + -1L, // lastAccessTime: Long + "", // createVersion: String + scalaProps, // properties: Map[String, String] + Option$.MODULE$.empty(), // stats: Option[CatalogStatistics] + Option$.MODULE$.empty(), // viewText: Option[String] + Option$.MODULE$.empty(), // comment: Option[String] + emptySeq, // unsupportedFeatures: Seq[String] + false, // tracksPartitionsInCatalog: Boolean + false, // schemaPreservesCase: Boolean + Map$.MODULE$.empty(), // ignoredProperties: Map[String, String] + Option$.MODULE$.empty() // viewOriginalText: Option[String] + ); } } From 030b548563d269cde2ea3483b933bebc00cea29e Mon Sep 17 00:00:00 2001 From: Timothy Wang Date: Wed, 12 Nov 2025 12:16:29 -0800 Subject: [PATCH 07/42] delete workspace Signed-off-by: Timothy Wang --- .../kernel/spark/utils/delta-uc.code-workspace | 18 +++++------------- 1 file changed, 5 insertions(+), 13 deletions(-) diff --git a/kernel-spark/src/main/java/io/delta/kernel/spark/utils/delta-uc.code-workspace b/kernel-spark/src/main/java/io/delta/kernel/spark/utils/delta-uc.code-workspace index 21426cccaaa..ebd79b0a59d 100644 --- a/kernel-spark/src/main/java/io/delta/kernel/spark/utils/delta-uc.code-workspace +++ b/kernel-spark/src/main/java/io/delta/kernel/spark/utils/delta-uc.code-workspace @@ -1,15 +1,7 @@ { - "folders": [ - { - "path": "../../../../../../../../.." - }, - { - "path": "../../../../../../../../../../unitycatalog" - } - ], - "settings": { - "files.watcherExclude": { - "**/target": true - } - } + "settings": { + "files.watcherExclude": { + "**/target": true + } + } } \ No newline at end of file From 85c45ebab91810fa908583a8fe98ba1abdecf55d Mon Sep 17 00:00:00 2001 From: Timothy Wang Date: Wed, 12 Nov 2025 12:16:52 -0800 Subject: [PATCH 08/42] delete Signed-off-by: Timothy Wang --- .../io/delta/kernel/spark/utils/delta-uc.code-workspace | 7 ------- 1 file changed, 7 deletions(-) delete mode 100644 kernel-spark/src/main/java/io/delta/kernel/spark/utils/delta-uc.code-workspace diff --git a/kernel-spark/src/main/java/io/delta/kernel/spark/utils/delta-uc.code-workspace b/kernel-spark/src/main/java/io/delta/kernel/spark/utils/delta-uc.code-workspace deleted file mode 100644 index ebd79b0a59d..00000000000 --- a/kernel-spark/src/main/java/io/delta/kernel/spark/utils/delta-uc.code-workspace +++ /dev/null @@ -1,7 +0,0 @@ -{ - "settings": { - "files.watcherExclude": { - "**/target": true - } - } -} \ No newline at end of file From 1ad766d6d90e5c1121dbaf9263608c58f169dd7d Mon Sep 17 00:00:00 2001 From: Timothy Wang Date: Wed, 12 Nov 2025 13:30:58 -0800 Subject: [PATCH 09/42] fmt Signed-off-by: Timothy Wang --- .../spark/utils/CatalogTableUtilsTest.java | 54 +++++++++++-------- 1 file changed, 31 insertions(+), 23 deletions(-) diff --git a/kernel-spark/src/test/java/io/delta/kernel/spark/utils/CatalogTableUtilsTest.java b/kernel-spark/src/test/java/io/delta/kernel/spark/utils/CatalogTableUtilsTest.java index 1048ccf8503..328aa613d02 100644 --- a/kernel-spark/src/test/java/io/delta/kernel/spark/utils/CatalogTableUtilsTest.java +++ b/kernel-spark/src/test/java/io/delta/kernel/spark/utils/CatalogTableUtilsTest.java @@ -36,11 +36,9 @@ class CatalogTableUtilsTest { @Test void catalogManagedFlagEnablesDetection() { - // Arrange: table with catalogManaged flag set to "supported" CatalogTable table = catalogTableWithProperties(Map.of(CatalogTableUtils.FEATURE_CATALOG_MANAGED, "supported")); - // Act & Assert assertTrue( CatalogTableUtils.isCatalogManaged(table), "Should detect catalog management with flag"); assertFalse( @@ -49,12 +47,10 @@ void catalogManagedFlagEnablesDetection() { @Test void previewFlagEnablesDetectionIgnoringCase() { - // Arrange: table with preview flag set to mixed case CatalogTable table = catalogTableWithProperties( Map.of(CatalogTableUtils.FEATURE_CATALOG_OWNED_PREVIEW, "SuPpOrTeD")); - // Act & Assert assertTrue( CatalogTableUtils.isCatalogManaged(table), "Should detect via preview flag ignoring case"); assertFalse( @@ -63,10 +59,8 @@ void previewFlagEnablesDetectionIgnoringCase() { @Test void noFlagsMeansNotManaged() { - // Arrange: empty properties CatalogTable table = catalogTableWithProperties(Collections.emptyMap()); - // Act & Assert assertFalse( CatalogTableUtils.isCatalogManaged(table), "Should not detect management without flags"); assertFalse( @@ -76,7 +70,6 @@ void noFlagsMeansNotManaged() { @Test void unityManagementRequiresFlagAndId() { - // Arrange: table with both flag and UC ID CatalogTable table = catalogTableWithProperties( Map.of( @@ -85,7 +78,6 @@ void unityManagementRequiresFlagAndId() { .UC_TABLE_ID_KEY, "abc-123")); - // Act & Assert assertTrue( CatalogTableUtils.isCatalogManaged(table), "Should detect general catalog management"); assertTrue( @@ -94,11 +86,9 @@ void unityManagementRequiresFlagAndId() { @Test void unityManagementFailsWithoutId() { - // Arrange: table with flag but no UC ID CatalogTable table = catalogTableWithProperties(Map.of(CatalogTableUtils.FEATURE_CATALOG_MANAGED, "supported")); - // Act & Assert assertTrue( CatalogTableUtils.isCatalogManaged(table), "Should detect general catalog management"); assertFalse( @@ -106,6 +96,13 @@ void unityManagementFailsWithoutId() { "Should fail Unity detection without ID"); } + /** + * Creates a CatalogTable with the given properties. This is a helper method to create a + * CatalogTable for testing purposes - see interface {@link CatalogTable} for more details. + * + * @param properties the properties to set on the CatalogTable + * @return a CatalogTable with the given properties + */ private static CatalogTable catalogTableWithProperties(Map properties) { scala.collection.immutable.Map scalaProps = properties.isEmpty() @@ -113,31 +110,42 @@ private static CatalogTable catalogTableWithProperties(Map prope : scala.collection.immutable.Map$.MODULE$.from( CollectionConverters.asScala(properties).toSeq()); - @SuppressWarnings("unchecked") - scala.collection.immutable.Seq emptySeq = - (scala.collection.immutable.Seq) scala.collection.immutable.Seq$.MODULE$.empty(); - return CatalogTable$.MODULE$.apply( new TableIdentifier("tbl", Option$.MODULE$.empty(), Option$.MODULE$.empty()), CatalogTableType$.MODULE$.MANAGED(), CatalogStorageFormat$.MODULE$.empty(), new StructType(), - Option$.MODULE$.empty(), // provider: Option[String] - emptySeq, // partitionColumnNames: Seq[String] - Option$.MODULE$.empty(), // bucketSpec: Option[BucketSpec] + scalaNone(), // provider: Option[String] + scalaEmptySeq(), // partitionColumnNames: Seq[String] + scalaNone(), // bucketSpec: Option[BucketSpec] "", // owner: String 0L, // createTime: Long -1L, // lastAccessTime: Long "", // createVersion: String scalaProps, // properties: Map[String, String] - Option$.MODULE$.empty(), // stats: Option[CatalogStatistics] - Option$.MODULE$.empty(), // viewText: Option[String] - Option$.MODULE$.empty(), // comment: Option[String] - emptySeq, // unsupportedFeatures: Seq[String] + scalaNone(), // stats: Option[CatalogStatistics] + scalaNone(), // viewText: Option[String] + scalaNone(), // comment: Option[String] + scalaEmptySeq(), // unsupportedFeatures: Seq[String] false, // tracksPartitionsInCatalog: Boolean false, // schemaPreservesCase: Boolean - Map$.MODULE$.empty(), // ignoredProperties: Map[String, String] - Option$.MODULE$.empty() // viewOriginalText: Option[String] + scalaEmptyMap(), // ignoredProperties: Map[String, String] + scalaNone() // viewOriginalText: Option[String] ); } + + @SuppressWarnings("unchecked") + private static scala.Option scalaNone() { + return (scala.Option) Option$.MODULE$.empty(); + } + + @SuppressWarnings("unchecked") + private static scala.collection.immutable.Seq scalaEmptySeq() { + return (scala.collection.immutable.Seq) scala.collection.immutable.Seq$.MODULE$.empty(); + } + + @SuppressWarnings("unchecked") + private static scala.collection.immutable.Map scalaEmptyMap() { + return (scala.collection.immutable.Map) Map$.MODULE$.empty(); + } } From d6c5b475b5e509ac0b6c45974fdc11e0f5d5ba7e Mon Sep 17 00:00:00 2001 From: Timothy Wang Date: Wed, 12 Nov 2025 17:10:37 -0800 Subject: [PATCH 10/42] test fix Signed-off-by: Timothy Wang --- .../spark/utils/CatalogTableUtilsTest.java | 43 +++++++++++-------- 1 file changed, 25 insertions(+), 18 deletions(-) diff --git a/kernel-spark/src/test/java/io/delta/kernel/spark/utils/CatalogTableUtilsTest.java b/kernel-spark/src/test/java/io/delta/kernel/spark/utils/CatalogTableUtilsTest.java index 328aa613d02..2a0e67f5f13 100644 --- a/kernel-spark/src/test/java/io/delta/kernel/spark/utils/CatalogTableUtilsTest.java +++ b/kernel-spark/src/test/java/io/delta/kernel/spark/utils/CatalogTableUtilsTest.java @@ -21,6 +21,8 @@ import java.util.Collections; import java.util.Map; import org.apache.spark.sql.catalyst.TableIdentifier; +import org.apache.spark.sql.catalyst.catalog.BucketSpec; +import org.apache.spark.sql.catalyst.catalog.CatalogStatistics; import org.apache.spark.sql.catalyst.catalog.CatalogStorageFormat$; import org.apache.spark.sql.catalyst.catalog.CatalogTable; import org.apache.spark.sql.catalyst.catalog.CatalogTable$; @@ -115,37 +117,42 @@ private static CatalogTable catalogTableWithProperties(Map prope CatalogTableType$.MODULE$.MANAGED(), CatalogStorageFormat$.MODULE$.empty(), new StructType(), - scalaNone(), // provider: Option[String] - scalaEmptySeq(), // partitionColumnNames: Seq[String] - scalaNone(), // bucketSpec: Option[BucketSpec] + noneString(), // provider: Option[String] + emptyStringSeq(), // partitionColumnNames: Seq[String] + noneBucketSpec(), // bucketSpec: Option[BucketSpec] "", // owner: String 0L, // createTime: Long -1L, // lastAccessTime: Long "", // createVersion: String scalaProps, // properties: Map[String, String] - scalaNone(), // stats: Option[CatalogStatistics] - scalaNone(), // viewText: Option[String] - scalaNone(), // comment: Option[String] - scalaEmptySeq(), // unsupportedFeatures: Seq[String] + noneCatalogStatistics(), // stats: Option[CatalogStatistics] + noneString(), // viewText: Option[String] + noneString(), // comment: Option[String] + emptyStringSeq(), // unsupportedFeatures: Seq[String] false, // tracksPartitionsInCatalog: Boolean false, // schemaPreservesCase: Boolean - scalaEmptyMap(), // ignoredProperties: Map[String, String] - scalaNone() // viewOriginalText: Option[String] + emptyStringMap(), // ignoredProperties: Map[String, String] + noneString() // viewOriginalText: Option[String] ); } - @SuppressWarnings("unchecked") - private static scala.Option scalaNone() { - return (scala.Option) Option$.MODULE$.empty(); + private static scala.Option noneString() { + return Option$.MODULE$.empty(); } - @SuppressWarnings("unchecked") - private static scala.collection.immutable.Seq scalaEmptySeq() { - return (scala.collection.immutable.Seq) scala.collection.immutable.Seq$.MODULE$.empty(); + private static scala.Option noneBucketSpec() { + return Option$.MODULE$.empty(); } - @SuppressWarnings("unchecked") - private static scala.collection.immutable.Map scalaEmptyMap() { - return (scala.collection.immutable.Map) Map$.MODULE$.empty(); + private static scala.Option noneCatalogStatistics() { + return Option$.MODULE$.empty(); + } + + private static scala.collection.immutable.Seq emptyStringSeq() { + return scala.collection.immutable.Seq$.MODULE$.empty(); + } + + private static scala.collection.immutable.Map emptyStringMap() { + return Map$.MODULE$.empty(); } } From 12d1cfd943f751cd69f186548c11d8928cdc385c Mon Sep 17 00:00:00 2001 From: TimothyW553 Date: Thu, 13 Nov 2025 23:53:27 +0000 Subject: [PATCH 11/42] fix catalogtable mcok Signed-off-by: TimothyW553 --- .../java/io/delta/kernel/spark/utils/CatalogTableUtilsTest.java | 1 + 1 file changed, 1 insertion(+) diff --git a/kernel-spark/src/test/java/io/delta/kernel/spark/utils/CatalogTableUtilsTest.java b/kernel-spark/src/test/java/io/delta/kernel/spark/utils/CatalogTableUtilsTest.java index 2a0e67f5f13..4f6834bce21 100644 --- a/kernel-spark/src/test/java/io/delta/kernel/spark/utils/CatalogTableUtilsTest.java +++ b/kernel-spark/src/test/java/io/delta/kernel/spark/utils/CatalogTableUtilsTest.java @@ -128,6 +128,7 @@ private static CatalogTable catalogTableWithProperties(Map prope noneCatalogStatistics(), // stats: Option[CatalogStatistics] noneString(), // viewText: Option[String] noneString(), // comment: Option[String] + noneString(), // viewDefaultDatabase: Option[String] emptyStringSeq(), // unsupportedFeatures: Seq[String] false, // tracksPartitionsInCatalog: Boolean false, // schemaPreservesCase: Boolean From b2b44284bf95160fc1a47ecf15a46a26b90cce5a Mon Sep 17 00:00:00 2001 From: TimothyW553 Date: Fri, 14 Nov 2025 18:53:27 +0000 Subject: [PATCH 12/42] storage props Signed-off-by: TimothyW553 --- .../kernel/spark/utils/CatalogTableUtils.java | 32 ++++---- .../delta/kernel/spark/utils/ScalaUtils.java | 10 +++ .../spark/utils/CatalogTableUtilsTest.java | 73 ++++++++++++++----- 3 files changed, 81 insertions(+), 34 deletions(-) diff --git a/kernel-spark/src/main/java/io/delta/kernel/spark/utils/CatalogTableUtils.java b/kernel-spark/src/main/java/io/delta/kernel/spark/utils/CatalogTableUtils.java index d6c95fae40b..12ebb1aa81d 100644 --- a/kernel-spark/src/main/java/io/delta/kernel/spark/utils/CatalogTableUtils.java +++ b/kernel-spark/src/main/java/io/delta/kernel/spark/utils/CatalogTableUtils.java @@ -3,10 +3,8 @@ import static java.util.Objects.requireNonNull; import io.delta.storage.commit.uccommitcoordinator.UCCommitCoordinatorClient; -import java.util.Collections; import java.util.Map; import org.apache.spark.sql.catalyst.catalog.CatalogTable; -import scala.jdk.javaapi.CollectionConverters; /** * Utility helpers for inspecting Delta-related metadata persisted on Spark {@link CatalogTable} @@ -36,20 +34,30 @@ private CatalogTableUtils() {} // if the catalogManaged/catalogOwned-preview flags are 'supported' public static boolean isCatalogManaged(CatalogTable table) { requireNonNull(table, "table is null"); - Map tableProperties = toJavaMap(table.properties()); - return isCatalogManagedFeatureEnabled(tableProperties, FEATURE_CATALOG_MANAGED) - || isCatalogManagedFeatureEnabled(tableProperties, FEATURE_CATALOG_OWNED_PREVIEW); + Map storageProperties = getStorageProperties(table); + return isCatalogManagedFeatureEnabled(storageProperties, FEATURE_CATALOG_MANAGED) + || isCatalogManagedFeatureEnabled(storageProperties, FEATURE_CATALOG_OWNED_PREVIEW); } - // Checks if table is *Unity Catalog* managed - meaning it isCatalogManaged and it contains + // Checks if table is *Unity Catalog* managed - meaning it isCatalogManaged, and it contains // a UC identifier (UC_TABLE_ID_KEY) public static boolean isUnityCatalogManagedTable(CatalogTable table) { requireNonNull(table, "table is null"); - Map tableProperties = toJavaMap(table.properties()); - boolean isUCBacked = tableProperties.containsKey(UCCommitCoordinatorClient.UC_TABLE_ID_KEY); + Map storageProperties = getStorageProperties(table); + boolean isUCBacked = storageProperties.containsKey(UCCommitCoordinatorClient.UC_TABLE_ID_KEY); return isUCBacked && isCatalogManaged(table); } + /** + * Returns storage-layer metadata published with a {@link CatalogTable}. Unity Catalog and other + * Spark connectors propagate connection credentials and format-specific options through the + * storage properties map rather than the logical table properties map. + */ + public static Map getStorageProperties(CatalogTable table) { + requireNonNull(table, "table is null"); + return ScalaUtils.toJavaMap(table.storage().properties()); + } + public static boolean isCatalogManagedFeatureEnabled( Map tableProperties, String featureKey) { requireNonNull(tableProperties, "tableProperties is null"); @@ -60,12 +68,4 @@ public static boolean isCatalogManagedFeatureEnabled( } return featureValue.equalsIgnoreCase(SUPPORTED); } - - private static Map toJavaMap( - scala.collection.immutable.Map scalaMap) { - if (scalaMap == null || scalaMap.isEmpty()) { - return Collections.emptyMap(); - } - return CollectionConverters.asJava(scalaMap); - } } diff --git a/kernel-spark/src/main/java/io/delta/kernel/spark/utils/ScalaUtils.java b/kernel-spark/src/main/java/io/delta/kernel/spark/utils/ScalaUtils.java index 3832dbd5fb3..39317f96b99 100644 --- a/kernel-spark/src/main/java/io/delta/kernel/spark/utils/ScalaUtils.java +++ b/kernel-spark/src/main/java/io/delta/kernel/spark/utils/ScalaUtils.java @@ -15,10 +15,12 @@ */ package io.delta.kernel.spark.utils; +import java.util.Collections; import java.util.Map; import scala.Tuple2; import scala.collection.immutable.Map$; import scala.collection.mutable.Builder; +import scala.jdk.javaapi.CollectionConverters; public final class ScalaUtils { public static scala.collection.immutable.Map toScalaMap( @@ -35,4 +37,12 @@ public static scala.collection.immutable.Map toScalaMap( } return b.result(); } + + public static Map toJavaMap( + scala.collection.immutable.Map scalaMap) { + if (scalaMap == null || scalaMap.isEmpty()) { + return Collections.emptyMap(); + } + return CollectionConverters.asJava(scalaMap); + } } diff --git a/kernel-spark/src/test/java/io/delta/kernel/spark/utils/CatalogTableUtilsTest.java b/kernel-spark/src/test/java/io/delta/kernel/spark/utils/CatalogTableUtilsTest.java index 4f6834bce21..0e89ccc9880 100644 --- a/kernel-spark/src/test/java/io/delta/kernel/spark/utils/CatalogTableUtilsTest.java +++ b/kernel-spark/src/test/java/io/delta/kernel/spark/utils/CatalogTableUtilsTest.java @@ -15,9 +15,11 @@ */ package io.delta.kernel.spark.utils; +import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertTrue; +import java.net.URI; import java.util.Collections; import java.util.Map; import org.apache.spark.sql.catalyst.TableIdentifier; @@ -39,7 +41,8 @@ class CatalogTableUtilsTest { @Test void catalogManagedFlagEnablesDetection() { CatalogTable table = - catalogTableWithProperties(Map.of(CatalogTableUtils.FEATURE_CATALOG_MANAGED, "supported")); + catalogTableWithProperties( + Collections.emptyMap(), Map.of(CatalogTableUtils.FEATURE_CATALOG_MANAGED, "supported")); assertTrue( CatalogTableUtils.isCatalogManaged(table), "Should detect catalog management with flag"); @@ -51,6 +54,7 @@ void catalogManagedFlagEnablesDetection() { void previewFlagEnablesDetectionIgnoringCase() { CatalogTable table = catalogTableWithProperties( + Collections.emptyMap(), Map.of(CatalogTableUtils.FEATURE_CATALOG_OWNED_PREVIEW, "SuPpOrTeD")); assertTrue( @@ -74,6 +78,7 @@ void noFlagsMeansNotManaged() { void unityManagementRequiresFlagAndId() { CatalogTable table = catalogTableWithProperties( + Collections.emptyMap(), Map.of( CatalogTableUtils.FEATURE_CATALOG_MANAGED, "supported", io.delta.storage.commit.uccommitcoordinator.UCCommitCoordinatorClient @@ -89,7 +94,8 @@ void unityManagementRequiresFlagAndId() { @Test void unityManagementFailsWithoutId() { CatalogTable table = - catalogTableWithProperties(Map.of(CatalogTableUtils.FEATURE_CATALOG_MANAGED, "supported")); + catalogTableWithProperties( + Collections.emptyMap(), Map.of(CatalogTableUtils.FEATURE_CATALOG_MANAGED, "supported")); assertTrue( CatalogTableUtils.isCatalogManaged(table), "Should detect general catalog management"); @@ -98,6 +104,17 @@ void unityManagementFailsWithoutId() { "Should fail Unity detection without ID"); } + @Test + void storagePropertiesExposeStorageMetadata() { + Map storageProps = Map.of("fs.test.option", "value", "dfs.conf.key", "abc"); + CatalogTable table = catalogTableWithProperties(Collections.emptyMap(), storageProps); + + assertEquals( + storageProps, + CatalogTableUtils.getStorageProperties(table), + "Should surface storage properties published by the catalog"); + } + /** * Creates a CatalogTable with the given properties. This is a helper method to create a * CatalogTable for testing purposes - see interface {@link CatalogTable} for more details. @@ -106,29 +123,45 @@ void unityManagementFailsWithoutId() { * @return a CatalogTable with the given properties */ private static CatalogTable catalogTableWithProperties(Map properties) { + return catalogTableWithProperties(properties, Collections.emptyMap()); + } + + private static CatalogTable catalogTableWithProperties( + Map properties, Map storageProperties) { scala.collection.immutable.Map scalaProps = properties.isEmpty() ? Map$.MODULE$.empty() : scala.collection.immutable.Map$.MODULE$.from( CollectionConverters.asScala(properties).toSeq()); + scala.collection.immutable.Map scalaStorageProps = + storageProperties.isEmpty() + ? Map$.MODULE$.empty() + : scala.collection.immutable.Map$.MODULE$.from( + CollectionConverters.asScala(storageProperties).toSeq()); return CatalogTable$.MODULE$.apply( - new TableIdentifier("tbl", Option$.MODULE$.empty(), Option$.MODULE$.empty()), - CatalogTableType$.MODULE$.MANAGED(), - CatalogStorageFormat$.MODULE$.empty(), - new StructType(), - noneString(), // provider: Option[String] - emptyStringSeq(), // partitionColumnNames: Seq[String] - noneBucketSpec(), // bucketSpec: Option[BucketSpec] - "", // owner: String - 0L, // createTime: Long - -1L, // lastAccessTime: Long - "", // createVersion: String - scalaProps, // properties: Map[String, String] - noneCatalogStatistics(), // stats: Option[CatalogStatistics] - noneString(), // viewText: Option[String] - noneString(), // comment: Option[String] - noneString(), // viewDefaultDatabase: Option[String] + new TableIdentifier( + "tbl", Option$.MODULE$.empty(), Option$.MODULE$.empty()), // id: TableIdentifier + CatalogTableType$.MODULE$.MANAGED(), // tableType: CatalogTableType + CatalogStorageFormat$.MODULE$.apply( + noneUri(), + noneString(), + noneString(), + noneString(), + false, + scalaStorageProps), // storage: CatalogStorageFormat + new StructType(), // schema: StructType + noneString(), // provider: Option[String] = None + emptyStringSeq(), // partitionColumnNames: Seq[String] = Seq.empty + noneBucketSpec(), // bucketSpec: Option[BucketSpec] = None + "", // owner: String = "" + 0L, // createTime: Long = System.currentTimeMillis + -1L, // lastAccessTime: Long = -1 + "", // createVersion: String = "" + scalaProps, // properties: Map[String, String] = Map.empty + noneCatalogStatistics(), // stats: Option[CatalogStatistics] = None + noneString(), // viewText: Option[String] = None + noneString(), // comment: Option[String] = None emptyStringSeq(), // unsupportedFeatures: Seq[String] false, // tracksPartitionsInCatalog: Boolean false, // schemaPreservesCase: Boolean @@ -141,6 +174,10 @@ private static scala.Option noneString() { return Option$.MODULE$.empty(); } + private static scala.Option noneUri() { + return Option$.MODULE$.empty(); + } + private static scala.Option noneBucketSpec() { return Option$.MODULE$.empty(); } From 08321383a345734ed2886f0dcd81b80e366f14f2 Mon Sep 17 00:00:00 2001 From: TimothyW553 Date: Fri, 14 Nov 2025 22:24:45 +0000 Subject: [PATCH 13/42] Add CatalogTable test helper --- .../spark/utils/CatalogTableUtilsTest.java | 76 +------------------ .../spark/utils/CatalogTableTestUtils.scala | 49 ++++++++++++ 2 files changed, 50 insertions(+), 75 deletions(-) create mode 100644 kernel-spark/src/test/scala/io/delta/kernel/spark/utils/CatalogTableTestUtils.scala diff --git a/kernel-spark/src/test/java/io/delta/kernel/spark/utils/CatalogTableUtilsTest.java b/kernel-spark/src/test/java/io/delta/kernel/spark/utils/CatalogTableUtilsTest.java index 0e89ccc9880..43c4f3d1cc8 100644 --- a/kernel-spark/src/test/java/io/delta/kernel/spark/utils/CatalogTableUtilsTest.java +++ b/kernel-spark/src/test/java/io/delta/kernel/spark/utils/CatalogTableUtilsTest.java @@ -19,21 +19,10 @@ import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertTrue; -import java.net.URI; import java.util.Collections; import java.util.Map; -import org.apache.spark.sql.catalyst.TableIdentifier; -import org.apache.spark.sql.catalyst.catalog.BucketSpec; -import org.apache.spark.sql.catalyst.catalog.CatalogStatistics; -import org.apache.spark.sql.catalyst.catalog.CatalogStorageFormat$; import org.apache.spark.sql.catalyst.catalog.CatalogTable; -import org.apache.spark.sql.catalyst.catalog.CatalogTable$; -import org.apache.spark.sql.catalyst.catalog.CatalogTableType$; -import org.apache.spark.sql.types.StructType; import org.junit.jupiter.api.Test; -import scala.Option$; -import scala.collection.immutable.Map$; -import scala.jdk.javaapi.CollectionConverters; /** Tests for {@link CatalogTableUtils}. */ class CatalogTableUtilsTest { @@ -128,69 +117,6 @@ private static CatalogTable catalogTableWithProperties(Map prope private static CatalogTable catalogTableWithProperties( Map properties, Map storageProperties) { - scala.collection.immutable.Map scalaProps = - properties.isEmpty() - ? Map$.MODULE$.empty() - : scala.collection.immutable.Map$.MODULE$.from( - CollectionConverters.asScala(properties).toSeq()); - scala.collection.immutable.Map scalaStorageProps = - storageProperties.isEmpty() - ? Map$.MODULE$.empty() - : scala.collection.immutable.Map$.MODULE$.from( - CollectionConverters.asScala(storageProperties).toSeq()); - - return CatalogTable$.MODULE$.apply( - new TableIdentifier( - "tbl", Option$.MODULE$.empty(), Option$.MODULE$.empty()), // id: TableIdentifier - CatalogTableType$.MODULE$.MANAGED(), // tableType: CatalogTableType - CatalogStorageFormat$.MODULE$.apply( - noneUri(), - noneString(), - noneString(), - noneString(), - false, - scalaStorageProps), // storage: CatalogStorageFormat - new StructType(), // schema: StructType - noneString(), // provider: Option[String] = None - emptyStringSeq(), // partitionColumnNames: Seq[String] = Seq.empty - noneBucketSpec(), // bucketSpec: Option[BucketSpec] = None - "", // owner: String = "" - 0L, // createTime: Long = System.currentTimeMillis - -1L, // lastAccessTime: Long = -1 - "", // createVersion: String = "" - scalaProps, // properties: Map[String, String] = Map.empty - noneCatalogStatistics(), // stats: Option[CatalogStatistics] = None - noneString(), // viewText: Option[String] = None - noneString(), // comment: Option[String] = None - emptyStringSeq(), // unsupportedFeatures: Seq[String] - false, // tracksPartitionsInCatalog: Boolean - false, // schemaPreservesCase: Boolean - emptyStringMap(), // ignoredProperties: Map[String, String] - noneString() // viewOriginalText: Option[String] - ); - } - - private static scala.Option noneString() { - return Option$.MODULE$.empty(); - } - - private static scala.Option noneUri() { - return Option$.MODULE$.empty(); - } - - private static scala.Option noneBucketSpec() { - return Option$.MODULE$.empty(); - } - - private static scala.Option noneCatalogStatistics() { - return Option$.MODULE$.empty(); - } - - private static scala.collection.immutable.Seq emptyStringSeq() { - return scala.collection.immutable.Seq$.MODULE$.empty(); - } - - private static scala.collection.immutable.Map emptyStringMap() { - return Map$.MODULE$.empty(); + return CatalogTableTestUtils$.MODULE$.catalogTableWithProperties(properties, storageProperties); } } diff --git a/kernel-spark/src/test/scala/io/delta/kernel/spark/utils/CatalogTableTestUtils.scala b/kernel-spark/src/test/scala/io/delta/kernel/spark/utils/CatalogTableTestUtils.scala new file mode 100644 index 00000000000..6ee24279bdd --- /dev/null +++ b/kernel-spark/src/test/scala/io/delta/kernel/spark/utils/CatalogTableTestUtils.scala @@ -0,0 +1,49 @@ +/* + * Copyright (2025) The Delta Lake Project Authors. + * + * Licensed 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 io.delta.kernel.spark.utils + +import scala.collection.immutable.Seq + +import org.apache.spark.sql.catalyst.TableIdentifier +import org.apache.spark.sql.catalyst.catalog.{CatalogStorageFormat, CatalogTable, CatalogTableType} +import org.apache.spark.sql.types.StructType + +/** Helpers for constructing [[CatalogTable]] instances inside Java tests. */ +object CatalogTableTestUtils { + + def catalogTableWithProperties( + properties: java.util.Map[String, String], + storageProperties: java.util.Map[String, String]): CatalogTable = { + val scalaProps = ScalaUtils.toScalaMap(properties) + val scalaStorageProps = ScalaUtils.toScalaMap(storageProperties) + + CatalogTable( + identifier = TableIdentifier("tbl"), + tableType = CatalogTableType.MANAGED, + storage = CatalogStorageFormat( + locationUri = None, + inputFormat = None, + outputFormat = None, + serde = None, + compressed = false, + properties = scalaStorageProps), + schema = new StructType(), + provider = None, + partitionColumnNames = Seq.empty, + bucketSpec = None, + properties = scalaProps) + } +} From 5830196ab3c131617f46a4625dfb15ec22eb0a85 Mon Sep 17 00:00:00 2001 From: TimothyW553 Date: Mon, 17 Nov 2025 18:08:06 +0000 Subject: [PATCH 14/42] merge props Signed-off-by: TimothyW553 --- .../kernel/spark/utils/CatalogTableUtils.java | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/kernel-spark/src/main/java/io/delta/kernel/spark/utils/CatalogTableUtils.java b/kernel-spark/src/main/java/io/delta/kernel/spark/utils/CatalogTableUtils.java index 12ebb1aa81d..68d679105d4 100644 --- a/kernel-spark/src/main/java/io/delta/kernel/spark/utils/CatalogTableUtils.java +++ b/kernel-spark/src/main/java/io/delta/kernel/spark/utils/CatalogTableUtils.java @@ -3,6 +3,7 @@ import static java.util.Objects.requireNonNull; import io.delta.storage.commit.uccommitcoordinator.UCCommitCoordinatorClient; +import java.util.HashMap; import java.util.Map; import org.apache.spark.sql.catalyst.catalog.CatalogTable; @@ -10,9 +11,9 @@ * Utility helpers for inspecting Delta-related metadata persisted on Spark {@link CatalogTable} * instances by Unity Catalog. * - *

Unity Catalog marks catalog-managed tables via feature flags stored in table properties. This - * helper centralises the logic for interpreting those properties so the Kernel connector can decide - * when to use catalog-owned (CCv2) behaviour. + *

Unity Catalog marks catalog-managed tables via feature flags stored in table storage + * properties. This helper centralises the logic for interpreting those properties so the Kernel + * connector can decide when to use catalog-owned (CCv2) behaviour. * *

    *
  • {@link #isCatalogManaged(CatalogTable)} checks whether either {@code @@ -48,14 +49,14 @@ public static boolean isUnityCatalogManagedTable(CatalogTable table) { return isUCBacked && isCatalogManaged(table); } - /** - * Returns storage-layer metadata published with a {@link CatalogTable}. Unity Catalog and other - * Spark connectors propagate connection credentials and format-specific options through the - * storage properties map rather than the logical table properties map. - */ + // We merge the storage and logical properties into a single map because we want to be able to + // access both sets of properties in a single method. public static Map getStorageProperties(CatalogTable table) { requireNonNull(table, "table is null"); - return ScalaUtils.toJavaMap(table.storage().properties()); + Map merged = new HashMap<>(); + merged.putAll(ScalaUtils.toJavaMap(table.storage().properties())); + merged.putAll(ScalaUtils.toJavaMap(table.properties())); + return merged; } public static boolean isCatalogManagedFeatureEnabled( From 8b5c96212754e48b94a2440d714ca5061c5512e7 Mon Sep 17 00:00:00 2001 From: TimothyW553 Date: Mon, 17 Nov 2025 21:27:18 +0000 Subject: [PATCH 15/42] address comments --- .../kernel/spark/utils/CatalogTableUtils.java | 30 ++++--- .../spark/utils/CatalogTableUtilsTest.java | 89 +++++++++---------- 2 files changed, 61 insertions(+), 58 deletions(-) diff --git a/kernel-spark/src/main/java/io/delta/kernel/spark/utils/CatalogTableUtils.java b/kernel-spark/src/main/java/io/delta/kernel/spark/utils/CatalogTableUtils.java index 68d679105d4..05f235207a6 100644 --- a/kernel-spark/src/main/java/io/delta/kernel/spark/utils/CatalogTableUtils.java +++ b/kernel-spark/src/main/java/io/delta/kernel/spark/utils/CatalogTableUtils.java @@ -3,7 +3,6 @@ import static java.util.Objects.requireNonNull; import io.delta.storage.commit.uccommitcoordinator.UCCommitCoordinatorClient; -import java.util.HashMap; import java.util.Map; import org.apache.spark.sql.catalyst.catalog.CatalogTable; @@ -31,8 +30,12 @@ public final class CatalogTableUtils { private CatalogTableUtils() {} - // Checks whether *any* catalog manages this table via CCv2 semantics by checking - // if the catalogManaged/catalogOwned-preview flags are 'supported' + /** + * Checks whether any catalog manages this table via CCv2 semantics. + * + * @param table Spark {@link CatalogTable} descriptor + * @return {@code true} when either catalog feature flag is set to {@code supported} + */ public static boolean isCatalogManaged(CatalogTable table) { requireNonNull(table, "table is null"); Map storageProperties = getStorageProperties(table); @@ -40,8 +43,12 @@ public static boolean isCatalogManaged(CatalogTable table) { || isCatalogManagedFeatureEnabled(storageProperties, FEATURE_CATALOG_OWNED_PREVIEW); } - // Checks if table is *Unity Catalog* managed - meaning it isCatalogManaged, and it contains - // a UC identifier (UC_TABLE_ID_KEY) + /** + * Checks whether the table is Unity Catalog managed. + * + * @param table Spark {@link CatalogTable} descriptor + * @return {@code true} when the table is catalog managed and contains the UC identifier + */ public static boolean isUnityCatalogManagedTable(CatalogTable table) { requireNonNull(table, "table is null"); Map storageProperties = getStorageProperties(table); @@ -49,14 +56,15 @@ public static boolean isUnityCatalogManagedTable(CatalogTable table) { return isUCBacked && isCatalogManaged(table); } - // We merge the storage and logical properties into a single map because we want to be able to - // access both sets of properties in a single method. + /** + * Returns the catalog storage properties published with a {@link CatalogTable}. + * + * @param table Spark {@link CatalogTable} descriptor + * @return Java map view of the storage properties + */ public static Map getStorageProperties(CatalogTable table) { requireNonNull(table, "table is null"); - Map merged = new HashMap<>(); - merged.putAll(ScalaUtils.toJavaMap(table.storage().properties())); - merged.putAll(ScalaUtils.toJavaMap(table.properties())); - return merged; + return ScalaUtils.toJavaMap(table.storage().properties()); } public static boolean isCatalogManagedFeatureEnabled( diff --git a/kernel-spark/src/test/java/io/delta/kernel/spark/utils/CatalogTableUtilsTest.java b/kernel-spark/src/test/java/io/delta/kernel/spark/utils/CatalogTableUtilsTest.java index 43c4f3d1cc8..5660b9ba5ab 100644 --- a/kernel-spark/src/test/java/io/delta/kernel/spark/utils/CatalogTableUtilsTest.java +++ b/kernel-spark/src/test/java/io/delta/kernel/spark/utils/CatalogTableUtilsTest.java @@ -19,6 +19,7 @@ import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertTrue; +import io.delta.storage.commit.uccommitcoordinator.UCCommitCoordinatorClient; import java.util.Collections; import java.util.Map; import org.apache.spark.sql.catalyst.catalog.CatalogTable; @@ -28,94 +29,88 @@ class CatalogTableUtilsTest { @Test - void catalogManagedFlagEnablesDetection() { + void testIsCatalogManaged_CatalogFlagEnabled_ReturnsTrue() { CatalogTable table = - catalogTableWithProperties( - Collections.emptyMap(), Map.of(CatalogTableUtils.FEATURE_CATALOG_MANAGED, "supported")); + catalogTable(Collections.emptyMap(), Map.of(CatalogTableUtils.FEATURE_CATALOG_MANAGED, "supported")); assertTrue( - CatalogTableUtils.isCatalogManaged(table), "Should detect catalog management with flag"); - assertFalse( - CatalogTableUtils.isUnityCatalogManagedTable(table), "Should not detect Unity without ID"); + CatalogTableUtils.isCatalogManaged(table), + "Catalog-managed flag should enable detection"); } @Test - void previewFlagEnablesDetectionIgnoringCase() { + void testIsCatalogManaged_PreviewFlagEnabled_ReturnsTrue() { CatalogTable table = - catalogTableWithProperties( + catalogTable( Collections.emptyMap(), Map.of(CatalogTableUtils.FEATURE_CATALOG_OWNED_PREVIEW, "SuPpOrTeD")); assertTrue( - CatalogTableUtils.isCatalogManaged(table), "Should detect via preview flag ignoring case"); - assertFalse( - CatalogTableUtils.isUnityCatalogManagedTable(table), "Should not detect Unity without ID"); + CatalogTableUtils.isCatalogManaged(table), + "Preview flag should enable detection ignoring case"); } @Test - void noFlagsMeansNotManaged() { - CatalogTable table = catalogTableWithProperties(Collections.emptyMap()); + void testIsCatalogManaged_NoFlags_ReturnsFalse() { + CatalogTable table = catalogTable(Collections.emptyMap(), Collections.emptyMap()); assertFalse( - CatalogTableUtils.isCatalogManaged(table), "Should not detect management without flags"); - assertFalse( - CatalogTableUtils.isUnityCatalogManagedTable(table), - "Should not detect Unity without ID or flags"); + CatalogTableUtils.isCatalogManaged(table), + "No catalog flags should disable detection"); } @Test - void unityManagementRequiresFlagAndId() { + void testIsUnityCatalogManaged_FlagAndIdPresent_ReturnsTrue() { CatalogTable table = - catalogTableWithProperties( + catalogTable( Collections.emptyMap(), Map.of( - CatalogTableUtils.FEATURE_CATALOG_MANAGED, "supported", - io.delta.storage.commit.uccommitcoordinator.UCCommitCoordinatorClient - .UC_TABLE_ID_KEY, - "abc-123")); + CatalogTableUtils.FEATURE_CATALOG_MANAGED, + "supported", + UCCommitCoordinatorClient.UC_TABLE_ID_KEY, + "abc-123")); assertTrue( - CatalogTableUtils.isCatalogManaged(table), "Should detect general catalog management"); - assertTrue( - CatalogTableUtils.isUnityCatalogManagedTable(table), "Should detect Unity with ID present"); + CatalogTableUtils.isUnityCatalogManagedTable(table), + "Unity Catalog detection should require flag and identifier"); } @Test - void unityManagementFailsWithoutId() { + void testIsUnityCatalogManaged_MissingId_ReturnsFalse() { CatalogTable table = - catalogTableWithProperties( - Collections.emptyMap(), Map.of(CatalogTableUtils.FEATURE_CATALOG_MANAGED, "supported")); + catalogTable( + Collections.emptyMap(), + Map.of(CatalogTableUtils.FEATURE_CATALOG_MANAGED, "supported")); + + assertFalse( + CatalogTableUtils.isUnityCatalogManagedTable(table), + "Missing table identifier should break Unity detection"); + } + + @Test + void testIsUnityCatalogManaged_PreviewFlagMissingId_ReturnsFalse() { + CatalogTable table = + catalogTable( + Collections.emptyMap(), + Map.of(CatalogTableUtils.FEATURE_CATALOG_OWNED_PREVIEW, "supported")); - assertTrue( - CatalogTableUtils.isCatalogManaged(table), "Should detect general catalog management"); assertFalse( CatalogTableUtils.isUnityCatalogManagedTable(table), - "Should fail Unity detection without ID"); + "Preview flag without ID should not be considered Unity managed"); } @Test - void storagePropertiesExposeStorageMetadata() { + void testGetStorageProperties_ReturnsPublishedMetadata() { Map storageProps = Map.of("fs.test.option", "value", "dfs.conf.key", "abc"); - CatalogTable table = catalogTableWithProperties(Collections.emptyMap(), storageProps); + CatalogTable table = catalogTable(Collections.emptyMap(), storageProps); assertEquals( storageProps, CatalogTableUtils.getStorageProperties(table), - "Should surface storage properties published by the catalog"); - } - - /** - * Creates a CatalogTable with the given properties. This is a helper method to create a - * CatalogTable for testing purposes - see interface {@link CatalogTable} for more details. - * - * @param properties the properties to set on the CatalogTable - * @return a CatalogTable with the given properties - */ - private static CatalogTable catalogTableWithProperties(Map properties) { - return catalogTableWithProperties(properties, Collections.emptyMap()); + "Storage properties should surface catalog-published metadata"); } - private static CatalogTable catalogTableWithProperties( + private static CatalogTable catalogTable( Map properties, Map storageProperties) { return CatalogTableTestUtils$.MODULE$.catalogTableWithProperties(properties, storageProperties); } From 4463c3eddf6a2bba1a8d404144cde2c90ec51192 Mon Sep 17 00:00:00 2001 From: TimothyW553 Date: Mon, 17 Nov 2025 21:31:00 +0000 Subject: [PATCH 16/42] fmt --- .../kernel/spark/utils/CatalogTableUtilsTest.java | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/kernel-spark/src/test/java/io/delta/kernel/spark/utils/CatalogTableUtilsTest.java b/kernel-spark/src/test/java/io/delta/kernel/spark/utils/CatalogTableUtilsTest.java index 5660b9ba5ab..809698dee3d 100644 --- a/kernel-spark/src/test/java/io/delta/kernel/spark/utils/CatalogTableUtilsTest.java +++ b/kernel-spark/src/test/java/io/delta/kernel/spark/utils/CatalogTableUtilsTest.java @@ -31,11 +31,11 @@ class CatalogTableUtilsTest { @Test void testIsCatalogManaged_CatalogFlagEnabled_ReturnsTrue() { CatalogTable table = - catalogTable(Collections.emptyMap(), Map.of(CatalogTableUtils.FEATURE_CATALOG_MANAGED, "supported")); + catalogTable( + Collections.emptyMap(), Map.of(CatalogTableUtils.FEATURE_CATALOG_MANAGED, "supported")); assertTrue( - CatalogTableUtils.isCatalogManaged(table), - "Catalog-managed flag should enable detection"); + CatalogTableUtils.isCatalogManaged(table), "Catalog-managed flag should enable detection"); } @Test @@ -55,8 +55,7 @@ void testIsCatalogManaged_NoFlags_ReturnsFalse() { CatalogTable table = catalogTable(Collections.emptyMap(), Collections.emptyMap()); assertFalse( - CatalogTableUtils.isCatalogManaged(table), - "No catalog flags should disable detection"); + CatalogTableUtils.isCatalogManaged(table), "No catalog flags should disable detection"); } @Test @@ -79,8 +78,7 @@ void testIsUnityCatalogManaged_FlagAndIdPresent_ReturnsTrue() { void testIsUnityCatalogManaged_MissingId_ReturnsFalse() { CatalogTable table = catalogTable( - Collections.emptyMap(), - Map.of(CatalogTableUtils.FEATURE_CATALOG_MANAGED, "supported")); + Collections.emptyMap(), Map.of(CatalogTableUtils.FEATURE_CATALOG_MANAGED, "supported")); assertFalse( CatalogTableUtils.isUnityCatalogManagedTable(table), From fed1e0a7f6e6d707056d4a2eaaa753df1e097ba9 Mon Sep 17 00:00:00 2001 From: TimothyW553 Date: Tue, 18 Nov 2025 00:00:53 +0000 Subject: [PATCH 17/42] address second round comments --- .../kernel/spark/utils/CatalogTableUtils.java | 40 ++++++++++++++----- 1 file changed, 31 insertions(+), 9 deletions(-) diff --git a/kernel-spark/src/main/java/io/delta/kernel/spark/utils/CatalogTableUtils.java b/kernel-spark/src/main/java/io/delta/kernel/spark/utils/CatalogTableUtils.java index 05f235207a6..d95f92473ad 100644 --- a/kernel-spark/src/main/java/io/delta/kernel/spark/utils/CatalogTableUtils.java +++ b/kernel-spark/src/main/java/io/delta/kernel/spark/utils/CatalogTableUtils.java @@ -1,3 +1,18 @@ +/* + * Copyright (2025) The Delta Lake Project Authors. + * + * Licensed 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 io.delta.kernel.spark.utils; import static java.util.Objects.requireNonNull; @@ -57,17 +72,13 @@ public static boolean isUnityCatalogManagedTable(CatalogTable table) { } /** - * Returns the catalog storage properties published with a {@link CatalogTable}. + * Checks whether the given feature key is enabled in the table properties. * - * @param table Spark {@link CatalogTable} descriptor - * @return Java map view of the storage properties + * @param tableProperties The table properties + * @param featureKey The feature key + * @return {@code true} when the feature key is set to {@code supported} */ - public static Map getStorageProperties(CatalogTable table) { - requireNonNull(table, "table is null"); - return ScalaUtils.toJavaMap(table.storage().properties()); - } - - public static boolean isCatalogManagedFeatureEnabled( + private static boolean isCatalogManagedFeatureEnabled( Map tableProperties, String featureKey) { requireNonNull(tableProperties, "tableProperties is null"); requireNonNull(featureKey, "featureKey is null"); @@ -77,4 +88,15 @@ public static boolean isCatalogManagedFeatureEnabled( } return featureValue.equalsIgnoreCase(SUPPORTED); } + + /** + * Returns the catalog storage properties published with a {@link CatalogTable}. + * + * @param table Spark {@link CatalogTable} descriptor + * @return Java map view of the storage properties + */ + private static Map getStorageProperties(CatalogTable table) { + requireNonNull(table, "table is null"); + return ScalaUtils.toJavaMap(table.storage().properties()); + } } From f88f46aa3700e1a50b5912dc1d71ab5ac3961f1b Mon Sep 17 00:00:00 2001 From: TimothyW553 Date: Tue, 18 Nov 2025 00:05:33 +0000 Subject: [PATCH 18/42] add reason for scala usage --- .../delta/kernel/spark/utils/CatalogTableTestUtils.scala | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/kernel-spark/src/test/scala/io/delta/kernel/spark/utils/CatalogTableTestUtils.scala b/kernel-spark/src/test/scala/io/delta/kernel/spark/utils/CatalogTableTestUtils.scala index 6ee24279bdd..05a1ae39357 100644 --- a/kernel-spark/src/test/scala/io/delta/kernel/spark/utils/CatalogTableTestUtils.scala +++ b/kernel-spark/src/test/scala/io/delta/kernel/spark/utils/CatalogTableTestUtils.scala @@ -21,7 +21,13 @@ import org.apache.spark.sql.catalyst.TableIdentifier import org.apache.spark.sql.catalyst.catalog.{CatalogStorageFormat, CatalogTable, CatalogTableType} import org.apache.spark.sql.types.StructType -/** Helpers for constructing [[CatalogTable]] instances inside Java tests. */ +/** + * Helpers for constructing [[CatalogTable]] instances inside Java tests. + * + * Spark's [[CatalogTable]] is defined in Scala and its constructor signature shifts between Spark + * releases. Centralising the construction in Scala keeps the kernel tests insulated from those + * binary changes and saves Java tests from manually wiring the many optional parameters. + */ object CatalogTableTestUtils { def catalogTableWithProperties( From aab04bb2d29ae4c55bc3c4b7f01f5983c8455e4a Mon Sep 17 00:00:00 2001 From: TimothyW553 Date: Tue, 18 Nov 2025 00:18:13 +0000 Subject: [PATCH 19/42] remove private method test --- .../kernel/spark/utils/CatalogTableUtilsTest.java | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/kernel-spark/src/test/java/io/delta/kernel/spark/utils/CatalogTableUtilsTest.java b/kernel-spark/src/test/java/io/delta/kernel/spark/utils/CatalogTableUtilsTest.java index 809698dee3d..a3b2f4a59f6 100644 --- a/kernel-spark/src/test/java/io/delta/kernel/spark/utils/CatalogTableUtilsTest.java +++ b/kernel-spark/src/test/java/io/delta/kernel/spark/utils/CatalogTableUtilsTest.java @@ -15,7 +15,6 @@ */ package io.delta.kernel.spark.utils; -import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertTrue; @@ -97,17 +96,6 @@ void testIsUnityCatalogManaged_PreviewFlagMissingId_ReturnsFalse() { "Preview flag without ID should not be considered Unity managed"); } - @Test - void testGetStorageProperties_ReturnsPublishedMetadata() { - Map storageProps = Map.of("fs.test.option", "value", "dfs.conf.key", "abc"); - CatalogTable table = catalogTable(Collections.emptyMap(), storageProps); - - assertEquals( - storageProps, - CatalogTableUtils.getStorageProperties(table), - "Storage properties should surface catalog-published metadata"); - } - private static CatalogTable catalogTable( Map properties, Map storageProperties) { return CatalogTableTestUtils$.MODULE$.catalogTableWithProperties(properties, storageProperties); From ec1a35b89c0f7e517b1f6fbc8bdee5d15132d4b5 Mon Sep 17 00:00:00 2001 From: TimothyW553 Date: Tue, 18 Nov 2025 03:40:50 +0000 Subject: [PATCH 20/42] null check for storage properties --- .../java/io/delta/kernel/spark/utils/CatalogTableUtils.java | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/kernel-spark/src/main/java/io/delta/kernel/spark/utils/CatalogTableUtils.java b/kernel-spark/src/main/java/io/delta/kernel/spark/utils/CatalogTableUtils.java index d95f92473ad..a8bba31b26c 100644 --- a/kernel-spark/src/main/java/io/delta/kernel/spark/utils/CatalogTableUtils.java +++ b/kernel-spark/src/main/java/io/delta/kernel/spark/utils/CatalogTableUtils.java @@ -18,6 +18,7 @@ import static java.util.Objects.requireNonNull; import io.delta.storage.commit.uccommitcoordinator.UCCommitCoordinatorClient; +import java.util.Collections; import java.util.Map; import org.apache.spark.sql.catalyst.catalog.CatalogTable; @@ -97,6 +98,8 @@ private static boolean isCatalogManagedFeatureEnabled( */ private static Map getStorageProperties(CatalogTable table) { requireNonNull(table, "table is null"); - return ScalaUtils.toJavaMap(table.storage().properties()); + Map storageProperties = + ScalaUtils.toJavaMap(table.storage().properties()); + return storageProperties == null ? Collections.emptyMap() : storageProperties; } } From 1b1fe5955b5edfcc14e7d15f453a276b25d8afc0 Mon Sep 17 00:00:00 2001 From: TimothyW553 Date: Tue, 18 Nov 2025 03:54:39 +0000 Subject: [PATCH 21/42] fmt --- .../java/io/delta/kernel/spark/utils/CatalogTableUtils.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/kernel-spark/src/main/java/io/delta/kernel/spark/utils/CatalogTableUtils.java b/kernel-spark/src/main/java/io/delta/kernel/spark/utils/CatalogTableUtils.java index a8bba31b26c..81d68f2ddeb 100644 --- a/kernel-spark/src/main/java/io/delta/kernel/spark/utils/CatalogTableUtils.java +++ b/kernel-spark/src/main/java/io/delta/kernel/spark/utils/CatalogTableUtils.java @@ -98,8 +98,7 @@ private static boolean isCatalogManagedFeatureEnabled( */ private static Map getStorageProperties(CatalogTable table) { requireNonNull(table, "table is null"); - Map storageProperties = - ScalaUtils.toJavaMap(table.storage().properties()); + Map storageProperties = ScalaUtils.toJavaMap(table.storage().properties()); return storageProperties == null ? Collections.emptyMap() : storageProperties; } } From 6686a97717fc22bc27d28599a2a0ad2b0a663d8b Mon Sep 17 00:00:00 2001 From: TimothyW553 Date: Tue, 18 Nov 2025 20:38:01 +0000 Subject: [PATCH 22/42] address comments --- .../kernel/spark/utils/CatalogTableUtils.java | 8 ++-- .../spark/utils/CatalogTableUtilsTest.java | 39 +++++++++++++++++++ .../spark/utils/CatalogTableTestUtils.scala | 15 +++++++ 3 files changed, 59 insertions(+), 3 deletions(-) diff --git a/kernel-spark/src/main/java/io/delta/kernel/spark/utils/CatalogTableUtils.java b/kernel-spark/src/main/java/io/delta/kernel/spark/utils/CatalogTableUtils.java index 81d68f2ddeb..8f6038c7483 100644 --- a/kernel-spark/src/main/java/io/delta/kernel/spark/utils/CatalogTableUtils.java +++ b/kernel-spark/src/main/java/io/delta/kernel/spark/utils/CatalogTableUtils.java @@ -94,11 +94,13 @@ private static boolean isCatalogManagedFeatureEnabled( * Returns the catalog storage properties published with a {@link CatalogTable}. * * @param table Spark {@link CatalogTable} descriptor - * @return Java map view of the storage properties + * @return Java map view of the storage properties, never null */ private static Map getStorageProperties(CatalogTable table) { requireNonNull(table, "table is null"); - Map storageProperties = ScalaUtils.toJavaMap(table.storage().properties()); - return storageProperties == null ? Collections.emptyMap() : storageProperties; + if (table.storage() == null) { + return Collections.emptyMap(); + } + return ScalaUtils.toJavaMap(table.storage().properties()); } } diff --git a/kernel-spark/src/test/java/io/delta/kernel/spark/utils/CatalogTableUtilsTest.java b/kernel-spark/src/test/java/io/delta/kernel/spark/utils/CatalogTableUtilsTest.java index a3b2f4a59f6..5336305e617 100644 --- a/kernel-spark/src/test/java/io/delta/kernel/spark/utils/CatalogTableUtilsTest.java +++ b/kernel-spark/src/test/java/io/delta/kernel/spark/utils/CatalogTableUtilsTest.java @@ -16,6 +16,7 @@ package io.delta.kernel.spark.utils; import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; import io.delta.storage.commit.uccommitcoordinator.UCCommitCoordinatorClient; @@ -96,8 +97,46 @@ void testIsUnityCatalogManaged_PreviewFlagMissingId_ReturnsFalse() { "Preview flag without ID should not be considered Unity managed"); } + @Test + void testIsCatalogManaged_NullTable_ThrowsException() { + assertThrows( + NullPointerException.class, + () -> CatalogTableUtils.isCatalogManaged(null), + "Null table should throw NullPointerException"); + } + + @Test + void testIsUnityCatalogManaged_NullTable_ThrowsException() { + assertThrows( + NullPointerException.class, + () -> CatalogTableUtils.isUnityCatalogManagedTable(null), + "Null table should throw NullPointerException"); + } + + @Test + void testIsCatalogManaged_NullStorage_ReturnsFalse() { + CatalogTable table = catalogTableWithNullStorage(Collections.emptyMap()); + + assertFalse( + CatalogTableUtils.isCatalogManaged(table), + "Null storage should not be considered catalog managed"); + } + + @Test + void testIsUnityCatalogManaged_NullStorage_ReturnsFalse() { + CatalogTable table = catalogTableWithNullStorage(Collections.emptyMap()); + + assertFalse( + CatalogTableUtils.isUnityCatalogManagedTable(table), + "Null storage should not be considered Unity managed"); + } + private static CatalogTable catalogTable( Map properties, Map storageProperties) { return CatalogTableTestUtils$.MODULE$.catalogTableWithProperties(properties, storageProperties); } + + private static CatalogTable catalogTableWithNullStorage(Map properties) { + return CatalogTableTestUtils$.MODULE$.catalogTableWithNullStorage(properties); + } } diff --git a/kernel-spark/src/test/scala/io/delta/kernel/spark/utils/CatalogTableTestUtils.scala b/kernel-spark/src/test/scala/io/delta/kernel/spark/utils/CatalogTableTestUtils.scala index 05a1ae39357..926d8ebe5e7 100644 --- a/kernel-spark/src/test/scala/io/delta/kernel/spark/utils/CatalogTableTestUtils.scala +++ b/kernel-spark/src/test/scala/io/delta/kernel/spark/utils/CatalogTableTestUtils.scala @@ -52,4 +52,19 @@ object CatalogTableTestUtils { bucketSpec = None, properties = scalaProps) } + + def catalogTableWithNullStorage( + properties: java.util.Map[String, String]): CatalogTable = { + val scalaProps = ScalaUtils.toScalaMap(properties) + + CatalogTable( + identifier = TableIdentifier("tbl"), + tableType = CatalogTableType.MANAGED, + storage = null, + schema = new StructType(), + provider = None, + partitionColumnNames = Seq.empty, + bucketSpec = None, + properties = scalaProps) + } } From 26ad1cd180e1e41968b0db5d8b7c6892fc7d01b4 Mon Sep 17 00:00:00 2001 From: TimothyW553 Date: Tue, 18 Nov 2025 22:04:48 +0000 Subject: [PATCH 23/42] address comments --- .../kernel/spark/utils/CatalogTableUtils.java | 3 +- .../delta/kernel/spark/utils/ScalaUtils.java | 5 +- .../spark/utils/CatalogTableUtilsTest.java | 23 ++++++++ .../kernel/spark/utils/ScalaUtilsTest.java | 52 +++++++++++++++++++ .../spark/utils/CatalogTableTestUtils.scala | 21 ++++++++ 5 files changed, 102 insertions(+), 2 deletions(-) create mode 100644 kernel-spark/src/test/java/io/delta/kernel/spark/utils/ScalaUtilsTest.java diff --git a/kernel-spark/src/main/java/io/delta/kernel/spark/utils/CatalogTableUtils.java b/kernel-spark/src/main/java/io/delta/kernel/spark/utils/CatalogTableUtils.java index 8f6038c7483..511e70b45da 100644 --- a/kernel-spark/src/main/java/io/delta/kernel/spark/utils/CatalogTableUtils.java +++ b/kernel-spark/src/main/java/io/delta/kernel/spark/utils/CatalogTableUtils.java @@ -101,6 +101,7 @@ private static Map getStorageProperties(CatalogTable table) { if (table.storage() == null) { return Collections.emptyMap(); } - return ScalaUtils.toJavaMap(table.storage().properties()); + Map javaStorageProperties = ScalaUtils.toJavaMap(table.storage().properties()); + return javaStorageProperties == null ? Collections.emptyMap() : javaStorageProperties; } } diff --git a/kernel-spark/src/main/java/io/delta/kernel/spark/utils/ScalaUtils.java b/kernel-spark/src/main/java/io/delta/kernel/spark/utils/ScalaUtils.java index 39317f96b99..9de26b09760 100644 --- a/kernel-spark/src/main/java/io/delta/kernel/spark/utils/ScalaUtils.java +++ b/kernel-spark/src/main/java/io/delta/kernel/spark/utils/ScalaUtils.java @@ -40,7 +40,10 @@ public static scala.collection.immutable.Map toScalaMap( public static Map toJavaMap( scala.collection.immutable.Map scalaMap) { - if (scalaMap == null || scalaMap.isEmpty()) { + if (scalaMap == null) { + return null; + } + if (scalaMap.isEmpty()) { return Collections.emptyMap(); } return CollectionConverters.asJava(scalaMap); diff --git a/kernel-spark/src/test/java/io/delta/kernel/spark/utils/CatalogTableUtilsTest.java b/kernel-spark/src/test/java/io/delta/kernel/spark/utils/CatalogTableUtilsTest.java index 5336305e617..24e6593890a 100644 --- a/kernel-spark/src/test/java/io/delta/kernel/spark/utils/CatalogTableUtilsTest.java +++ b/kernel-spark/src/test/java/io/delta/kernel/spark/utils/CatalogTableUtilsTest.java @@ -131,6 +131,24 @@ void testIsUnityCatalogManaged_NullStorage_ReturnsFalse() { "Null storage should not be considered Unity managed"); } + @Test + void testIsCatalogManaged_NullStorageProperties_ReturnsFalse() { + CatalogTable table = catalogTableWithNullStorageProperties(Collections.emptyMap()); + + assertFalse( + CatalogTableUtils.isCatalogManaged(table), + "Null storage properties should not be considered catalog managed"); + } + + @Test + void testIsUnityCatalogManaged_NullStorageProperties_ReturnsFalse() { + CatalogTable table = catalogTableWithNullStorageProperties(Collections.emptyMap()); + + assertFalse( + CatalogTableUtils.isUnityCatalogManagedTable(table), + "Null storage properties should not be considered Unity managed"); + } + private static CatalogTable catalogTable( Map properties, Map storageProperties) { return CatalogTableTestUtils$.MODULE$.catalogTableWithProperties(properties, storageProperties); @@ -139,4 +157,9 @@ private static CatalogTable catalogTable( private static CatalogTable catalogTableWithNullStorage(Map properties) { return CatalogTableTestUtils$.MODULE$.catalogTableWithNullStorage(properties); } + + private static CatalogTable catalogTableWithNullStorageProperties( + Map properties) { + return CatalogTableTestUtils$.MODULE$.catalogTableWithNullStorageProperties(properties); + } } diff --git a/kernel-spark/src/test/java/io/delta/kernel/spark/utils/ScalaUtilsTest.java b/kernel-spark/src/test/java/io/delta/kernel/spark/utils/ScalaUtilsTest.java new file mode 100644 index 00000000000..67e71e1e4d7 --- /dev/null +++ b/kernel-spark/src/test/java/io/delta/kernel/spark/utils/ScalaUtilsTest.java @@ -0,0 +1,52 @@ +/* + * Copyright (2025) The Delta Lake Project Authors. + * + * Licensed 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 io.delta.kernel.spark.utils; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.util.Collections; +import java.util.Map; +import org.junit.jupiter.api.Test; + +class ScalaUtilsTest { + + @Test + void testToJavaMap_NullInput_ReturnsNull() { + assertNull(ScalaUtils.toJavaMap(null), "Null scala maps should return null"); + } + + @Test + void testToJavaMap_EmptyInput_ReturnsEmptyMap() { + scala.collection.immutable.Map emptyScalaMap = + ScalaUtils.toScalaMap(Collections.emptyMap()); + + Map javaMap = ScalaUtils.toJavaMap(emptyScalaMap); + + assertTrue(javaMap.isEmpty(), "Empty scala maps should convert to empty java maps"); + } + + @Test + void testToJavaMap_PopulatedInput_PreservesEntries() { + scala.collection.immutable.Map scalaMap = + ScalaUtils.toScalaMap(Map.of("foo", "bar")); + + Map javaMap = ScalaUtils.toJavaMap(scalaMap); + + assertEquals(Map.of("foo", "bar"), javaMap, "Scala map entries should be preserved"); + } +} diff --git a/kernel-spark/src/test/scala/io/delta/kernel/spark/utils/CatalogTableTestUtils.scala b/kernel-spark/src/test/scala/io/delta/kernel/spark/utils/CatalogTableTestUtils.scala index 926d8ebe5e7..4f80f346bb8 100644 --- a/kernel-spark/src/test/scala/io/delta/kernel/spark/utils/CatalogTableTestUtils.scala +++ b/kernel-spark/src/test/scala/io/delta/kernel/spark/utils/CatalogTableTestUtils.scala @@ -67,4 +67,25 @@ object CatalogTableTestUtils { bucketSpec = None, properties = scalaProps) } + + def catalogTableWithNullStorageProperties( + properties: java.util.Map[String, String]): CatalogTable = { + val scalaProps = ScalaUtils.toScalaMap(properties) + + CatalogTable( + identifier = TableIdentifier("tbl"), + tableType = CatalogTableType.MANAGED, + storage = CatalogStorageFormat( + locationUri = None, + inputFormat = None, + outputFormat = None, + serde = None, + compressed = false, + properties = null), + schema = new StructType(), + provider = None, + partitionColumnNames = Seq.empty, + bucketSpec = None, + properties = scalaProps) + } } From 3f3cd9c22c5a44f9207e37f54bcd9c199e03828f Mon Sep 17 00:00:00 2001 From: TimothyW553 Date: Thu, 20 Nov 2025 00:30:19 +0000 Subject: [PATCH 24/42] constants --- .../kernel/spark/utils/CatalogTableUtils.java | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/kernel-spark/src/main/java/io/delta/kernel/spark/utils/CatalogTableUtils.java b/kernel-spark/src/main/java/io/delta/kernel/spark/utils/CatalogTableUtils.java index 511e70b45da..39e31c48b00 100644 --- a/kernel-spark/src/main/java/io/delta/kernel/spark/utils/CatalogTableUtils.java +++ b/kernel-spark/src/main/java/io/delta/kernel/spark/utils/CatalogTableUtils.java @@ -17,6 +17,7 @@ import static java.util.Objects.requireNonNull; +import io.delta.kernel.internal.tablefeatures.TableFeatures; import io.delta.storage.commit.uccommitcoordinator.UCCommitCoordinatorClient; import java.util.Collections; import java.util.Map; @@ -40,9 +41,17 @@ *
*/ public final class CatalogTableUtils { - static final String FEATURE_CATALOG_MANAGED = "delta.feature.catalogManaged"; - static final String FEATURE_CATALOG_OWNED_PREVIEW = "delta.feature.catalogOwned-preview"; - private static final String SUPPORTED = "supported"; + /** + * Property key for catalog-managed feature flag. + * Constructed from {@link TableFeatures#CATALOG_MANAGED_RW_FEATURE} (delta.feature.catalogManaged) + * and preview variant (delta.feature.catalogOwned-preview) + */ + static final String FEATURE_CATALOG_MANAGED = + TableFeatures.SET_TABLE_FEATURE_SUPPORTED_PREFIX + + TableFeatures.CATALOG_MANAGED_RW_FEATURE.featureName(); + static final String FEATURE_CATALOG_OWNED_PREVIEW = + TableFeatures.SET_TABLE_FEATURE_SUPPORTED_PREFIX + "catalogOwned-preview"; + private static final String SUPPORTED = TableFeatures.SET_TABLE_FEATURE_SUPPORTED_VALUE; private CatalogTableUtils() {} @@ -104,4 +113,4 @@ private static Map getStorageProperties(CatalogTable table) { Map javaStorageProperties = ScalaUtils.toJavaMap(table.storage().properties()); return javaStorageProperties == null ? Collections.emptyMap() : javaStorageProperties; } -} +} \ No newline at end of file From f4d4f13dc3cb92714fc865569c6bef1fe5d4dc92 Mon Sep 17 00:00:00 2001 From: TimothyW553 Date: Thu, 20 Nov 2025 00:42:39 +0000 Subject: [PATCH 25/42] space --- .../java/io/delta/kernel/spark/utils/CatalogTableUtils.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/kernel-spark/src/main/java/io/delta/kernel/spark/utils/CatalogTableUtils.java b/kernel-spark/src/main/java/io/delta/kernel/spark/utils/CatalogTableUtils.java index 39e31c48b00..bb1a6a22c3e 100644 --- a/kernel-spark/src/main/java/io/delta/kernel/spark/utils/CatalogTableUtils.java +++ b/kernel-spark/src/main/java/io/delta/kernel/spark/utils/CatalogTableUtils.java @@ -113,4 +113,4 @@ private static Map getStorageProperties(CatalogTable table) { Map javaStorageProperties = ScalaUtils.toJavaMap(table.storage().properties()); return javaStorageProperties == null ? Collections.emptyMap() : javaStorageProperties; } -} \ No newline at end of file +} From 6359268dbee8d1a114e3f66620c6585bc0bdb6eb Mon Sep 17 00:00:00 2001 From: TimothyW553 Date: Thu, 20 Nov 2025 00:51:06 +0000 Subject: [PATCH 26/42] fmt --- .../io/delta/kernel/spark/utils/CatalogTableUtils.java | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/kernel-spark/src/main/java/io/delta/kernel/spark/utils/CatalogTableUtils.java b/kernel-spark/src/main/java/io/delta/kernel/spark/utils/CatalogTableUtils.java index bb1a6a22c3e..9c56003476c 100644 --- a/kernel-spark/src/main/java/io/delta/kernel/spark/utils/CatalogTableUtils.java +++ b/kernel-spark/src/main/java/io/delta/kernel/spark/utils/CatalogTableUtils.java @@ -42,13 +42,14 @@ */ public final class CatalogTableUtils { /** - * Property key for catalog-managed feature flag. - * Constructed from {@link TableFeatures#CATALOG_MANAGED_RW_FEATURE} (delta.feature.catalogManaged) - * and preview variant (delta.feature.catalogOwned-preview) + * Property key for catalog-managed feature flag. Constructed from {@link + * TableFeatures#CATALOG_MANAGED_RW_FEATURE} (delta.feature.catalogManaged) and preview variant + * (delta.feature.catalogOwned-preview) */ static final String FEATURE_CATALOG_MANAGED = TableFeatures.SET_TABLE_FEATURE_SUPPORTED_PREFIX + TableFeatures.CATALOG_MANAGED_RW_FEATURE.featureName(); + static final String FEATURE_CATALOG_OWNED_PREVIEW = TableFeatures.SET_TABLE_FEATURE_SUPPORTED_PREFIX + "catalogOwned-preview"; private static final String SUPPORTED = TableFeatures.SET_TABLE_FEATURE_SUPPORTED_VALUE; From 4571d963f5f39efcbd28f5977dd120ada2275024 Mon Sep 17 00:00:00 2001 From: TimothyW553 Date: Thu, 20 Nov 2025 19:04:05 +0000 Subject: [PATCH 27/42] config and extraction --- .../kernel/spark/utils/CatalogTableUtils.java | 49 +++++++++++++ .../delta/kernel/spark/utils/ScalaUtils.java | 31 ++++++++ .../kernel/spark/utils/UCCatalogConfig.java | 72 +++++++++++++++++++ .../spark/utils/CatalogTableUtilsTest.java | 16 +++++ .../kernel/spark/utils/ScalaUtilsTest.java | 9 +++ .../spark/utils/UCCatalogConfigTest.java | 71 ++++++++++++++++++ 6 files changed, 248 insertions(+) create mode 100644 kernel-spark/src/main/java/io/delta/kernel/spark/utils/UCCatalogConfig.java create mode 100644 kernel-spark/src/test/java/io/delta/kernel/spark/utils/UCCatalogConfigTest.java diff --git a/kernel-spark/src/main/java/io/delta/kernel/spark/utils/CatalogTableUtils.java b/kernel-spark/src/main/java/io/delta/kernel/spark/utils/CatalogTableUtils.java index 9c56003476c..01cc5136e5c 100644 --- a/kernel-spark/src/main/java/io/delta/kernel/spark/utils/CatalogTableUtils.java +++ b/kernel-spark/src/main/java/io/delta/kernel/spark/utils/CatalogTableUtils.java @@ -20,7 +20,10 @@ import io.delta.kernel.internal.tablefeatures.TableFeatures; import io.delta.storage.commit.uccommitcoordinator.UCCommitCoordinatorClient; import java.util.Collections; +import java.util.List; import java.util.Map; +import java.util.Optional; +import org.apache.spark.sql.SparkSession; import org.apache.spark.sql.catalyst.catalog.CatalogTable; /** @@ -114,4 +117,50 @@ private static Map getStorageProperties(CatalogTable table) { Map javaStorageProperties = ScalaUtils.toJavaMap(table.storage().properties()); return javaStorageProperties == null ? Collections.emptyMap() : javaStorageProperties; } + + /** + * Extracts the catalog name from a {@link CatalogTable}. + * + *

If the table identifier contains a catalog name, that is returned. Otherwise, falls back to + * the current catalog name from the SparkSession. + * + * @param table Spark {@link CatalogTable} descriptor + * @param spark the SparkSession to use for fallback catalog resolution + * @return the catalog name + */ + public static String getCatalogName(CatalogTable table, SparkSession spark) { + requireNonNull(table, "table is null"); + requireNonNull(spark, "spark is null"); + + scala.Option catalogOption = table.identifier().catalog(); + if (catalogOption.isDefined()) { + return catalogOption.get(); + } + + // Fall back to current catalog if not specified in table identifier + return spark.sessionState().catalogManager().currentCatalog().name(); + } + + /** + * Retrieves Unity Catalog configuration for the catalog owning this table. + * + *

Resolves the catalog name from the table identifier and looks up the corresponding Unity + * Catalog configuration from the SparkSession. + * + * @param table Spark {@link CatalogTable} descriptor + * @param spark the SparkSession containing catalog configurations + * @return UC catalog configuration if found, empty otherwise + */ + public static Optional getUCCatalogConfig( + CatalogTable table, SparkSession spark) { + requireNonNull(table, "table is null"); + requireNonNull(spark, "spark is null"); + + String catalogName = getCatalogName(table, spark); + List allConfigs = ScalaUtils.getUCCatalogConfigs(spark); + + return allConfigs.stream() + .filter(config -> config.getCatalogName().equals(catalogName)) + .findFirst(); + } } diff --git a/kernel-spark/src/main/java/io/delta/kernel/spark/utils/ScalaUtils.java b/kernel-spark/src/main/java/io/delta/kernel/spark/utils/ScalaUtils.java index 9de26b09760..abc04b51dbe 100644 --- a/kernel-spark/src/main/java/io/delta/kernel/spark/utils/ScalaUtils.java +++ b/kernel-spark/src/main/java/io/delta/kernel/spark/utils/ScalaUtils.java @@ -16,8 +16,12 @@ package io.delta.kernel.spark.utils; import java.util.Collections; +import java.util.List; import java.util.Map; +import java.util.stream.Collectors; +import org.apache.spark.sql.SparkSession; import scala.Tuple2; +import scala.Tuple3; import scala.collection.immutable.Map$; import scala.collection.mutable.Builder; import scala.jdk.javaapi.CollectionConverters; @@ -48,4 +52,31 @@ public static Map toJavaMap( } return CollectionConverters.asJava(scalaMap); } + + /** + * Retrieves all Unity Catalog configurations from the SparkSession. + * + *

This method bridges to Scala code that extracts UC catalog configurations from Spark's + * catalog manager settings. The Scala implementation is in {@link + * org.apache.spark.sql.delta.coordinatedcommits.UCCommitCoordinatorBuilder#getCatalogConfigs}. + * + * @param spark the SparkSession containing catalog configurations + * @return list of UC catalog configurations (catalog name, URI, token) + * @throws NullPointerException if spark is null + */ + public static List getUCCatalogConfigs(SparkSession spark) { + if (spark == null) { + throw new NullPointerException("spark is null"); + } + + // Call Scala code to get catalog configs + scala.collection.immutable.List> scalaConfigs = + org.apache.spark.sql.delta.coordinatedcommits.UCCommitCoordinatorBuilder$.MODULE$ + .getCatalogConfigs(spark); + + // Convert Scala List to Java List + return CollectionConverters.asJava(scalaConfigs).stream() + .map(tuple -> new UCCatalogConfig(tuple._1(), tuple._2(), tuple._3())) + .collect(Collectors.toList()); + } } diff --git a/kernel-spark/src/main/java/io/delta/kernel/spark/utils/UCCatalogConfig.java b/kernel-spark/src/main/java/io/delta/kernel/spark/utils/UCCatalogConfig.java new file mode 100644 index 00000000000..cbb122cf511 --- /dev/null +++ b/kernel-spark/src/main/java/io/delta/kernel/spark/utils/UCCatalogConfig.java @@ -0,0 +1,72 @@ +/* + * Copyright (2025) The Delta Lake Project Authors. + * + * Licensed 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 io.delta.kernel.spark.utils; + +import static java.util.Objects.requireNonNull; + +/** + * Immutable data class holding Unity Catalog configuration extracted from Spark catalog settings. + * + *

Contains the catalog name, URI, and authentication token needed to communicate with a Unity + * Catalog instance. + */ +public final class UCCatalogConfig { + private final String catalogName; + private final String uri; + private final String token; + + /** + * Creates a new Unity Catalog configuration. + * + * @param catalogName the catalog name (e.g., "unity") + * @param uri the Unity Catalog server URI (e.g., "https://uc-server:8080") + * @param token the authentication token + * @throws NullPointerException if any parameter is null + */ + public UCCatalogConfig(String catalogName, String uri, String token) { + this.catalogName = requireNonNull(catalogName, "catalogName is null"); + this.uri = requireNonNull(uri, "uri is null"); + this.token = requireNonNull(token, "token is null"); + } + + /** @return the catalog name */ + public String getCatalogName() { + return catalogName; + } + + /** @return the Unity Catalog server URI */ + public String getUri() { + return uri; + } + + /** @return the authentication token */ + public String getToken() { + return token; + } + + @Override + public String toString() { + return "UCCatalogConfig{" + + "catalogName='" + + catalogName + + '\'' + + ", uri='" + + uri + + '\'' + + ", token='***'" + + '}'; + } +} diff --git a/kernel-spark/src/test/java/io/delta/kernel/spark/utils/CatalogTableUtilsTest.java b/kernel-spark/src/test/java/io/delta/kernel/spark/utils/CatalogTableUtilsTest.java index 24e6593890a..77c96206d06 100644 --- a/kernel-spark/src/test/java/io/delta/kernel/spark/utils/CatalogTableUtilsTest.java +++ b/kernel-spark/src/test/java/io/delta/kernel/spark/utils/CatalogTableUtilsTest.java @@ -149,6 +149,22 @@ void testIsUnityCatalogManaged_NullStorageProperties_ReturnsFalse() { "Null storage properties should not be considered Unity managed"); } + @Test + void testGetCatalogName_NullTable_ThrowsException() { + assertThrows( + NullPointerException.class, + () -> CatalogTableUtils.getCatalogName(null, null), + "Null table should throw NullPointerException"); + } + + @Test + void testGetUCCatalogConfig_NullTable_ThrowsException() { + assertThrows( + NullPointerException.class, + () -> CatalogTableUtils.getUCCatalogConfig(null, null), + "Null table should throw NullPointerException"); + } + private static CatalogTable catalogTable( Map properties, Map storageProperties) { return CatalogTableTestUtils$.MODULE$.catalogTableWithProperties(properties, storageProperties); diff --git a/kernel-spark/src/test/java/io/delta/kernel/spark/utils/ScalaUtilsTest.java b/kernel-spark/src/test/java/io/delta/kernel/spark/utils/ScalaUtilsTest.java index 67e71e1e4d7..1bc955ce381 100644 --- a/kernel-spark/src/test/java/io/delta/kernel/spark/utils/ScalaUtilsTest.java +++ b/kernel-spark/src/test/java/io/delta/kernel/spark/utils/ScalaUtilsTest.java @@ -17,6 +17,7 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; import java.util.Collections; @@ -49,4 +50,12 @@ void testToJavaMap_PopulatedInput_PreservesEntries() { assertEquals(Map.of("foo", "bar"), javaMap, "Scala map entries should be preserved"); } + + @Test + void testGetUCCatalogConfigs_NullSpark_ThrowsException() { + assertThrows( + NullPointerException.class, + () -> ScalaUtils.getUCCatalogConfigs(null), + "Null spark should throw NullPointerException"); + } } diff --git a/kernel-spark/src/test/java/io/delta/kernel/spark/utils/UCCatalogConfigTest.java b/kernel-spark/src/test/java/io/delta/kernel/spark/utils/UCCatalogConfigTest.java new file mode 100644 index 00000000000..2308993eaa5 --- /dev/null +++ b/kernel-spark/src/test/java/io/delta/kernel/spark/utils/UCCatalogConfigTest.java @@ -0,0 +1,71 @@ +/* + * Copyright (2025) The Delta Lake Project Authors. + * + * Licensed 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 io.delta.kernel.spark.utils; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import org.junit.jupiter.api.Test; + +/** Tests for {@link UCCatalogConfig}. */ +class UCCatalogConfigTest { + + @Test + void testConstructor_ValidInputs_Success() { + UCCatalogConfig config = new UCCatalogConfig("unity", "https://uc:8080", "token123"); + + assertEquals("unity", config.getCatalogName()); + assertEquals("https://uc:8080", config.getUri()); + assertEquals("token123", config.getToken()); + } + + @Test + void testConstructor_NullCatalogName_ThrowsException() { + assertThrows( + NullPointerException.class, + () -> new UCCatalogConfig(null, "https://uc:8080", "token"), + "Null catalogName should throw NullPointerException"); + } + + @Test + void testConstructor_NullUri_ThrowsException() { + assertThrows( + NullPointerException.class, + () -> new UCCatalogConfig("unity", null, "token"), + "Null uri should throw NullPointerException"); + } + + @Test + void testConstructor_NullToken_ThrowsException() { + assertThrows( + NullPointerException.class, + () -> new UCCatalogConfig("unity", "https://uc:8080", null), + "Null token should throw NullPointerException"); + } + + @Test + void testToString_MasksToken() { + UCCatalogConfig config = new UCCatalogConfig("unity", "https://uc:8080", "secret-token"); + String toString = config.toString(); + + assertTrue(toString.contains("unity"), "toString should contain catalog name"); + assertTrue(toString.contains("https://uc:8080"), "toString should contain URI"); + assertTrue(toString.contains("***"), "toString should contain masked token placeholder"); + assertFalse(toString.contains("secret-token"), "toString should NOT contain actual token"); + } +} From f66e0cd149bf9afe414ca9d2be9756b050999873 Mon Sep 17 00:00:00 2001 From: TimothyW553 Date: Fri, 21 Nov 2025 05:38:24 +0000 Subject: [PATCH 28/42] decouple --- build.sbt | 1 + .../kernel/spark/catalog/SparkTable.java | 10 +- .../CatalogManagedSnapshotManager.java | 318 ++++++++++++++++++ .../snapshot/DeltaSnapshotManagerFactory.java | 102 ++++++ .../kernel/spark/utils/CatalogTableUtils.java | 49 --- .../delta/kernel/spark/utils/ScalaUtils.java | 31 -- .../kernel/spark/utils/UCCatalogConfig.java | 72 ---- .../CatalogManagedSnapshotManagerTest.java | 85 +++++ .../DeltaSnapshotManagerFactoryTest.java | 80 +++++ .../spark/utils/CatalogTableUtilsTest.java | 16 - .../kernel/spark/utils/ScalaUtilsTest.java | 9 - .../spark/utils/UCCatalogConfigTest.java | 71 ---- 12 files changed, 592 insertions(+), 252 deletions(-) create mode 100644 kernel-spark/src/main/java/io/delta/kernel/spark/snapshot/CatalogManagedSnapshotManager.java create mode 100644 kernel-spark/src/main/java/io/delta/kernel/spark/snapshot/DeltaSnapshotManagerFactory.java delete mode 100644 kernel-spark/src/main/java/io/delta/kernel/spark/utils/UCCatalogConfig.java create mode 100644 kernel-spark/src/test/java/io/delta/kernel/spark/snapshot/CatalogManagedSnapshotManagerTest.java create mode 100644 kernel-spark/src/test/java/io/delta/kernel/spark/snapshot/DeltaSnapshotManagerFactoryTest.java delete mode 100644 kernel-spark/src/test/java/io/delta/kernel/spark/utils/UCCatalogConfigTest.java diff --git a/build.sbt b/build.sbt index cd9a3d8eca7..683cb5ab58e 100644 --- a/build.sbt +++ b/build.sbt @@ -462,6 +462,7 @@ lazy val sparkV1Filtered = (project in file("spark-v1-filtered")) lazy val sparkV2 = (project in file("kernel-spark")) .dependsOn(sparkV1Filtered) .dependsOn(kernelDefaults) + .dependsOn(unity) .dependsOn(goldenTables % "test") .settings( name := "delta-spark-v2", diff --git a/kernel-spark/src/main/java/io/delta/kernel/spark/catalog/SparkTable.java b/kernel-spark/src/main/java/io/delta/kernel/spark/catalog/SparkTable.java index bef04d4f1f7..cb831d99dcd 100644 --- a/kernel-spark/src/main/java/io/delta/kernel/spark/catalog/SparkTable.java +++ b/kernel-spark/src/main/java/io/delta/kernel/spark/catalog/SparkTable.java @@ -21,7 +21,7 @@ import io.delta.kernel.Snapshot; import io.delta.kernel.spark.read.SparkScanBuilder; import io.delta.kernel.spark.snapshot.DeltaSnapshotManager; -import io.delta.kernel.spark.snapshot.PathBasedSnapshotManager; +import io.delta.kernel.spark.snapshot.DeltaSnapshotManagerFactory; import io.delta.kernel.spark.utils.SchemaUtils; import java.util.*; import org.apache.hadoop.conf.Configuration; @@ -138,9 +138,11 @@ private SparkTable( merged.putAll(userOptions); this.options = Collections.unmodifiableMap(merged); - this.hadoopConf = - SparkSession.active().sessionState().newHadoopConfWithOptions(toScalaMap(options)); - this.snapshotManager = new PathBasedSnapshotManager(tablePath, hadoopConf); + SparkSession spark = SparkSession.active(); + this.hadoopConf = spark.sessionState().newHadoopConfWithOptions(toScalaMap(options)); + // Use factory to create appropriate snapshot manager (UC-managed vs path-based) + this.snapshotManager = + DeltaSnapshotManagerFactory.create(tablePath, catalogTable, spark, hadoopConf); // Load the initial snapshot through the manager this.initialSnapshot = snapshotManager.loadLatestSnapshot(); this.schema = SchemaUtils.convertKernelSchemaToSparkSchema(initialSnapshot.getSchema()); diff --git a/kernel-spark/src/main/java/io/delta/kernel/spark/snapshot/CatalogManagedSnapshotManager.java b/kernel-spark/src/main/java/io/delta/kernel/spark/snapshot/CatalogManagedSnapshotManager.java new file mode 100644 index 00000000000..5beaac22c71 --- /dev/null +++ b/kernel-spark/src/main/java/io/delta/kernel/spark/snapshot/CatalogManagedSnapshotManager.java @@ -0,0 +1,318 @@ +/* + * Copyright (2025) The Delta Lake Project Authors. + * + * Licensed 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 io.delta.kernel.spark.snapshot; + +import static io.delta.kernel.internal.util.Preconditions.checkArgument; +import static java.util.Objects.requireNonNull; + +import io.delta.kernel.CommitRange; +import io.delta.kernel.Snapshot; +import io.delta.kernel.defaults.engine.DefaultEngine; +import io.delta.kernel.engine.Engine; +import io.delta.kernel.exceptions.KernelException; +import io.delta.kernel.internal.DeltaHistoryManager; +import io.delta.kernel.spark.exception.VersionNotFoundException; +import io.delta.kernel.spark.utils.CatalogTableUtils; +import io.delta.storage.commit.uccommitcoordinator.UCClient; +import io.delta.storage.commit.uccommitcoordinator.UCCommitCoordinatorClient; +import io.delta.storage.commit.uccommitcoordinator.UCTokenBasedRestClient; +import io.delta.unity.UCCatalogManagedClient; +import java.util.Optional; +import org.apache.hadoop.conf.Configuration; +import org.apache.spark.annotation.Experimental; +import org.apache.spark.sql.SparkSession; +import org.apache.spark.sql.catalyst.catalog.CatalogTable; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +/** + * Implementation of DeltaSnapshotManager for Unity Catalog managed tables (CCv2). + * + *

This snapshot manager handles tables where Unity Catalog acts as the commit coordinator. It + * manages the lifecycle of UC client connections and delegates snapshot operations to {@link + * UCCatalogManagedClient}. + * + *

The manager automatically extracts table metadata (ucTableId, tablePath) from the {@link + * CatalogTable} and creates the necessary UC client for communication with Unity Catalog. + * + *

Resource Management: This class implements {@link AutoCloseable} and must be + * properly closed to release UC client resources. Use try-with-resources: + * + * 

{@code
+ * try (CatalogManagedSnapshotManager manager = new CatalogManagedSnapshotManager(...)) {
+ *   Snapshot snapshot = manager.loadLatestSnapshot();
+ *   // Use snapshot...
+ * }
+ * }
+ * + *

TODO (Next PR): Integrate proper lifecycle management into {@link + * io.delta.kernel.spark.table.SparkTable}. Currently, SparkTable doesn't implement close() or + * properly manage snapshot manager resources. Options to consider: + * + *

    + *
  • Make SparkTable implement AutoCloseable and propagate close() to snapshot manager + *
  • Add {@link AutoCloseable} to {@link DeltaSnapshotManager} interface with default no-op + *
  • Use finalization or shutdown hooks as safety net (not recommended as primary solution) + *
+ * + *

Without proper lifecycle management, UC client connections may leak in long-running Spark + * sessions. This is acceptable for this initial PR but must be addressed before production use. + */ +@Experimental +public class CatalogManagedSnapshotManager implements DeltaSnapshotManager, AutoCloseable { + + private static final Logger logger = LoggerFactory.getLogger(CatalogManagedSnapshotManager.class); + + private final UCClient ucClient; + private final UCCatalogManagedClient ucManagedClient; + private final Engine kernelEngine; + private final String ucTableId; + private final String tablePath; + + /** + * Creates a snapshot manager for a Unity Catalog managed table. + * + * @param catalogTable the Spark catalog table (must be Unity Catalog managed) + * @param spark the SparkSession containing Unity Catalog configurations + * @param hadoopConf Hadoop configuration for the Delta Kernel engine + * @throws NullPointerException if any parameter is null + * @throws IllegalArgumentException if table is not Unity Catalog managed + * @throws IllegalArgumentException if Unity Catalog configuration is not found + */ + public CatalogManagedSnapshotManager( + CatalogTable catalogTable, SparkSession spark, Configuration hadoopConf) { + requireNonNull(catalogTable, "catalogTable is null"); + requireNonNull(spark, "spark is null"); + requireNonNull(hadoopConf, "hadoopConf is null"); + + // Validate table is Unity Catalog managed + if (!CatalogTableUtils.isUnityCatalogManagedTable(catalogTable)) { + throw new IllegalArgumentException( + "Cannot create CatalogManagedSnapshotManager: table is not Unity Catalog managed. " + + "Table identifier: " + + catalogTable.identifier()); + } + + // Extract table metadata + this.ucTableId = extractUCTableId(catalogTable); + this.tablePath = extractTablePath(catalogTable); + + // Create UC client and managed client + this.ucClient = createUCClientInternal(catalogTable, spark); + this.ucManagedClient = new UCCatalogManagedClient(ucClient); + this.kernelEngine = DefaultEngine.create(hadoopConf); + + logger.info( + "Created CatalogManagedSnapshotManager for table {} at path {}", ucTableId, tablePath); + } + + /** + * Loads the latest snapshot of the Unity Catalog managed Delta table. + * + * @return the latest snapshot + */ + @Override + public Snapshot loadLatestSnapshot() { + return ucManagedClient.loadSnapshot( + kernelEngine, ucTableId, tablePath, Optional.empty(), Optional.empty()); + } + + /** + * Loads a specific version of the Unity Catalog managed Delta table. + * + * @param version the version to load (must be >= 0) + * @return the snapshot at the specified version + */ + @Override + public Snapshot loadSnapshotAt(long version) { + checkArgument(version >= 0, "version must be non-negative"); + return ucManagedClient.loadSnapshot( + kernelEngine, ucTableId, tablePath, Optional.of(version), Optional.empty()); + } + + /** + * Finds the active commit at a specific timestamp. + * + *

Note: This operation is not yet supported for Unity Catalog managed tables + * because it requires filesystem-based commit history which is not accessible for catalog-managed + * tables. Unity Catalog coordinates commits differently than traditional Delta tables. + * + * @throws UnsupportedOperationException always - not yet implemented for catalog-managed tables + */ + @Override + public DeltaHistoryManager.Commit getActiveCommitAtTime( + long timestampMillis, + boolean canReturnLastCommit, + boolean mustBeRecreatable, + boolean canReturnEarliestCommit) { + throw new UnsupportedOperationException( + "getActiveCommitAtTime not yet implemented for catalog-managed tables. " + + "This operation requires filesystem-based commit history which is not " + + "accessible for Unity Catalog managed tables."); + } + + /** + * Checks if a specific version exists and is accessible. + * + *

Performance Note: For Unity Catalog managed tables, version checking + * requires loading the full snapshot including all file metadata. This is less efficient than + * filesystem-based checks which can verify log file existence without reading contents. + * + *

TODO (Next PR): Add lightweight version checking API to + * UCCatalogManagedClient to avoid loading full snapshots for existence checks. + * + * @throws VersionNotFoundException if the version is not available + */ + @Override + public void checkVersionExists(long version, boolean mustBeRecreatable, boolean allowOutOfRange) + throws VersionNotFoundException { + checkArgument(version >= 0, "version must be non-negative"); + + try { + // Attempt to load the snapshot at the specified version + // Note: This loads the full snapshot - see performance note above + loadSnapshotAt(version); + } catch (KernelException e) { + // Specific Kernel exceptions indicate version doesn't exist or isn't accessible + // Let other exceptions (network failures, auth errors, etc.) propagate to caller + long latestVersion = loadLatestSnapshot().getVersion(); + throw new VersionNotFoundException(version, 0, latestVersion); + } + } + + /** + * Gets a range of table changes between versions. + * + *

Note: This operation delegates to {@link UCCatalogManagedClient} for Unity + * Catalog managed tables. + * + * @throws UnsupportedOperationException if not yet implemented for catalog-managed tables + */ + @Override + public CommitRange getTableChanges(Engine engine, long startVersion, Optional endVersion) { + // TODO: Implement getTableChanges for UC-managed tables + // This requires UCCatalogManagedClient to expose commit range functionality + throw new UnsupportedOperationException( + "getTableChanges not yet implemented for catalog-managed tables"); + } + + /** + * Closes the UC client and releases resources. + * + *

This method should be called when the snapshot manager is no longer needed. Prefer using + * try-with-resources to ensure proper cleanup. + */ + @Override + public void close() { + try { + ucClient.close(); + logger.info("Closed CatalogManagedSnapshotManager for table {}", ucTableId); + } catch (Exception e) { + logger.warn("Error closing UC client for table {}", ucTableId, e); + } + } + + /** + * Creates a UC client for the table's catalog (internal helper). + * + *

This method resolves the catalog configuration directly from Spark and creates a {@link + * UCTokenBasedRestClient}. All UC-specific logic is encapsulated here to avoid polluting + * kernel-spark utilities with UC coupling. + * + * @param catalogTable the catalog table + * @param spark the SparkSession + * @return configured UC client + * @throws IllegalArgumentException if catalog configuration is not found + */ + private static UCClient createUCClientInternal(CatalogTable catalogTable, SparkSession spark) { + // 1. Extract catalog name from table identifier (or use default) + scala.Option catalogOption = catalogTable.identifier().catalog(); + String catalogName = + catalogOption.isDefined() + ? catalogOption.get() + : spark.sessionState().catalogManager().currentCatalog().name(); + + // 2. Get all UC catalog configs from Scala code (inlined, not abstracted) + scala.collection.immutable.List> scalaConfigs = + org.apache.spark.sql.delta.coordinatedcommits.UCCommitCoordinatorBuilder$.MODULE$ + .getCatalogConfigs(spark); + + // 3. Find matching config for this catalog + Optional> configTuple = + scala.jdk.javaapi.CollectionConverters.asJava(scalaConfigs).stream() + .filter(tuple -> tuple._1().equals(catalogName)) + .findFirst(); + + if (!configTuple.isPresent()) { + throw new IllegalArgumentException( + "Cannot create UC client: Unity Catalog configuration not found " + + "for catalog '" + + catalogName + + "'. " + + "Ensure spark.sql.catalog." + + catalogName + + ".uri and " + + "spark.sql.catalog." + + catalogName + + ".token are configured."); + } + + // 4. Extract URI and token from tuple (catalogName, uri, token) + scala.Tuple3 config = configTuple.get(); + String uri = config._2(); + String token = config._3(); + + // 5. Create UC client + return new UCTokenBasedRestClient(uri, token); + } + + /** + * Extracts the Unity Catalog table ID from catalog table properties. + * + * @param catalogTable the catalog table + * @return the UC table ID + * @throws IllegalArgumentException if ucTableId is not found + */ + private static String extractUCTableId(CatalogTable catalogTable) { + java.util.Map storageProperties = + scala.collection.JavaConverters.mapAsJavaMap(catalogTable.storage().properties()); + + String ucTableId = storageProperties.get(UCCommitCoordinatorClient.UC_TABLE_ID_KEY); + if (ucTableId == null || ucTableId.isEmpty()) { + throw new IllegalArgumentException( + "Cannot extract ucTableId: " + + UCCommitCoordinatorClient.UC_TABLE_ID_KEY + + " not found in table storage properties for table " + + catalogTable.identifier()); + } + return ucTableId; + } + + /** + * Extracts the table path from the catalog table location. + * + * @param catalogTable the catalog table + * @return the table path + * @throws IllegalArgumentException if location is not available + */ + private static String extractTablePath(CatalogTable catalogTable) { + if (catalogTable.location() == null) { + throw new IllegalArgumentException( + "Cannot extract table path: location is null for table " + catalogTable.identifier()); + } + return catalogTable.location().toString(); + } +} diff --git a/kernel-spark/src/main/java/io/delta/kernel/spark/snapshot/DeltaSnapshotManagerFactory.java b/kernel-spark/src/main/java/io/delta/kernel/spark/snapshot/DeltaSnapshotManagerFactory.java new file mode 100644 index 00000000000..75058e82d2c --- /dev/null +++ b/kernel-spark/src/main/java/io/delta/kernel/spark/snapshot/DeltaSnapshotManagerFactory.java @@ -0,0 +1,102 @@ +/* + * Copyright (2025) The Delta Lake Project Authors. + * + * Licensed 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 io.delta.kernel.spark.snapshot; + +import static java.util.Objects.requireNonNull; + +import io.delta.kernel.spark.utils.CatalogTableUtils; +import java.util.Optional; +import org.apache.hadoop.conf.Configuration; +import org.apache.spark.annotation.Experimental; +import org.apache.spark.sql.SparkSession; +import org.apache.spark.sql.catalyst.catalog.CatalogTable; + +/** + * Factory for creating {@link DeltaSnapshotManager} instances. + * + *

This factory determines the appropriate snapshot manager implementation based on table + * characteristics and automatically handles the selection between: + * + *

    + *
  • {@link CatalogManagedSnapshotManager} - for Unity Catalog managed tables (CCv2) + *
  • {@link PathBasedSnapshotManager} - for regular filesystem-based Delta tables + *
+ * + *

The factory encapsulates the decision logic so that callers (e.g., {@code SparkTable}) don't + * need to know about specific manager implementations. + * + *

Example usage: + * + * 

{@code
+ * DeltaSnapshotManager manager = DeltaSnapshotManagerFactory.create(
+ *     tablePath,
+ *     Optional.of(catalogTable),
+ *     spark,
+ *     hadoopConf
+ * );
+ * Snapshot snapshot = manager.loadLatestSnapshot();
+ * }
+ */ +@Experimental +public final class DeltaSnapshotManagerFactory { + + // Utility class - no instances + private DeltaSnapshotManagerFactory() {} + + /** + * Creates the appropriate snapshot manager for a Delta table. + * + *

Selection logic: + * + *

    + *
  • If {@code catalogTable} is present and Unity Catalog managed → {@link + * CatalogManagedSnapshotManager} + *
  • Otherwise → {@link PathBasedSnapshotManager} + *
+ * + *

Unity Catalog managed tables are identified by checking for catalog-managed feature flags + * and the presence of a Unity Catalog table ID. See {@link + * CatalogTableUtils#isUnityCatalogManagedTable(CatalogTable)} for details. + * + * @param tablePath filesystem path to the Delta table root + * @param catalogTable optional Spark catalog table metadata + * @param spark SparkSession for resolving Unity Catalog configurations + * @param hadoopConf Hadoop configuration for the Delta Kernel engine + * @return appropriate snapshot manager implementation + * @throws NullPointerException if tablePath, spark, or hadoopConf is null + * @throws IllegalArgumentException if catalogTable is UC-managed but configuration is invalid + */ + public static DeltaSnapshotManager create( + String tablePath, + Optional catalogTable, + SparkSession spark, + Configuration hadoopConf) { + + requireNonNull(tablePath, "tablePath is null"); + requireNonNull(catalogTable, "catalogTable is null"); + requireNonNull(spark, "spark is null"); + requireNonNull(hadoopConf, "hadoopConf is null"); + + // Check if table is Unity Catalog managed + if (catalogTable.isPresent() + && CatalogTableUtils.isUnityCatalogManagedTable(catalogTable.get())) { + return new CatalogManagedSnapshotManager(catalogTable.get(), spark, hadoopConf); + } + + // Default to path-based snapshot manager + return new PathBasedSnapshotManager(tablePath, hadoopConf); + } +} diff --git a/kernel-spark/src/main/java/io/delta/kernel/spark/utils/CatalogTableUtils.java b/kernel-spark/src/main/java/io/delta/kernel/spark/utils/CatalogTableUtils.java index 01cc5136e5c..9c56003476c 100644 --- a/kernel-spark/src/main/java/io/delta/kernel/spark/utils/CatalogTableUtils.java +++ b/kernel-spark/src/main/java/io/delta/kernel/spark/utils/CatalogTableUtils.java @@ -20,10 +20,7 @@ import io.delta.kernel.internal.tablefeatures.TableFeatures; import io.delta.storage.commit.uccommitcoordinator.UCCommitCoordinatorClient; import java.util.Collections; -import java.util.List; import java.util.Map; -import java.util.Optional; -import org.apache.spark.sql.SparkSession; import org.apache.spark.sql.catalyst.catalog.CatalogTable; /** @@ -117,50 +114,4 @@ private static Map getStorageProperties(CatalogTable table) { Map javaStorageProperties = ScalaUtils.toJavaMap(table.storage().properties()); return javaStorageProperties == null ? Collections.emptyMap() : javaStorageProperties; } - - /** - * Extracts the catalog name from a {@link CatalogTable}. - * - *

If the table identifier contains a catalog name, that is returned. Otherwise, falls back to - * the current catalog name from the SparkSession. - * - * @param table Spark {@link CatalogTable} descriptor - * @param spark the SparkSession to use for fallback catalog resolution - * @return the catalog name - */ - public static String getCatalogName(CatalogTable table, SparkSession spark) { - requireNonNull(table, "table is null"); - requireNonNull(spark, "spark is null"); - - scala.Option catalogOption = table.identifier().catalog(); - if (catalogOption.isDefined()) { - return catalogOption.get(); - } - - // Fall back to current catalog if not specified in table identifier - return spark.sessionState().catalogManager().currentCatalog().name(); - } - - /** - * Retrieves Unity Catalog configuration for the catalog owning this table. - * - *

Resolves the catalog name from the table identifier and looks up the corresponding Unity - * Catalog configuration from the SparkSession. - * - * @param table Spark {@link CatalogTable} descriptor - * @param spark the SparkSession containing catalog configurations - * @return UC catalog configuration if found, empty otherwise - */ - public static Optional getUCCatalogConfig( - CatalogTable table, SparkSession spark) { - requireNonNull(table, "table is null"); - requireNonNull(spark, "spark is null"); - - String catalogName = getCatalogName(table, spark); - List allConfigs = ScalaUtils.getUCCatalogConfigs(spark); - - return allConfigs.stream() - .filter(config -> config.getCatalogName().equals(catalogName)) - .findFirst(); - } } diff --git a/kernel-spark/src/main/java/io/delta/kernel/spark/utils/ScalaUtils.java b/kernel-spark/src/main/java/io/delta/kernel/spark/utils/ScalaUtils.java index abc04b51dbe..9de26b09760 100644 --- a/kernel-spark/src/main/java/io/delta/kernel/spark/utils/ScalaUtils.java +++ b/kernel-spark/src/main/java/io/delta/kernel/spark/utils/ScalaUtils.java @@ -16,12 +16,8 @@ package io.delta.kernel.spark.utils; import java.util.Collections; -import java.util.List; import java.util.Map; -import java.util.stream.Collectors; -import org.apache.spark.sql.SparkSession; import scala.Tuple2; -import scala.Tuple3; import scala.collection.immutable.Map$; import scala.collection.mutable.Builder; import scala.jdk.javaapi.CollectionConverters; @@ -52,31 +48,4 @@ public static Map toJavaMap( } return CollectionConverters.asJava(scalaMap); } - - /** - * Retrieves all Unity Catalog configurations from the SparkSession. - * - *

This method bridges to Scala code that extracts UC catalog configurations from Spark's - * catalog manager settings. The Scala implementation is in {@link - * org.apache.spark.sql.delta.coordinatedcommits.UCCommitCoordinatorBuilder#getCatalogConfigs}. - * - * @param spark the SparkSession containing catalog configurations - * @return list of UC catalog configurations (catalog name, URI, token) - * @throws NullPointerException if spark is null - */ - public static List getUCCatalogConfigs(SparkSession spark) { - if (spark == null) { - throw new NullPointerException("spark is null"); - } - - // Call Scala code to get catalog configs - scala.collection.immutable.List> scalaConfigs = - org.apache.spark.sql.delta.coordinatedcommits.UCCommitCoordinatorBuilder$.MODULE$ - .getCatalogConfigs(spark); - - // Convert Scala List to Java List - return CollectionConverters.asJava(scalaConfigs).stream() - .map(tuple -> new UCCatalogConfig(tuple._1(), tuple._2(), tuple._3())) - .collect(Collectors.toList()); - } } diff --git a/kernel-spark/src/main/java/io/delta/kernel/spark/utils/UCCatalogConfig.java b/kernel-spark/src/main/java/io/delta/kernel/spark/utils/UCCatalogConfig.java deleted file mode 100644 index cbb122cf511..00000000000 --- a/kernel-spark/src/main/java/io/delta/kernel/spark/utils/UCCatalogConfig.java +++ /dev/null @@ -1,72 +0,0 @@ -/* - * Copyright (2025) The Delta Lake Project Authors. - * - * Licensed 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 io.delta.kernel.spark.utils; - -import static java.util.Objects.requireNonNull; - -/** - * Immutable data class holding Unity Catalog configuration extracted from Spark catalog settings. - * - *

Contains the catalog name, URI, and authentication token needed to communicate with a Unity - * Catalog instance. - */ -public final class UCCatalogConfig { - private final String catalogName; - private final String uri; - private final String token; - - /** - * Creates a new Unity Catalog configuration. - * - * @param catalogName the catalog name (e.g., "unity") - * @param uri the Unity Catalog server URI (e.g., "https://uc-server:8080") - * @param token the authentication token - * @throws NullPointerException if any parameter is null - */ - public UCCatalogConfig(String catalogName, String uri, String token) { - this.catalogName = requireNonNull(catalogName, "catalogName is null"); - this.uri = requireNonNull(uri, "uri is null"); - this.token = requireNonNull(token, "token is null"); - } - - /** @return the catalog name */ - public String getCatalogName() { - return catalogName; - } - - /** @return the Unity Catalog server URI */ - public String getUri() { - return uri; - } - - /** @return the authentication token */ - public String getToken() { - return token; - } - - @Override - public String toString() { - return "UCCatalogConfig{" - + "catalogName='" - + catalogName - + '\'' - + ", uri='" - + uri - + '\'' - + ", token='***'" - + '}'; - } -} diff --git a/kernel-spark/src/test/java/io/delta/kernel/spark/snapshot/CatalogManagedSnapshotManagerTest.java b/kernel-spark/src/test/java/io/delta/kernel/spark/snapshot/CatalogManagedSnapshotManagerTest.java new file mode 100644 index 00000000000..863c7dc0e81 --- /dev/null +++ b/kernel-spark/src/test/java/io/delta/kernel/spark/snapshot/CatalogManagedSnapshotManagerTest.java @@ -0,0 +1,85 @@ +/* + * Copyright (2025) The Delta Lake Project Authors. + * + * Licensed 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 io.delta.kernel.spark.snapshot; + +import static org.junit.jupiter.api.Assertions.assertThrows; + +import io.delta.kernel.spark.utils.CatalogTableTestUtils$; +import java.util.Collections; +import java.util.Map; +import org.apache.hadoop.conf.Configuration; +import org.apache.spark.sql.catalyst.catalog.CatalogTable; +import org.junit.jupiter.api.Test; + +/** Tests for {@link CatalogManagedSnapshotManager}. */ +class CatalogManagedSnapshotManagerTest { + + @Test + void testConstructor_NullCatalogTable_ThrowsException() { + assertThrows( + NullPointerException.class, + () -> new CatalogManagedSnapshotManager(null, null, new Configuration()), + "Null catalogTable should throw NullPointerException"); + } + + @Test + void testConstructor_NullSpark_ThrowsException() { + CatalogTable table = catalogTable(Collections.emptyMap(), Collections.emptyMap()); + + assertThrows( + NullPointerException.class, + () -> new CatalogManagedSnapshotManager(table, null, new Configuration()), + "Null spark should throw NullPointerException"); + } + + @Test + void testConstructor_NullHadoopConf_ThrowsException() { + CatalogTable table = catalogTable(Collections.emptyMap(), Collections.emptyMap()); + + assertThrows( + NullPointerException.class, + () -> new CatalogManagedSnapshotManager(table, null, null), + "Null hadoopConf should throw NullPointerException"); + } + + // Note: Additional constructor validation tests require integration test setup. + // The following tests cannot be implemented as unit tests because they require + // a real SparkSession to properly test UC table validation logic: + // + // - testConstructor_NonUCTable_ThrowsException: Verify non-UC table rejection + // - testConstructor_CatalogManagedWithoutUCTableId_ThrowsException: Verify UC table ID + // requirement + // + // These validations happen after null checks, so passing null SparkSession causes + // NullPointerException before reaching the UC validation logic. Cannot mock SparkSession + // effectively for these tests. + // + // Note: Full integration tests for CatalogManagedSnapshotManager require: + // 1. Real SparkSession with Unity Catalog configured + // 2. Unity Catalog tables with proper feature flags and table IDs + // 3. Unity Catalog endpoint available for snapshot loading + // + // Such tests should be added when integration test infrastructure is available: + // - testConstructor_ValidUCTable_Success: Happy path with valid UC table and config + // - testLoadLatestSnapshot_Success: Test snapshot loading from UC + // - testLoadSnapshotAt_Success: Test version-specific snapshot loading + // - testClose_Success: Test UC client cleanup + + private static CatalogTable catalogTable( + Map properties, Map storageProperties) { + return CatalogTableTestUtils$.MODULE$.catalogTableWithProperties(properties, storageProperties); + } +} diff --git a/kernel-spark/src/test/java/io/delta/kernel/spark/snapshot/DeltaSnapshotManagerFactoryTest.java b/kernel-spark/src/test/java/io/delta/kernel/spark/snapshot/DeltaSnapshotManagerFactoryTest.java new file mode 100644 index 00000000000..61c80acac08 --- /dev/null +++ b/kernel-spark/src/test/java/io/delta/kernel/spark/snapshot/DeltaSnapshotManagerFactoryTest.java @@ -0,0 +1,80 @@ +/* + * Copyright (2025) The Delta Lake Project Authors. + * + * Licensed 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 io.delta.kernel.spark.snapshot; + +import static org.junit.jupiter.api.Assertions.assertThrows; + +import java.util.Optional; +import org.apache.hadoop.conf.Configuration; +import org.junit.jupiter.api.Test; + +/** Tests for {@link DeltaSnapshotManagerFactory}. */ +class DeltaSnapshotManagerFactoryTest { + + @Test + void testCreate_NullTablePath_ThrowsException() { + assertThrows( + NullPointerException.class, + () -> DeltaSnapshotManagerFactory.create(null, Optional.empty(), null, new Configuration()), + "Null tablePath should throw NullPointerException"); + } + + @Test + void testCreate_NullCatalogTable_ThrowsException() { + assertThrows( + NullPointerException.class, + () -> DeltaSnapshotManagerFactory.create("/tmp/test", null, null, new Configuration()), + "Null catalogTable should throw NullPointerException"); + } + + @Test + void testCreate_NullSpark_ThrowsException() { + assertThrows( + NullPointerException.class, + () -> + DeltaSnapshotManagerFactory.create( + "/tmp/test", Optional.empty(), null, new Configuration()), + "Null spark should throw NullPointerException"); + } + + @Test + void testCreate_NullHadoopConf_ThrowsException() { + assertThrows( + NullPointerException.class, + () -> DeltaSnapshotManagerFactory.create("/tmp/test", Optional.empty(), null, null), + "Null hadoopConf should throw NullPointerException"); + } + + // Note: Factory behavior tests (which manager type is created) require integration test setup. + // The following tests cannot be implemented as unit tests because the factory requires + // a non-null SparkSession parameter: + // + // - testCreate_NonCatalogManagedTable_ReturnsPathBasedManager: Verify non-UC tables use PathBased + // - testCreate_EmptyCatalogTable_ReturnsPathBasedManager: Verify empty catalogTable uses + // PathBased + // - testCreate_UCManagedTable_ReturnsCatalogManagedManager: Verify UC tables use CatalogManaged + // + // While PathBasedSnapshotManager doesn't technically need SparkSession, the factory API requires + // it to maintain a clean, consistent interface (always available in production via SparkTable). + // Cannot mock SparkSession effectively for these tests. + // + // Note: Testing CatalogManagedSnapshotManager creation requires integration tests with + // real SparkSession and Unity Catalog configuration. This is because: + // 1. CatalogManagedSnapshotManager constructor validates UC table and extracts metadata + // 2. It requires configured UC catalog (spark.sql.catalog.*.uri/token) + // 3. Unit tests cannot easily mock SparkSession's catalog manager + // See CatalogManagedSnapshotManagerTest for integration tests. +} diff --git a/kernel-spark/src/test/java/io/delta/kernel/spark/utils/CatalogTableUtilsTest.java b/kernel-spark/src/test/java/io/delta/kernel/spark/utils/CatalogTableUtilsTest.java index 77c96206d06..24e6593890a 100644 --- a/kernel-spark/src/test/java/io/delta/kernel/spark/utils/CatalogTableUtilsTest.java +++ b/kernel-spark/src/test/java/io/delta/kernel/spark/utils/CatalogTableUtilsTest.java @@ -149,22 +149,6 @@ void testIsUnityCatalogManaged_NullStorageProperties_ReturnsFalse() { "Null storage properties should not be considered Unity managed"); } - @Test - void testGetCatalogName_NullTable_ThrowsException() { - assertThrows( - NullPointerException.class, - () -> CatalogTableUtils.getCatalogName(null, null), - "Null table should throw NullPointerException"); - } - - @Test - void testGetUCCatalogConfig_NullTable_ThrowsException() { - assertThrows( - NullPointerException.class, - () -> CatalogTableUtils.getUCCatalogConfig(null, null), - "Null table should throw NullPointerException"); - } - private static CatalogTable catalogTable( Map properties, Map storageProperties) { return CatalogTableTestUtils$.MODULE$.catalogTableWithProperties(properties, storageProperties); diff --git a/kernel-spark/src/test/java/io/delta/kernel/spark/utils/ScalaUtilsTest.java b/kernel-spark/src/test/java/io/delta/kernel/spark/utils/ScalaUtilsTest.java index 1bc955ce381..67e71e1e4d7 100644 --- a/kernel-spark/src/test/java/io/delta/kernel/spark/utils/ScalaUtilsTest.java +++ b/kernel-spark/src/test/java/io/delta/kernel/spark/utils/ScalaUtilsTest.java @@ -17,7 +17,6 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNull; -import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; import java.util.Collections; @@ -50,12 +49,4 @@ void testToJavaMap_PopulatedInput_PreservesEntries() { assertEquals(Map.of("foo", "bar"), javaMap, "Scala map entries should be preserved"); } - - @Test - void testGetUCCatalogConfigs_NullSpark_ThrowsException() { - assertThrows( - NullPointerException.class, - () -> ScalaUtils.getUCCatalogConfigs(null), - "Null spark should throw NullPointerException"); - } } diff --git a/kernel-spark/src/test/java/io/delta/kernel/spark/utils/UCCatalogConfigTest.java b/kernel-spark/src/test/java/io/delta/kernel/spark/utils/UCCatalogConfigTest.java deleted file mode 100644 index 2308993eaa5..00000000000 --- a/kernel-spark/src/test/java/io/delta/kernel/spark/utils/UCCatalogConfigTest.java +++ /dev/null @@ -1,71 +0,0 @@ -/* - * Copyright (2025) The Delta Lake Project Authors. - * - * Licensed 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 io.delta.kernel.spark.utils; - -import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertFalse; -import static org.junit.jupiter.api.Assertions.assertThrows; -import static org.junit.jupiter.api.Assertions.assertTrue; - -import org.junit.jupiter.api.Test; - -/** Tests for {@link UCCatalogConfig}. */ -class UCCatalogConfigTest { - - @Test - void testConstructor_ValidInputs_Success() { - UCCatalogConfig config = new UCCatalogConfig("unity", "https://uc:8080", "token123"); - - assertEquals("unity", config.getCatalogName()); - assertEquals("https://uc:8080", config.getUri()); - assertEquals("token123", config.getToken()); - } - - @Test - void testConstructor_NullCatalogName_ThrowsException() { - assertThrows( - NullPointerException.class, - () -> new UCCatalogConfig(null, "https://uc:8080", "token"), - "Null catalogName should throw NullPointerException"); - } - - @Test - void testConstructor_NullUri_ThrowsException() { - assertThrows( - NullPointerException.class, - () -> new UCCatalogConfig("unity", null, "token"), - "Null uri should throw NullPointerException"); - } - - @Test - void testConstructor_NullToken_ThrowsException() { - assertThrows( - NullPointerException.class, - () -> new UCCatalogConfig("unity", "https://uc:8080", null), - "Null token should throw NullPointerException"); - } - - @Test - void testToString_MasksToken() { - UCCatalogConfig config = new UCCatalogConfig("unity", "https://uc:8080", "secret-token"); - String toString = config.toString(); - - assertTrue(toString.contains("unity"), "toString should contain catalog name"); - assertTrue(toString.contains("https://uc:8080"), "toString should contain URI"); - assertTrue(toString.contains("***"), "toString should contain masked token placeholder"); - assertFalse(toString.contains("secret-token"), "toString should NOT contain actual token"); - } -} From 0bd57d343eac44f769beb1450f44e23d97106b41 Mon Sep 17 00:00:00 2001 From: TimothyW553 Date: Fri, 21 Nov 2025 05:40:28 +0000 Subject: [PATCH 29/42] Remove UC-specific utilities and inline logic in CatalogManagedSnapshotManager --- .bazelbsp/bazel.log | 1 + .bazelbsp/bsp.log | 2 + .bazelbsp/rpc.log | 0 CLAUDE.md | 615 ++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 618 insertions(+) create mode 100644 .bazelbsp/bazel.log create mode 100644 .bazelbsp/bsp.log create mode 100644 .bazelbsp/rpc.log create mode 100644 CLAUDE.md diff --git a/.bazelbsp/bazel.log b/.bazelbsp/bazel.log new file mode 100644 index 00000000000..8ee18baa1be --- /dev/null +++ b/.bazelbsp/bazel.log @@ -0,0 +1 @@ +{"time":"2025-11-21T05:39:37.262904969Z","level":"INFO","msg":"Bazel info created","cmd":"/usr/local/bin/bazel info workspace"} diff --git a/.bazelbsp/bsp.log b/.bazelbsp/bsp.log new file mode 100644 index 00000000000..3361c51520e --- /dev/null +++ b/.bazelbsp/bsp.log @@ -0,0 +1,2 @@ +{"time":"2025-11-21T05:39:37.262774602Z","level":"INFO","msg":"No project file found","path":"/home/timothy.wang/delta/.bazelbsp/project.json"} +{"time":"2025-11-21T05:39:37.271003496Z","level":"ERROR","msg":"failed to load project","error":"exit status 1"} diff --git a/.bazelbsp/rpc.log b/.bazelbsp/rpc.log new file mode 100644 index 00000000000..e69de29bb2d diff --git a/CLAUDE.md b/CLAUDE.md new file mode 100644 index 00000000000..87c3b3edd13 --- /dev/null +++ b/CLAUDE.md @@ -0,0 +1,615 @@ +# CLAUDE.md + +This file provides guidance to Claude Code (claude.ai/code) when working with code in this repository. + +## Project Overview + +Delta Lake is an open-source storage framework providing ACID transactions and scalable metadata handling for data lakes. This repository contains multiple components: + +- **Delta Spark (spark/)**: The core Spark connector for Delta Lake (v1 implementation using DeltaLog) +- **Delta Kernel (kernel/)**: A protocol-agnostic library for building Delta connectors + - `kernel-api/`: Public APIs for Table and Engine interfaces + - `kernel-defaults/`: Default Engine implementation using Hadoop libraries + - `kernel-benchmarks/`: Performance benchmarking suite +- **Delta Kernel Spark (kernel-spark/)**: Spark connector built using Delta Kernel (v2 pure DSv2) +- **Spark Unified (spark-unified/)**: Final published `delta-spark` JAR combining v1 and v2 +- **Unity (unity/)**: Unity Catalog integration with catalog-managed commit coordination +- **Storage (storage/, storage-s3-dynamodb/)**: Storage layer abstractions and S3 DynamoDB support +- **Connectors (connectors/)**: Various Delta Lake connectors (Flink, standalone, Hudi, Iceberg) +- **Python (python/)**: Python bindings for Delta Lake + +## Build System + +The project uses **SBT** (Scala Build Tool) with Java 11 as the target JVM. + +### Common Build Commands + +```bash +# Build the project +build/sbt compile + +# Generate artifacts +build/sbt package + +# Run all tests +build/sbt test + +# Run tests for a specific project group +build/sbt sparkGroup/test +build/sbt kernelGroup/test + +# Run a single test suite +build/sbt spark/'testOnly org.apache.spark.sql.delta.optimize.OptimizeCompactionSQLSuite' + +# Run a specific test within a suite +build/sbt spark/'testOnly *.OptimizeCompactionSQLSuite -- -z "optimize command: on partitioned table - all partitions"' + +# Run tests with coverage +build/sbt coverage test coverageAggregate coverageOff + +# Run with different Spark version +build/sbt -DsparkVersion=master compile +build/sbt -DsparkVersion=3.5.7 test +``` + +### Code Formatting + +**Java code** must follow Google Java Style: +```bash +# Check Java formatting +build/sbt javafmtCheckAll + +# Fix Java formatting +build/sbt javafmtAll +``` + +**Scala code** uses Scalafmt (config in `.scalafmt.conf`): +```bash +# Check Scala formatting +build/sbt scalafmtCheckAll + +# Fix Scala formatting +build/sbt scalafmtAll + +# Check Scala style +build/sbt scalastyle +``` + +### Python Tests + +```bash +# Setup conda environment +conda env create --name delta_python_tests --file=/python/environment.yml +conda activate delta_python_tests + +# Run Python tests +python3 /python/run-tests.py +``` + +### Test Groups + +The codebase is organized into test groups that can be run independently: +- `spark`: Spark-related tests +- `kernel`: Delta Kernel tests +- `iceberg`: Iceberg compatibility tests +- `spark-python`: Python integration tests + +Use `build/sbt Group/test` to run a specific group. + +## Architecture + +### Delta Kernel Architecture + +Delta Kernel provides two sets of APIs: + +1. **Table APIs** (`io.delta.kernel.*`): High-level interfaces for reading/writing Delta tables + - `Table`: Entry point for accessing Delta tables + - `Snapshot`: Point-in-time view of a Delta table + - `Scan`: API for reading data with filters and column pruning + - `Transaction`: API for writing data to Delta tables + +2. **Engine APIs** (`io.delta.kernel.engine.*`): Pluggable interfaces for compute-intensive operations + - `Engine`: Main interface for connector-specific optimizations + - `ParquetHandler`: Parquet file reading/writing + - `JsonHandler`: JSON processing for Delta logs + - `FileSystemClient`: File system operations + - Default implementations provided in `kernel-defaults` module + +### V1 vs V2 Architecture + +Delta Lake provides two Spark connector implementations that coexist in the published JAR: + +#### V1 Implementation (DeltaLog-based) +Located in `spark/`, this is the traditional connector using: +- **DeltaLog**: Core entry point for transaction log access and management +- **Snapshot**: In-memory representation of table state at a version +- **OptimisticTransaction**: Handles ACID transactions with conflict detection +- **DeltaTable**: Public Scala/Java/Python API +- Hybrid DataSource V1 and V2 APIs for compatibility + +**Core Components**: +- `DeltaLog.scala`: Transaction log management and caching +- `Snapshot.scala`: Table state with file lists and metadata +- `OptimisticTransaction.scala`: Write path with conflict resolution +- `commands/`: Command implementations (WriteIntoDelta, MergeIntoCommand, etc.) +- `catalog/`: AbstractDeltaCatalog with table resolution + +#### V2 Implementation (Kernel-based) +Located in `kernel-spark/`, this is a pure DataSource V2 connector using Delta Kernel: +- Uses Delta Kernel APIs (`io.delta.kernel.*`) for protocol handling +- Depends on `sparkV1Filtered` for shared utilities (excluding DeltaLog) +- Designed as a reference implementation for building Delta connectors +- Leverages Spark's ParquetFileFormat for actual data reads + +**Architecture**: +``` +Spark Driver → Delta Kernel Connector → Delta Kernel API + ↓ + (reads Delta logs) + ↓ +Spark Engine ← file splits ← Delta Kernel returns scan files + ↓ +(Parquet scans using Spark's built-in reader) +``` + +#### Unified Connector (spark-unified/) +The published `delta-spark` JAR combines both implementations: +- Provides single entry points: `DeltaCatalog` and `DeltaSparkSessionExtension` +- V1 handles most operations (writes, commands) +- V2 provides alternative read path via DSv2 +- No duplicate classes - sparkV1Filtered ensures clean merge + +### Module Dependencies and Build Flow + +The repository uses a sophisticated multi-module build structure: + +``` +storage (base storage abstraction) + ↓ +kernelApi (kernel/kernel-api) → kernelDefaults (kernel/kernel-defaults) + ↓ ↓ + | sparkV2 (kernel-spark/) + | ↓ + └──────────────────────────────> unity (unity/) + +sparkV1 (spark/) ──────> sparkV1Filtered (spark-v1-filtered/) + ↓ + sparkV2 (kernel-spark/) + ↓ + spark (spark-unified/) [PUBLISHED JAR] +``` + +**Key Points:** +- `sparkV1` contains the complete v1 implementation with DeltaLog, Snapshot, OptimisticTransaction +- `sparkV1Filtered` is a filtered version excluding DeltaLog classes (to avoid conflicts) +- `sparkV2` (kernel-spark) depends on sparkV1Filtered for shared utilities +- `spark` (spark-unified) is the final published artifact merging all three modules +- `unity` provides Unity Catalog integration for catalog-managed tables (CCv2) + +**Published vs Internal Modules:** +- Published: `delta-spark` (spark-unified), `delta-kernel-api`, `delta-kernel-defaults`, `delta-unity` +- Internal (not published): `delta-spark-v1`, `delta-spark-v1-filtered`, `delta-spark-v2` + +### Unity Catalog Integration (unity/) + +The Unity module provides integration with Unity Catalog for catalog-managed tables: + +**Key Components**: +- `UCCatalogManagedCommitter`: Implements coordinated commits version 2 (CCv2) +- `UCCatalogManagedClient`: Client for Unity Catalog commit coordination +- Catalog-managed tables use feature flags in table properties: + - `delta.feature.catalogManaged = supported` + - `delta.feature.catalogOwned-preview = supported` + +**Detection**: Use `CatalogTableUtils` (in kernel-spark) to check if a table is catalog-managed: +- `isCatalogManaged(CatalogTable)`: Checks for catalog management feature +- `isUnityCatalogManagedTable(CatalogTable)`: Verifies UC-specific table ID + +### Protocol Implementation + +The Delta transaction log protocol is defined in `PROTOCOL.md`. Key concepts: + +- **Actions**: Metadata changes recorded in JSON log entries (AddFile, RemoveFile, Protocol, etc.) +- **Checkpoints**: Parquet files that snapshot the table state for fast access +- **Table Features**: Protocol capabilities (column mapping, deletion vectors, row tracking, etc.) + - Defined in `TableFeature.scala` with reader/writer version requirements + - Features can be legacy (version-based) or name-based +- **Concurrency Control**: Optimistic concurrency with conflict detection and retry logic + +## Key File Locations + +### Spark V1 Implementation (spark/) +- Core Delta classes: `spark/src/main/scala/org/apache/spark/sql/delta/` + - `DeltaLog.scala`: Transaction log management + - `Snapshot.scala`: Table state snapshots + - `OptimisticTransaction.scala`: ACID transaction handling + - `commands/`: DML operations (MERGE, UPDATE, DELETE, OPTIMIZE, VACUUM) + - `catalog/`: Catalog integration (AbstractDeltaCatalog) + - `actions/`: Protocol actions (AddFile, RemoveFile, Metadata, etc.) + +### Delta Kernel (kernel/) +- API definitions: `kernel/kernel-api/src/main/java/io/delta/kernel/` + - `Table.java`, `Snapshot.java`, `Scan.java`, `Transaction.java` + - `engine/Engine.java`: Pluggable engine interface +- Default implementations: `kernel/kernel-defaults/src/main/java/io/delta/kernel/defaults/` + - `DefaultEngine.java`: Hadoop-based Engine + - Parquet/JSON handlers, file system client + +### Kernel-Spark Connector (kernel-spark/) +- Connector code: `kernel-spark/src/main/java/io/delta/kernel/spark/` + - `read/`: DSv2 read support + - `snapshot/`: Snapshot management + - `catalog/`: Catalog integration for kernel + - `utils/`: Bridge utilities between Kernel and Spark + - `CatalogTableUtils.java`: Unity Catalog metadata helpers + - `SchemaUtils.java`: Schema conversion utilities + +### Spark Unified (spark-unified/) +- Entry points: `spark-unified/src/main/` + - `java/.../DeltaCatalog.java`: Unified catalog (extends AbstractDeltaCatalog) + - `scala/.../DeltaSparkSessionExtension.scala`: Session extension + +### Unity Catalog Integration (unity/) +- UC support: `unity/src/main/java/io/delta/unity/` + - `UCCatalogManagedCommitter.java`: Catalog-managed commits (CCv2) + - `UCCatalogManagedClient.java`: Unity Catalog client + +### Tests +- Tests mirror source structure: `/src/test/{scala,java}/` +- Shared test utilities: `/src/test/scala/.../testUtils/` + +## Development Workflow + +### IntelliJ Setup + +1. Import project: `File` > `New Project` > `Project from Existing Sources` > select delta directory +2. Choose `sbt` as external model, use Java 11 JDK +3. Run `build/sbt clean package` to generate necessary files +4. If you see parser errors, run `build/sbt clean compile` and refresh IntelliJ +5. For source folder issues: `File` > `Project Structure` > `Modules` > remove target folders + +### Contributing + +- Sign commits with `git commit -s` (Developer Certificate of Origin required) +- Follow Apache Spark Scala Style Guide +- Major features (>100 LOC) require discussion via GitHub issue first +- Run formatting checks before committing: `build/sbt javafmtAll scalafmtAll` + +## Testing Strategy + +### Test Naming Conventions + +- Scala tests: `*Suite.scala` (e.g., `DeltaLogSuite`, `OptimizeCompactionSQLSuite`) +- Java tests: `*Test.java` (e.g., `CatalogTableUtilsTest`) + +### Running Tests Efficiently + +When working on a specific module: +```bash +# Test only kernel module +build/sbt kernel-api/test + +# Test specific package within spark +build/sbt spark/'testOnly org.apache.spark.sql.delta.commands.*' + +# Run with increased logging +build/sbt 'set logLevel := Level.Debug' spark/test +``` + +### Test Configuration + +Tests are configured for local execution with: +- Spark UI disabled +- Reduced shuffle partitions (5) +- Limited memory (1GB) +- Small log cache sizes + +See `build.sbt` `commonSettings` section for full test JVM options. + +## Protocol and Table Features + +When working on protocol changes or table features: + +1. Review `PROTOCOL.md` for specification details +2. Table features are defined in `spark/src/main/scala/org/apache/spark/sql/delta/TableFeature.scala` +3. Protocol version changes require updating reader/writer version requirements +4. Add feature documentation to `protocol_rfcs/` for major changes + +## Common Pitfalls + +### Java Version +- Must use Java 11 for building +- Set `JAVA_HOME` appropriately: `export JAVA_HOME=$(/usr/libexec/java_home -v 11)` (macOS) + +### SBT Commands +- Always use `build/sbt` not bare `sbt` (uses project-specific version) +- Use single quotes for commands with spaces: `build/sbt 'testOnly *.Suite'` + +### Cross-Scala Builds +- Default Scala version is 2.13.16 +- Use `+ ` to run for all Scala versions: `build/sbt "+ compile"` +- Use `++ 2.13.16` to set specific version: `build/sbt "++ 2.13.16" compile` + +### Test Parallelization +- Tests run in parallel by default +- Some tests may be flaky when run in parallel +- Use `Test / parallelExecution := false` in specific project if needed + +## Git Workflow with Stacked Branches + +This project uses **git stack** for managing stacked branches. Common commands: + +```bash +# Visualize stack +git stack ls + +# Navigate between branches +git stack jump BRANCH + +# Create child branch from current +git stack create BRANCH + +# Daily workflow + +git stack commit # or regular git commit +git stack sync # rebase children onto updated parent +git stack push origin # push branches + manage PRs +git stack push origin --publish # mark all PRs ready for review + +# Branch management +git stack remove BRANCH # drop a branch from stack +git stack clear # forget stored PR links +git stack abort # recover from failed sync/rebase +``` + +**Important**: Never add "Co-Authored-By: Claude" to commits. + +## CCv2 Project Context + +**Current Focus**: Implementing Coordinated Commits V2 (CCv2) support in Delta Kernel's DSv2 Connector for Unity Catalog-managed tables. + +### Architecture Overview + +CCv2 enables Unity Catalog to manage Delta table commits through coordinated commit coordination: + +1. **Catalog-managed tables** have feature flags in table properties: + - `delta.feature.catalogManaged = supported` + - `delta.feature.catalogOwned-preview = supported` + - `ucTableId` identifier for Unity Catalog + +2. **Components**: + - `UCCatalogManagedClient` (unity/): Loads snapshots for UC-managed tables + - `UCCatalogManagedCommitter` (unity/): Handles commits via UC + - `CatalogTableUtils` (kernel-spark/): Detection utilities for catalog management + - `UCClientFactory`: Creates UC clients (currently token-based) + +3. **Current Work**: Extending CCv2 support with additional features and integration tests. + +### Completed Work + +- **PR #5477**: Added utilities for detecting CCv2 tables + - `CatalogTableUtils.java`: Methods to check if table is catalog-managed + - `ScalaUtils.java`: Scala-Java conversion utilities + - Test utilities in kernel-spark module + +- **PR #5520**: Implemented snapshot manager infrastructure for UC-managed tables + - `CatalogManagedSnapshotManager`: DeltaSnapshotManager implementation for UC tables + - `DeltaSnapshotManagerFactory`: Factory pattern for creating appropriate snapshot managers + - `SparkTable` integration: Automatically uses correct manager based on table type + - Added `unity` as dependency to `kernel-spark` module + - Unit tests for null validation and factory behavior + +### Key Files for CCv2 + +- `unity/src/main/java/io/delta/unity/` + - `UCCatalogManagedClient.java`: Client for UC-managed Delta tables + - `UCCatalogManagedCommitter.java`: Commit coordination logic + - `adapters/`: Protocol and metadata adapters between Kernel and storage layers +- `spark/src/main/scala/.../coordinatedcommits/UCCommitCoordinatorBuilder.scala` + - `UCClientFactory` trait and `UCTokenBasedRestClientFactory` +- `kernel-spark/src/main/java/io/delta/kernel/spark/` + - `utils/CatalogTableUtils.java`: Detection utilities for catalog-managed tables + - `snapshot/CatalogManagedSnapshotManager.java`: Snapshot manager for UC tables + - `snapshot/DeltaSnapshotManagerFactory.java`: Factory for manager selection + - `table/SparkTable.java`: Uses factory to create appropriate snapshot manager + +### Known Limitations and Future Work + +**Resource Lifecycle Management (TODO for next PR):** +- `CatalogManagedSnapshotManager` implements `AutoCloseable` but `DeltaSnapshotManager` interface doesn't +- `SparkTable` doesn't implement close() or manage snapshot manager resources +- UC client connections may leak in long-running Spark sessions +- Options to consider: + - Make SparkTable implement AutoCloseable + - Add AutoCloseable to DeltaSnapshotManager interface with default no-op + - Use shutdown hooks as safety net (not recommended as primary solution) + +**Unsupported Operations (Architectural Limitations):** +- `getActiveCommitAtTime()`: Throws `UnsupportedOperationException` + - Requires filesystem-based commit history (not available for UC tables) + - Unity Catalog coordinates commits differently +- `getTableChanges()`: Throws `UnsupportedOperationException` + - Needs UC API support for commit range functionality + +**Performance Optimizations Needed:** +- `checkVersionExists()` loads full snapshot just to check version existence + - Very inefficient for tables with large file lists + - TODO: Add lightweight version checking API to `UCCatalogManagedClient` +- No connection pooling for UC clients (new HTTP client per manager instance) +- No snapshot caching strategy (loads fresh from UC every time) + +**Module Dependencies:** +- Added `unity` as dependency to `kernel-spark` (sparkV2) +- This makes kernel-spark Unity-aware (appropriate for UC-managed tables) +- Creates tight coupling to Unity implementation +- Consider if UC support should be pluggable via SPI pattern in the future + +## Working with Different Modules + +### When to modify which module: + +**spark/** - Modify when: +- Adding/fixing V1 connector features (DeltaLog, OptimisticTransaction) +- Implementing new commands (DML operations) +- Changing Spark catalog integration +- Working on most production Spark features + +**kernel/kernel-api/** - Modify when: +- Adding new Table or Engine API methods +- Changing public Kernel interfaces +- This requires careful API compatibility considerations + +**kernel/kernel-defaults/** - Modify when: +- Improving default Engine implementations +- Fixing Parquet/JSON handling in default engine +- Adding new default utilities + +**kernel-spark/** - Modify when: +- Working on V2 DSv2 connector +- Adding Kernel-specific Spark integration +- Bridging Kernel and Spark types (utils/) +- Testing Kernel with Spark + +**spark-unified/** - Rarely modified: +- Only for entry point changes +- Usually just extends classes from spark/ + +**unity/** - Modify when: +- Working on Unity Catalog integration +- Implementing catalog-managed commit coordination +- Adding UC-specific features + +### Module Testing Strategy + +When working across modules: +1. Test individual module first: `build/sbt /test` +2. Test the group: `build/sbt Group/test` +3. Test integration in spark-unified if needed +4. Remember that kernel-spark tests may use sparkV1Filtered classes + +**For CCv2/kernel-spark work**, use: +```bash +build/sbt -DsparkVersion=master "++ 2.13.16" clean sparkV2/test +``` +Note: Test output can be very large, monitor carefully. + +### Testing Unity Catalog Managed Tables + +**Unit Test Limitations:** +Testing UC-managed table logic has inherent limitations in unit tests: +- UC table validation requires real `SparkSession` with catalog configuration +- Cannot mock `SparkSession` effectively for UC catalog access +- Constructor null checks happen before business logic validation +- Factory pattern requires non-null SparkSession for consistent API + +**What CAN be unit tested:** +- ✅ Null parameter validation (requireNonNull checks) +- ✅ Factory creation with mock catalog tables +- ✅ Interface contract validation +- ✅ Exception type verification + +**What REQUIRES integration tests:** +- ❌ UC table detection and validation logic +- ❌ Snapshot loading from Unity Catalog +- ❌ UC client creation and lifecycle +- ❌ Manager selection based on table type +- ❌ End-to-end flows with real UC tables + +**Integration Test Requirements:** +1. Real SparkSession with Unity Catalog configured +2. `spark.sql.catalog..uri` and `.token` properties set +3. UC tables with proper feature flags (`delta.feature.catalogManaged`) +4. Unity Catalog endpoint available for testing + +**Pattern to Follow:** +- Unit tests: Focus on null validation and basic contract verification +- Document integration test needs clearly in comments +- Avoid tests that require SparkSession but pass null (they'll fail on null checks) +- Accept that some validations can only be tested with real UC setup + +**Example from CatalogManagedSnapshotManagerTest:** +```java +// ✅ Good unit test - validates null check +@Test +void testConstructor_NullSpark_ThrowsException() { + assertThrows(NullPointerException.class, ...); +} + +// ❌ Cannot be unit tested - requires real SparkSession +// Documented as needing integration test instead +// - testConstructor_NonUCTable_ThrowsException +// - testConstructor_ValidUCTable_Success +``` + +## Development Best Practices + +### Before Adding New Code + +**Always check if similar code exists** before creating new utilities or implementations: +- Search the codebase for similar functionality +- Look for existing patterns in related modules +- Reuse existing utilities when possible + +Example: Before creating new UC configuration extraction utilities, check if `spark/` or `unity/` already has them. + +### Naming and Documentation + +- Prefix all Delta-specific APIs with "Delta" +- Use precise terminology: "commit" vs "version", "DAO" vs "model" +- Keep config keys aligned with canonical definitions +- Add inline comments explaining: + - Onboarding states + - Why helper fields exist + - Prerequisite PRs or follow-up work +- Document adapter/wrapper classes explaining their purpose +- Delete unused imports, constructors, helpers immediately + +### Configuration + +- Only configure what's needed - avoid unused properties +- Don't duplicate settings that are set centrally +- Maintain single naming scheme: spec → SDK → server → connector +- Centralize config constants and reuse everywhere +- Document meaning, units, and defaults at declaration site + +### Code Structure + +- Avoid one-off interfaces +- Collapse duplicate setters +- Encapsulate internal setup inside constructors +- Keep call sites minimal +- Prefer composition over inheritance + +### Testing Requirements + +For every behavioral change: +- Add regression tests covering both new and existing behavior +- Test success and failure paths +- Test with non-default config values (ask: "would test pass if this config were ignored?") +- Parameterize across error codes, statuses, exceptions +- Exercise public APIs rather than internal repositories +- Clean up shared resources after tests + +### Retry and Resilience + +When adding retry logic: +- Centralize in shared transport layers +- Classify retryable failures by inspecting full cause chain +- Record attempt counts and elapsed time +- Keep loops simple: detect, guard, backoff, rethrow +- Use injected clock abstraction for time handling + +### Error Handling + +- Use validation helpers consistently (`ValidationUtils.checkArgument`, Guava `Preconditions`) +- Enforce strict bounds (no zero-delay retries, no negative attempts) +- Surface follow-up work via issues, not by expanding current PR + +## Explaining Codebase Concepts + +When explaining any complex concept: +1. Search the repository first +2. Include a simple diagram or conversational flow +3. Use "When X happens, respond with Y; if Z occurs, do A otherwise B" patterns +4. Aim for ELI10 (Explain Like I'm 10) clarity +5. Don't just repeat the question From fc302139610a4b5d25e0a4cc416b3ea4c8ea745b Mon Sep 17 00:00:00 2001 From: TimothyW553 Date: Fri, 21 Nov 2025 06:12:02 +0000 Subject: [PATCH 30/42] change unity --- build.sbt | 10 +++------- .../spark/snapshot/CatalogManagedSnapshotManager.java | 2 +- 2 files changed, 4 insertions(+), 8 deletions(-) diff --git a/build.sbt b/build.sbt index 683cb5ab58e..69e44d9c9df 100644 --- a/build.sbt +++ b/build.sbt @@ -462,7 +462,6 @@ lazy val sparkV1Filtered = (project in file("spark-v1-filtered")) lazy val sparkV2 = (project in file("kernel-spark")) .dependsOn(sparkV1Filtered) .dependsOn(kernelDefaults) - .dependsOn(unity) .dependsOn(goldenTables % "test") .settings( name := "delta-spark-v2", @@ -586,10 +585,7 @@ lazy val spark = (project in file("spark-unified")) override def transform(n: Node): Seq[Node] = n match { case e: Elem if e.label == "dependency" => val artifactId = (e \ "artifactId").text - // Check if artifactId starts with any internal module name - // (e.g., "delta-spark-v1_4.1_2.13" starts with "delta-spark-v1") - val isInternal = internalModules.exists(module => artifactId.startsWith(module)) - if (isInternal) Seq.empty else Seq(n) + if (internalModules.contains(artifactId)) Seq.empty else Seq(n) case _ => Seq(n) } }).transform(node).head @@ -909,12 +905,12 @@ lazy val kernelBenchmarks = (project in file("kernel/kernel-benchmarks")) ), ) -lazy val kernelUnityCatalog = (project in file("kernel/unitycatalog")) +lazy val unity = (project in file("unity")) .enablePlugins(ScalafmtPlugin) .dependsOn(kernelDefaults % "test->test") .dependsOn(storage) .settings ( - name := "delta-kernel-unitycatalog", + name := "delta-unity", commonSettings, javaOnlyReleaseSettings, javafmtCheckSettings, diff --git a/kernel-spark/src/main/java/io/delta/kernel/spark/snapshot/CatalogManagedSnapshotManager.java b/kernel-spark/src/main/java/io/delta/kernel/spark/snapshot/CatalogManagedSnapshotManager.java index 5beaac22c71..b5fb0573f74 100644 --- a/kernel-spark/src/main/java/io/delta/kernel/spark/snapshot/CatalogManagedSnapshotManager.java +++ b/kernel-spark/src/main/java/io/delta/kernel/spark/snapshot/CatalogManagedSnapshotManager.java @@ -26,10 +26,10 @@ import io.delta.kernel.internal.DeltaHistoryManager; import io.delta.kernel.spark.exception.VersionNotFoundException; import io.delta.kernel.spark.utils.CatalogTableUtils; +import io.delta.kernel.unitycatalog.UCCatalogManagedClient; import io.delta.storage.commit.uccommitcoordinator.UCClient; import io.delta.storage.commit.uccommitcoordinator.UCCommitCoordinatorClient; import io.delta.storage.commit.uccommitcoordinator.UCTokenBasedRestClient; -import io.delta.unity.UCCatalogManagedClient; import java.util.Optional; import org.apache.hadoop.conf.Configuration; import org.apache.spark.annotation.Experimental; From 3bd43573f4338c191409c23f9149ea2587efbda3 Mon Sep 17 00:00:00 2001 From: TimothyW553 Date: Fri, 21 Nov 2025 06:12:28 +0000 Subject: [PATCH 31/42] change unity --- .bazelbsp/bazel.log | 1 - .bazelbsp/bsp.log | 2 -- .bazelbsp/rpc.log | 0 3 files changed, 3 deletions(-) delete mode 100644 .bazelbsp/bazel.log delete mode 100644 .bazelbsp/bsp.log delete mode 100644 .bazelbsp/rpc.log diff --git a/.bazelbsp/bazel.log b/.bazelbsp/bazel.log deleted file mode 100644 index 8ee18baa1be..00000000000 --- a/.bazelbsp/bazel.log +++ /dev/null @@ -1 +0,0 @@ -{"time":"2025-11-21T05:39:37.262904969Z","level":"INFO","msg":"Bazel info created","cmd":"/usr/local/bin/bazel info workspace"} diff --git a/.bazelbsp/bsp.log b/.bazelbsp/bsp.log deleted file mode 100644 index 3361c51520e..00000000000 --- a/.bazelbsp/bsp.log +++ /dev/null @@ -1,2 +0,0 @@ -{"time":"2025-11-21T05:39:37.262774602Z","level":"INFO","msg":"No project file found","path":"/home/timothy.wang/delta/.bazelbsp/project.json"} -{"time":"2025-11-21T05:39:37.271003496Z","level":"ERROR","msg":"failed to load project","error":"exit status 1"} diff --git a/.bazelbsp/rpc.log b/.bazelbsp/rpc.log deleted file mode 100644 index e69de29bb2d..00000000000 From 2ad9fc0426f690acf71bd17969e42faace836ad0 Mon Sep 17 00:00:00 2001 From: TimothyW553 Date: Fri, 21 Nov 2025 07:02:16 +0000 Subject: [PATCH 32/42] docs: tidy root metadata --- CLAUDE.md | 615 ------------------------------------------------------ 1 file changed, 615 deletions(-) delete mode 100644 CLAUDE.md diff --git a/CLAUDE.md b/CLAUDE.md deleted file mode 100644 index 87c3b3edd13..00000000000 --- a/CLAUDE.md +++ /dev/null @@ -1,615 +0,0 @@ -# CLAUDE.md - -This file provides guidance to Claude Code (claude.ai/code) when working with code in this repository. - -## Project Overview - -Delta Lake is an open-source storage framework providing ACID transactions and scalable metadata handling for data lakes. This repository contains multiple components: - -- **Delta Spark (spark/)**: The core Spark connector for Delta Lake (v1 implementation using DeltaLog) -- **Delta Kernel (kernel/)**: A protocol-agnostic library for building Delta connectors - - `kernel-api/`: Public APIs for Table and Engine interfaces - - `kernel-defaults/`: Default Engine implementation using Hadoop libraries - - `kernel-benchmarks/`: Performance benchmarking suite -- **Delta Kernel Spark (kernel-spark/)**: Spark connector built using Delta Kernel (v2 pure DSv2) -- **Spark Unified (spark-unified/)**: Final published `delta-spark` JAR combining v1 and v2 -- **Unity (unity/)**: Unity Catalog integration with catalog-managed commit coordination -- **Storage (storage/, storage-s3-dynamodb/)**: Storage layer abstractions and S3 DynamoDB support -- **Connectors (connectors/)**: Various Delta Lake connectors (Flink, standalone, Hudi, Iceberg) -- **Python (python/)**: Python bindings for Delta Lake - -## Build System - -The project uses **SBT** (Scala Build Tool) with Java 11 as the target JVM. - -### Common Build Commands - -```bash -# Build the project -build/sbt compile - -# Generate artifacts -build/sbt package - -# Run all tests -build/sbt test - -# Run tests for a specific project group -build/sbt sparkGroup/test -build/sbt kernelGroup/test - -# Run a single test suite -build/sbt spark/'testOnly org.apache.spark.sql.delta.optimize.OptimizeCompactionSQLSuite' - -# Run a specific test within a suite -build/sbt spark/'testOnly *.OptimizeCompactionSQLSuite -- -z "optimize command: on partitioned table - all partitions"' - -# Run tests with coverage -build/sbt coverage test coverageAggregate coverageOff - -# Run with different Spark version -build/sbt -DsparkVersion=master compile -build/sbt -DsparkVersion=3.5.7 test -``` - -### Code Formatting - -**Java code** must follow Google Java Style: -```bash -# Check Java formatting -build/sbt javafmtCheckAll - -# Fix Java formatting -build/sbt javafmtAll -``` - -**Scala code** uses Scalafmt (config in `.scalafmt.conf`): -```bash -# Check Scala formatting -build/sbt scalafmtCheckAll - -# Fix Scala formatting -build/sbt scalafmtAll - -# Check Scala style -build/sbt scalastyle -``` - -### Python Tests - -```bash -# Setup conda environment -conda env create --name delta_python_tests --file=/python/environment.yml -conda activate delta_python_tests - -# Run Python tests -python3 /python/run-tests.py -``` - -### Test Groups - -The codebase is organized into test groups that can be run independently: -- `spark`: Spark-related tests -- `kernel`: Delta Kernel tests -- `iceberg`: Iceberg compatibility tests -- `spark-python`: Python integration tests - -Use `build/sbt Group/test` to run a specific group. - -## Architecture - -### Delta Kernel Architecture - -Delta Kernel provides two sets of APIs: - -1. **Table APIs** (`io.delta.kernel.*`): High-level interfaces for reading/writing Delta tables - - `Table`: Entry point for accessing Delta tables - - `Snapshot`: Point-in-time view of a Delta table - - `Scan`: API for reading data with filters and column pruning - - `Transaction`: API for writing data to Delta tables - -2. **Engine APIs** (`io.delta.kernel.engine.*`): Pluggable interfaces for compute-intensive operations - - `Engine`: Main interface for connector-specific optimizations - - `ParquetHandler`: Parquet file reading/writing - - `JsonHandler`: JSON processing for Delta logs - - `FileSystemClient`: File system operations - - Default implementations provided in `kernel-defaults` module - -### V1 vs V2 Architecture - -Delta Lake provides two Spark connector implementations that coexist in the published JAR: - -#### V1 Implementation (DeltaLog-based) -Located in `spark/`, this is the traditional connector using: -- **DeltaLog**: Core entry point for transaction log access and management -- **Snapshot**: In-memory representation of table state at a version -- **OptimisticTransaction**: Handles ACID transactions with conflict detection -- **DeltaTable**: Public Scala/Java/Python API -- Hybrid DataSource V1 and V2 APIs for compatibility - -**Core Components**: -- `DeltaLog.scala`: Transaction log management and caching -- `Snapshot.scala`: Table state with file lists and metadata -- `OptimisticTransaction.scala`: Write path with conflict resolution -- `commands/`: Command implementations (WriteIntoDelta, MergeIntoCommand, etc.) -- `catalog/`: AbstractDeltaCatalog with table resolution - -#### V2 Implementation (Kernel-based) -Located in `kernel-spark/`, this is a pure DataSource V2 connector using Delta Kernel: -- Uses Delta Kernel APIs (`io.delta.kernel.*`) for protocol handling -- Depends on `sparkV1Filtered` for shared utilities (excluding DeltaLog) -- Designed as a reference implementation for building Delta connectors -- Leverages Spark's ParquetFileFormat for actual data reads - -**Architecture**: -``` -Spark Driver → Delta Kernel Connector → Delta Kernel API - ↓ - (reads Delta logs) - ↓ -Spark Engine ← file splits ← Delta Kernel returns scan files - ↓ -(Parquet scans using Spark's built-in reader) -``` - -#### Unified Connector (spark-unified/) -The published `delta-spark` JAR combines both implementations: -- Provides single entry points: `DeltaCatalog` and `DeltaSparkSessionExtension` -- V1 handles most operations (writes, commands) -- V2 provides alternative read path via DSv2 -- No duplicate classes - sparkV1Filtered ensures clean merge - -### Module Dependencies and Build Flow - -The repository uses a sophisticated multi-module build structure: - -``` -storage (base storage abstraction) - ↓ -kernelApi (kernel/kernel-api) → kernelDefaults (kernel/kernel-defaults) - ↓ ↓ - | sparkV2 (kernel-spark/) - | ↓ - └──────────────────────────────> unity (unity/) - -sparkV1 (spark/) ──────> sparkV1Filtered (spark-v1-filtered/) - ↓ - sparkV2 (kernel-spark/) - ↓ - spark (spark-unified/) [PUBLISHED JAR] -``` - -**Key Points:** -- `sparkV1` contains the complete v1 implementation with DeltaLog, Snapshot, OptimisticTransaction -- `sparkV1Filtered` is a filtered version excluding DeltaLog classes (to avoid conflicts) -- `sparkV2` (kernel-spark) depends on sparkV1Filtered for shared utilities -- `spark` (spark-unified) is the final published artifact merging all three modules -- `unity` provides Unity Catalog integration for catalog-managed tables (CCv2) - -**Published vs Internal Modules:** -- Published: `delta-spark` (spark-unified), `delta-kernel-api`, `delta-kernel-defaults`, `delta-unity` -- Internal (not published): `delta-spark-v1`, `delta-spark-v1-filtered`, `delta-spark-v2` - -### Unity Catalog Integration (unity/) - -The Unity module provides integration with Unity Catalog for catalog-managed tables: - -**Key Components**: -- `UCCatalogManagedCommitter`: Implements coordinated commits version 2 (CCv2) -- `UCCatalogManagedClient`: Client for Unity Catalog commit coordination -- Catalog-managed tables use feature flags in table properties: - - `delta.feature.catalogManaged = supported` - - `delta.feature.catalogOwned-preview = supported` - -**Detection**: Use `CatalogTableUtils` (in kernel-spark) to check if a table is catalog-managed: -- `isCatalogManaged(CatalogTable)`: Checks for catalog management feature -- `isUnityCatalogManagedTable(CatalogTable)`: Verifies UC-specific table ID - -### Protocol Implementation - -The Delta transaction log protocol is defined in `PROTOCOL.md`. Key concepts: - -- **Actions**: Metadata changes recorded in JSON log entries (AddFile, RemoveFile, Protocol, etc.) -- **Checkpoints**: Parquet files that snapshot the table state for fast access -- **Table Features**: Protocol capabilities (column mapping, deletion vectors, row tracking, etc.) - - Defined in `TableFeature.scala` with reader/writer version requirements - - Features can be legacy (version-based) or name-based -- **Concurrency Control**: Optimistic concurrency with conflict detection and retry logic - -## Key File Locations - -### Spark V1 Implementation (spark/) -- Core Delta classes: `spark/src/main/scala/org/apache/spark/sql/delta/` - - `DeltaLog.scala`: Transaction log management - - `Snapshot.scala`: Table state snapshots - - `OptimisticTransaction.scala`: ACID transaction handling - - `commands/`: DML operations (MERGE, UPDATE, DELETE, OPTIMIZE, VACUUM) - - `catalog/`: Catalog integration (AbstractDeltaCatalog) - - `actions/`: Protocol actions (AddFile, RemoveFile, Metadata, etc.) - -### Delta Kernel (kernel/) -- API definitions: `kernel/kernel-api/src/main/java/io/delta/kernel/` - - `Table.java`, `Snapshot.java`, `Scan.java`, `Transaction.java` - - `engine/Engine.java`: Pluggable engine interface -- Default implementations: `kernel/kernel-defaults/src/main/java/io/delta/kernel/defaults/` - - `DefaultEngine.java`: Hadoop-based Engine - - Parquet/JSON handlers, file system client - -### Kernel-Spark Connector (kernel-spark/) -- Connector code: `kernel-spark/src/main/java/io/delta/kernel/spark/` - - `read/`: DSv2 read support - - `snapshot/`: Snapshot management - - `catalog/`: Catalog integration for kernel - - `utils/`: Bridge utilities between Kernel and Spark - - `CatalogTableUtils.java`: Unity Catalog metadata helpers - - `SchemaUtils.java`: Schema conversion utilities - -### Spark Unified (spark-unified/) -- Entry points: `spark-unified/src/main/` - - `java/.../DeltaCatalog.java`: Unified catalog (extends AbstractDeltaCatalog) - - `scala/.../DeltaSparkSessionExtension.scala`: Session extension - -### Unity Catalog Integration (unity/) -- UC support: `unity/src/main/java/io/delta/unity/` - - `UCCatalogManagedCommitter.java`: Catalog-managed commits (CCv2) - - `UCCatalogManagedClient.java`: Unity Catalog client - -### Tests -- Tests mirror source structure: `/src/test/{scala,java}/` -- Shared test utilities: `/src/test/scala/.../testUtils/` - -## Development Workflow - -### IntelliJ Setup - -1. Import project: `File` > `New Project` > `Project from Existing Sources` > select delta directory -2. Choose `sbt` as external model, use Java 11 JDK -3. Run `build/sbt clean package` to generate necessary files -4. If you see parser errors, run `build/sbt clean compile` and refresh IntelliJ -5. For source folder issues: `File` > `Project Structure` > `Modules` > remove target folders - -### Contributing - -- Sign commits with `git commit -s` (Developer Certificate of Origin required) -- Follow Apache Spark Scala Style Guide -- Major features (>100 LOC) require discussion via GitHub issue first -- Run formatting checks before committing: `build/sbt javafmtAll scalafmtAll` - -## Testing Strategy - -### Test Naming Conventions - -- Scala tests: `*Suite.scala` (e.g., `DeltaLogSuite`, `OptimizeCompactionSQLSuite`) -- Java tests: `*Test.java` (e.g., `CatalogTableUtilsTest`) - -### Running Tests Efficiently - -When working on a specific module: -```bash -# Test only kernel module -build/sbt kernel-api/test - -# Test specific package within spark -build/sbt spark/'testOnly org.apache.spark.sql.delta.commands.*' - -# Run with increased logging -build/sbt 'set logLevel := Level.Debug' spark/test -``` - -### Test Configuration - -Tests are configured for local execution with: -- Spark UI disabled -- Reduced shuffle partitions (5) -- Limited memory (1GB) -- Small log cache sizes - -See `build.sbt` `commonSettings` section for full test JVM options. - -## Protocol and Table Features - -When working on protocol changes or table features: - -1. Review `PROTOCOL.md` for specification details -2. Table features are defined in `spark/src/main/scala/org/apache/spark/sql/delta/TableFeature.scala` -3. Protocol version changes require updating reader/writer version requirements -4. Add feature documentation to `protocol_rfcs/` for major changes - -## Common Pitfalls - -### Java Version -- Must use Java 11 for building -- Set `JAVA_HOME` appropriately: `export JAVA_HOME=$(/usr/libexec/java_home -v 11)` (macOS) - -### SBT Commands -- Always use `build/sbt` not bare `sbt` (uses project-specific version) -- Use single quotes for commands with spaces: `build/sbt 'testOnly *.Suite'` - -### Cross-Scala Builds -- Default Scala version is 2.13.16 -- Use `+ ` to run for all Scala versions: `build/sbt "+ compile"` -- Use `++ 2.13.16` to set specific version: `build/sbt "++ 2.13.16" compile` - -### Test Parallelization -- Tests run in parallel by default -- Some tests may be flaky when run in parallel -- Use `Test / parallelExecution := false` in specific project if needed - -## Git Workflow with Stacked Branches - -This project uses **git stack** for managing stacked branches. Common commands: - -```bash -# Visualize stack -git stack ls - -# Navigate between branches -git stack jump BRANCH - -# Create child branch from current -git stack create BRANCH - -# Daily workflow - -git stack commit # or regular git commit -git stack sync # rebase children onto updated parent -git stack push origin # push branches + manage PRs -git stack push origin --publish # mark all PRs ready for review - -# Branch management -git stack remove BRANCH # drop a branch from stack -git stack clear # forget stored PR links -git stack abort # recover from failed sync/rebase -``` - -**Important**: Never add "Co-Authored-By: Claude" to commits. - -## CCv2 Project Context - -**Current Focus**: Implementing Coordinated Commits V2 (CCv2) support in Delta Kernel's DSv2 Connector for Unity Catalog-managed tables. - -### Architecture Overview - -CCv2 enables Unity Catalog to manage Delta table commits through coordinated commit coordination: - -1. **Catalog-managed tables** have feature flags in table properties: - - `delta.feature.catalogManaged = supported` - - `delta.feature.catalogOwned-preview = supported` - - `ucTableId` identifier for Unity Catalog - -2. **Components**: - - `UCCatalogManagedClient` (unity/): Loads snapshots for UC-managed tables - - `UCCatalogManagedCommitter` (unity/): Handles commits via UC - - `CatalogTableUtils` (kernel-spark/): Detection utilities for catalog management - - `UCClientFactory`: Creates UC clients (currently token-based) - -3. **Current Work**: Extending CCv2 support with additional features and integration tests. - -### Completed Work - -- **PR #5477**: Added utilities for detecting CCv2 tables - - `CatalogTableUtils.java`: Methods to check if table is catalog-managed - - `ScalaUtils.java`: Scala-Java conversion utilities - - Test utilities in kernel-spark module - -- **PR #5520**: Implemented snapshot manager infrastructure for UC-managed tables - - `CatalogManagedSnapshotManager`: DeltaSnapshotManager implementation for UC tables - - `DeltaSnapshotManagerFactory`: Factory pattern for creating appropriate snapshot managers - - `SparkTable` integration: Automatically uses correct manager based on table type - - Added `unity` as dependency to `kernel-spark` module - - Unit tests for null validation and factory behavior - -### Key Files for CCv2 - -- `unity/src/main/java/io/delta/unity/` - - `UCCatalogManagedClient.java`: Client for UC-managed Delta tables - - `UCCatalogManagedCommitter.java`: Commit coordination logic - - `adapters/`: Protocol and metadata adapters between Kernel and storage layers -- `spark/src/main/scala/.../coordinatedcommits/UCCommitCoordinatorBuilder.scala` - - `UCClientFactory` trait and `UCTokenBasedRestClientFactory` -- `kernel-spark/src/main/java/io/delta/kernel/spark/` - - `utils/CatalogTableUtils.java`: Detection utilities for catalog-managed tables - - `snapshot/CatalogManagedSnapshotManager.java`: Snapshot manager for UC tables - - `snapshot/DeltaSnapshotManagerFactory.java`: Factory for manager selection - - `table/SparkTable.java`: Uses factory to create appropriate snapshot manager - -### Known Limitations and Future Work - -**Resource Lifecycle Management (TODO for next PR):** -- `CatalogManagedSnapshotManager` implements `AutoCloseable` but `DeltaSnapshotManager` interface doesn't -- `SparkTable` doesn't implement close() or manage snapshot manager resources -- UC client connections may leak in long-running Spark sessions -- Options to consider: - - Make SparkTable implement AutoCloseable - - Add AutoCloseable to DeltaSnapshotManager interface with default no-op - - Use shutdown hooks as safety net (not recommended as primary solution) - -**Unsupported Operations (Architectural Limitations):** -- `getActiveCommitAtTime()`: Throws `UnsupportedOperationException` - - Requires filesystem-based commit history (not available for UC tables) - - Unity Catalog coordinates commits differently -- `getTableChanges()`: Throws `UnsupportedOperationException` - - Needs UC API support for commit range functionality - -**Performance Optimizations Needed:** -- `checkVersionExists()` loads full snapshot just to check version existence - - Very inefficient for tables with large file lists - - TODO: Add lightweight version checking API to `UCCatalogManagedClient` -- No connection pooling for UC clients (new HTTP client per manager instance) -- No snapshot caching strategy (loads fresh from UC every time) - -**Module Dependencies:** -- Added `unity` as dependency to `kernel-spark` (sparkV2) -- This makes kernel-spark Unity-aware (appropriate for UC-managed tables) -- Creates tight coupling to Unity implementation -- Consider if UC support should be pluggable via SPI pattern in the future - -## Working with Different Modules - -### When to modify which module: - -**spark/** - Modify when: -- Adding/fixing V1 connector features (DeltaLog, OptimisticTransaction) -- Implementing new commands (DML operations) -- Changing Spark catalog integration -- Working on most production Spark features - -**kernel/kernel-api/** - Modify when: -- Adding new Table or Engine API methods -- Changing public Kernel interfaces -- This requires careful API compatibility considerations - -**kernel/kernel-defaults/** - Modify when: -- Improving default Engine implementations -- Fixing Parquet/JSON handling in default engine -- Adding new default utilities - -**kernel-spark/** - Modify when: -- Working on V2 DSv2 connector -- Adding Kernel-specific Spark integration -- Bridging Kernel and Spark types (utils/) -- Testing Kernel with Spark - -**spark-unified/** - Rarely modified: -- Only for entry point changes -- Usually just extends classes from spark/ - -**unity/** - Modify when: -- Working on Unity Catalog integration -- Implementing catalog-managed commit coordination -- Adding UC-specific features - -### Module Testing Strategy - -When working across modules: -1. Test individual module first: `build/sbt /test` -2. Test the group: `build/sbt Group/test` -3. Test integration in spark-unified if needed -4. Remember that kernel-spark tests may use sparkV1Filtered classes - -**For CCv2/kernel-spark work**, use: -```bash -build/sbt -DsparkVersion=master "++ 2.13.16" clean sparkV2/test -``` -Note: Test output can be very large, monitor carefully. - -### Testing Unity Catalog Managed Tables - -**Unit Test Limitations:** -Testing UC-managed table logic has inherent limitations in unit tests: -- UC table validation requires real `SparkSession` with catalog configuration -- Cannot mock `SparkSession` effectively for UC catalog access -- Constructor null checks happen before business logic validation -- Factory pattern requires non-null SparkSession for consistent API - -**What CAN be unit tested:** -- ✅ Null parameter validation (requireNonNull checks) -- ✅ Factory creation with mock catalog tables -- ✅ Interface contract validation -- ✅ Exception type verification - -**What REQUIRES integration tests:** -- ❌ UC table detection and validation logic -- ❌ Snapshot loading from Unity Catalog -- ❌ UC client creation and lifecycle -- ❌ Manager selection based on table type -- ❌ End-to-end flows with real UC tables - -**Integration Test Requirements:** -1. Real SparkSession with Unity Catalog configured -2. `spark.sql.catalog..uri` and `.token` properties set -3. UC tables with proper feature flags (`delta.feature.catalogManaged`) -4. Unity Catalog endpoint available for testing - -**Pattern to Follow:** -- Unit tests: Focus on null validation and basic contract verification -- Document integration test needs clearly in comments -- Avoid tests that require SparkSession but pass null (they'll fail on null checks) -- Accept that some validations can only be tested with real UC setup - -**Example from CatalogManagedSnapshotManagerTest:** -```java -// ✅ Good unit test - validates null check -@Test -void testConstructor_NullSpark_ThrowsException() { - assertThrows(NullPointerException.class, ...); -} - -// ❌ Cannot be unit tested - requires real SparkSession -// Documented as needing integration test instead -// - testConstructor_NonUCTable_ThrowsException -// - testConstructor_ValidUCTable_Success -``` - -## Development Best Practices - -### Before Adding New Code - -**Always check if similar code exists** before creating new utilities or implementations: -- Search the codebase for similar functionality -- Look for existing patterns in related modules -- Reuse existing utilities when possible - -Example: Before creating new UC configuration extraction utilities, check if `spark/` or `unity/` already has them. - -### Naming and Documentation - -- Prefix all Delta-specific APIs with "Delta" -- Use precise terminology: "commit" vs "version", "DAO" vs "model" -- Keep config keys aligned with canonical definitions -- Add inline comments explaining: - - Onboarding states - - Why helper fields exist - - Prerequisite PRs or follow-up work -- Document adapter/wrapper classes explaining their purpose -- Delete unused imports, constructors, helpers immediately - -### Configuration - -- Only configure what's needed - avoid unused properties -- Don't duplicate settings that are set centrally -- Maintain single naming scheme: spec → SDK → server → connector -- Centralize config constants and reuse everywhere -- Document meaning, units, and defaults at declaration site - -### Code Structure - -- Avoid one-off interfaces -- Collapse duplicate setters -- Encapsulate internal setup inside constructors -- Keep call sites minimal -- Prefer composition over inheritance - -### Testing Requirements - -For every behavioral change: -- Add regression tests covering both new and existing behavior -- Test success and failure paths -- Test with non-default config values (ask: "would test pass if this config were ignored?") -- Parameterize across error codes, statuses, exceptions -- Exercise public APIs rather than internal repositories -- Clean up shared resources after tests - -### Retry and Resilience - -When adding retry logic: -- Centralize in shared transport layers -- Classify retryable failures by inspecting full cause chain -- Record attempt counts and elapsed time -- Keep loops simple: detect, guard, backoff, rethrow -- Use injected clock abstraction for time handling - -### Error Handling - -- Use validation helpers consistently (`ValidationUtils.checkArgument`, Guava `Preconditions`) -- Enforce strict bounds (no zero-delay retries, no negative attempts) -- Surface follow-up work via issues, not by expanding current PR - -## Explaining Codebase Concepts - -When explaining any complex concept: -1. Search the repository first -2. Include a simple diagram or conversational flow -3. Use "When X happens, respond with Y; if Z occurs, do A otherwise B" patterns -4. Aim for ELI10 (Explain Like I'm 10) clarity -5. Don't just repeat the question From 6126080cbdd9c2fc3ff8e6fb61c3a6cb84b050c6 Mon Sep 17 00:00:00 2001 From: TimothyW553 Date: Fri, 21 Nov 2025 07:12:34 +0000 Subject: [PATCH 33/42] build --- build.sbt | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/build.sbt b/build.sbt index 69e44d9c9df..84019516af7 100644 --- a/build.sbt +++ b/build.sbt @@ -462,6 +462,7 @@ lazy val sparkV1Filtered = (project in file("spark-v1-filtered")) lazy val sparkV2 = (project in file("kernel-spark")) .dependsOn(sparkV1Filtered) .dependsOn(kernelDefaults) + .dependsOn(kernelUnityCatalog) .dependsOn(goldenTables % "test") .settings( name := "delta-spark-v2", @@ -585,7 +586,10 @@ lazy val spark = (project in file("spark-unified")) override def transform(n: Node): Seq[Node] = n match { case e: Elem if e.label == "dependency" => val artifactId = (e \ "artifactId").text - if (internalModules.contains(artifactId)) Seq.empty else Seq(n) + // Check if artifactId starts with any internal module name + // (e.g., "delta-spark-v1_4.1_2.13" starts with "delta-spark-v1") + val isInternal = internalModules.exists(module => artifactId.startsWith(module)) + if (isInternal) Seq.empty else Seq(n) case _ => Seq(n) } }).transform(node).head @@ -905,12 +909,12 @@ lazy val kernelBenchmarks = (project in file("kernel/kernel-benchmarks")) ), ) -lazy val unity = (project in file("unity")) +lazy val kernelUnityCatalog = (project in file("kernel/unitycatalog")) .enablePlugins(ScalafmtPlugin) .dependsOn(kernelDefaults % "test->test") .dependsOn(storage) .settings ( - name := "delta-unity", + name := "delta-kernel-unitycatalog", commonSettings, javaOnlyReleaseSettings, javafmtCheckSettings, From 54bf26df21b62eecce61323ab57e13d4b83a3eda Mon Sep 17 00:00:00 2001 From: TimothyW553 Date: Mon, 24 Nov 2025 23:25:26 +0000 Subject: [PATCH 34/42] generics --- .../CatalogManagedSnapshotManager.java | 208 +++--------------- .../snapshot/DeltaSnapshotManagerFactory.java | 22 +- .../spark/snapshot/ManagedCommitClient.java | 47 ++++ .../uc/UnityCatalogManagedCommitClient.java | 150 +++++++++++++ .../CatalogManagedSnapshotManagerTest.java | 79 +++---- 5 files changed, 265 insertions(+), 241 deletions(-) create mode 100644 kernel-spark/src/main/java/io/delta/kernel/spark/snapshot/ManagedCommitClient.java create mode 100644 kernel-spark/src/main/java/io/delta/kernel/spark/snapshot/uc/UnityCatalogManagedCommitClient.java diff --git a/kernel-spark/src/main/java/io/delta/kernel/spark/snapshot/CatalogManagedSnapshotManager.java b/kernel-spark/src/main/java/io/delta/kernel/spark/snapshot/CatalogManagedSnapshotManager.java index b5fb0573f74..6d3dfc80431 100644 --- a/kernel-spark/src/main/java/io/delta/kernel/spark/snapshot/CatalogManagedSnapshotManager.java +++ b/kernel-spark/src/main/java/io/delta/kernel/spark/snapshot/CatalogManagedSnapshotManager.java @@ -25,109 +25,41 @@ import io.delta.kernel.exceptions.KernelException; import io.delta.kernel.internal.DeltaHistoryManager; import io.delta.kernel.spark.exception.VersionNotFoundException; -import io.delta.kernel.spark.utils.CatalogTableUtils; -import io.delta.kernel.unitycatalog.UCCatalogManagedClient; -import io.delta.storage.commit.uccommitcoordinator.UCClient; -import io.delta.storage.commit.uccommitcoordinator.UCCommitCoordinatorClient; -import io.delta.storage.commit.uccommitcoordinator.UCTokenBasedRestClient; import java.util.Optional; import org.apache.hadoop.conf.Configuration; import org.apache.spark.annotation.Experimental; -import org.apache.spark.sql.SparkSession; -import org.apache.spark.sql.catalyst.catalog.CatalogTable; import org.slf4j.Logger; import org.slf4j.LoggerFactory; /** - * Implementation of DeltaSnapshotManager for Unity Catalog managed tables (CCv2). + * Implementation of DeltaSnapshotManager for catalog-managed tables (e.g., UC). * - *

This snapshot manager handles tables where Unity Catalog acts as the commit coordinator. It - * manages the lifecycle of UC client connections and delegates snapshot operations to {@link - * UCCatalogManagedClient}. - * - *

The manager automatically extracts table metadata (ucTableId, tablePath) from the {@link - * CatalogTable} and creates the necessary UC client for communication with Unity Catalog. - * - *

Resource Management: This class implements {@link AutoCloseable} and must be - * properly closed to release UC client resources. Use try-with-resources: - * - * 

{@code
- * try (CatalogManagedSnapshotManager manager = new CatalogManagedSnapshotManager(...)) {
- *   Snapshot snapshot = manager.loadLatestSnapshot();
- *   // Use snapshot...
- * }
- * }
- * - *

TODO (Next PR): Integrate proper lifecycle management into {@link - * io.delta.kernel.spark.table.SparkTable}. Currently, SparkTable doesn't implement close() or - * properly manage snapshot manager resources. Options to consider: - * - *

    - *
  • Make SparkTable implement AutoCloseable and propagate close() to snapshot manager - *
  • Add {@link AutoCloseable} to {@link DeltaSnapshotManager} interface with default no-op - *
  • Use finalization or shutdown hooks as safety net (not recommended as primary solution) - *
- * - *

Without proper lifecycle management, UC client connections may leak in long-running Spark - * sessions. This is acceptable for this initial PR but must be addressed before production use. + *

This snapshot manager is agnostic to the underlying catalog implementation. It delegates to a + * {@link ManagedCommitClient}, keeping catalog-specific wiring out of the manager itself. */ @Experimental public class CatalogManagedSnapshotManager implements DeltaSnapshotManager, AutoCloseable { private static final Logger logger = LoggerFactory.getLogger(CatalogManagedSnapshotManager.class); - private final UCClient ucClient; - private final UCCatalogManagedClient ucManagedClient; + private final ManagedCommitClient commitClient; private final Engine kernelEngine; - private final String ucTableId; - private final String tablePath; - /** - * Creates a snapshot manager for a Unity Catalog managed table. - * - * @param catalogTable the Spark catalog table (must be Unity Catalog managed) - * @param spark the SparkSession containing Unity Catalog configurations - * @param hadoopConf Hadoop configuration for the Delta Kernel engine - * @throws NullPointerException if any parameter is null - * @throws IllegalArgumentException if table is not Unity Catalog managed - * @throws IllegalArgumentException if Unity Catalog configuration is not found - */ - public CatalogManagedSnapshotManager( - CatalogTable catalogTable, SparkSession spark, Configuration hadoopConf) { - requireNonNull(catalogTable, "catalogTable is null"); - requireNonNull(spark, "spark is null"); + public CatalogManagedSnapshotManager(ManagedCommitClient commitClient, Configuration hadoopConf) { + this.commitClient = requireNonNull(commitClient, "commitClient is null"); requireNonNull(hadoopConf, "hadoopConf is null"); - // Validate table is Unity Catalog managed - if (!CatalogTableUtils.isUnityCatalogManagedTable(catalogTable)) { - throw new IllegalArgumentException( - "Cannot create CatalogManagedSnapshotManager: table is not Unity Catalog managed. " - + "Table identifier: " - + catalogTable.identifier()); - } - - // Extract table metadata - this.ucTableId = extractUCTableId(catalogTable); - this.tablePath = extractTablePath(catalogTable); - - // Create UC client and managed client - this.ucClient = createUCClientInternal(catalogTable, spark); - this.ucManagedClient = new UCCatalogManagedClient(ucClient); this.kernelEngine = DefaultEngine.create(hadoopConf); - logger.info( - "Created CatalogManagedSnapshotManager for table {} at path {}", ucTableId, tablePath); + "Created CatalogManagedSnapshotManager for table {} at path {}", + commitClient.getTableId(), + commitClient.getTablePath()); } - /** - * Loads the latest snapshot of the Unity Catalog managed Delta table. - * - * @return the latest snapshot - */ + /** Loads the latest snapshot of the catalog-managed Delta table. */ @Override public Snapshot loadLatestSnapshot() { - return ucManagedClient.loadSnapshot( - kernelEngine, ucTableId, tablePath, Optional.empty(), Optional.empty()); + return commitClient.loadSnapshot(kernelEngine, Optional.empty(), Optional.empty()); } /** @@ -139,8 +71,8 @@ public Snapshot loadLatestSnapshot() { @Override public Snapshot loadSnapshotAt(long version) { checkArgument(version >= 0, "version must be non-negative"); - return ucManagedClient.loadSnapshot( - kernelEngine, ucTableId, tablePath, Optional.of(version), Optional.empty()); + return commitClient.loadSnapshot( + kernelEngine, Optional.of(version), Optional.empty()); } /** @@ -160,8 +92,8 @@ public DeltaHistoryManager.Commit getActiveCommitAtTime( boolean canReturnEarliestCommit) { throw new UnsupportedOperationException( "getActiveCommitAtTime not yet implemented for catalog-managed tables. " - + "This operation requires filesystem-based commit history which is not " - + "accessible for Unity Catalog managed tables."); + + "This operation requires filesystem-based commit history which may not be " + + "available for catalog-managed tables."); } /** @@ -196,17 +128,18 @@ public void checkVersionExists(long version, boolean mustBeRecreatable, boolean /** * Gets a range of table changes between versions. * - *

Note: This operation delegates to {@link UCCatalogManagedClient} for Unity - * Catalog managed tables. + *

Note: This operation delegates to the managed commit client. * * @throws UnsupportedOperationException if not yet implemented for catalog-managed tables */ @Override public CommitRange getTableChanges(Engine engine, long startVersion, Optional endVersion) { - // TODO: Implement getTableChanges for UC-managed tables - // This requires UCCatalogManagedClient to expose commit range functionality - throw new UnsupportedOperationException( - "getTableChanges not yet implemented for catalog-managed tables"); + requireNonNull(engine, "engine is null"); + checkArgument(startVersion >= 0, "startVersion must be non-negative"); + endVersion.ifPresent(v -> checkArgument(v >= 0, "endVersion must be non-negative")); + + return commitClient.loadCommitRange( + engine, Optional.of(startVersion), Optional.empty(), endVersion, Optional.empty()); } /** @@ -218,101 +151,10 @@ public CommitRange getTableChanges(Engine engine, long startVersion, OptionalThis method resolves the catalog configuration directly from Spark and creates a {@link - * UCTokenBasedRestClient}. All UC-specific logic is encapsulated here to avoid polluting - * kernel-spark utilities with UC coupling. - * - * @param catalogTable the catalog table - * @param spark the SparkSession - * @return configured UC client - * @throws IllegalArgumentException if catalog configuration is not found - */ - private static UCClient createUCClientInternal(CatalogTable catalogTable, SparkSession spark) { - // 1. Extract catalog name from table identifier (or use default) - scala.Option catalogOption = catalogTable.identifier().catalog(); - String catalogName = - catalogOption.isDefined() - ? catalogOption.get() - : spark.sessionState().catalogManager().currentCatalog().name(); - - // 2. Get all UC catalog configs from Scala code (inlined, not abstracted) - scala.collection.immutable.List> scalaConfigs = - org.apache.spark.sql.delta.coordinatedcommits.UCCommitCoordinatorBuilder$.MODULE$ - .getCatalogConfigs(spark); - - // 3. Find matching config for this catalog - Optional> configTuple = - scala.jdk.javaapi.CollectionConverters.asJava(scalaConfigs).stream() - .filter(tuple -> tuple._1().equals(catalogName)) - .findFirst(); - - if (!configTuple.isPresent()) { - throw new IllegalArgumentException( - "Cannot create UC client: Unity Catalog configuration not found " - + "for catalog '" - + catalogName - + "'. " - + "Ensure spark.sql.catalog." - + catalogName - + ".uri and " - + "spark.sql.catalog." - + catalogName - + ".token are configured."); - } - - // 4. Extract URI and token from tuple (catalogName, uri, token) - scala.Tuple3 config = configTuple.get(); - String uri = config._2(); - String token = config._3(); - - // 5. Create UC client - return new UCTokenBasedRestClient(uri, token); - } - - /** - * Extracts the Unity Catalog table ID from catalog table properties. - * - * @param catalogTable the catalog table - * @return the UC table ID - * @throws IllegalArgumentException if ucTableId is not found - */ - private static String extractUCTableId(CatalogTable catalogTable) { - java.util.Map storageProperties = - scala.collection.JavaConverters.mapAsJavaMap(catalogTable.storage().properties()); - - String ucTableId = storageProperties.get(UCCommitCoordinatorClient.UC_TABLE_ID_KEY); - if (ucTableId == null || ucTableId.isEmpty()) { - throw new IllegalArgumentException( - "Cannot extract ucTableId: " - + UCCommitCoordinatorClient.UC_TABLE_ID_KEY - + " not found in table storage properties for table " - + catalogTable.identifier()); - } - return ucTableId; - } - - /** - * Extracts the table path from the catalog table location. - * - * @param catalogTable the catalog table - * @return the table path - * @throws IllegalArgumentException if location is not available - */ - private static String extractTablePath(CatalogTable catalogTable) { - if (catalogTable.location() == null) { - throw new IllegalArgumentException( - "Cannot extract table path: location is null for table " + catalogTable.identifier()); + logger.warn("Error closing catalog-managed client for table {}", commitClient.getTableId(), e); } - return catalogTable.location().toString(); } } diff --git a/kernel-spark/src/main/java/io/delta/kernel/spark/snapshot/DeltaSnapshotManagerFactory.java b/kernel-spark/src/main/java/io/delta/kernel/spark/snapshot/DeltaSnapshotManagerFactory.java index 75058e82d2c..4144dded9a9 100644 --- a/kernel-spark/src/main/java/io/delta/kernel/spark/snapshot/DeltaSnapshotManagerFactory.java +++ b/kernel-spark/src/main/java/io/delta/kernel/spark/snapshot/DeltaSnapshotManagerFactory.java @@ -17,6 +17,7 @@ import static java.util.Objects.requireNonNull; +import io.delta.kernel.spark.snapshot.uc.UnityCatalogManagedCommitClient; import io.delta.kernel.spark.utils.CatalogTableUtils; import java.util.Optional; import org.apache.hadoop.conf.Configuration; @@ -56,21 +57,16 @@ public final class DeltaSnapshotManagerFactory { // Utility class - no instances private DeltaSnapshotManagerFactory() {} - /** - * Creates the appropriate snapshot manager for a Delta table. - * + /** + * Creates the appropriate snapshot manager for a Delta table. + * *

Selection logic: * *

    - *
  • If {@code catalogTable} is present and Unity Catalog managed → {@link - * CatalogManagedSnapshotManager} + *
  • If {@code catalogTable} is present and UC-managed → {@link CatalogManagedSnapshotManager} *
  • Otherwise → {@link PathBasedSnapshotManager} *
* - *

Unity Catalog managed tables are identified by checking for catalog-managed feature flags - * and the presence of a Unity Catalog table ID. See {@link - * CatalogTableUtils#isUnityCatalogManagedTable(CatalogTable)} for details. - * * @param tablePath filesystem path to the Delta table root * @param catalogTable optional Spark catalog table metadata * @param spark SparkSession for resolving Unity Catalog configurations @@ -90,10 +86,10 @@ public static DeltaSnapshotManager create( requireNonNull(spark, "spark is null"); requireNonNull(hadoopConf, "hadoopConf is null"); - // Check if table is Unity Catalog managed - if (catalogTable.isPresent() - && CatalogTableUtils.isUnityCatalogManagedTable(catalogTable.get())) { - return new CatalogManagedSnapshotManager(catalogTable.get(), spark, hadoopConf); + if (catalogTable.isPresent() && CatalogTableUtils.isUnityCatalogManagedTable(catalogTable.get())) { + ManagedCommitClient client = + UnityCatalogManagedCommitClient.fromCatalog(catalogTable.get(), spark); + return new CatalogManagedSnapshotManager(client, hadoopConf); } // Default to path-based snapshot manager diff --git a/kernel-spark/src/main/java/io/delta/kernel/spark/snapshot/ManagedCommitClient.java b/kernel-spark/src/main/java/io/delta/kernel/spark/snapshot/ManagedCommitClient.java new file mode 100644 index 00000000000..e44cb57ddf7 --- /dev/null +++ b/kernel-spark/src/main/java/io/delta/kernel/spark/snapshot/ManagedCommitClient.java @@ -0,0 +1,47 @@ +/* + * Copyright (2025) The Delta Lake Project Authors. + * + * Licensed 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 io.delta.kernel.spark.snapshot; + +import io.delta.kernel.CommitRange; +import io.delta.kernel.Snapshot; +import io.delta.kernel.engine.Engine; +import java.util.Optional; + +/** + * Catalog-managed commit client that knows how to load snapshots and commit ranges for a specific + * table. + */ +public interface ManagedCommitClient extends AutoCloseable { + + /** @return catalog-managed table identifier (for logging/telemetry). */ + String getTableId(); + + /** @return physical table path used by Delta Kernel. */ + String getTablePath(); + + Snapshot loadSnapshot( + Engine engine, Optional versionOpt, Optional timestampOpt); + + CommitRange loadCommitRange( + Engine engine, + Optional startVersionOpt, + Optional startTimestampOpt, + Optional endVersionOpt, + Optional endTimestampOpt); + + @Override + void close(); +} diff --git a/kernel-spark/src/main/java/io/delta/kernel/spark/snapshot/uc/UnityCatalogManagedCommitClient.java b/kernel-spark/src/main/java/io/delta/kernel/spark/snapshot/uc/UnityCatalogManagedCommitClient.java new file mode 100644 index 00000000000..f87a89bc9a2 --- /dev/null +++ b/kernel-spark/src/main/java/io/delta/kernel/spark/snapshot/uc/UnityCatalogManagedCommitClient.java @@ -0,0 +1,150 @@ +/* + * Copyright (2025) The Delta Lake Project Authors. + * + * Licensed 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 io.delta.kernel.spark.snapshot.uc; + +import static java.util.Objects.requireNonNull; + +import io.delta.kernel.CommitRange; +import io.delta.kernel.Snapshot; +import io.delta.kernel.engine.Engine; +import io.delta.kernel.spark.snapshot.ManagedCommitClient; +import io.delta.kernel.unitycatalog.UCCatalogManagedClient; +import io.delta.storage.commit.uccommitcoordinator.UCClient; +import io.delta.storage.commit.uccommitcoordinator.UCTokenBasedRestClient; +import java.util.Optional; +import org.apache.spark.sql.SparkSession; +import org.apache.spark.sql.catalyst.catalog.CatalogTable; + +/** UC-backed implementation of {@link ManagedCommitClient}. */ +public final class UnityCatalogManagedCommitClient implements ManagedCommitClient { + + private final String tableId; + private final String tablePath; + private final UCClient ucClient; + private final UCCatalogManagedClient ucManagedClient; + + public UnityCatalogManagedCommitClient( + String tableId, String tablePath, UCClient ucClient) { + this.tableId = requireNonNull(tableId, "tableId is null"); + this.tablePath = requireNonNull(tablePath, "tablePath is null"); + this.ucClient = requireNonNull(ucClient, "ucClient is null"); + this.ucManagedClient = new UCCatalogManagedClient(ucClient); + } + + public static UnityCatalogManagedCommitClient fromCatalog( + CatalogTable catalogTable, SparkSession spark) { + requireNonNull(catalogTable, "catalogTable is null"); + requireNonNull(spark, "spark is null"); + + String tableId = extractUCTableId(catalogTable); + String tablePath = extractTablePath(catalogTable); + UCClient client = createUCClient(catalogTable, spark); + return new UnityCatalogManagedCommitClient(tableId, tablePath, client); + } + + @Override + public String getTableId() { + return tableId; + } + + @Override + public String getTablePath() { + return tablePath; + } + + @Override + public Snapshot loadSnapshot( + Engine engine, Optional versionOpt, Optional timestampOpt) { + return ucManagedClient.loadSnapshot(engine, tableId, tablePath, versionOpt, timestampOpt); + } + + @Override + public CommitRange loadCommitRange( + Engine engine, + Optional startVersionOpt, + Optional startTimestampOpt, + Optional endVersionOpt, + Optional endTimestampOpt) { + return ucManagedClient.loadCommitRange( + engine, + tableId, + tablePath, + startVersionOpt, + startTimestampOpt, + endVersionOpt, + endTimestampOpt); + } + + @Override + public void close() { + try { + ucClient.close(); + } catch (Exception e) { + // Swallow close errors to avoid disrupting caller cleanup + } + } + + private static String extractUCTableId(CatalogTable catalogTable) { + java.util.Map storageProperties = + scala.collection.JavaConverters.mapAsJavaMap(catalogTable.storage().properties()); + + String ucTableId = storageProperties.get(io.delta.storage.commit.uccommitcoordinator + .UCCommitCoordinatorClient.UC_TABLE_ID_KEY); + if (ucTableId == null || ucTableId.isEmpty()) { + throw new IllegalArgumentException( + "Cannot extract ucTableId from table " + catalogTable.identifier()); + } + return ucTableId; + } + + private static String extractTablePath(CatalogTable catalogTable) { + if (catalogTable.location() == null) { + throw new IllegalArgumentException( + "Cannot extract table path: location is null for table " + catalogTable.identifier()); + } + return catalogTable.location().toString(); + } + + private static UCClient createUCClient(CatalogTable catalogTable, SparkSession spark) { + scala.Option catalogOption = catalogTable.identifier().catalog(); + String catalogName = + catalogOption.isDefined() + ? catalogOption.get() + : spark.sessionState().catalogManager().currentCatalog().name(); + + scala.collection.immutable.List> scalaConfigs = + org.apache.spark.sql.delta.coordinatedcommits.UCCommitCoordinatorBuilder$.MODULE$ + .getCatalogConfigs(spark); + + Optional> configTuple = + scala.jdk.javaapi.CollectionConverters.asJava(scalaConfigs).stream() + .filter(tuple -> tuple._1().equals(catalogName)) + .findFirst(); + + if (!configTuple.isPresent()) { + throw new IllegalArgumentException( + "Cannot create UC client: Unity Catalog configuration not found for catalog '" + + catalogName + + "'."); + } + + scala.Tuple3 config = configTuple.get(); + String uri = config._2(); + String token = config._3(); + + return new UCTokenBasedRestClient(uri, token); + } +} diff --git a/kernel-spark/src/test/java/io/delta/kernel/spark/snapshot/CatalogManagedSnapshotManagerTest.java b/kernel-spark/src/test/java/io/delta/kernel/spark/snapshot/CatalogManagedSnapshotManagerTest.java index 863c7dc0e81..a2913160109 100644 --- a/kernel-spark/src/test/java/io/delta/kernel/spark/snapshot/CatalogManagedSnapshotManagerTest.java +++ b/kernel-spark/src/test/java/io/delta/kernel/spark/snapshot/CatalogManagedSnapshotManagerTest.java @@ -17,69 +17,58 @@ import static org.junit.jupiter.api.Assertions.assertThrows; -import io.delta.kernel.spark.utils.CatalogTableTestUtils$; -import java.util.Collections; -import java.util.Map; import org.apache.hadoop.conf.Configuration; -import org.apache.spark.sql.catalyst.catalog.CatalogTable; import org.junit.jupiter.api.Test; /** Tests for {@link CatalogManagedSnapshotManager}. */ class CatalogManagedSnapshotManagerTest { @Test - void testConstructor_NullCatalogTable_ThrowsException() { + void testConstructor_NullHadoopConf_ThrowsException() { assertThrows( NullPointerException.class, - () -> new CatalogManagedSnapshotManager(null, null, new Configuration()), - "Null catalogTable should throw NullPointerException"); + () -> new CatalogManagedSnapshotManager(new NoOpClient(), null), + "Null hadoopConf should throw NullPointerException"); } @Test - void testConstructor_NullSpark_ThrowsException() { - CatalogTable table = catalogTable(Collections.emptyMap(), Collections.emptyMap()); - + void testConstructor_NullClient_ThrowsException() { assertThrows( NullPointerException.class, - () -> new CatalogManagedSnapshotManager(table, null, new Configuration()), - "Null spark should throw NullPointerException"); + () -> new CatalogManagedSnapshotManager(null, new Configuration()), + "Null commitClient should throw NullPointerException"); } - @Test - void testConstructor_NullHadoopConf_ThrowsException() { - CatalogTable table = catalogTable(Collections.emptyMap(), Collections.emptyMap()); + private static final class NoOpClient implements ManagedCommitClient { + @Override + public String getTableId() { + return "dummy"; + } - assertThrows( - NullPointerException.class, - () -> new CatalogManagedSnapshotManager(table, null, null), - "Null hadoopConf should throw NullPointerException"); - } + @Override + public String getTablePath() { + return "/tmp/dummy"; + } + + @Override + public io.delta.kernel.Snapshot loadSnapshot( + io.delta.kernel.engine.Engine engine, + java.util.Optional versionOpt, + java.util.Optional timestampOpt) { + throw new UnsupportedOperationException("noop"); + } - // Note: Additional constructor validation tests require integration test setup. - // The following tests cannot be implemented as unit tests because they require - // a real SparkSession to properly test UC table validation logic: - // - // - testConstructor_NonUCTable_ThrowsException: Verify non-UC table rejection - // - testConstructor_CatalogManagedWithoutUCTableId_ThrowsException: Verify UC table ID - // requirement - // - // These validations happen after null checks, so passing null SparkSession causes - // NullPointerException before reaching the UC validation logic. Cannot mock SparkSession - // effectively for these tests. - // - // Note: Full integration tests for CatalogManagedSnapshotManager require: - // 1. Real SparkSession with Unity Catalog configured - // 2. Unity Catalog tables with proper feature flags and table IDs - // 3. Unity Catalog endpoint available for snapshot loading - // - // Such tests should be added when integration test infrastructure is available: - // - testConstructor_ValidUCTable_Success: Happy path with valid UC table and config - // - testLoadLatestSnapshot_Success: Test snapshot loading from UC - // - testLoadSnapshotAt_Success: Test version-specific snapshot loading - // - testClose_Success: Test UC client cleanup + @Override + public io.delta.kernel.CommitRange loadCommitRange( + io.delta.kernel.engine.Engine engine, + java.util.Optional startVersionOpt, + java.util.Optional startTimestampOpt, + java.util.Optional endVersionOpt, + java.util.Optional endTimestampOpt) { + throw new UnsupportedOperationException("noop"); + } - private static CatalogTable catalogTable( - Map properties, Map storageProperties) { - return CatalogTableTestUtils$.MODULE$.catalogTableWithProperties(properties, storageProperties); + @Override + public void close() {} } } From ec45077bb2205b0cead8e3746ea37718a2951f98 Mon Sep 17 00:00:00 2001 From: TimothyW553 Date: Mon, 24 Nov 2025 23:38:29 +0000 Subject: [PATCH 35/42] clean up --- .../uc/UnityCatalogManagedCommitClient.java | 61 +++++++++++++++---- 1 file changed, 49 insertions(+), 12 deletions(-) diff --git a/kernel-spark/src/main/java/io/delta/kernel/spark/snapshot/uc/UnityCatalogManagedCommitClient.java b/kernel-spark/src/main/java/io/delta/kernel/spark/snapshot/uc/UnityCatalogManagedCommitClient.java index f87a89bc9a2..28e49241322 100644 --- a/kernel-spark/src/main/java/io/delta/kernel/spark/snapshot/uc/UnityCatalogManagedCommitClient.java +++ b/kernel-spark/src/main/java/io/delta/kernel/spark/snapshot/uc/UnityCatalogManagedCommitClient.java @@ -23,10 +23,12 @@ import io.delta.kernel.spark.snapshot.ManagedCommitClient; import io.delta.kernel.unitycatalog.UCCatalogManagedClient; import io.delta.storage.commit.uccommitcoordinator.UCClient; -import io.delta.storage.commit.uccommitcoordinator.UCTokenBasedRestClient; +import java.util.HashMap; +import java.util.Map; import java.util.Optional; import org.apache.spark.sql.SparkSession; import org.apache.spark.sql.catalyst.catalog.CatalogTable; +import org.apache.spark.sql.delta.coordinatedcommits.UCTokenBasedRestClientFactory$; /** UC-backed implementation of {@link ManagedCommitClient}. */ public final class UnityCatalogManagedCommitClient implements ManagedCommitClient { @@ -119,19 +121,43 @@ private static String extractTablePath(CatalogTable catalogTable) { } private static UCClient createUCClient(CatalogTable catalogTable, SparkSession spark) { + final String SPARK_SQL_CATALOG_PREFIX = "spark.sql.catalog."; + final String UC_CONNECTOR_CLASS = "io.unitycatalog.spark.UCSingleCatalog"; + scala.Option catalogOption = catalogTable.identifier().catalog(); String catalogName = catalogOption.isDefined() ? catalogOption.get() : spark.sessionState().catalogManager().currentCatalog().name(); - scala.collection.immutable.List> scalaConfigs = - org.apache.spark.sql.delta.coordinatedcommits.UCCommitCoordinatorBuilder$.MODULE$ - .getCatalogConfigs(spark); - - Optional> configTuple = - scala.jdk.javaapi.CollectionConverters.asJava(scalaConfigs).stream() - .filter(tuple -> tuple._1().equals(catalogName)) + Map entries = new HashMap<>(); + spark.conf() + .getAll() + .forEach( + (k, v) -> { + if (!k.startsWith(SPARK_SQL_CATALOG_PREFIX)) { + return; + } + String remainder = k.substring(SPARK_SQL_CATALOG_PREFIX.length()); + int dotIdx = remainder.indexOf('.'); + String name = dotIdx == -1 ? remainder : remainder.substring(0, dotIdx); + String keySuffix = dotIdx == -1 ? "" : remainder.substring(dotIdx + 1); + CatalogEntry entry = entries.computeIfAbsent(name, CatalogEntry::new); + if (keySuffix.isEmpty()) { + entry.connectorClass = v; + } else if ("uri".equals(keySuffix)) { + entry.uri = v; + } else if ("token".equals(keySuffix)) { + entry.token = v; + } + }); + + Optional configTuple = + entries.values().stream() + .filter(e -> catalogName.equals(e.name)) + .filter(e -> UC_CONNECTOR_CLASS.equals(e.connectorClass)) + .filter(e -> e.uri != null && !e.uri.isEmpty()) + .filter(e -> e.token != null && !e.token.isEmpty()) .findFirst(); if (!configTuple.isPresent()) { @@ -141,10 +167,21 @@ private static UCClient createUCClient(CatalogTable catalogTable, SparkSession s + "'."); } - scala.Tuple3 config = configTuple.get(); - String uri = config._2(); - String token = config._3(); + CatalogEntry config = configTuple.get(); + String uri = config.uri; + String token = config.token; + + return UCTokenBasedRestClientFactory$.MODULE$.createUCClient(uri, token); + } + + private static final class CatalogEntry { + final String name; + String connectorClass; + String uri; + String token; - return new UCTokenBasedRestClient(uri, token); + CatalogEntry(String name) { + this.name = name; + } } } From d6d604ed9aa5b1baa90e79cc2a2c0a0472a9ed06 Mon Sep 17 00:00:00 2001 From: TimothyW553 Date: Mon, 24 Nov 2025 23:38:54 +0000 Subject: [PATCH 36/42] fmt --- .../spark/snapshot/CatalogManagedSnapshotManager.java | 6 +++--- .../spark/snapshot/DeltaSnapshotManagerFactory.java | 9 +++++---- .../kernel/spark/snapshot/ManagedCommitClient.java | 3 +-- .../snapshot/uc/UnityCatalogManagedCommitClient.java | 11 ++++++----- 4 files changed, 15 insertions(+), 14 deletions(-) diff --git a/kernel-spark/src/main/java/io/delta/kernel/spark/snapshot/CatalogManagedSnapshotManager.java b/kernel-spark/src/main/java/io/delta/kernel/spark/snapshot/CatalogManagedSnapshotManager.java index 6d3dfc80431..9ef166cdc42 100644 --- a/kernel-spark/src/main/java/io/delta/kernel/spark/snapshot/CatalogManagedSnapshotManager.java +++ b/kernel-spark/src/main/java/io/delta/kernel/spark/snapshot/CatalogManagedSnapshotManager.java @@ -71,8 +71,7 @@ public Snapshot loadLatestSnapshot() { @Override public Snapshot loadSnapshotAt(long version) { checkArgument(version >= 0, "version must be non-negative"); - return commitClient.loadSnapshot( - kernelEngine, Optional.of(version), Optional.empty()); + return commitClient.loadSnapshot(kernelEngine, Optional.of(version), Optional.empty()); } /** @@ -154,7 +153,8 @@ public void close() { commitClient.close(); logger.info("Closed CatalogManagedSnapshotManager for table {}", commitClient.getTableId()); } catch (Exception e) { - logger.warn("Error closing catalog-managed client for table {}", commitClient.getTableId(), e); + logger.warn( + "Error closing catalog-managed client for table {}", commitClient.getTableId(), e); } } } diff --git a/kernel-spark/src/main/java/io/delta/kernel/spark/snapshot/DeltaSnapshotManagerFactory.java b/kernel-spark/src/main/java/io/delta/kernel/spark/snapshot/DeltaSnapshotManagerFactory.java index 4144dded9a9..f7f958e2a93 100644 --- a/kernel-spark/src/main/java/io/delta/kernel/spark/snapshot/DeltaSnapshotManagerFactory.java +++ b/kernel-spark/src/main/java/io/delta/kernel/spark/snapshot/DeltaSnapshotManagerFactory.java @@ -57,9 +57,9 @@ public final class DeltaSnapshotManagerFactory { // Utility class - no instances private DeltaSnapshotManagerFactory() {} - /** - * Creates the appropriate snapshot manager for a Delta table. - * + /** + * Creates the appropriate snapshot manager for a Delta table. + * *

Selection logic: * *

    @@ -86,7 +86,8 @@ public static DeltaSnapshotManager create( requireNonNull(spark, "spark is null"); requireNonNull(hadoopConf, "hadoopConf is null"); - if (catalogTable.isPresent() && CatalogTableUtils.isUnityCatalogManagedTable(catalogTable.get())) { + if (catalogTable.isPresent() + && CatalogTableUtils.isUnityCatalogManagedTable(catalogTable.get())) { ManagedCommitClient client = UnityCatalogManagedCommitClient.fromCatalog(catalogTable.get(), spark); return new CatalogManagedSnapshotManager(client, hadoopConf); diff --git a/kernel-spark/src/main/java/io/delta/kernel/spark/snapshot/ManagedCommitClient.java b/kernel-spark/src/main/java/io/delta/kernel/spark/snapshot/ManagedCommitClient.java index e44cb57ddf7..ad2088b79ff 100644 --- a/kernel-spark/src/main/java/io/delta/kernel/spark/snapshot/ManagedCommitClient.java +++ b/kernel-spark/src/main/java/io/delta/kernel/spark/snapshot/ManagedCommitClient.java @@ -32,8 +32,7 @@ public interface ManagedCommitClient extends AutoCloseable { /** @return physical table path used by Delta Kernel. */ String getTablePath(); - Snapshot loadSnapshot( - Engine engine, Optional versionOpt, Optional timestampOpt); + Snapshot loadSnapshot(Engine engine, Optional versionOpt, Optional timestampOpt); CommitRange loadCommitRange( Engine engine, diff --git a/kernel-spark/src/main/java/io/delta/kernel/spark/snapshot/uc/UnityCatalogManagedCommitClient.java b/kernel-spark/src/main/java/io/delta/kernel/spark/snapshot/uc/UnityCatalogManagedCommitClient.java index 28e49241322..51fd76502f8 100644 --- a/kernel-spark/src/main/java/io/delta/kernel/spark/snapshot/uc/UnityCatalogManagedCommitClient.java +++ b/kernel-spark/src/main/java/io/delta/kernel/spark/snapshot/uc/UnityCatalogManagedCommitClient.java @@ -38,8 +38,7 @@ public final class UnityCatalogManagedCommitClient implements ManagedCommitClien private final UCClient ucClient; private final UCCatalogManagedClient ucManagedClient; - public UnityCatalogManagedCommitClient( - String tableId, String tablePath, UCClient ucClient) { + public UnityCatalogManagedCommitClient(String tableId, String tablePath, UCClient ucClient) { this.tableId = requireNonNull(tableId, "tableId is null"); this.tablePath = requireNonNull(tablePath, "tablePath is null"); this.ucClient = requireNonNull(ucClient, "ucClient is null"); @@ -103,8 +102,9 @@ private static String extractUCTableId(CatalogTable catalogTable) { java.util.Map storageProperties = scala.collection.JavaConverters.mapAsJavaMap(catalogTable.storage().properties()); - String ucTableId = storageProperties.get(io.delta.storage.commit.uccommitcoordinator - .UCCommitCoordinatorClient.UC_TABLE_ID_KEY); + String ucTableId = + storageProperties.get( + io.delta.storage.commit.uccommitcoordinator.UCCommitCoordinatorClient.UC_TABLE_ID_KEY); if (ucTableId == null || ucTableId.isEmpty()) { throw new IllegalArgumentException( "Cannot extract ucTableId from table " + catalogTable.identifier()); @@ -131,7 +131,8 @@ private static UCClient createUCClient(CatalogTable catalogTable, SparkSession s : spark.sessionState().catalogManager().currentCatalog().name(); Map entries = new HashMap<>(); - spark.conf() + spark + .conf() .getAll() .forEach( (k, v) -> { From 5635ab6a98c14747e730d5501e32374454f7c1c9 Mon Sep 17 00:00:00 2001 From: TimothyW553 Date: Mon, 24 Nov 2025 23:59:20 +0000 Subject: [PATCH 37/42] refactor --- .../snapshot/DeltaSnapshotManagerFactory.java | 14 +-- .../uc/UnityCatalogManagedCommitClient.java | 103 ------------------ ...nityCatalogManagedCommitClientFactory.java | 99 +++++++++++++++++ 3 files changed, 106 insertions(+), 110 deletions(-) create mode 100644 kernel-spark/src/main/java/io/delta/kernel/spark/snapshot/uc/UnityCatalogManagedCommitClientFactory.java diff --git a/kernel-spark/src/main/java/io/delta/kernel/spark/snapshot/DeltaSnapshotManagerFactory.java b/kernel-spark/src/main/java/io/delta/kernel/spark/snapshot/DeltaSnapshotManagerFactory.java index f7f958e2a93..a887ceac3a2 100644 --- a/kernel-spark/src/main/java/io/delta/kernel/spark/snapshot/DeltaSnapshotManagerFactory.java +++ b/kernel-spark/src/main/java/io/delta/kernel/spark/snapshot/DeltaSnapshotManagerFactory.java @@ -17,8 +17,7 @@ import static java.util.Objects.requireNonNull; -import io.delta.kernel.spark.snapshot.uc.UnityCatalogManagedCommitClient; -import io.delta.kernel.spark.utils.CatalogTableUtils; +import io.delta.kernel.spark.snapshot.uc.UnityCatalogManagedCommitClientFactory; import java.util.Optional; import org.apache.hadoop.conf.Configuration; import org.apache.spark.annotation.Experimental; @@ -86,11 +85,12 @@ public static DeltaSnapshotManager create( requireNonNull(spark, "spark is null"); requireNonNull(hadoopConf, "hadoopConf is null"); - if (catalogTable.isPresent() - && CatalogTableUtils.isUnityCatalogManagedTable(catalogTable.get())) { - ManagedCommitClient client = - UnityCatalogManagedCommitClient.fromCatalog(catalogTable.get(), spark); - return new CatalogManagedSnapshotManager(client, hadoopConf); + if (catalogTable.isPresent()) { + Optional clientOpt = + UnityCatalogManagedCommitClientFactory.create(catalogTable.get(), spark); + if (clientOpt.isPresent()) { + return new CatalogManagedSnapshotManager(clientOpt.get(), hadoopConf); + } } // Default to path-based snapshot manager diff --git a/kernel-spark/src/main/java/io/delta/kernel/spark/snapshot/uc/UnityCatalogManagedCommitClient.java b/kernel-spark/src/main/java/io/delta/kernel/spark/snapshot/uc/UnityCatalogManagedCommitClient.java index 51fd76502f8..11e2c9ed756 100644 --- a/kernel-spark/src/main/java/io/delta/kernel/spark/snapshot/uc/UnityCatalogManagedCommitClient.java +++ b/kernel-spark/src/main/java/io/delta/kernel/spark/snapshot/uc/UnityCatalogManagedCommitClient.java @@ -23,12 +23,7 @@ import io.delta.kernel.spark.snapshot.ManagedCommitClient; import io.delta.kernel.unitycatalog.UCCatalogManagedClient; import io.delta.storage.commit.uccommitcoordinator.UCClient; -import java.util.HashMap; -import java.util.Map; import java.util.Optional; -import org.apache.spark.sql.SparkSession; -import org.apache.spark.sql.catalyst.catalog.CatalogTable; -import org.apache.spark.sql.delta.coordinatedcommits.UCTokenBasedRestClientFactory$; /** UC-backed implementation of {@link ManagedCommitClient}. */ public final class UnityCatalogManagedCommitClient implements ManagedCommitClient { @@ -45,17 +40,6 @@ public UnityCatalogManagedCommitClient(String tableId, String tablePath, UCClien this.ucManagedClient = new UCCatalogManagedClient(ucClient); } - public static UnityCatalogManagedCommitClient fromCatalog( - CatalogTable catalogTable, SparkSession spark) { - requireNonNull(catalogTable, "catalogTable is null"); - requireNonNull(spark, "spark is null"); - - String tableId = extractUCTableId(catalogTable); - String tablePath = extractTablePath(catalogTable); - UCClient client = createUCClient(catalogTable, spark); - return new UnityCatalogManagedCommitClient(tableId, tablePath, client); - } - @Override public String getTableId() { return tableId; @@ -98,91 +82,4 @@ public void close() { } } - private static String extractUCTableId(CatalogTable catalogTable) { - java.util.Map storageProperties = - scala.collection.JavaConverters.mapAsJavaMap(catalogTable.storage().properties()); - - String ucTableId = - storageProperties.get( - io.delta.storage.commit.uccommitcoordinator.UCCommitCoordinatorClient.UC_TABLE_ID_KEY); - if (ucTableId == null || ucTableId.isEmpty()) { - throw new IllegalArgumentException( - "Cannot extract ucTableId from table " + catalogTable.identifier()); - } - return ucTableId; - } - - private static String extractTablePath(CatalogTable catalogTable) { - if (catalogTable.location() == null) { - throw new IllegalArgumentException( - "Cannot extract table path: location is null for table " + catalogTable.identifier()); - } - return catalogTable.location().toString(); - } - - private static UCClient createUCClient(CatalogTable catalogTable, SparkSession spark) { - final String SPARK_SQL_CATALOG_PREFIX = "spark.sql.catalog."; - final String UC_CONNECTOR_CLASS = "io.unitycatalog.spark.UCSingleCatalog"; - - scala.Option catalogOption = catalogTable.identifier().catalog(); - String catalogName = - catalogOption.isDefined() - ? catalogOption.get() - : spark.sessionState().catalogManager().currentCatalog().name(); - - Map entries = new HashMap<>(); - spark - .conf() - .getAll() - .forEach( - (k, v) -> { - if (!k.startsWith(SPARK_SQL_CATALOG_PREFIX)) { - return; - } - String remainder = k.substring(SPARK_SQL_CATALOG_PREFIX.length()); - int dotIdx = remainder.indexOf('.'); - String name = dotIdx == -1 ? remainder : remainder.substring(0, dotIdx); - String keySuffix = dotIdx == -1 ? "" : remainder.substring(dotIdx + 1); - CatalogEntry entry = entries.computeIfAbsent(name, CatalogEntry::new); - if (keySuffix.isEmpty()) { - entry.connectorClass = v; - } else if ("uri".equals(keySuffix)) { - entry.uri = v; - } else if ("token".equals(keySuffix)) { - entry.token = v; - } - }); - - Optional configTuple = - entries.values().stream() - .filter(e -> catalogName.equals(e.name)) - .filter(e -> UC_CONNECTOR_CLASS.equals(e.connectorClass)) - .filter(e -> e.uri != null && !e.uri.isEmpty()) - .filter(e -> e.token != null && !e.token.isEmpty()) - .findFirst(); - - if (!configTuple.isPresent()) { - throw new IllegalArgumentException( - "Cannot create UC client: Unity Catalog configuration not found for catalog '" - + catalogName - + "'."); - } - - CatalogEntry config = configTuple.get(); - String uri = config.uri; - String token = config.token; - - return UCTokenBasedRestClientFactory$.MODULE$.createUCClient(uri, token); - } - - private static final class CatalogEntry { - final String name; - String connectorClass; - String uri; - String token; - - CatalogEntry(String name) { - this.name = name; - } - } } diff --git a/kernel-spark/src/main/java/io/delta/kernel/spark/snapshot/uc/UnityCatalogManagedCommitClientFactory.java b/kernel-spark/src/main/java/io/delta/kernel/spark/snapshot/uc/UnityCatalogManagedCommitClientFactory.java new file mode 100644 index 00000000000..20cb31233b2 --- /dev/null +++ b/kernel-spark/src/main/java/io/delta/kernel/spark/snapshot/uc/UnityCatalogManagedCommitClientFactory.java @@ -0,0 +1,99 @@ +/* + * Copyright (2025) The Delta Lake Project Authors. + * + * Licensed 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 io.delta.kernel.spark.snapshot.uc; + +import static java.util.Objects.requireNonNull; + +import io.delta.kernel.spark.snapshot.ManagedCommitClient; +import io.delta.kernel.spark.utils.CatalogTableUtils; +import io.delta.storage.commit.uccommitcoordinator.UCClient; +import java.util.Map; +import java.util.Optional; +import org.apache.spark.sql.SparkSession; +import org.apache.spark.sql.catalyst.catalog.CatalogTable; +import org.apache.spark.sql.delta.coordinatedcommits.UCCommitCoordinatorBuilder$; +import org.apache.spark.sql.delta.coordinatedcommits.UCTokenBasedRestClientFactory$; + +/** Factory for UC-backed {@link ManagedCommitClient} instances. */ +public final class UnityCatalogManagedCommitClientFactory { + + private UnityCatalogManagedCommitClientFactory() {} + + public static Optional create(CatalogTable catalogTable, SparkSession spark) { + requireNonNull(catalogTable, "catalogTable is null"); + requireNonNull(spark, "spark is null"); + + if (!CatalogTableUtils.isUnityCatalogManagedTable(catalogTable)) { + return Optional.empty(); + } + + String tableId = extractUCTableId(catalogTable); + String tablePath = extractTablePath(catalogTable); + UCClient client = createUCClient(catalogTable, spark); + return Optional.of(new UnityCatalogManagedCommitClient(tableId, tablePath, client)); + } + + private static String extractUCTableId(CatalogTable catalogTable) { + Map storageProperties = + scala.collection.JavaConverters.mapAsJavaMap(catalogTable.storage().properties()); + + String ucTableId = + storageProperties.get( + io.delta.storage.commit.uccommitcoordinator.UCCommitCoordinatorClient.UC_TABLE_ID_KEY); + if (ucTableId == null || ucTableId.isEmpty()) { + throw new IllegalArgumentException( + "Cannot extract ucTableId from table " + catalogTable.identifier()); + } + return ucTableId; + } + + private static String extractTablePath(CatalogTable catalogTable) { + if (catalogTable.location() == null) { + throw new IllegalArgumentException( + "Cannot extract table path: location is null for table " + catalogTable.identifier()); + } + return catalogTable.location().toString(); + } + + private static UCClient createUCClient(CatalogTable catalogTable, SparkSession spark) { + scala.Option catalogOption = catalogTable.identifier().catalog(); + String catalogName = + catalogOption.isDefined() + ? catalogOption.get() + : spark.sessionState().catalogManager().currentCatalog().name(); + + scala.collection.immutable.List> scalaConfigs = + UCCommitCoordinatorBuilder$.MODULE$.getCatalogConfigs(spark); + + Optional> configTuple = + scala.jdk.javaapi.CollectionConverters.asJava(scalaConfigs).stream() + .filter(tuple -> tuple._1().equals(catalogName)) + .findFirst(); + + if (!configTuple.isPresent()) { + throw new IllegalArgumentException( + "Cannot create UC client: Unity Catalog configuration not found for catalog '" + + catalogName + + "'."); + } + + scala.Tuple3 config = configTuple.get(); + String uri = config._2(); + String token = config._3(); + + return UCTokenBasedRestClientFactory$.MODULE$.createUCClient(uri, token); + } +} From 2d056e50ec36290a9546a8465d62d04a7165f1a3 Mon Sep 17 00:00:00 2001 From: TimothyW553 Date: Tue, 25 Nov 2025 00:40:14 +0000 Subject: [PATCH 38/42] cleaner --- .../snapshot/DeltaSnapshotManagerFactory.java | 4 +- .../uc/UnityCatalogManagedCommitClient.java | 85 ------------------- .../UnityCatalogManagedCommitClient.java} | 73 ++++++++++++++-- 3 files changed, 70 insertions(+), 92 deletions(-) delete mode 100644 kernel-spark/src/main/java/io/delta/kernel/spark/snapshot/uc/UnityCatalogManagedCommitClient.java rename kernel-spark/src/main/java/io/delta/kernel/spark/snapshot/{uc/UnityCatalogManagedCommitClientFactory.java => unitycatalog/UnityCatalogManagedCommitClient.java} (62%) diff --git a/kernel-spark/src/main/java/io/delta/kernel/spark/snapshot/DeltaSnapshotManagerFactory.java b/kernel-spark/src/main/java/io/delta/kernel/spark/snapshot/DeltaSnapshotManagerFactory.java index a887ceac3a2..bb86b7fc5ea 100644 --- a/kernel-spark/src/main/java/io/delta/kernel/spark/snapshot/DeltaSnapshotManagerFactory.java +++ b/kernel-spark/src/main/java/io/delta/kernel/spark/snapshot/DeltaSnapshotManagerFactory.java @@ -17,7 +17,7 @@ import static java.util.Objects.requireNonNull; -import io.delta.kernel.spark.snapshot.uc.UnityCatalogManagedCommitClientFactory; +import io.delta.kernel.spark.snapshot.unitycatalog.UnityCatalogManagedCommitClient; import java.util.Optional; import org.apache.hadoop.conf.Configuration; import org.apache.spark.annotation.Experimental; @@ -87,7 +87,7 @@ public static DeltaSnapshotManager create( if (catalogTable.isPresent()) { Optional clientOpt = - UnityCatalogManagedCommitClientFactory.create(catalogTable.get(), spark); + UnityCatalogManagedCommitClient.fromCatalog(catalogTable.get(), spark); if (clientOpt.isPresent()) { return new CatalogManagedSnapshotManager(clientOpt.get(), hadoopConf); } diff --git a/kernel-spark/src/main/java/io/delta/kernel/spark/snapshot/uc/UnityCatalogManagedCommitClient.java b/kernel-spark/src/main/java/io/delta/kernel/spark/snapshot/uc/UnityCatalogManagedCommitClient.java deleted file mode 100644 index 11e2c9ed756..00000000000 --- a/kernel-spark/src/main/java/io/delta/kernel/spark/snapshot/uc/UnityCatalogManagedCommitClient.java +++ /dev/null @@ -1,85 +0,0 @@ -/* - * Copyright (2025) The Delta Lake Project Authors. - * - * Licensed 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 io.delta.kernel.spark.snapshot.uc; - -import static java.util.Objects.requireNonNull; - -import io.delta.kernel.CommitRange; -import io.delta.kernel.Snapshot; -import io.delta.kernel.engine.Engine; -import io.delta.kernel.spark.snapshot.ManagedCommitClient; -import io.delta.kernel.unitycatalog.UCCatalogManagedClient; -import io.delta.storage.commit.uccommitcoordinator.UCClient; -import java.util.Optional; - -/** UC-backed implementation of {@link ManagedCommitClient}. */ -public final class UnityCatalogManagedCommitClient implements ManagedCommitClient { - - private final String tableId; - private final String tablePath; - private final UCClient ucClient; - private final UCCatalogManagedClient ucManagedClient; - - public UnityCatalogManagedCommitClient(String tableId, String tablePath, UCClient ucClient) { - this.tableId = requireNonNull(tableId, "tableId is null"); - this.tablePath = requireNonNull(tablePath, "tablePath is null"); - this.ucClient = requireNonNull(ucClient, "ucClient is null"); - this.ucManagedClient = new UCCatalogManagedClient(ucClient); - } - - @Override - public String getTableId() { - return tableId; - } - - @Override - public String getTablePath() { - return tablePath; - } - - @Override - public Snapshot loadSnapshot( - Engine engine, Optional versionOpt, Optional timestampOpt) { - return ucManagedClient.loadSnapshot(engine, tableId, tablePath, versionOpt, timestampOpt); - } - - @Override - public CommitRange loadCommitRange( - Engine engine, - Optional startVersionOpt, - Optional startTimestampOpt, - Optional endVersionOpt, - Optional endTimestampOpt) { - return ucManagedClient.loadCommitRange( - engine, - tableId, - tablePath, - startVersionOpt, - startTimestampOpt, - endVersionOpt, - endTimestampOpt); - } - - @Override - public void close() { - try { - ucClient.close(); - } catch (Exception e) { - // Swallow close errors to avoid disrupting caller cleanup - } - } - -} diff --git a/kernel-spark/src/main/java/io/delta/kernel/spark/snapshot/uc/UnityCatalogManagedCommitClientFactory.java b/kernel-spark/src/main/java/io/delta/kernel/spark/snapshot/unitycatalog/UnityCatalogManagedCommitClient.java similarity index 62% rename from kernel-spark/src/main/java/io/delta/kernel/spark/snapshot/uc/UnityCatalogManagedCommitClientFactory.java rename to kernel-spark/src/main/java/io/delta/kernel/spark/snapshot/unitycatalog/UnityCatalogManagedCommitClient.java index 20cb31233b2..ca1868f6112 100644 --- a/kernel-spark/src/main/java/io/delta/kernel/spark/snapshot/uc/UnityCatalogManagedCommitClientFactory.java +++ b/kernel-spark/src/main/java/io/delta/kernel/spark/snapshot/unitycatalog/UnityCatalogManagedCommitClient.java @@ -13,12 +13,16 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -package io.delta.kernel.spark.snapshot.uc; +package io.delta.kernel.spark.snapshot.unitycatalog; import static java.util.Objects.requireNonNull; +import io.delta.kernel.CommitRange; +import io.delta.kernel.Snapshot; +import io.delta.kernel.engine.Engine; import io.delta.kernel.spark.snapshot.ManagedCommitClient; import io.delta.kernel.spark.utils.CatalogTableUtils; +import io.delta.kernel.unitycatalog.UCCatalogManagedClient; import io.delta.storage.commit.uccommitcoordinator.UCClient; import java.util.Map; import java.util.Optional; @@ -27,12 +31,29 @@ import org.apache.spark.sql.delta.coordinatedcommits.UCCommitCoordinatorBuilder$; import org.apache.spark.sql.delta.coordinatedcommits.UCTokenBasedRestClientFactory$; -/** Factory for UC-backed {@link ManagedCommitClient} instances. */ -public final class UnityCatalogManagedCommitClientFactory { +/** UC-backed implementation of {@link ManagedCommitClient}. */ +public final class UnityCatalogManagedCommitClient implements ManagedCommitClient { - private UnityCatalogManagedCommitClientFactory() {} + private final String tableId; + private final String tablePath; + private final UCClient ucClient; + private final UCCatalogManagedClient ucManagedClient; - public static Optional create(CatalogTable catalogTable, SparkSession spark) { + public UnityCatalogManagedCommitClient(String tableId, String tablePath, UCClient ucClient) { + this.tableId = requireNonNull(tableId, "tableId is null"); + this.tablePath = requireNonNull(tablePath, "tablePath is null"); + this.ucClient = requireNonNull(ucClient, "ucClient is null"); + this.ucManagedClient = new UCCatalogManagedClient(ucClient); + } + + /** + * Builds a UC-backed {@link ManagedCommitClient} for a UC-managed table. + * + * @throws IllegalArgumentException if the table lacks UC identifiers or catalog config is + * missing + */ + public static Optional fromCatalog( + CatalogTable catalogTable, SparkSession spark) { requireNonNull(catalogTable, "catalogTable is null"); requireNonNull(spark, "spark is null"); @@ -46,6 +67,48 @@ public static Optional create(CatalogTable catalogTable, Sp return Optional.of(new UnityCatalogManagedCommitClient(tableId, tablePath, client)); } + @Override + public String getTableId() { + return tableId; + } + + @Override + public String getTablePath() { + return tablePath; + } + + @Override + public Snapshot loadSnapshot( + Engine engine, Optional versionOpt, Optional timestampOpt) { + return ucManagedClient.loadSnapshot(engine, tableId, tablePath, versionOpt, timestampOpt); + } + + @Override + public CommitRange loadCommitRange( + Engine engine, + Optional startVersionOpt, + Optional startTimestampOpt, + Optional endVersionOpt, + Optional endTimestampOpt) { + return ucManagedClient.loadCommitRange( + engine, + tableId, + tablePath, + startVersionOpt, + startTimestampOpt, + endVersionOpt, + endTimestampOpt); + } + + @Override + public void close() { + try { + ucClient.close(); + } catch (Exception e) { + // Swallow close errors to avoid disrupting caller cleanup + } + } + private static String extractUCTableId(CatalogTable catalogTable) { Map storageProperties = scala.collection.JavaConverters.mapAsJavaMap(catalogTable.storage().properties()); From 4a1d8fa93e56d68b5971fb32970bdeaa5799abdc Mon Sep 17 00:00:00 2001 From: TimothyW553 Date: Tue, 25 Nov 2025 00:42:10 +0000 Subject: [PATCH 39/42] fix comment --- .../src/main/java/io/delta/kernel/spark/catalog/SparkTable.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/kernel-spark/src/main/java/io/delta/kernel/spark/catalog/SparkTable.java b/kernel-spark/src/main/java/io/delta/kernel/spark/catalog/SparkTable.java index cb831d99dcd..afe8abd98ec 100644 --- a/kernel-spark/src/main/java/io/delta/kernel/spark/catalog/SparkTable.java +++ b/kernel-spark/src/main/java/io/delta/kernel/spark/catalog/SparkTable.java @@ -140,7 +140,7 @@ private SparkTable( SparkSession spark = SparkSession.active(); this.hadoopConf = spark.sessionState().newHadoopConfWithOptions(toScalaMap(options)); - // Use factory to create appropriate snapshot manager (UC-managed vs path-based) + // Use factory to create appropriate snapshot manager (catalog-managed vs path-based) this.snapshotManager = DeltaSnapshotManagerFactory.create(tablePath, catalogTable, spark, hadoopConf); // Load the initial snapshot through the manager From 2f792d81fe71410096186b7a9d8c0c7d49bad08b Mon Sep 17 00:00:00 2001 From: TimothyW553 Date: Tue, 25 Nov 2025 19:14:23 +0000 Subject: [PATCH 40/42] fmt --- .../snapshot/unitycatalog/UnityCatalogManagedCommitClient.java | 3 +-- .../org/apache/spark/sql/delta/sources/DeltaSQLConfV2.scala | 3 +-- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/kernel-spark/src/main/java/io/delta/kernel/spark/snapshot/unitycatalog/UnityCatalogManagedCommitClient.java b/kernel-spark/src/main/java/io/delta/kernel/spark/snapshot/unitycatalog/UnityCatalogManagedCommitClient.java index ca1868f6112..5f3c1b55003 100644 --- a/kernel-spark/src/main/java/io/delta/kernel/spark/snapshot/unitycatalog/UnityCatalogManagedCommitClient.java +++ b/kernel-spark/src/main/java/io/delta/kernel/spark/snapshot/unitycatalog/UnityCatalogManagedCommitClient.java @@ -49,8 +49,7 @@ public UnityCatalogManagedCommitClient(String tableId, String tablePath, UCClien /** * Builds a UC-backed {@link ManagedCommitClient} for a UC-managed table. * - * @throws IllegalArgumentException if the table lacks UC identifiers or catalog config is - * missing + * @throws IllegalArgumentException if the table lacks UC identifiers or catalog config is missing */ public static Optional fromCatalog( CatalogTable catalogTable, SparkSession spark) { diff --git a/kernel-spark/src/main/scala/org/apache/spark/sql/delta/sources/DeltaSQLConfV2.scala b/kernel-spark/src/main/scala/org/apache/spark/sql/delta/sources/DeltaSQLConfV2.scala index f42f1a44048..e2f8aac7459 100644 --- a/kernel-spark/src/main/scala/org/apache/spark/sql/delta/sources/DeltaSQLConfV2.scala +++ b/kernel-spark/src/main/scala/org/apache/spark/sql/delta/sources/DeltaSQLConfV2.scala @@ -39,9 +39,8 @@ object DeltaSQLConfV2 extends DeltaSQLConfUtils { buildConf("v2.enableMode") .doc( "Controls the Delta V2 connector enable mode. " + - "Valid values: NONE (disabled, default), STRICT (should ONLY be enabled for testing).") + "Valid values: NONE (disabled, default), STRICT (should ONLY be enabled for testing).") .stringConf .checkValues(Set("NONE", "STRICT")) .createWithDefault("NONE") } - From 82c38377b8480d3cb69a1f670d1fc38864bd3b70 Mon Sep 17 00:00:00 2001 From: TimothyW553 Date: Tue, 25 Nov 2025 19:38:56 +0000 Subject: [PATCH 41/42] Remove wiring from catalog-config base --- .../java/io/delta/kernel/spark/catalog/SparkTable.java | 10 ++++------ .../spark/sql/delta/sources/DeltaSQLConfV2.scala | 3 ++- 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/kernel-spark/src/main/java/io/delta/kernel/spark/catalog/SparkTable.java b/kernel-spark/src/main/java/io/delta/kernel/spark/catalog/SparkTable.java index afe8abd98ec..bef04d4f1f7 100644 --- a/kernel-spark/src/main/java/io/delta/kernel/spark/catalog/SparkTable.java +++ b/kernel-spark/src/main/java/io/delta/kernel/spark/catalog/SparkTable.java @@ -21,7 +21,7 @@ import io.delta.kernel.Snapshot; import io.delta.kernel.spark.read.SparkScanBuilder; import io.delta.kernel.spark.snapshot.DeltaSnapshotManager; -import io.delta.kernel.spark.snapshot.DeltaSnapshotManagerFactory; +import io.delta.kernel.spark.snapshot.PathBasedSnapshotManager; import io.delta.kernel.spark.utils.SchemaUtils; import java.util.*; import org.apache.hadoop.conf.Configuration; @@ -138,11 +138,9 @@ private SparkTable( merged.putAll(userOptions); this.options = Collections.unmodifiableMap(merged); - SparkSession spark = SparkSession.active(); - this.hadoopConf = spark.sessionState().newHadoopConfWithOptions(toScalaMap(options)); - // Use factory to create appropriate snapshot manager (catalog-managed vs path-based) - this.snapshotManager = - DeltaSnapshotManagerFactory.create(tablePath, catalogTable, spark, hadoopConf); + this.hadoopConf = + SparkSession.active().sessionState().newHadoopConfWithOptions(toScalaMap(options)); + this.snapshotManager = new PathBasedSnapshotManager(tablePath, hadoopConf); // Load the initial snapshot through the manager this.initialSnapshot = snapshotManager.loadLatestSnapshot(); this.schema = SchemaUtils.convertKernelSchemaToSparkSchema(initialSnapshot.getSchema()); diff --git a/kernel-spark/src/main/scala/org/apache/spark/sql/delta/sources/DeltaSQLConfV2.scala b/kernel-spark/src/main/scala/org/apache/spark/sql/delta/sources/DeltaSQLConfV2.scala index e2f8aac7459..f42f1a44048 100644 --- a/kernel-spark/src/main/scala/org/apache/spark/sql/delta/sources/DeltaSQLConfV2.scala +++ b/kernel-spark/src/main/scala/org/apache/spark/sql/delta/sources/DeltaSQLConfV2.scala @@ -39,8 +39,9 @@ object DeltaSQLConfV2 extends DeltaSQLConfUtils { buildConf("v2.enableMode") .doc( "Controls the Delta V2 connector enable mode. " + - "Valid values: NONE (disabled, default), STRICT (should ONLY be enabled for testing).") + "Valid values: NONE (disabled, default), STRICT (should ONLY be enabled for testing).") .stringConf .checkValues(Set("NONE", "STRICT")) .createWithDefault("NONE") } + From b2f2b5f8d59adce70d2d2f3de5dc53e761738c60 Mon Sep 17 00:00:00 2001 From: TimothyW553 Date: Tue, 25 Nov 2025 19:40:08 +0000 Subject: [PATCH 42/42] Wire SparkTable to snapshot manager factory --- .../java/io/delta/kernel/spark/catalog/SparkTable.java | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/kernel-spark/src/main/java/io/delta/kernel/spark/catalog/SparkTable.java b/kernel-spark/src/main/java/io/delta/kernel/spark/catalog/SparkTable.java index bef04d4f1f7..afe8abd98ec 100644 --- a/kernel-spark/src/main/java/io/delta/kernel/spark/catalog/SparkTable.java +++ b/kernel-spark/src/main/java/io/delta/kernel/spark/catalog/SparkTable.java @@ -21,7 +21,7 @@ import io.delta.kernel.Snapshot; import io.delta.kernel.spark.read.SparkScanBuilder; import io.delta.kernel.spark.snapshot.DeltaSnapshotManager; -import io.delta.kernel.spark.snapshot.PathBasedSnapshotManager; +import io.delta.kernel.spark.snapshot.DeltaSnapshotManagerFactory; import io.delta.kernel.spark.utils.SchemaUtils; import java.util.*; import org.apache.hadoop.conf.Configuration; @@ -138,9 +138,11 @@ private SparkTable( merged.putAll(userOptions); this.options = Collections.unmodifiableMap(merged); - this.hadoopConf = - SparkSession.active().sessionState().newHadoopConfWithOptions(toScalaMap(options)); - this.snapshotManager = new PathBasedSnapshotManager(tablePath, hadoopConf); + SparkSession spark = SparkSession.active(); + this.hadoopConf = spark.sessionState().newHadoopConfWithOptions(toScalaMap(options)); + // Use factory to create appropriate snapshot manager (catalog-managed vs path-based) + this.snapshotManager = + DeltaSnapshotManagerFactory.create(tablePath, catalogTable, spark, hadoopConf); // Load the initial snapshot through the manager this.initialSnapshot = snapshotManager.loadLatestSnapshot(); this.schema = SchemaUtils.convertKernelSchemaToSparkSchema(initialSnapshot.getSchema());