HeartSaVioR commented on PR #59909: URL: https://github.com/apache/spark/pull/59909#issuecomment-6513022608
> [!NOTE] > **DISCLAIMER:** This comment was written by an LLM, Claude Opus 5.5 (`claude-opus-5-5`, 1M context), posting on behalf of @HeartSaVioR. Thanks for the review. Changes are in [73b62b7c559](https://github.com/apache/spark/pull/59909/commits/73b62b7c559b90caf5ac9088417679591118c5d5). **Where the lookup join forms.** Lookup joins now form at the start of optimization, so the analyzed plan stays a `Join`. One classification, `ExternalLookupJoinEligibility`, backs both steps, so the analysis-time errors and the conversion always agree: - `CheckExternalLookupJoin` is a check function over the analyzed plan that leaves the plan unchanged. It raises `STATIC_LEFT_SIDE`, `LOOKUP_TABLE_ON_LEFT`, `JOIN_TYPE` and `CONFLICTING_JOIN_STRATEGY` when the DataFrame is built. It considers only resolved joins (`Join.resolved`), so `DeduplicateRelations` and CheckAnalysis's own join checks run and report first. - `FormExternalLookupJoin`, which replaces `ResolveExternalLookupJoin`, converts each eligible join into an `ExternalLookupJoin` at the start of optimization, before Finish Analysis. It therefore sees the same shapes the check saw: no hint or alias eliminated, no predicate pushed into the join and no column pruned. It also raises `MISSING_JOIN_CONDITION` for a join without a condition, including one whose condition is in a later filter. `Dataset.joinWith`, self-join resolution, `joinAsOf` and the other analyzer rules and checks that match a `Join` see an ordinary analyzed `Join`. SPARK-60690 registers the check next to `TableCapabilityCheck` and the rule ahead of Finish Analysis. It adds end-to-end tests for `joinWith`, one lookup DataFrame used in two joins, `joinAsOf`, the ambiguous self-join and a non-boolean condition. **Shared repair plan 1 (read-schema mapping).** `PrepareExternalLookupJoin` now binds each read-schema field to the table column with the same exact name, and the data type must match ignoring nullability, with CHAR and VARCHAR read as STRING. `lookupInput` describes the connector's value rows: it follows the read schema's order, types and nullability, and keeps the relation's exprIds and CHAR/VARCHAR metadata. `lookupOutput` is the analyzed lookup attributes in analyzed order, so the prepared node has the same output as the join it replaces, and the left outer widening is unchanged. The read schema may list the columns in any order, but it must contain every table column, since the lookup builder is not pruned yet. A missing column fails with the new `DATA_SOURCE_INVALID_EXTERNAL_LOOKUP.MISSING_SCAN_FIELD`, which names it. This also covers the residual's lookup columns. Weaker nullability is still accepted, and the table's NOT NULL stays authoritative above the node, as it does a bove an ordinary V2 scan. The connector API documentation in SPARK-60688 states the same contract. **Shared repair plan 2 (error texts).** `CONFLICTING_JOIN_STRATEGY` and `LOOKUP_PLAN` now describe only what these rules accept. `UNKNOWN_SCAN_FIELD` says the field matches no table column by name and data type. When Spark has nothing to offer, the join fails with the new `EXTERNAL_LOOKUP_JOIN_UNSUPPORTED.NO_LOOKUP_CANDIDATE` before the connector is called. `NO_LOOKUP_PREDICATE` is left for a connector that selects none of a non-empty offer. **Tests.** `PrepareExternalLookupJoinSuite` now extends `PlanTest` and uses a configurable stub over an `ExternalLookupRelation`, and `FormExternalLookupJoinSuite` (formerly `ResolveExternalLookupJoinSuite`) analyzes its plans, so hints are `ResolvedHint` nodes and the lookup side is an `ExternalLookupRelation`. The connector-contract errors that SPARK-60690 already covers end to end are not repeated here. -- 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]
