peter-toth opened a new pull request, #57599:
URL: https://github.com/apache/spark/pull/57599
### What changes were proposed in this pull request?
Backport of #57563 to `branch-4.1`.
`RelationResolution.resolveRelation` answers a hit in the per-query
`AnalysisContext.relationCache` before reaching the branch that calls
`CatalogV2Util.loadTable(catalog, ident, timeTravelSpec, writePrivileges)`.
This PR stops serving a reference that carries `REQUIRED_WRITE_PRIVILEGES` from
that cache, so a write target always goes through
`TableCatalog.loadTable(ident, writePrivileges)`:
```scala
val writePrivileges =
u.options.get(UnresolvedRelation.REQUIRED_WRITE_PRIVILEGES)
val cached = if (writePrivileges == null) relationCache.get(key) else None
cached.map(adaptCachedRelation(_, planId)).orElse { ... }
```
Loaded relations are still *stored* in the cache, so a self-MERGE's source
keeps sharing the target's `Table` instance (one snapshot).
### Backport notes
The cherry-pick of `a1c3f310a8c` does not apply cleanly, so the change was
re-applied by hand:
- On `master` the affected code lives in
`RelationResolution.tryResolvePersistent`, extracted alongside
`relationResolutionSteps` and the `RelationCatalog.loadRelation` fast path.
None of that exists on `branch-4.1`, where the same cache-then-load sequence
still sits inline in `resolveRelation`, one nesting level deeper inside
`resolveTempView(...).orElse { expandIdentifier(...) match ... }`. The change
itself is unchanged: hoist `writePrivileges` above the cache lookup and skip
the lookup when it is set.
- The comment block is rewrapped, because the extra two spaces of
indentation push one line past 100 characters.
The test hunk applies unchanged.
### Why are the changes needed?
`TableCatalog.loadTable(Identifier, Set<TableWritePrivilege>)` (`@since
3.5.3`) is where a catalog authorizes a write; the only other caller in the
tree is `ResolveSchemaEvolution`. A write target hits the relation cache
whenever another reference to the same table was resolved earlier in the same
analyzer run, which is exactly the case for `InsertIntoStatement` and
`V2WriteCommand` - their target is resolved at the command node *after* its
query. So the catalog is never asked to authorize these statements:
```sql
INSERT INTO t SELECT * FROM t
INSERT OVERWRITE t SELECT * FROM t WHERE 1 = 0 -- empties the table
INSERT INTO t REPLACE WHERE i = 0 SELECT * FROM t
INSERT INTO t SELECT i FROM (SELECT i FROM t) x
WITH c AS (SELECT i FROM t) INSERT INTO t SELECT i FROM c
```
```scala
spark.table("t").writeTo("t").append() // DataFrameWriterV2
passes the unanalyzed plan
```
`INSERT INTO t SELECT 1` on the same table is authorized normally, and
`UPDATE` / `DELETE` / `MERGE` are unaffected (their target is a plain child,
resolved before the source, so it is a cache miss), as is
`DataFrameWriter.insertInto` (it passes the already-analyzed plan).
### Does this PR introduce _any_ user-facing change?
Yes. On a catalog that enforces privileges in `loadTable(ident,
writePrivileges)`, the statements above are now authorized like every other
write, so a user without the required privilege gets the catalog's error
instead of silently writing. Catalogs that ignore the privileges argument
(including Spark's built-in `V2SessionCatalog`) are unaffected. The write
target of a self-referencing INSERT is now loaded once more, which is what a
non-self-referencing INSERT already does.
### How was this patch tested?
New test in `DataSourceV2SQLSuite`, identical to the one on `master`. It
fails on `branch-4.1` without the fix ("Expected exception
java.lang.RuntimeException to be thrown, but no exception was thrown") and
passes with it.
Also green on this branch: 17 suites / 1162 tests - `DataSourceV2SQLSuite`
(V1+V2 filter), `DataSourceV2DataFrameSuite`, `DataSourceV2OptionSuite`,
`PlanResolutionSuite`, `CachedTableSuite`, `*ViewTestSuite`, group/delta-based
MERGE/UPDATE/DELETE suites, `MergeIntoDataFrameSuite`, `InsertSuite`.
`dev/lint-scala` clean.
### 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]
