cloud-fan commented on code in PR #52334:
URL: https://github.com/apache/spark/pull/52334#discussion_r2429726444
##########
sql/core/src/main/scala/org/apache/spark/sql/execution/SparkSqlParser.scala:
##########
@@ -50,9 +54,152 @@ class SparkSqlParser extends AbstractSqlParser {
private val substitutor = new VariableSubstitution()
+ /**
+ * Parse SQL with explicit parameter context, avoiding thread-local usage.
+ * This is the preferred method for parsing SQL with parameters.
+ *
+ * @param command The SQL text to parse
+ * @param parameterContext The parameter context containing parameter values
+ * @param toResult Function to convert the parser result
+ * @return The parsed result
+ */
+ def parseWithParameters[T](
+ command: String,
+ parameterContext: ParameterContext)
+ (toResult: SqlBaseParser => T): T = {
+ parseInternal(command, Some(parameterContext), isTopLevel = true)(toResult)
+ }
+
+ /**
+ * Parse SQL plan with explicit parameter context, avoiding thread-local
usage.
+ * This is the preferred method for parsing SQL plans with parameters.
+ *
+ * @param sqlText The SQL text to parse
+ * @param parameterContext The parameter context containing parameter values
+ * @return The parsed logical plan
+ */
+ override def parsePlanWithParameters(
+ sqlText: String,
+ parameterContext: ParameterContext): LogicalPlan = {
+ parseWithParameters(sqlText, parameterContext) { parser =>
+ val ctx = parser.compoundOrSingleStatement()
+ withErrorHandling(ctx, Some(sqlText)) {
+ astBuilder.visitCompoundOrSingleStatement(ctx) match {
+ case compoundBody: CompoundPlanStatement => compoundBody
+ case plan: LogicalPlan => plan
+ case _ =>
+ val position = Origin(None, None)
+ throw QueryParsingErrors.sqlStatementUnsupportedError(sqlText,
position)
+ }
+ }
+ }
+ }
+
protected override def parse[T](command: String)(toResult: SqlBaseParser =>
T): T = {
- super.parse(substitutor.substitute(command))(toResult)
+ parseInternal(command, None, isTopLevel = true)(toResult)
}
+
+ /**
+ * Internal parse method that handles both parameter substitution and
regular parsing.
+ *
+ * @param command The SQL text to parse
+ * @param parameterContext Optional parameter context for parameter
substitution
+ * @param isTopLevel Whether this is a top-level parse (vs identifier/data
type parsing)
+ * @param toResult Function to convert the parser result
+ * @return The parsed result
+ */
+ private def parseInternal[T](
+ command: String,
+ parameterContext: Option[ParameterContext],
+ isTopLevel: Boolean)
+ (toResult: SqlBaseParser => T): T = {
+
+ // Step 1: Check if parameter substitution should occur.
+ val (paramSubstituted, substitutionOccurred, hasParameters) =
parameterContext match {
+ case Some(context) if isTopLevel =>
+ if (SQLConf.get.legacyParameterSubstitutionConstantsOnly) {
+ // Legacy mode: skip parameter substitution but still detect
parameters for context.
+ // Parameters detected but substitution skipped in legacy mode.
+ // Position mapping will be set up below.
+ (command, false, true)
+ } else {
+ // Modern mode: perform parameter substitution if parameters are
present.
+ val substituted = substituteParametersOrSetupCallback(command,
context)
+ (substituted, substituted != command, true) // Track if substitution
occurred.
+ }
+ case _ =>
+ // No parameter context or not top-level - no parameter substitution.
+ (command, false, false)
+ }
+
+ // Step 2: Apply existing variable substitution.
+ val variableSubstituted = substitutor.substitute(paramSubstituted)
+
+ // Step 3: Set up origin with original SQL text for parameter-aware error
reporting.
+ val currentOrigin = CurrentOrigin.get
+ val originToUse = if ((substitutionOccurred || hasParameters) &&
isTopLevel) {
+ // Set up origin with the substituted SQL text for proper error reporting
+ currentOrigin.copy(
+ sqlText = Some(variableSubstituted), // Use substituted SQL text
+ startIndex = Some(0),
+ stopIndex = Some(variableSubstituted.length - 1)
+ )
+ } else {
+ // No substitution or nested call - use existing origin unchanged.
+ currentOrigin
+ }
+
+ CurrentOrigin.withOrigin(originToUse) {
+ super.parse(variableSubstituted)(toResult)
+ }
+ }
+
+ private def substituteParametersOrSetupCallback(
+ command: String,
+ context: ParameterContext): String = {
+
+ // Check legacy configuration - if true, skip parameter substitution
during parsing.
+ if (SQLConf.get.legacyParameterSubstitutionConstantsOnly) {
+ // In legacy mode, return original command without substitution.
+ command
+ } else {
+ ParameterHandler.substituteParameters(command, context)
+ }
+ }
+
+ /**
+ * Internal parse method for identifiers and data types that bypasses
parameter logic.
+ * This ensures clean parsing without parameter substitution side effects.
Review Comment:
do we need to? The `def parse[T]` calls `parseInternal` without parameter
context.
--
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]
