Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
43 commits
Select commit Hold shift + click to select a range
6b42040
Add UnboundParameter
MaxGekk Nov 18, 2022
fe3ca7a
Add a test
MaxGekk Nov 18, 2022
ab3c615
Merge remote-tracking branch 'origin/master' into parameterized-sql
MaxGekk Nov 18, 2022
3cfef61
Merge remote-tracking branch 'origin/master' into parameterized-sql
MaxGekk Nov 19, 2022
c264476
Refactoring + add more parsing tests
MaxGekk Nov 21, 2022
17b9ea3
Merge remote-tracking branch 'origin/master' into parameterized-sql
MaxGekk Nov 21, 2022
0f32b75
NamedParameter should be bound
MaxGekk Nov 22, 2022
2806334
Add new method bind() to Dataset
MaxGekk Nov 23, 2022
d16e85b
Return back Literal and fix for scala 2.13
MaxGekk Nov 23, 2022
e48e2e1
Add the BindParameters rule
MaxGekk Nov 24, 2022
16b9a27
Merge remote-tracking branch 'origin/master' into parameterized-sql
MaxGekk Nov 26, 2022
693cd92
Add an end-to-end test
MaxGekk Nov 28, 2022
4e55269
Add a config to control the feature
MaxGekk Nov 30, 2022
4d1198f
Merge remote-tracking branch 'origin/master' into parameterized-sql
MaxGekk Dec 1, 2022
fcdf5f2
Support parametrized SQL queries by sql()
MaxGekk Dec 1, 2022
10c7680
Just strip any marker
MaxGekk Dec 1, 2022
cddff4a
Refactoring
MaxGekk Dec 1, 2022
a0f568a
Remove the Bind node
MaxGekk Dec 1, 2022
2e93bec
Add the error class NON_FOLDABLE_SQL_ARG
MaxGekk Dec 2, 2022
cf03c3b
Improve comments
MaxGekk Dec 2, 2022
0605f22
Add unboundError()
MaxGekk Dec 2, 2022
210d23c
Remove the BIND tag
MaxGekk Dec 2, 2022
16966c9
Fix UNBOUND_PARAMETER
MaxGekk Dec 2, 2022
b8a6fd9
Add Java-specific method
MaxGekk Dec 3, 2022
89c5371
Add tests for double quotes
MaxGekk Dec 6, 2022
e883aad
Add one more test for a foldable expr - cast
MaxGekk Dec 6, 2022
d05526c
Convert the internal error to an user-facing one
MaxGekk Dec 6, 2022
ce19640
Add a test for ignored args
MaxGekk Dec 7, 2022
5a1f33d
Allow literals only.
MaxGekk Dec 7, 2022
43fd1b9
Fix the order of error classes in error-classes.json
MaxGekk Dec 7, 2022
5ad46f8
Switch the parameter marker from @ to :
MaxGekk Dec 8, 2022
637778c
Fix parsing error
MaxGekk Dec 8, 2022
d2ce096
Use COLON
MaxGekk Dec 8, 2022
d3ac69c
a literal value -> a SQL literal statement
MaxGekk Dec 8, 2022
08e6dfa
Address comments related to error classes
MaxGekk Dec 8, 2022
27927fb
Add ParametersSuite
MaxGekk Dec 9, 2022
c390260
Address Wenchen's review comments
MaxGekk Dec 9, 2022
81cb619
Use identifier() to get parameter name
MaxGekk Dec 12, 2022
4b7cb23
Extend Unevaluable
MaxGekk Dec 12, 2022
0a3e500
Throw an internal error for dataType() and nullable()
MaxGekk Dec 12, 2022
15e2b27
Remove the SQL config
MaxGekk Dec 12, 2022
165a21d
Remove a test from JavaSparkSessionSuite.java
MaxGekk Dec 12, 2022
2857350
Update sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expr…
MaxGekk Dec 15, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions core/src/main/resources/error/error-classes.json
Original file line number Diff line number Diff line change
Expand Up @@ -795,6 +795,11 @@
}
}
},
"INVALID_SQL_ARG" : {
"message" : [
"The argument <name> of `sql()` is invalid. Consider to replace it by a SQL literal statement."

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Can we more explicit about why it is invalid?
"The argument of sql() is not a literal." What is a "SQL Literal statement"?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

What is a "SQL Literal statement"?

Any SQL statement that can produce a literal, see https://spark.apache.org/docs/latest/sql-ref-literals.html

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

That's an expression then. Statements are top level. (Like SELECT, UPDATE, CRAETE, SET).

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

how about just say SQL literal?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I will change this in #39183

]
},
"INVALID_SQL_SYNTAX" : {
"message" : [
"Invalid SQL syntax: <inputString>"
Expand Down Expand Up @@ -1130,6 +1135,11 @@
"Unable to convert SQL type <toType> to Protobuf type <protobufType>."
]
},
"UNBOUND_SQL_PARAMETER" : {
"message" : [
"Found the unbound parameter: <name>. Please, fix `args` and provide a mapping of the parameter to a SQL literal statement."

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Won't this same error be used fro all other APIs (JDBC, SQL (when we support). So we may not want to refer to args.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

So we may not want to refer to args.

This is a premature generalisation. Let's make it more generic when we will need that.

]
},
"UNCLOSED_BRACKETED_COMMENT" : {
"message" : [
"Found an unclosed bracketed comment. Please, append */ at the end of the comment."
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -930,6 +930,7 @@ primaryExpression

constant
: NULL #nullLiteral
| COLON identifier #parameterLiteral
| interval #intervalLiteral
| identifier stringLit #typeConstructor
| number #numericLiteral
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -325,6 +325,11 @@ trait CheckAnalysis extends PredicateHelper with LookupCatalog with QueryErrorsB
errorClass = "_LEGACY_ERROR_TEMP_2413",
messageParameters = Map("argName" -> e.prettyName))

case p: Parameter =>
p.failAnalysis(
errorClass = "UNBOUND_SQL_PARAMETER",
messageParameters = Map("name" -> toSQLId(p.name)))

case _ =>
})

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
/*
* Licensed to the Apache Software Foundation (ASF) under one or more
* contributor license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright ownership.
* The ASF licenses this file to You under the Apache License, Version 2.0
* (the "License"); you may not use this file except in compliance with
* the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package org.apache.spark.sql.catalyst.expressions

import org.apache.spark.SparkException
import org.apache.spark.sql.catalyst.analysis.AnalysisErrorAt
import org.apache.spark.sql.catalyst.plans.logical.LogicalPlan
import org.apache.spark.sql.catalyst.trees.TreePattern.{PARAMETER, TreePattern}
import org.apache.spark.sql.errors.QueryErrorsBase
import org.apache.spark.sql.types.DataType

/**
* The expression represents a named parameter that should be replaced by a literal.
*
* @param name The identifier of the parameter without the marker.
*/
case class Parameter(name: String) extends LeafExpression with Unevaluable {
override lazy val resolved: Boolean = false

private def unboundError(methodName: String): Nothing = {
throw SparkException.internalError(
s"Cannot call `$methodName()` of the unbound parameter `$name`.")
}
override def dataType: DataType = unboundError("dataType")
override def nullable: Boolean = unboundError("nullable")

final override val nodePatterns: Seq[TreePattern] = Seq(PARAMETER)
}


/**
* Finds all named parameters in the given plan and substitutes them by literals of `args` values.
*/
object Parameter extends QueryErrorsBase {
def bind(plan: LogicalPlan, args: Map[String, Expression]): LogicalPlan = {
if (!args.isEmpty) {
args.filter(!_._2.isInstanceOf[Literal]).headOption.foreach { case (name, expr) =>
expr.failAnalysis(
errorClass = "INVALID_SQL_ARG",
messageParameters = Map("name" -> toSQLId(name)))
}
plan.transformAllExpressionsWithPruning(_.containsPattern(PARAMETER)) {
case Parameter(name) if args.contains(name) => args(name)
}
} else {
plan
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -4837,4 +4837,11 @@ class AstBuilder extends SqlBaseParserBaseVisitor[AnyRef] with SQLConfHelper wit
override def visitTimestampdiff(ctx: TimestampdiffContext): Expression = withOrigin(ctx) {
TimestampDiff(ctx.unit.getText, expression(ctx.startTimestamp), expression(ctx.endTimestamp))
}

/**
* Create a named parameter which represents a literal with a non-bound value and unknown type.
* */
override def visitParameterLiteral(ctx: ParameterLiteralContext): Expression = withOrigin(ctx) {
Parameter(ctx.identifier().getText)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ object TreePattern extends Enumeration {
val NULL_LITERAL: Value = Value
val SERIALIZE_FROM_OBJECT: Value = Value
val OUTER_REFERENCE: Value = Value
val PARAMETER: Value = Value
val PIVOT: Value = Value
val PLAN_EXPRESSION: Value = Value
val PYTHON_UDF: Value = Value
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1295,4 +1295,18 @@ class AnalysisSuite extends AnalysisTest with Matchers {

assertAnalysisSuccess(finalPlan)
}

test("SPARK-41271: bind named parameters to literals") {
comparePlans(
Parameter.bind(
plan = parsePlan("SELECT * FROM a LIMIT :limitA"),
args = Map("limitA" -> Literal(10))),
parsePlan("SELECT * FROM a LIMIT 10"))
// Ignore unused arguments
comparePlans(
Parameter.bind(
plan = parsePlan("SELECT c FROM a WHERE c < :param2"),
args = Map("param1" -> Literal(10), "param2" -> Literal(20))),
parsePlan("SELECT c FROM a WHERE c < 20"))
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -1568,4 +1568,30 @@ class PlanParserSuite extends AnalysisTest {
.toAggregateExpression(false, Some(GreaterThan(UnresolvedAttribute("id"), Literal(10))))
)
}

test("SPARK-41271: parsing of named parameters") {
comparePlans(
parsePlan("SELECT :param_1"),
Project(UnresolvedAlias(Parameter("param_1"), None) :: Nil, OneRowRelation()))
comparePlans(
parsePlan("SELECT abs(:1Abc)"),
Project(UnresolvedAlias(
UnresolvedFunction(
"abs" :: Nil,
Parameter("1Abc") :: Nil,
isDistinct = false), None) :: Nil,
OneRowRelation()))
comparePlans(
parsePlan("SELECT * FROM a LIMIT :limitA"),
table("a").select(star()).limit(Parameter("limitA")))
// Invalid empty name and invalid symbol in a name
checkError(
exception = parseException(s"SELECT :-"),
errorClass = "PARSE_SYNTAX_ERROR",
parameters = Map("error" -> "'-'", "hint" -> ""))
checkError(
exception = parseException(s"SELECT :"),
errorClass = "PARSE_SYNTAX_ERROR",
parameters = Map("error" -> "end of input", "hint" -> ""))
}
}
40 changes: 35 additions & 5 deletions sql/core/src/main/scala/org/apache/spark/sql/SparkSession.scala
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ import org.apache.spark.sql.catalog.Catalog
import org.apache.spark.sql.catalyst._
import org.apache.spark.sql.catalyst.analysis.UnresolvedRelation
import org.apache.spark.sql.catalyst.encoders._
import org.apache.spark.sql.catalyst.expressions.AttributeReference
import org.apache.spark.sql.catalyst.expressions.{AttributeReference, Parameter}
import org.apache.spark.sql.catalyst.plans.logical.{LocalRelation, Range}
import org.apache.spark.sql.catalyst.util.CharVarcharUtils
import org.apache.spark.sql.connector.ExternalCommandRunner
Expand Down Expand Up @@ -609,19 +609,49 @@ class SparkSession private(
* ----------------- */

/**
* Executes a SQL query using Spark, returning the result as a `DataFrame`.
* Executes a SQL query substituting named parameters by the given arguments,
* returning the result as a `DataFrame`.
* This API eagerly runs DDL/DML commands, but not for SELECT queries.
*
* @since 2.0.0
* @param sqlText A SQL statement with named parameters to execute.
* @param args A map of parameter names to literal values.
*
* @since 3.4.0
*/
def sql(sqlText: String): DataFrame = withActive {
@Experimental
def sql(sqlText: String, args: Map[String, String]): DataFrame = withActive {
val tracker = new QueryPlanningTracker
val plan = tracker.measurePhase(QueryPlanningTracker.PARSING) {
sessionState.sqlParser.parsePlan(sqlText)
val parser = sessionState.sqlParser
val parsedArgs = args.mapValues(parser.parseExpression).toMap
Parameter.bind(parser.parsePlan(sqlText), parsedArgs)
}
Dataset.ofRows(self, plan, tracker)
}

/**
* Executes a SQL query substituting named parameters by the given arguments,
* returning the result as a `DataFrame`.
* This API eagerly runs DDL/DML commands, but not for SELECT queries.
*
* @param sqlText A SQL statement with named parameters to execute.
* @param args A map of parameter names to literal values.
*
* @since 3.4.0
*/
@Experimental
def sql(sqlText: String, args: java.util.Map[String, String]): DataFrame = {
sql(sqlText, args.asScala.toMap)
}

/**
* Executes a SQL query using Spark, returning the result as a `DataFrame`.
* This API eagerly runs DDL/DML commands, but not for SELECT queries.
*
* @since 2.0.0
*/
def sql(sqlText: String): DataFrame = sql(sqlText, Map.empty[String, String])

/**
* Execute an arbitrary string command inside an external execution engine rather than Spark.
* This could be useful when user wants to execute some commands out of Spark. For
Expand Down
78 changes: 78 additions & 0 deletions sql/core/src/test/scala/org/apache/spark/sql/ParametersSuite.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
/*
* Licensed to the Apache Software Foundation (ASF) under one or more
* contributor license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright ownership.
* The ASF licenses this file to You under the Apache License, Version 2.0
* (the "License"); you may not use this file except in compliance with
* the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package org.apache.spark.sql

import org.apache.spark.sql.test.SharedSparkSession

class ParametersSuite extends QueryTest with SharedSparkSession {

test("bind parameters") {
val sqlText =
"""
|SELECT id, id % :div as c0
|FROM VALUES (0), (1), (2), (3), (4), (5), (6), (7), (8), (9) AS t(id)
|WHERE id < :constA
|""".stripMargin
val args = Map("div" -> "3", "constA" -> "4L")
checkAnswer(
spark.sql(sqlText, args),
Row(0, 0) :: Row(1, 1) :: Row(2, 2) :: Row(3, 0) :: Nil)

checkAnswer(
spark.sql("""SELECT contains('Spark \'SQL\'', :subStr)""", Map("subStr" -> "'SQL'")),
Row(true))
}

test("non-substituted parameters") {
checkError(
exception = intercept[AnalysisException] {
spark.sql("select :abc, :def", Map("abc" -> "1"))
},
errorClass = "UNBOUND_SQL_PARAMETER",
parameters = Map("name" -> "`def`"),
context = ExpectedContext(
fragment = ":def",
start = 13,
stop = 16))
checkError(
exception = intercept[AnalysisException] {
sql("select :abc").collect()
},
errorClass = "UNBOUND_SQL_PARAMETER",
parameters = Map("name" -> "`abc`"),
context = ExpectedContext(
fragment = ":abc",
start = 7,
stop = 10))
}

test("non-literal argument of `sql()`") {
Seq("col1 + 1", "CAST('100' AS INT)", "map('a', 1, 'b', 2)", "array(1)").foreach { arg =>
checkError(
exception = intercept[AnalysisException] {
spark.sql("SELECT :param1 FROM VALUES (1) AS t(col1)", Map("param1" -> arg))
},
errorClass = "INVALID_SQL_ARG",
parameters = Map("name" -> "`param1`"),
context = ExpectedContext(
fragment = arg,
start = 0,
stop = arg.length - 1))
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -229,7 +229,7 @@ private[sql] trait SQLTestUtilsBase
protected def sparkContext = spark.sparkContext

// Shorthand for running a query using our SQLContext
protected lazy val sql = spark.sql _
protected lazy val sql: String => DataFrame = spark.sql _

/**
* A helper object for importing SQL implicits.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
package org.apache.spark.sql.execution.benchmark

import org.apache.spark.benchmark.Benchmark
import org.apache.spark.sql.SparkSession
import org.apache.spark.sql.{DataFrame, SparkSession}
import org.apache.spark.sql.hive.test.TestHive

/**
Expand All @@ -40,7 +40,7 @@ object InsertIntoHiveTableBenchmark extends SqlBasedBenchmark {

val tempView = "temp"
val numRows = 1024 * 10
val sql = spark.sql _
val sql: String => DataFrame = spark.sql _

// scalastyle:off hadoopconfiguration
private val hadoopConf = spark.sparkContext.hadoopConfiguration
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import scala.concurrent.duration._
import org.apache.hadoop.hive.ql.udf.generic.GenericUDAFPercentileApprox

import org.apache.spark.benchmark.Benchmark
import org.apache.spark.sql.{Column, SparkSession}
import org.apache.spark.sql.{Column, DataFrame, SparkSession}
import org.apache.spark.sql.catalyst.expressions.Literal
import org.apache.spark.sql.catalyst.expressions.aggregate.ApproximatePercentile
import org.apache.spark.sql.hive.execution.TestingTypedCount
Expand All @@ -46,7 +46,7 @@ object ObjectHashAggregateExecBenchmark extends SqlBasedBenchmark {

override def getSparkSession: SparkSession = TestHive.sparkSession

private val sql = spark.sql _
private val sql: String => DataFrame = spark.sql _
import spark.implicits._

private def hiveUDAFvsSparkAF(N: Int): Unit = {
Expand Down