Skip to content

Conversation

szehon-ho
Copy link
Member

@szehon-ho szehon-ho commented Oct 2, 2025

What changes were proposed in this pull request?

Fix the analysis of default value expression to not include column names

Why are the changes needed?

The following query:

CREATE TABLE t (current_timestamp DEFAULT current_timestamp)

fails with an exception:

[INVALID_DEFAULT_VALUE.NOT_CONSTANT] Failed to execute CREATE TABLE command because the destination column or variable `current_timestamp` has a DEFAULT value CURRENT_TIMESTAMP, which is not a constant expression whose equivalent value is known at query planning time. SQLSTATE: 42623;

This is because , to create a default value DSV2 expression, the code now uses the main analyzer to analyze the default value, which resolves it to the column current_timestamp. However, analyzer should not try to resolve default value to other columns.

Does this PR introduce any user-facing change?

Should fix a regression

How was this patch tested?

Add new unit test in DataSourceV2SQLSuite

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

No

@github-actions github-actions bot added the SQL label Oct 2, 2025
u.copy(child = newChild)
}

case d @ DefaultValueExpression(u: UnresolvedAttribute, _, _) =>
Copy link
Member Author

@szehon-ho szehon-ho Oct 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note: before this fix, Default value expression would fall to UnresolvedAttribute above. It would then think the default value refers to the conflicting column name and fail.

case u @ UnresolvedAttribute(nameParts) =>
          val result = withPosition(u) {
            resolveColumnByName(nameParts)
              .orElse(LiteralFunctionResolution.resolve(nameParts))
              .map {
                // We trim unnecessary alias here. Note that, we cannot trim the alias at top-level,
                // as we should resolve `UnresolvedAttribute` to a named expression. The caller side
                // can trim the top-level alias if it's safe to do so. Since we will call
                // CleanupAliases later in Analyzer, trim non top-level unnecessary alias is safe.
                case Alias(child, _) if !isTopLevel => child
                case other => other
              }
              .getOrElse(u)
          }
          logDebug(s"Resolving $u to $result")
          result

@szehon-ho szehon-ho force-pushed the default_value_conflict branch from b0350a5 to da190b3 Compare October 2, 2025 06:19
case d @ DefaultValueExpression(u: UnresolvedAttribute, _, _) =>
d.copy(child = LiteralFunctionResolution.resolve(u.nameParts)
.map {
case Alias(child, _) if !isTopLevel => child
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just copied this from the other code, we dont need it right?

@szehon-ho
Copy link
Member Author

failure may not be related:

[error] org.apache.spark.sql.kafka010.KafkaMicroBatchV1SourceWithConsumerSuite, rerunning to verify

@szehon-ho szehon-ho changed the title [SPARK-53786][SQL] Default value should not conflict with special column name [SPARK-53786][SQL] Default value with special column name should not conflict with real column Oct 2, 2025
@szehon-ho
Copy link
Member Author

a better approach is here: #52530

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant