Vivek1106-04 opened a new pull request, #58821: URL: https://github.com/apache/spark/pull/58821
### What changes were proposed in this pull request? During routine resolution, `FunctionResolution` caught `AnalysisException` whose condition is `FORBIDDEN_OPERATION` and treated it as a "not found" miss, continuing to the next candidate in the search path. This PR makes a denial distinguishable from an absence, and reports it when resolution ultimately fails. The JIRA lists two options for single-part names searched across a multi-entry path: (a) always propagate `FORBIDDEN_OPERATION`, or (b) propagate it only when no other candidate resolves. This PR implements **(b)**, the defer-and-rethrow option, so that one locked-down catalog on the path cannot break resolution of a name that legitimately lives in a later entry. Changes: - a small `CandidateResult` ADT (`Resolved` / `Missing` / `Forbidden`) replaces the `Option` returned by the per-candidate helpers, which could not distinguish "denied" from "absent"; - each candidate loop remembers the first denial and throws it in place of `UNRESOLVED_ROUTINE` if no candidate resolves; - `FunctionType.Forbidden(error)` is added and thrown by the `LookupFunctions` analyzer rule. That rule raises `UNRESOLVED_ROUTINE` before `resolveFunction` runs, so the fix is incomplete without it. The five sites named in the JIRA are covered: `resolveFunctionCandidate`, `resolveTableFunctionCandidate`, `tryRethrowNotTableFunction`, `lookupFunctionType`, `resolveProcedure`. Two notes for reviewers: - The table-function sites are changed for consistency with the JIRA's list, but I could not exercise them in a test: they fire only when the v1 session catalog raises `FORBIDDEN_OPERATION` during persistent TVF lookup, which needs a custom `ExternalCatalog`. Happy to add coverage if you can point me at an existing harness. - While tracing that, the `else if (catalog.asFunctionCatalog.functionExists(ident))` branch in `tryRethrowNotTableFunction` appears unreachable today: for a non-session catalog the `try` throws the missing-TVF-ability error rather than `NoSuchFunctionException`, so that `catch` never runs with a non-session catalog. Pre-existing, left untouched here. ### Why are the changes needed? A genuine permission error was reported as a missing routine. A user querying a function in a catalog they cannot access was told the function does not exist, rather than that they are not allowed to use it, which sends debugging in the wrong direction and hides an access-control problem. ### Does this PR introduce _any_ user-facing change? Yes, an error-condition change. When a catalog denies a routine lookup with `FORBIDDEN_OPERATION` and no other candidate on the path resolves the name: Before: ``` [UNRESOLVED_ROUTINE] Cannot resolve routine `no_such_function` on search path [`denycat`.`fns`, `system`.`builtin`]. SQLSTATE: 42883 ``` After: the catalog's own `FORBIDDEN_OPERATION` error (SQLSTATE 42809) is reported. This affects only catalogs that raise `FORBIDDEN_OPERATION` during `loadFunction` / `functionExists` / `loadProcedure`; no built-in catalog does, so queries against the session catalog and the built-in registry are unchanged. Resolution that previously succeeded still succeeds: a denial does not stop the path search. ### How was this patch tested? New suite `ForbiddenRoutineResolutionSuite`, with a `DenyingRoutineCatalog` that refuses every routine lookup the way an access-controlled catalog does: - qualified function name in a denying catalog reports `FORBIDDEN_OPERATION`; - unqualified name still resolves past a denying PATH entry to a later catalog that holds it; - unqualified name reports `FORBIDDEN_OPERATION` when no candidate resolves; - same for an unqualified procedure name via `CALL`. Control run with the fix reverted and the tests kept: tests 3 and 4 fail with `"[UNRESOLVED_ROUTINE]" did not equal "[FORBIDDEN_OPERATION]"`, reproducing the reported symptom. Tests 1 and 2 pass either way and act as regression guards. Existing suites, all passing: `LookupFunctionsSuite`, `FunctionQualificationSuite`, `SqlPathV2CatalogSuite`, `ProcedureSuite`, `StaticProcedureSuite`, `DataSourceV2FunctionSuite`, `SetPathSuite` (231 tests), plus the whole `org.apache.spark.sql.catalyst.analysis` package (1255 tests). Scalastyle clean on both modules. ### Was this patch authored or co-authored using generative AI tooling? Generated-by: Claude Code (Claude Opus 5) -- 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]
