MaxGekk commented on code in PR #48055:
URL: https://github.com/apache/spark/pull/48055#discussion_r1751909597
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/parser/AstBuilder.scala:
##########
@@ -5081,6 +5081,13 @@ class AstBuilder extends DataTypeAstBuilder
val options =
Option(ctx.options).map(visitPropertyKeyValues).getOrElse(Map.empty)
val isLazy = ctx.LAZY != null
if (query.isDefined) {
+ // Disallow parameter markers in the body of the cache.
Review Comment:
Let's use the term from docs/gramma `query` instead of `body`.
##########
sql/core/src/test/scala/org/apache/spark/sql/ParametersSuite.scala:
##########
@@ -715,4 +715,30 @@ class ParametersSuite extends QueryTest with
SharedSparkSession with PlanTest {
spark.sessionState.analyzer.executeAndCheck(analyzedPlan,
df.queryExecution.tracker)
checkAnswer(df, Row(11))
}
+
+ test("SPARK-49398: Cache Table with parameter markers in select query should
throw " +
+ "UNSUPPORTED_FEATURE.PARAMETER_MARKER_IN_UNEXPECTED_STATEMENT") {
+ val sqlText = "CACHE TABLE CacheTable as SELECT 1 + :param1"
+ checkError(
+ exception = intercept[AnalysisException] {
+ spark.sql(sqlText, Map("param1" -> "1")).show()
+ },
+ errorClass =
"UNSUPPORTED_FEATURE.PARAMETER_MARKER_IN_UNEXPECTED_STATEMENT",
+ parameters = Map("statement" -> "CACHE TABLE body"),
+ context = ExpectedContext(
+ fragment = sqlText,
+ start = 0,
+ stop = sqlText.length - 1)
+ )
+ }
+
+ test("SPARK-49398: Cache Table with parameter in identifier should work") {
+ val cacheName = "MyCacheTable"
+ withCache(cacheName) {
+ spark.sql("CACHE TABLE IDENTIFIER(:param) as SELECT 1 as c1",
Map("param" -> cacheName))
Review Comment:
The error message confuses me a little. It says:
```
The feature is not supported: Parameter markers are not allowed in CACHE
TABLE body.
```
but this test shows that parameters are allowed in some parts of `CACHE
TABLE`. Could you rewrite `statement` a little bit:
```
The feature is not supported: Parameter markers are not allowed in the query
of CACHE TABLE.
```
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/parser/AstBuilder.scala:
##########
@@ -5677,4 +5684,24 @@ class AstBuilder extends DataTypeAstBuilder
withOrigin(ctx) {
visitSetVariableImpl(ctx.query(), ctx.multipartIdentifierList(),
ctx.assignmentList())
}
+
+ /**
+ * Check plan for any parameters.
+ * If it finds any throws
UNSUPPORTED_FEATURE.PARAMETER_MARKER_IN_UNEXPECTED_STATEMENT.
+ * This method is used to ban parameters in some contexts.
+ */
+ protected def checkInvalidParameter(plan: LogicalPlan, statement: String):
+ Unit = {
Review Comment:
I see you just copy-pasted the code, but this is unusual indentation, I
think, see
https://github.com/databricks/scala-style-guide?tab=readme-ov-file#spacing-and-indentation
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
