[SPARK-51987][SQL] DSv2 expressions in column defaults on write #50792

aokolnychyi · 2025-05-05T20:41:52Z

What changes were proposed in this pull request?

This PR allows connectors to expose expression-based defaults on write.

Why are the changes needed?

These changes are needed to avoid the requirement of producing Spark SQL dialect in connectors.

Does this PR introduce any user-facing change?

No.

How was this patch tested?

This PR comes with tests.

Was this patch authored or co-authored using generative AI tooling?

No.

aokolnychyi · 2025-05-05T20:57:35Z

sql/catalyst/src/test/scala/org/apache/spark/sql/connector/catalog/InMemoryBaseTable.scala

@@ -49,7 +49,7 @@ import org.apache.spark.util.ArrayImplicits._
 */
 abstract class InMemoryBaseTable(
    val name: String,
-    val schema: StructType,
+    override val columns: Array[Column],


I had to change this to expose passed columns directly, rather than going through a round of conversion.

aokolnychyi · 2025-05-06T20:25:38Z

cc @cloud-fan @dongjoon-hyun @viirya @szehon-ho @gengliangwang

dongjoon-hyun · 2025-05-06T20:32:23Z

sql/catalyst/src/test/scala/org/apache/spark/sql/connector/catalog/InMemoryTable.scala

    ordering, numPartitions, advisoryPartitionSize, isDistributionStrictlyRequired,
    numRowsPerSplit) with SupportsDelete {

+  def this(
+      name: String,
+      schema: StructType,


Thank you for adding this constructor.

dongjoon-hyun

+1, LGTM. Thank you, @aokolnychyi .

dongjoon-hyun · 2025-05-06T20:33:22Z

cc @huaxingao , too.

aokolnychyi · 2025-05-06T20:43:52Z

sql/core/src/test/scala/org/apache/spark/sql/connector/DataSourceV2DataFrameSuite.scala

@@ -483,6 +488,32 @@ class DataSourceV2DataFrameSuite
    }
  }

+  test("write with expression-based default values") {


This test would previously fail.

gengliangwang · 2025-05-06T21:28:49Z

sql/catalyst/src/main/scala/org/apache/spark/sql/connector/catalog/CatalogV2Util.scala

+        case None if defaultValue.getSql != null =>
+          defaultValue.getSql
+        case _ =>
+          throw SparkException.internalError(


Shall we add a test for this branch?

viirya · 2025-05-06T22:02:15Z

sql/catalyst/src/main/scala/org/apache/spark/sql/connector/catalog/CatalogV2Util.scala

    }.getOrElse(f)
  }

+  private def toSql(defaultValue: DefaultValue): String = {
+    if (defaultValue.getExpression != null) {
+      V2ExpressionUtils.toCatalyst(defaultValue.getExpression) match {


Hmm, when it is possible that a default expression cannot be converted back to catalyst one?

Dynamic expressions other than literals?

Can we also add a test case that falls back to defaultValue.getSql when toCatalyst returns None?

V2 expressions are quite flexible, users can represent a function with an arbitrary name. In practice, I expect it to be rare but you never know.

I'll add a test.

Added more tests.

szehon-ho · 2025-05-06T22:22:35Z

sql/catalyst/src/main/scala/org/apache/spark/sql/connector/catalog/CatalogV2Util.scala

@@ -598,10 +598,28 @@ private[sql] object CatalogV2Util {
      //       data unchanged and let the data reader to return "exist default" for missing
      //       columns.
      val existingDefault = Literal(default.getValue.value(), default.getValue.dataType()).sql
-      f.withExistenceDefaultValue(existingDefault).withCurrentDefaultValue(default.getSql)
+      f.withExistenceDefaultValue(existingDefault).withCurrentDefaultValue(toSql(defaultValue))


is this method comment outdated (its about built-in file sources)?

It's for all data sources that can add column default values with existing data

It seems to still apply as it describes the concept of "exists" default that we don't change here.

i mean the method javadoc:

// For built-in file sources, we encode the default value in StructField metadata. An analyzer // rule will check the special metadata and change the DML input plan to fill the default value.

do we need to update it?

That still seems accurate even after this PR. Spark always converts DefaultValue to StructField metadata. It is a good question whether that's strictly required or desirable but reconsidering that will be a big effort.

szehon-ho · 2025-05-06T22:23:29Z

sql/catalyst/src/main/scala/org/apache/spark/sql/connector/catalog/CatalogV2Util.scala

    }.getOrElse(f)
  }

+  private def toSql(defaultValue: DefaultValue): String = {


style question: did we consider putting this logic in DefaultValue?

DefaultValue is a user-facing DS v2 API. We should put these internal util functions in internal classes.

Correct, the idea is to keep DefaultValue as simple as possible.

szehon-ho · 2025-05-06T22:26:24Z

sql/catalyst/src/main/scala/org/apache/spark/sql/connector/catalog/CatalogV2Util.scala

    }.getOrElse(f)
  }

+  private def toSql(defaultValue: DefaultValue): String = {
+    if (defaultValue.getExpression != null) {


iiuc, this is prioritizing expression over sql , if we get both. Is that desired? We could put a comment somewhere if so

Yea I have the same question. It seems more reasonable to use the original SQL string first.

Based on DefaultValue document, it uses expression first and fallbacks to SQL string if the expression cannot be converted.

When we need to evaluate, it makes sense to use expression first and fallback to SQL string if the expression cannot be converted. But here we just need to get the SQL string.

The problem is that whatever we encode here will be picked up and used on write. Spark doesn't currently use expressions to represent default values. We persist the expression SQL in struct field metadata. What this logic does is saying if I a connector provides an expression, I am going to convert it to Catalyst and trust Spark to generate an accurate SQL representation. If we flip this, it means we prioritize the connector-provided SQL string over our own SQL generation for Catalyst expressions.

Does this make sense, @szehon-ho @viirya @cloud-fan?

But Spark is not capable of generating an executable SQL string from a query plan/expression. The .sql only generates a pretty string for display. We don't even have a test to parse back the .sql result.

On the other hand, the DefaultValue.sql is usually the original user-specified SQL, or the connector is responsible to give an executable SQL if it generates it by itself.

Ah, I misinterpreted the promise behind .sql. It was my understanding it is actually reliable. Let me think more about this tomorrow. If .sql is not reliable, then it does change things a bit, I guess. The problem that if we prefer the string over the expression it kind of breaks the promise behind DefaultValue, which states the expression takes precedence.

There is a way to produce reliable SQL strings from v2 expressions: V2ExpressionSQLBuilder

[SPARK-51987][SQL] DSv2 expressions in column defaults on write

b14cc46

github-actions bot added the SQL label May 5, 2025

aokolnychyi commented May 5, 2025

View reviewed changes

aokolnychyi closed this May 6, 2025

aokolnychyi reopened this May 6, 2025

aokolnychyi closed this May 6, 2025

aokolnychyi reopened this May 6, 2025

aokolnychyi mentioned this pull request May 6, 2025

[SPARK-51902][SQL] Enforce check constraint on table insertion #50761

Closed

dongjoon-hyun reviewed May 6, 2025

View reviewed changes

dongjoon-hyun approved these changes May 6, 2025

View reviewed changes

aokolnychyi commented May 6, 2025

View reviewed changes

gengliangwang reviewed May 6, 2025

View reviewed changes

viirya reviewed May 6, 2025

View reviewed changes

szehon-ho reviewed May 6, 2025

View reviewed changes

More tests

bd7bd68

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[SPARK-51987][SQL] DSv2 expressions in column defaults on write #50792

[SPARK-51987][SQL] DSv2 expressions in column defaults on write #50792

aokolnychyi commented May 5, 2025

aokolnychyi May 5, 2025 •

edited

Loading

dongjoon-hyun May 6, 2025

aokolnychyi commented May 6, 2025

dongjoon-hyun May 6, 2025

dongjoon-hyun left a comment

dongjoon-hyun commented May 6, 2025

aokolnychyi May 6, 2025

gengliangwang May 6, 2025

aokolnychyi May 7, 2025

viirya May 6, 2025

viirya May 6, 2025

viirya May 6, 2025

aokolnychyi May 7, 2025

aokolnychyi May 7, 2025

szehon-ho May 6, 2025

cloud-fan May 7, 2025

aokolnychyi May 7, 2025

szehon-ho May 7, 2025 •

edited

Loading

aokolnychyi May 7, 2025

szehon-ho May 6, 2025

cloud-fan May 7, 2025

aokolnychyi May 7, 2025

szehon-ho May 6, 2025

cloud-fan May 7, 2025

viirya May 7, 2025

cloud-fan May 7, 2025

aokolnychyi May 7, 2025

aokolnychyi May 7, 2025

cloud-fan May 8, 2025

aokolnychyi May 8, 2025 •

edited

Loading

cloud-fan May 8, 2025

[SPARK-51987][SQL] DSv2 expressions in column defaults on write #50792

Are you sure you want to change the base?

[SPARK-51987][SQL] DSv2 expressions in column defaults on write #50792

Conversation

aokolnychyi commented May 5, 2025

What changes were proposed in this pull request?

Why are the changes needed?

Does this PR introduce any user-facing change?

How was this patch tested?

Was this patch authored or co-authored using generative AI tooling?

aokolnychyi May 5, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aokolnychyi commented May 6, 2025

Choose a reason for hiding this comment

dongjoon-hyun left a comment

Choose a reason for hiding this comment

dongjoon-hyun commented May 6, 2025

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

szehon-ho May 7, 2025 • edited Loading

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

aokolnychyi May 8, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aokolnychyi May 5, 2025 •

edited

Loading

szehon-ho May 7, 2025 •

edited

Loading

aokolnychyi May 8, 2025 •

edited

Loading