[Spark] Drop feature support in DeltaTable Scala/Python APIs #3952

andreaschat-db · 2024-12-11T08:24:07Z

Which Delta project/connector is this regarding?

Description

This PR adds drop feature support in the DeltaTable API for both scala and python APIs.

How was this patch tested?

Added UTs.

Does this PR introduce any user-facing changes?

Yes. See description.

python/delta/tables.py

longvu-db · 2024-12-11T17:09:49Z

python/delta/tables.py

+        higher than necessary.
+
+        Normalization can also decrease the reader version of a table features protocol when it is
+        higher than necessary.


Question: Should we use "Example::", ":param", ":return:" and "rtype", with a Example io.delta.tables.DeltaTable.dropFeatureSupport("RowTracking")?

longvu-db · 2024-12-11T17:09:57Z

python/delta/tables.py

+        Normalization can also decrease the reader version of a table features protocol when it is
+        higher than necessary.
+
+        Normalization can also decrease the reader version of a table features protocol when it is


Is the repeat of " Normalization can also decrease the reader version of a table features protocol when it is
higher than necessary." intended?

No. Fixed it.

longvu-db · 2024-12-11T17:11:13Z

python/delta/tests/test_deltatable.py

@@ -1188,16 +1188,19 @@ def test_protocolUpgrade(self) -> None:
        with self.assertRaisesRegex(ValueError, "writerVersion"):
            dt.upgradeTableProtocol(1, {})  # type: ignore[arg-type]

-    def test_addFeatureSupport(self) -> None:
+    def __create_df_for_feature_tests(self) -> DeltaTable:


Should we also use this refactor __create_df_for_feature_tests in other place like test_protocolUpgrade?

Ahh I did not see the code is (almost) the same there. Yes.

python/delta/tables.py

longvu-db · 2024-12-11T17:27:57Z

spark/src/test/scala/io/delta/tables/DeltaTableSuite.scala

+        TestRemovableWriterFeature)))
+
+      // Drop feature.
+      table.dropFeatureSupport(TestRemovableWriterFeature.name)


Should we add a test where after dropping the feature, the protocol doesn't change? Like dropping a readerWriterFeature from a set of 2 readerWriterFeatures should remain (3, 7)

All there cases (and more) are covered in the DeltaProtocolVersionSuite. Here I tried to focus on whether the DeltaTable API works by testing some basic functionality.

python/delta/tables.py

longvu-db · 2024-12-11T17:31:26Z

python/delta/tables.py

+    @since(4.0)  # type: ignore[arg-type]
+    def dropFeatureSupport(self, featureName: str) -> None:
+        """
+        Modify the protocol to drop a supported feature. The operation always normalizes the


Suggested change

Modify the protocol to drop a supported feature. The operation always normalizes the

Modify the protocol to drop an existing supported feature. The operation always normalizes the

Nit: Similar to the alterDeltaTableCommand's description.

I could not find where this is mentioned in alterDeltaTableCommand but "existing" is redundant in that case since we are already using "supported."

longvu-db · 2024-12-11T17:35:15Z

spark/src/main/scala/io/delta/tables/DeltaTable.scala

+   *
+   * For example, consider protocol (1, 7, None, {Invariants, AppendOnly, TestWriter}.
+   * Dropping the testWriter feature results to protocol (1, 2). This is because the implicit
+   * features of the legacy protocol (1, 2) exactly match the explicit features of the


By "exactly match", we are referring to the protocol versions right?

No. We are referring to the feature set. For example:

Legacy Protocol(1, 2) implicitly supports invariants and appendOnly features.

Table Features protocol (1, 7, invariants, appendOnly) explicitly supports invariants and appendOnly features.

These two protocols are considered equivalent but in different form.

longvu-db · 2024-12-11T17:36:14Z

python/delta/tables.py

+        protocol to the weakest possible form. This primarily refers to converting a table features
+        protocol to a legacy protocol. A Table Features protocol can be represented with the legacy
+        representation only when the features set of the former exactly matches a legacy protocol.
+        Normalization can also decrease the reader version of a table features protocol when it is


Why don't we make the Scala text description and Python text description identical?

That was my initial intention but I diverted later on :D.

longvu-db · 2024-12-11T17:38:44Z

Could we add "[Spark]" to the description and also specify that this is for Spark Classic in the title and description as well?

spark/src/main/scala/io/delta/tables/DeltaTable.scala

spark/src/test/scala/io/delta/tables/DeltaTableSuite.scala

longvu-db · 2024-12-11T17:44:04Z

spark/src/main/scala/org/apache/spark/sql/delta/commands/alterDeltaTableCommands.scala

@@ -322,7 +322,7 @@ case class AlterTableDropFeatureDeltaCommand(
    // Check whether the protocol contains the feature in either the writer features list or
    // the reader+writer features list. Note, protocol needs to denormalized to allow dropping
    // features from legacy protocols.
-    val protocol = table.initialSnapshot.protocol
+    val protocol = table.deltaLog.update().protocol


I'm happy with changing this to deltaLog.update(), but just wondering if there was any risk before with using initialSnapshot. Don't know why we haven't ran into a problem with this before since we should always get the latest protocol, not the "The snapshot initially associated with this table. "

Yeah good question. That command was until now primarily only accessible with spark SQL. It seems that when using SQL the command would get table with a fresh snapshot. However, now with the DeltaTable API the user directly controls which table instance is using. So unless the user cares to properly refresh the table instance, we can get a stale snapshot.

longvu-db

LGTM!

longvu-db · 2024-12-12T17:00:40Z

spark/src/main/scala/io/delta/tables/DeltaTable.scala

+   *
+   * See online documentation for more details.
+   *
+   * @param featureName: The name of the feature to drop.


@andreaschat-db It should be "featureName" instead of "featureName:"

longvu-db · 2024-12-12T17:02:43Z

python/delta/tables.py

@@ -594,6 +594,30 @@ def addFeatureSupport(self, featureName: str) -> None:
        DeltaTable._verify_type_str(featureName, "featureName")
        self._jdt.addFeatureSupport(featureName)

+    @since(4.0)  # type: ignore[arg-type]


@andreaschat-db I believe there will be a Delta 3.4 version before 4.0

longvu-db · 2024-12-12T17:02:58Z

spark/src/main/scala/io/delta/tables/DeltaTable.scala

+   * @param featureName: The name of the feature to drop.
+   * @return None.
+   *
+   * @since 4.0.0


Ditto, 3.4.0

longvu-db reviewed Dec 11, 2024

View reviewed changes

python/delta/tables.py Outdated Show resolved Hide resolved

longvu-db reviewed Dec 11, 2024

View reviewed changes

spark/src/main/scala/io/delta/tables/DeltaTable.scala Show resolved Hide resolved

longvu-db reviewed Dec 11, 2024

View reviewed changes

spark/src/test/scala/io/delta/tables/DeltaTableSuite.scala Outdated Show resolved Hide resolved

longvu-db reviewed Dec 11, 2024

View reviewed changes

andreaschat-db changed the title ~~Drop feature support in DeltaTable API~~ [Spark] Drop feature support in DeltaTable Scala/Python APIs Dec 12, 2024

andreaschat-db force-pushed the addDropFeatureToDeltaTableAPI branch from 68567c1 to d5991d7 Compare December 12, 2024 15:15

longvu-db approved these changes Dec 12, 2024

View reviewed changes

longvu-db reviewed Dec 12, 2024

View reviewed changes

improvements

075d86d

andreaschat-db force-pushed the addDropFeatureToDeltaTableAPI branch from 416e097 to 075d86d Compare December 18, 2024 18:08

andreaschat-db added 2 commits December 18, 2024 19:28

fix type annotation

85f99c3

type annotations

69b30a1

scottsand-db merged commit 1cd6fed into delta-io:master Dec 18, 2024
16 of 19 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Spark] Drop feature support in DeltaTable Scala/Python APIs #3952

[Spark] Drop feature support in DeltaTable Scala/Python APIs #3952

andreaschat-db commented Dec 11, 2024 •

edited

Loading

longvu-db Dec 11, 2024

longvu-db Dec 11, 2024

andreaschat-db Dec 12, 2024

longvu-db Dec 11, 2024

andreaschat-db Dec 12, 2024

longvu-db Dec 11, 2024

andreaschat-db Dec 12, 2024

longvu-db Dec 11, 2024

andreaschat-db Dec 12, 2024

longvu-db Dec 11, 2024

andreaschat-db Dec 12, 2024

longvu-db Dec 11, 2024

andreaschat-db Dec 12, 2024

longvu-db commented Dec 11, 2024

longvu-db Dec 11, 2024

andreaschat-db Dec 12, 2024

longvu-db left a comment

longvu-db Dec 12, 2024 •

edited

Loading

longvu-db Dec 12, 2024

longvu-db Dec 12, 2024

	Modify the protocol to drop a supported feature. The operation always normalizes the
	Modify the protocol to drop an existing supported feature. The operation always normalizes the

[Spark] Drop feature support in DeltaTable Scala/Python APIs #3952

[Spark] Drop feature support in DeltaTable Scala/Python APIs #3952

Conversation

andreaschat-db commented Dec 11, 2024 • edited Loading

Which Delta project/connector is this regarding?

Description

How was this patch tested?

Does this PR introduce any user-facing changes?

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

longvu-db commented Dec 11, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

longvu-db left a comment

Choose a reason for hiding this comment

longvu-db Dec 12, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

andreaschat-db commented Dec 11, 2024 •

edited

Loading

longvu-db Dec 12, 2024 •

edited

Loading