-
Notifications
You must be signed in to change notification settings - Fork 28.9k
[SPARK-53786][SQL] Default value with special column name should not conflict with real column #52504
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[SPARK-53786][SQL] Default value with special column name should not conflict with real column #52504
Changes from 5 commits
da190b3
ad8f55e
5bb76b4
2caf43f
8f7179c
e1b38bb
c6c30c8
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -175,6 +175,9 @@ trait ColumnResolutionHelper extends Logging with DataTypeErrorsBase { | |
u.copy(child = newChild) | ||
} | ||
|
||
case d @ DefaultValueExpression(c: Expression, _, _) => | ||
szehon-ho marked this conversation as resolved.
Show resolved
Hide resolved
|
||
d.copy(child = resolveLiteralColumns(c)) | ||
|
||
case _ => e.mapChildren(innerResolve(_, isTopLevel = false)) | ||
} | ||
resolved.copyTagsFrom(e) | ||
|
@@ -195,6 +198,13 @@ trait ColumnResolutionHelper extends Logging with DataTypeErrorsBase { | |
} | ||
} | ||
|
||
private def resolveLiteralColumns(e: Expression) = { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I am a little bit worried that this doesn't fix the root problem that ResolvedIdentifier output in CREATE actually is used as candidates for resolving default values. Let me think. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Actually, maybe it does solve the root problem. Am I right we will not recurse into DefaultValueExpression child so we effectively ensure that default value resolution doesn't have access to attributes? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yes, this is matched before children |
||
e.transformWithPruning(_.containsPattern(UNRESOLVED_ATTRIBUTE)) { | ||
case u @ UnresolvedAttribute(nameParts) => | ||
LiteralFunctionResolution.resolve(nameParts).getOrElse(u) | ||
} | ||
} | ||
|
||
// Resolves `UnresolvedAttribute` to `OuterReference`. | ||
protected def resolveOuterRef(e: Expression): Expression = { | ||
val outerPlan = AnalysisContext.get.outerPlan | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3870,6 +3870,60 @@ class DataSourceV2SQLSuiteV1Filter | |
} | ||
} | ||
|
||
test("test default value special column name conflicting with real column name") { | ||
|
||
val t = "testcat.ns.t" | ||
withTable("t") { | ||
sql(s"""CREATE table $t ( | ||
c1 STRING, | ||
current_date DATE DEFAULT CURRENT_DATE, | ||
current_timestamp TIMESTAMP DEFAULT CURRENT_TIMESTAMP, | ||
current_time time DEFAULT CURRENT_TIME, | ||
current_user STRING DEFAULT CURRENT_USER, | ||
session_user STRING DEFAULT SESSION_USER, | ||
user STRING DEFAULT USER, | ||
current_database STRING DEFAULT CURRENT_DATABASE(), | ||
current_catalog STRING DEFAULT CURRENT_CATALOG())""") | ||
sql(s"INSERT INTO $t (c1) VALUES ('a')") | ||
val result = sql(s"SELECT * FROM $t").collect() | ||
assert(result.length == 1) | ||
szehon-ho marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
assert(result(0).getString(0) == "a") | ||
Seq(1 to 8: _*).foreach(i => assert(result(0).get(i) != null)) | ||
} | ||
} | ||
|
||
test("test default value special column name nested in function") { | ||
szehon-ho marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
val t = "testcat.ns.t" | ||
withTable("t") { | ||
sql(s"""CREATE table $t ( | ||
c1 STRING, | ||
current_date DATE DEFAULT DATE_ADD(current_date, 7))""") | ||
sql(s"INSERT INTO $t (c1) VALUES ('a')") | ||
val result = sql(s"SELECT * FROM $t").collect() | ||
assert(result.length == 1) | ||
assert(result(0).getString(0) == "a") | ||
} | ||
} | ||
|
||
test("test default value should not refer to real column") { | ||
val t = "testcat.ns.t" | ||
withTable("t") { | ||
checkError( | ||
exception = intercept[AnalysisException] { | ||
sql(s"""CREATE table $t ( | ||
c1 timestamp, | ||
current_timestamp TIMESTAMP DEFAULT c1)""") | ||
}, | ||
condition = "INVALID_DEFAULT_VALUE.UNRESOLVED_EXPRESSION", | ||
parameters = Map( | ||
"statement" -> "CREATE TABLE", | ||
"colName" -> "`current_timestamp`", | ||
"defaultValue" -> "c1" | ||
), | ||
sqlState = Some("42623") | ||
) | ||
} | ||
} | ||
|
||
private def testNotSupportedV2Command( | ||
sqlCommand: String, | ||
sqlParams: String, | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I assume this works because we resolve expressions top to bottom, hence we see DefaultValueExpression before we see the unresolved attribute?