uros-b commented on code in PR #58450:
URL: https://github.com/apache/spark/pull/58450#discussion_r3904556193
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/Analyzer.scala:
##########
@@ -2100,31 +2100,51 @@ class Analyzer(
case _ => false
}
+ private val sqlJsonFunctionsWithoutStarExpansion =
+ Set("json_array", "json_exists", "json_query", "json_value")
+
/**
* Checks if the given function name parts match the expected builtin
function name.
* This is used for special syntax transformations (e.g., COUNT(*) ->
COUNT(1)) that
* should only apply to builtin functions, not to user-defined functions.
*
- * When the effective SQL PATH puts `system.session` before
`system.builtin`, temp
- * functions shadow builtins, so an unqualified name that matches a temp
function
- * should NOT be treated as builtin.
+ * Mirrors function resolution precedence, including SQL PATH shadowing
for unqualified names
+ * and `spark.sql.legacy.persistentCatalogFirst` for two-part
`builtin.name` references.
*/
- private def matchesFunctionName(nameParts: Seq[String], expectedName:
String): Boolean = {
- if (!FunctionResolution.isUnqualifiedOrBuiltinFunctionName(nameParts,
expectedName)) {
- return false
- }
- if (nameParts.size == 1 &&
functionResolution.isSessionBeforeBuiltinInPath) {
- val v1Catalog = catalogManager.v1SessionCatalog
- !v1Catalog.isTemporaryFunction(FunctionIdentifier(nameParts.head))
- } else {
- true
+ private def matchesFunctionName(nameParts: Seq[String], expectedName:
String): Boolean =
+ functionResolution.functionNameResolvesToBuiltin(nameParts, expectedName)
+
+ private def isSqlJsonFunctionWithStarExpansionDisallowed(nameParts:
Seq[String]): Boolean = {
+ sqlJsonFunctionsWithoutStarExpansion.exists { functionName =>
+ matchesFunctionName(nameParts, functionName)
}
}
/**
* Expands the matching attribute.*'s in `child`'s output.
*/
def expandStarExpression(expr: Expression, child: LogicalPlan): Expression
= {
Review Comment:
The expr.foreach pre-check uses containsStar(f.arguments) which recurses
into every sub-expression, including nested aggregate calls. As a result,
SELECT json_array(count(*)) FROM t (which was valid before this PR, the parser
built JsonArray([count(*)]) and the count-star rule converted count(*) to
count(1)) now throws INVALID_USAGE_OF_STAR_OR_REGEX. The foreach fires on the
outer json_array node before the transformUp count-star normalization can run.
There is no test covering json_array(count(*)). The fix is to either (a) run
the count-star normalization in a preparatory transformUp pass before the JSON
star check, or (b) restrict the foreach check to cases where the star would
actually be expanded rather than converted (i.e., exclude arguments where the
only stars are inside count(...) aggregate calls). This is a genuine behavioral
regression against the JSON constructor functions that were just merged.
--
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]
