s-maling-celonis opened a new pull request, #58856:
URL: https://github.com/apache/spark/pull/58856

   ### What changes were proposed in this pull request?
   
   https://issues.apache.org/jira/browse/SPARK-59574
   
   The single-pass Resolver never applies the hint resolution rules injected 
through `SparkSessionExtensions.injectHintResolutionRule`. 
`HybridAnalyzer.fromLegacyAnalyzer` forwards `singlePassResolverExtensions`, 
`singlePassMetadataResolverExtensions`, `singlePassPostHocResolutionRules` and 
`singlePassExtendedResolutionChecks`, but not `Analyzer.hintResolutionRules`, 
so those rules are silently dropped.
   
   This PR forwards them into `Resolver` and applies them in 
`lookupMetadataAndResolve`, after IDENTIFIER/CTE substitution and before the 
`MetadataResolver` pre-pass. That mirrors the fixed-point ordering:
   
   ```
   Substitution -> Hints -> ... -> Resolution (ResolveSQLOnFile)
   ```
   
   `HintResolutionRunner` is a small wrapper on top of `RuleExecutor` (modelled 
on the existing `PlanRewriter`) that:
   
   - runs the rules in a `"Hints"` batch with the same `FixedPoint` strategy 
the fixed-point Analyzer uses for that batch, so injected rules see identical 
iteration semantics and the same `spark.sql.analyzer.maxIterations` error;
   - applies them to the main plan, the CTE definitions and the subquery plans. 
The fixed-point Analyzer re-enters all of its batches for view bodies, CTE 
definitions and subquery plans via `executeSameContext`, so covering only the 
top-level plan would leave a parity gap. Views are covered for free, because 
`ViewResolver` re-enters `lookupMetadataAndResolve` for the view body. 
`UnresolvedWith` needs to be handled explicitly, since it does not expose its 
CTE definitions as children — the same reason `MetadataResolver` matches it 
explicitly.
   
   Ordering relative to the metadata pre-pass is the important part: the 
pre-pass resolves relation metadata eagerly, and for path-based relations 
`FileResolver` lists files there. Anything that has to rewrite an 
`UnresolvedRelation` must run before it.
   
   When no extension registers hint resolution rules, which is the default, the 
plan is returned untouched and no `RuleExecutor` runs.
   
   ### Why are the changes needed?
   
   1. Rules registered through a public, documented extension API do not run 
under the single-pass Analyzer, i.e. with 
`spark.sql.analyzer.singlePassResolver.enabled=true`, with 
`enabledTentatively=true`, or in the single-pass half of `dualRunWithLegacy`.
   
   2. Rules that must run *before* relation metadata lookup cannot work at all. 
Example: path SQL such as ``SELECT * FROM parquet.`s3://bucket/dir` ``. Under 
fixed-point a hint rule sees the `UnresolvedRelation` and can rewrite it (for 
instance to attach storage credentials) before `ResolveSQLOnFile`. Under 
single-pass, `FileResolver` has already resolved and listed the path in the 
metadata pre-pass. `injectPostHocResolutionRule` is not a substitute, as it 
runs after resolution, and `singlePassMetadataResolverExtensions` is not 
injectable through `SparkSessionExtensions`.
   
   3. Dual-run diverges. Without this change the dual-run of such a query fails 
with:
   
   ```
   [HYBRID_ANALYZER_EXCEPTION.SINGLE_PASS_FAILED_FIXED_POINT_SUCCEEDED] ...
   Single-pass resolution failed, but fixed-point resolution succeeded.
   ```
   
   This is the same class of gap as SPARK-59081 and SPARK-56453: an analyzer 
rule list that was not updated when a second analyzer path was introduced.
   
   ### Does this PR introduce _any_ user-facing change?
   
   Yes. Rules injected with `injectHintResolutionRule` now also run under the 
single-pass Resolver and in dual-run, matching the fixed-point Analyzer. 
Previously they were silently dropped. There is no change for sessions that do 
not register such rules.
   
   ### How was this patch tested?
   
   New tests in `SparkSessionExtensionSuite`:
   
   - `SPARK-59574: inject custom hint rule with single-pass resolver` — the 
existing `CONVERT_TO_EMPTY` -> `LocalRelation` hint rule, with the single-pass 
resolver enabled.
   - `SPARK-59574: hint rule rewrites path relations` (fixed-point, regression 
guard), `... with single-pass resolver` and `... in dual-run` — a hint rule 
redirects ``parquet`.`<path>`` to another directory holding a different number 
of rows, so the rewrite is observable in the query result. Each variant covers 
a plain relation, a CTE and a scalar subquery.
   
   New tests in `HiveSparkSessionExtensionSuite` (single-pass and dual-run 
variants of the existing SPARK-59081 path SQL test), covering the same 
behaviour under `enableHiveSupport()`.
   
   Verified the tests are meaningful: with the test changes kept and the three 
production files reverted to `master`, the single-pass and dual-run tests fail 
(single-pass reads the original directory, `1 did not equal 3`; dual-run raises 
`SINGLE_PASS_FAILED_FIXED_POINT_SUCCEEDED`) while the fixed-point test passes.
   
   Locally, on Java 17:
   
   - `build/sbt 'sql/testOnly org.apache.spark.sql.SparkSessionExtensionSuite'` 
— 37 tests passed.
   - `build/sbt 'hive/testOnly 
org.apache.spark.sql.hive.HiveSparkSessionExtensionSuite'` — 4 tests passed.
   - `build/sbt 'sql/testOnly org.apache.spark.sql.analysis.resolver.*'` — 220 
tests passed.
   - `catalyst/scalastyle`, `sql/Test/scalastyle`, `hive/Test/scalastyle` — no 
errors.
   
   ### Was this patch authored or co-authored using generative AI tooling?
   
   Generated-by: Cursor 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]

Reply via email to