peter-toth opened a new pull request, #57601:
URL: https://github.com/apache/spark/pull/57601
### What changes were proposed in this pull request?
Backport of #57563 to `branch-3.5`.
`ResolveRelations.resolveRelation` in `Analyzer.scala` 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 writePrivilegesString =
Option(u.options.get(UnresolvedRelation.REQUIRED_WRITE_PRIVILEGES))
val cached = if (writePrivilegesString.isEmpty) {
AnalysisContext.get.relationCache.get(key)
} else {
None
}
cached.map { cache => ... }.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:
- `branch-3.5` has no `RelationResolution.scala` at all - relation
resolution still lives in the `ResolveRelations` rule in `Analyzer.scala`, so
that is where the fix goes. The cache-then-load sequence, and therefore the
bug, is the same.
- The local is `writePrivilegesString: Option[String]` here, not a raw
`String`, so the guard reads `if (writePrivilegesString.isEmpty)` rather than
`if (writePrivileges == null)`.
- The comment block is wrapped for this file's indentation.
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-3.5` 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: 15 suites / 794 tests - `DataSourceV2SQLSuite`
(V1+V2 filter), `DataSourceV2DataFrameSuite`, `PlanResolutionSuite`,
`CachedTableSuite`, `*ViewTestSuite`, group/delta-based MERGE/UPDATE/DELETE
suites, `InsertSuite`. `DataSourceV2OptionSuite` and `MergeIntoDataFrameSuite`
do not exist on this branch. `dev/lint-scala` clean. Built and tested on JDK 17.
### 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]
