Skip to content

Commit 1659122

Browse files
committed
Revert "[SPARK-56750] default path config"
This reverts commit 6cc6e8a.
1 parent a5a8aa4 commit 1659122

23 files changed

Lines changed: 145 additions & 817 deletions

File tree

.cursor/rules/apache-spark-scala-lint-before-commit.mdc

Lines changed: 0 additions & 11 deletions
This file was deleted.

AGENTS.md

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,6 @@ Before the first code edit or running test in a session, ensure a clean working
1212
- **New edits**: ask the user to choose: create a new git worktree from `<upstream>/master` and work from there (recommended), or create and switch to a new branch from `<upstream>/master`.
1313
- **Running tests**: use `<upstream>/master`.
1414

15-
5. **Commits:** Before committing or pushing changes that touch Scala (or when unsure), run `./dev/lint-scala` from the repo root and fix all failures. Do not consider a change merge-ready until lint passes; CI runs the same checks.
16-
1715
## Development Notes
1816

1917
SQL golden file tests are managed by `SQLQueryTestSuite` and its variants. Read the class documentation before running or updating these tests. DO NOT edit the generated golden files (`.sql.out`) directly. Always regenerate them when needed, and carefully review the diff to make sure it's expected.

sql/api/src/main/antlr4/org/apache/spark/sql/catalyst/parser/SqlBaseParser.g4

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -217,10 +217,6 @@ singleTableSchema
217217
: colTypeList EOF
218218
;
219219

220-
singlePathElementList
221-
: pathElement (COMMA pathElement)* EOF
222-
;
223-
224220
singleRoutineParamList
225221
: colDefinitionList EOF
226222
;

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/Analyzer.scala

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1976,15 +1976,14 @@ class Analyzer(
19761976
* This is used for special syntax transformations (e.g., COUNT(*) -> COUNT(1)) that
19771977
* should only apply to builtin functions, not to user-defined functions.
19781978
*
1979-
* When the effective SQL PATH puts `system.session` before `system.builtin`, temp
1980-
* functions shadow builtins, so an unqualified name that matches a temp function
1981-
* should NOT be treated as builtin.
1979+
* In legacy mode (sessionOrder="first"), temp functions shadow builtins, so an
1980+
* unqualified name that matches a temp function should NOT be treated as builtin.
19821981
*/
19831982
private def matchesFunctionName(nameParts: Seq[String], expectedName: String): Boolean = {
19841983
if (!FunctionResolution.isUnqualifiedOrBuiltinFunctionName(nameParts, expectedName)) {
19851984
return false
19861985
}
1987-
if (nameParts.size == 1 && functionResolution.isSessionBeforeBuiltinInPath) {
1986+
if (nameParts.size == 1 && conf.sessionFunctionResolutionOrder == "first") {
19881987
val v1Catalog = catalogManager.v1SessionCatalog
19891988
!v1Catalog.isTemporaryFunction(FunctionIdentifier(nameParts.head))
19901989
} else {

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/FunctionResolution.scala

Lines changed: 0 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -76,22 +76,6 @@ class FunctionResolution(
7676
nameParts.length == 3 &&
7777
nameParts.head.equalsIgnoreCase(CatalogManager.SYSTEM_CATALOG_NAME)
7878

79-
/**
80-
* True iff `system.session` is searched before `system.builtin` in the effective SQL PATH.
81-
*
82-
* Drives the `count(*) -> count(1)` rewrite (which must skip transformation when a temp
83-
* `count` shadows the builtin) and the `SessionCatalog` security check that blocks creating
84-
* a temp function with a builtin's name. Reads the live PATH via `CatalogManager` and
85-
* applies the same kinds extraction that drives `SessionCatalog`'s fast-path provider, so
86-
* the predicate stays in sync with the lookup loop's actual order.
87-
*/
88-
def isSessionBeforeBuiltinInPath: Boolean = {
89-
val path = catalogManager.sqlResolutionPathEntries(
90-
catalogManager.currentCatalog.name(), catalogManager.currentNamespace.toSeq)
91-
CatalogManager.systemFunctionKindsFromPath(path).headOption
92-
.contains(org.apache.spark.sql.catalyst.catalog.SessionCatalog.Temp)
93-
}
94-
9579
/**
9680
* Produces the ordered list of candidate names for resolution. Expansion happens in two cases:
9781
*

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/resolver/FunctionResolver.scala

Lines changed: 5 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -20,20 +20,17 @@ package org.apache.spark.sql.catalyst.analysis.resolver
2020
import scala.util.Random
2121

2222
import org.apache.spark.sql.AnalysisException
23-
import org.apache.spark.sql.catalyst.FunctionIdentifier
2423
import org.apache.spark.sql.catalyst.analysis.{
2524
FunctionResolution,
2625
UnresolvedFunction,
27-
UnresolvedSeed,
28-
UnresolvedStar
26+
UnresolvedSeed
2927
}
3028
import org.apache.spark.sql.catalyst.expressions.{
3129
BinaryArithmetic,
3230
Collate,
3331
Expression,
3432
ExpressionWithRandomSeed,
3533
InheritAnalysisRules,
36-
Literal,
3734
ResolvedCollation,
3835
TryEval,
3936
UnresolvedCollation,
@@ -63,7 +60,7 @@ import org.apache.spark.sql.catalyst.util.CollationFactory
6360
*/
6461
class FunctionResolver(
6562
expressionResolver: ExpressionResolver,
66-
protected val functionResolution: FunctionResolution,
63+
functionResolution: FunctionResolution,
6764
aggregateExpressionResolver: AggregateExpressionResolver,
6865
binaryArithmeticResolver: BinaryArithmeticResolver)
6966
extends TreeNodeResolver[UnresolvedFunction, Expression]
@@ -120,10 +117,9 @@ class FunctionResolver(
120117
* - Apply timezone, if the resulting expression is [[TimeZoneAwareExpression]].
121118
*/
122119
override def resolve(unresolvedFunction: UnresolvedFunction): Expression = {
123-
val effectiveUnresolved = restoreCountStarAfterParserForTempShadow(unresolvedFunction)
124120
val expressionInfo = functionResolution.lookupBuiltinOrTempFunction(
125-
nameParts = effectiveUnresolved.nameParts,
126-
unresolvedFunc = Some(effectiveUnresolved)
121+
nameParts = unresolvedFunction.nameParts,
122+
unresolvedFunc = Some(unresolvedFunction)
127123
)
128124
val expressionResolutionContext = expressionResolutionContextStack.peek()
129125

@@ -137,7 +133,7 @@ class FunctionResolver(
137133

138134
val functionWithResolvedChildren =
139135
withResolvedChildren(
140-
unresolvedExpression = effectiveUnresolved,
136+
unresolvedExpression = unresolvedFunction,
141137
resolveChild = expressionResolver.resolve _
142138
).asInstanceOf[UnresolvedFunction]
143139

@@ -149,33 +145,6 @@ class FunctionResolver(
149145
}
150146
}
151147

152-
/**
153-
* SQL parsing turns `COUNT(*)` into `COUNT(1)` before analysis ([[AstBuilder]]). When PATH
154-
* orders session before builtin and a temporary `count` shadows the builtin, that rewrite must
155-
* be undone so arguments expand from the child operator output (aligning with fixed-point
156-
* [[org.apache.spark.sql.catalyst.analysis.Analyzer.ResolveReferences]] / `matchesFunctionName`).
157-
*/
158-
private def restoreCountStarAfterParserForTempShadow(
159-
unresolvedFunction: UnresolvedFunction): UnresolvedFunction = {
160-
if (unresolvedFunction.nameParts.length != 1 ||
161-
unresolvedFunction.isDistinct ||
162-
!unresolvedFunction.nameParts.head.equalsIgnoreCase("count")) {
163-
return unresolvedFunction
164-
}
165-
unresolvedFunction.arguments match {
166-
case Seq(lit: Literal)
167-
if lit.value != null &&
168-
lit.value == 1 &&
169-
functionResolution.isSessionBeforeBuiltinInPath &&
170-
functionResolution.catalogManager.v1SessionCatalog.isTemporaryFunction(
171-
FunctionIdentifier(unresolvedFunction.nameParts.head)) =>
172-
val expanded = expressionResolver.expandStarExpressions(Seq(UnresolvedStar(None)))
173-
unresolvedFunction.copy(arguments = expanded)
174-
case _ =>
175-
unresolvedFunction
176-
}
177-
}
178-
179148
private def handlePartiallyResolvedFunction(partiallyResolvedFunction: Expression): Expression = {
180149
val expressionResolutionContext = expressionResolutionContextStack.peek()
181150
val resolvedFunction = partiallyResolvedFunction match {

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/resolver/FunctionResolverUtils.scala

Lines changed: 1 addition & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -19,9 +19,7 @@ package org.apache.spark.sql.catalyst.analysis.resolver
1919

2020
import java.util.Locale
2121

22-
import org.apache.spark.sql.catalyst.FunctionIdentifier
2322
import org.apache.spark.sql.catalyst.analysis.{
24-
FunctionResolution,
2523
ResolvedStar,
2624
Star,
2725
UnresolvedFunction,
@@ -37,7 +35,6 @@ import org.apache.spark.sql.internal.SQLConf
3735
*/
3836
trait FunctionResolverUtils {
3937
protected def expressionResolver: ExpressionResolver
40-
protected def functionResolution: FunctionResolution
4138
protected def conf: SQLConf
4239

4340
private val scopes = expressionResolver.getNameScopes
@@ -102,21 +99,7 @@ trait FunctionResolverUtils {
10299
unresolvedFunction: UnresolvedFunction,
103100
normalizeFunctionName: Boolean = true
104101
): Boolean = {
105-
!unresolvedFunction.isDistinct &&
106-
isCount(unresolvedFunction, normalizeFunctionName) &&
107-
!isUnqualifiedCountShadowedByTemp(unresolvedFunction)
108-
}
109-
110-
/**
111-
* Keep single-pass behavior aligned with fixed-point: when PATH puts system.session before
112-
* system.builtin and a temp `count` exists, unqualified `count(*)` must not be rewritten to
113-
* `count(1)`.
114-
*/
115-
private def isUnqualifiedCountShadowedByTemp(unresolvedFunction: UnresolvedFunction): Boolean = {
116-
unresolvedFunction.nameParts.length == 1 &&
117-
functionResolution.isSessionBeforeBuiltinInPath &&
118-
functionResolution.catalogManager.v1SessionCatalog
119-
.isTemporaryFunction(FunctionIdentifier(unresolvedFunction.nameParts.head))
102+
!unresolvedFunction.isDistinct && isCount(unresolvedFunction, normalizeFunctionName)
120103
}
121104

122105
private def isCount(

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/resolver/HigherOrderFunctionResolver.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ import org.apache.spark.sql.errors.QueryCompilationErrors
3333
*/
3434
class HigherOrderFunctionResolver(
3535
protected val expressionResolver: ExpressionResolver,
36-
protected val functionResolution: FunctionResolution)
36+
functionResolution: FunctionResolution)
3737
extends TreeNodeResolver[UnresolvedFunction, Expression]
3838
with ProducesUnresolvedSubtree
3939
with CoercesExpressionTypes

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/resolver/ResolutionValidator.scala

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -107,9 +107,6 @@ class ResolutionValidator {
107107
validateRepartitionByExpression(repartitionByExpression)
108108
case sample: Sample =>
109109
validateSample(sample)
110-
case deserializeToObject: DeserializeToObject =>
111-
validate(deserializeToObject.child)
112-
handleOperatorOutput(deserializeToObject)
113110
case generate: Generate =>
114111
validateGenerate(generate)
115112
case expand: Expand =>
@@ -121,9 +118,6 @@ class ResolutionValidator {
121118
validateRelation(multiInstanceRelation)
122119
case supervisingCommand: SupervisingCommand =>
123120
validateSupervisingCommand(supervisingCommand)
124-
case cmd: Command if cmd.children.isEmpty =>
125-
// Session side-effect commands (e.g. SET PATH) are leaves with no query subtree.
126-
()
127121
}
128122

129123
operator match {

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/resolver/Resolver.scala

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -136,15 +136,6 @@ class Resolver(
136136
*/
137137
private val planRewriter = new PlanRewriter(planRewriteRules, extendedRewriteRules)
138138

139-
/**
140-
* Runs session post-resolution rules (see [[extendedRewriteRules]]) on a subtree. Used for
141-
* operators such as [[DeserializeToObject]] whose deserializer is resolved by the same rules as
142-
* the end-of-resolution [[PlanRewriter]] batch, but which must be applied before single-pass
143-
* validation asserts the operator is fully resolved.
144-
*/
145-
private def applyExtendedRewriteRulesLocally(plan: LogicalPlan): LogicalPlan =
146-
extendedRewriteRules.foldLeft(plan) { case (p, rule) => rule(p) }
147-
148139
/**
149140
* [[relationMetadataProvider]] is used to resolve metadata for relations. It's initialized with
150141
* the default implementation [[MetadataResolver]] here and is called in
@@ -334,9 +325,6 @@ class Resolver(
334325
resolveRepartition(repartition)
335326
case sample: Sample =>
336327
resolveSample(sample)
337-
case deserializeToObject: DeserializeToObject =>
338-
applyExtendedRewriteRulesLocally(
339-
deserializeToObject.copy(child = resolve(deserializeToObject.child)))
340328
case _ =>
341329
tryDelegateResolutionToExtension(unresolvedPlan).getOrElse {
342330
handleUnmatchedOperator(unresolvedPlan)

0 commit comments

Comments
 (0)