cloud-fan commented on PR #58204:
URL: https://github.com/apache/spark/pull/58204#issuecomment-5425388828
Thanks for the thorough review. I addressed the actionable items in
`9588570d7b9`:
1. The separate source/target `loadTable` behavior is intentional. The PR
description now calls
out the two loads, distinct `Table` instances, and the
authorization/table-state implication
under the user-facing-change section.
2. I rewrote the description to cover `UnresolvedWriteTarget`, the
metadata-only resolved
carriers, `tableOptions`, write-target resolution, V1/Hive lowering,
temp-view rendering, and
the refreshed test matrix.
3. `PreprocessTableInsertion` is now session-aware and installs the derived
provider relation once.
`DataSourceAnalysis` and `PreWriteCheck` reuse that target; the check no
longer resolves a
relation or updates the session cache. The new `SparkSession.active` uses
are gone.
4. `RelationConversions` now returns the converted target to the normal
post-hoc pipeline instead
of invoking `PreprocessTableInsertion` and `DataSourceAnalysis` inline.
The stats helper moved to
the `DetermineTableStats` companion, and the redundant
already-materialized branches were
removed from `InsertWriteRelation`.
5. Persistent read/write resolution now shares `loadTableOrView`,
`mapPersistentRelation`, and the
V2 relation-construction helper, including V1 view, delegating-table, and
view-catalog handling.
6. `ResolvedTempView.stringArgs` again includes `metadata.identifier`.
Regenerating the affected
cases removed the unrelated temp-view golden diffs.
7. Non-empty `InsertIntoStatement.tableOptions` are visible in plan strings,
with a focused
assertion.
8. I kept `OpaqueLogicalPlan`. A directly typed `TemporaryViewRelation`
constructor field remains
discoverable by Catalyst's product-based tree/copy machinery even with
shallow
`equals`/`hashCode`; the non-plan wrapper keeps the stored tree opaque
while making comparison
and hashing explicitly identity-based.
9. The unrelated `CTESubstitution` import change is reverted, and the other
touched imports use the
existing single-line style.
10. The V1 fallback without catalog/identifier is restored;
`ifPartitionNotExists` is explicitly
`false`; the resolution-order comment and dangling cast formatting are
fixed; and builtin
functions are enabled only within the function-based IDENTIFIER tests.
I also caught and fixed a Hive regression during verification: an
unconverted partitioned Hive
write target was being revisited by the read-conversion branch in the same
top-down rule pass. The
rule now preserves the metadata carrier when write conversion is disabled,
and the later
preprocessing stage materializes the Hive target once.
Verification included `hive/Test/compile`, the full `HiveSQLInsertTestSuite`
and
`HiveTableRelationResolverSuite`, `PlanResolutionSuite`,
`DataSourceV2OptionSuite`,
`DataSourceAnalysisSuite`, focused `DataSourceV2SQLSuiteV1Filter` coverage,
and the six affected
execution/analyzer SQL golden cases.
--
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]
