HeartSaVioR commented on code in PR #59910: URL: https://github.com/apache/spark/pull/59910#discussion_r4419819032
########## sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/ResolveExternalLookupJoin.scala: ########## @@ -0,0 +1,100 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.spark.sql.catalyst.analysis + +import org.apache.spark.sql.catalyst.optimizer.EliminateResolvedHint +import org.apache.spark.sql.catalyst.plans.{Inner, LeftOuter} +import org.apache.spark.sql.catalyst.plans.logical.{ExternalLookupJoin, Join, JoinHint, + JoinStrategyHint, LogicalPlan} +import org.apache.spark.sql.catalyst.rules.Rule +import org.apache.spark.sql.catalyst.trees.TreePattern.JOIN +import org.apache.spark.sql.errors.{QueryCompilationErrors, QueryErrorsBase} +import org.apache.spark.sql.execution.datasources.v2.ExternalLookupRelations + +/** + * Replaces an eligible join with a lookup join holder. + * + * A join is planned as an external lookup join when its right side reads a table that supports + * external lookup but not batch read. It must be a stream-static join: the left side is a + * streaming query, whose micro-batches tolerate lookup results that change between requests, and + * the lookup-only table is on the right side. + */ +object ResolveExternalLookupJoin extends Rule[LogicalPlan] with QueryErrorsBase { + private val SupportedJoinTypes = Seq(Inner, LeftOuter) + + override def apply(plan: LogicalPlan): LogicalPlan = + plan.resolveOperatorsUpWithPruning(_.containsPattern(JOIN), ruleId) { + case join: Join if join.left.resolved && join.right.resolved => resolveLookupJoin(join) + } + + private def resolveLookupJoin(join: Join): LogicalPlan = { + val Join(left, right, joinType, condition, hint) = join + // The lookup-only table must be on the right side; the sides are never swapped. + ExternalLookupRelations.lookupOnlyTable(left).foreach { table => + throw QueryCompilationErrors.unsupportedExternalLookupJoinError( + "LOOKUP_TABLE_ON_LEFT", Map("relation" -> toSQLId(table.name()))) + } + // TODO: look through projections, filters and aliases above the lookup relation. + val lookupTable = ExternalLookupRelations.table(right) + // TODO: allow the EXTERNAL_LOOKUP hint once it is introduced, and apply these checks to the + // joins it opts in. + if (!lookupTable.exists(ExternalLookupRelations.isLookupOnly)) return join + val relation = lookupTable.map(table => toSQLId(table.name())).getOrElse("the lookup side") + if (!left.isStreaming) { + throw QueryCompilationErrors.unsupportedExternalLookupJoinError( + "STATIC_LEFT_SIDE", Map("relation" -> relation)) + } + // A join hint the user wrote is never silently dropped: an external lookup join cannot run a + // physical join strategy, and a lookup-only table has no batch scan for one. + joinStrategyHint(hint, left, right).foreach { strategy => + throw QueryCompilationErrors.unsupportedExternalLookupJoinError( + "CONFLICTING_JOIN_STRATEGY", + Map("strategy" -> toSQLStmt(strategy.displayName), "relation" -> relation)) + } + // A missing condition is reported by PrepareExternalLookupJoin, on the final plan. + if (condition.isEmpty) return join + // The join reads a lookup-only table that no other join can read, so a join that cannot be a + // lookup join fails here with the reason. + if (!SupportedJoinTypes.contains(joinType)) { + throw QueryCompilationErrors.unsupportedExternalLookupJoinError( + "JOIN_TYPE", + Map( + "joinType" -> toSQLStmt(joinType.sql), + "relation" -> relation, + "supportedJoinTypes" -> SupportedJoinTypes.map(t => toSQLStmt(t.sql)).mkString(", "))) + } + ExternalLookupJoin( Review Comment: > [!NOTE] > **DISCLAIMER:** This comment was written by an LLM, Claude Opus 5.5 (`claude-opus-5-5`, 1M context), posting on behalf of @HeartSaVioR. Fixed in #59909 ([73b62b7c559](https://github.com/apache/spark/commit/73b62b7c559b90caf5ac9088417679591118c5d5)) and [01060c7f72e](https://github.com/apache/spark/commit/01060c7f72e4e7256c177939e0e2306e463dfae0), as recommended. The analyzed plan stays a `Join`. `CheckExternalLookupJoin` raises `STATIC_LEFT_SIDE`, `LOOKUP_TABLE_ON_LEFT`, `JOIN_TYPE` and `CONFLICTING_JOIN_STRATEGY` while the DataFrame is analyzed. `FormExternalLookupJoin` converts the join at the start of optimization, before Finish Analysis. This PR registers the check next to `TableCapabilityCheck` (in both the base and Hive session state builders), puts the conversion in its own optimizer batch ahead of the pre-optimization batches, makes it non-excludable, and removes the analyzer batch. New end-to-end tests: - `joinWith` over a lookup-only table plans a lookup join (classic only; I did not add a Connect test). - A lookup DataFrame reused in a second join fails with the same ambiguous self-join error as any other reused DataFrame, with `spark.sql.selfJoinAutoResolveAmbiguity` both on and off. Loading the table twice enriches the stream with two lookup joins. - `joinAsOf` fails with `UNSUPPORTED_FEATURE.TABLE_OPERATION`, because the AS OF rewrite reads the table outside a lookup join. - A non-boolean condition fails with `JOIN_CONDITION_IS_NOT_BOOLEAN_TYPE`, as for an ordinary join. - The existing USING join test still plans a lookup join. ########## sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/v2/PrepareExternalLookupJoin.scala: ########## @@ -0,0 +1,233 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.spark.sql.execution.datasources.v2 + +import java.util.{Collections, IdentityHashMap} + +import scala.collection.mutable + +import org.apache.spark.sql.catalyst.expressions.{Alias, And, Attribute, EqualTo, Expression, + Literal, PredicateHelper} +import org.apache.spark.sql.catalyst.plans.LeftOuter +import org.apache.spark.sql.catalyst.plans.logical.{ExternalLookupJoin, Filter, Join, LocalRelation, + LogicalPlan, PreparedExternalLookupJoin, Project} +import org.apache.spark.sql.catalyst.rules.Rule +import org.apache.spark.sql.catalyst.trees.TreePattern.EXTERNAL_LOOKUP_JOIN +import org.apache.spark.sql.catalyst.util.CharVarcharUtils +import org.apache.spark.sql.connector.catalog.{SupportsRead, TableCapability} +import org.apache.spark.sql.connector.expressions.{Expression => V2Expression, FieldReference} +import org.apache.spark.sql.connector.expressions.filter.{Predicate => V2Predicate} +import org.apache.spark.sql.connector.read.{LookupParameter, SupportsExternalLookup} +import org.apache.spark.sql.errors.{QueryCompilationErrors, QueryErrorsBase} +import org.apache.spark.sql.internal.SQLConf +import org.apache.spark.sql.types.{BooleanType, DataType} + +/** Finalizes an optimized external lookup side immediately before ordinary V2 scan preparation. */ +object PrepareExternalLookupJoin extends Rule[LogicalPlan] with PredicateHelper + with QueryErrorsBase { + override def apply(plan: LogicalPlan): LogicalPlan = { + val prepared = plan.transformUpWithPruning(_.containsPattern(EXTERNAL_LOOKUP_JOIN)) { + case join: ExternalLookupJoin if provablyEmpty(join) => emptyJoinResult(join) + case join: ExternalLookupJoin => prepare(join) + } + // Prepared lookup joins no longer contain their lookup relation, so a lookup-only table that + // is still read here cannot run. Pre-order visits a join before the relation it reads. + prepared.foreach { + // Classic Dataset.join(right, usingColumns) first analyzes Join(left, right, joinType, None), + // including CheckAnalysis, to disambiguate a self-join, and only then builds the USING join. + // That plan cannot be told apart from a join without a condition, so analysis keeps it, and + // a missing condition is reported here, on the final plan. + case Join(_, right, _, None, _) if readsLookupOnlyTable(right) => + throw QueryCompilationErrors.unsupportedExternalLookupJoinError("MISSING_JOIN_CONDITION") + case node => + ExternalLookupRelations.lookupOnlyTable(node).foreach { table => + throw QueryCompilationErrors.unsupportedBatchReadError(table) + } + } + prepared + } + + private def readsLookupOnlyTable(plan: LogicalPlan): Boolean = plan match { + case node @ (_: Project | _: Filter) => readsLookupOnlyTable(node.children.head) + case node => ExternalLookupRelations.lookupOnlyTable(node).isDefined + } + + // The optimizer folds `WHERE false` on the lookup side into an empty relation, and a condition + // such as `s.id = r.id AND false` into `false`. Such a join needs no connector call. + private def provablyEmpty(join: ExternalLookupJoin): Boolean = + join.right.maxRows.contains(0L) || (join.condition match { + case Some(Literal(value, BooleanType)) => value == null || value == false + case _ => false + }) + + private def emptyJoinResult(join: ExternalLookupJoin): LogicalPlan = join.joinType match { + case LeftOuter => + val nullLookupColumns = join.right.output.map { attr => + Alias(Literal(null, attr.dataType), attr.name)(attr.exprId) + } + Project(join.left.output ++ nullLookupColumns, join.left) + case _ => LocalRelation(join.output, Seq.empty, join.isStreaming) Review Comment: > [!NOTE] > **DISCLAIMER:** This comment was written by an LLM, Claude Opus 5.5 (`claude-opus-5-5`, 1M context), posting on behalf of @HeartSaVioR. Fixed in [01060c7f72e](https://github.com/apache/spark/commit/01060c7f72e4e7256c177939e0e2306e463dfae0). An inner lookup join whose condition folds to false or null, or whose lookup side is empty, now keeps the stream side: it becomes the same null-padded projection as the left outer case under `Filter(false, ...)`, so `PruneFilters` applies its streaming guard. Still no lookup is prepared and no reader is created. The folded-false test now uses a `dropDuplicates` stream side and checks that the deduplication runs and holds its state, for both inner and left outer joins, with unchanged results. ########## sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/ApplyCharTypePadding.scala: ########## @@ -68,6 +68,11 @@ object ApplyCharTypePadding extends Rule[LogicalPlan] { case r: DataSourceV2Relation => ApplyCharTypePaddingHelper.readSidePadding(r, () => r.copy(output = r.output.map(CharVarcharUtils.cleanAttrMetadata))) + // A lookup-only table is read through an external lookup join and needs the same padding. + case r: ExternalLookupRelation => Review Comment: > [!NOTE] > **DISCLAIMER:** This comment was written by an LLM, Claude Opus 5.5 (`claude-opus-5-5`, 1M context), posting on behalf of @HeartSaVioR. Fixed in [01060c7f72e](https://github.com/apache/spark/commit/01060c7f72e4e7256c177939e0e2306e463dfae0), with a narrower change than suggested. CHAR lookup columns need lookup-side projections, which come in SPARK-60692, so this PR adds no recognizer that looks through the padding projection. Instead, `ApplyCharTypePadding` rejects a lookup-only table whose columns need read-side CHAR padding, or a VARCHAR check under standard CHAR/VARCHAR semantics. The new error is `EXTERNAL_LOOKUP_JOIN_UNSUPPORTED.CHAR_VARCHAR_COLUMN`, which names the table and the column. The table is rejected when it is read, so classic and Connect reads both get the error, instead of the batch-scan error. VARCHAR columns under default semantics still join, and `LOOKUP_PLAN` (#59909, [73b62b7c559](https://github.com/apache/spark/commit/73b62b7c559b90caf5ac9088417679591118c5d5)) now describes only what is supported. New test: CHAR, and VARCHAR under standard semantics. ########## sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/v2/PrepareExternalLookupJoin.scala: ########## @@ -0,0 +1,233 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.spark.sql.execution.datasources.v2 + +import java.util.{Collections, IdentityHashMap} + +import scala.collection.mutable + +import org.apache.spark.sql.catalyst.expressions.{Alias, And, Attribute, EqualTo, Expression, + Literal, PredicateHelper} +import org.apache.spark.sql.catalyst.plans.LeftOuter +import org.apache.spark.sql.catalyst.plans.logical.{ExternalLookupJoin, Filter, Join, LocalRelation, + LogicalPlan, PreparedExternalLookupJoin, Project} +import org.apache.spark.sql.catalyst.rules.Rule +import org.apache.spark.sql.catalyst.trees.TreePattern.EXTERNAL_LOOKUP_JOIN +import org.apache.spark.sql.catalyst.util.CharVarcharUtils +import org.apache.spark.sql.connector.catalog.{SupportsRead, TableCapability} +import org.apache.spark.sql.connector.expressions.{Expression => V2Expression, FieldReference} +import org.apache.spark.sql.connector.expressions.filter.{Predicate => V2Predicate} +import org.apache.spark.sql.connector.read.{LookupParameter, SupportsExternalLookup} +import org.apache.spark.sql.errors.{QueryCompilationErrors, QueryErrorsBase} +import org.apache.spark.sql.internal.SQLConf +import org.apache.spark.sql.types.{BooleanType, DataType} + +/** Finalizes an optimized external lookup side immediately before ordinary V2 scan preparation. */ +object PrepareExternalLookupJoin extends Rule[LogicalPlan] with PredicateHelper + with QueryErrorsBase { + override def apply(plan: LogicalPlan): LogicalPlan = { + val prepared = plan.transformUpWithPruning(_.containsPattern(EXTERNAL_LOOKUP_JOIN)) { + case join: ExternalLookupJoin if provablyEmpty(join) => emptyJoinResult(join) + case join: ExternalLookupJoin => prepare(join) + } + // Prepared lookup joins no longer contain their lookup relation, so a lookup-only table that + // is still read here cannot run. Pre-order visits a join before the relation it reads. + prepared.foreach { + // Classic Dataset.join(right, usingColumns) first analyzes Join(left, right, joinType, None), + // including CheckAnalysis, to disambiguate a self-join, and only then builds the USING join. + // That plan cannot be told apart from a join without a condition, so analysis keeps it, and + // a missing condition is reported here, on the final plan. + case Join(_, right, _, None, _) if readsLookupOnlyTable(right) => + throw QueryCompilationErrors.unsupportedExternalLookupJoinError("MISSING_JOIN_CONDITION") + case node => + ExternalLookupRelations.lookupOnlyTable(node).foreach { table => + throw QueryCompilationErrors.unsupportedBatchReadError(table) + } + } + prepared + } + + private def readsLookupOnlyTable(plan: LogicalPlan): Boolean = plan match { + case node @ (_: Project | _: Filter) => readsLookupOnlyTable(node.children.head) + case node => ExternalLookupRelations.lookupOnlyTable(node).isDefined + } + + // The optimizer folds `WHERE false` on the lookup side into an empty relation, and a condition + // such as `s.id = r.id AND false` into `false`. Such a join needs no connector call. + private def provablyEmpty(join: ExternalLookupJoin): Boolean = + join.right.maxRows.contains(0L) || (join.condition match { + case Some(Literal(value, BooleanType)) => value == null || value == false + case _ => false + }) + + private def emptyJoinResult(join: ExternalLookupJoin): LogicalPlan = join.joinType match { + case LeftOuter => + val nullLookupColumns = join.right.output.map { attr => + Alias(Literal(null, attr.dataType), attr.name)(attr.exprId) + } + Project(join.left.output ++ nullLookupColumns, join.left) + case _ => LocalRelation(join.output, Seq.empty, join.isStreaming) + } + + private def prepare(join: ExternalLookupJoin): PreparedExternalLookupJoin = { + val relation = extractLookupPlan(join.right).getOrElse { + throw QueryCompilationErrors.unsupportedExternalLookupJoinError("LOOKUP_PLAN") + } + val tableName = relation.table.name() + val supportsRead = relation.table match { + case table: SupportsRead + if table.capabilities().contains(TableCapability.EXTERNAL_LOOKUP) => table + case _ => + throw QueryCompilationErrors.unsupportedExternalLookupJoinError( + "TABLE", Map("table" -> toSQLId(tableName))) + } + val scanBuilder = supportsRead.newScanBuilder(relation.options) + val lookupBuilder = scanBuilder match { + case builder: SupportsExternalLookup => builder + case _ => + throw QueryCompilationErrors.unsupportedExternalLookupJoinError( + "SCAN_BUILDER", Map("table" -> toSQLId(tableName))) + } + + val conjuncts = join.condition.toSeq.flatMap(splitConjunctivePredicates) + val offeredCandidates = mutable.ArrayBuffer.empty[OfferedLookupPredicate] + val residual = mutable.ArrayBuffer.empty[Expression] + conjuncts.foreach { + case equality @ EqualTo(leftExpr, rightAttr: Attribute) + if leftExpr.references.subsetOf(join.left.outputSet) && + join.right.outputSet.contains(rightAttr) => + offeredCandidates += OfferedLookupPredicate(leftExpr, rightAttr, equality, "=", false) + case equality @ EqualTo(rightAttr: Attribute, leftExpr) + if leftExpr.references.subsetOf(join.left.outputSet) && + join.right.outputSet.contains(rightAttr) => + offeredCandidates += OfferedLookupPredicate(leftExpr, rightAttr, equality, "=", false) + case other => residual += other + } + + val offeredPredicates = offeredCandidates.map { candidate => + new V2Predicate(candidate.predicateName, Array[V2Expression]( + FieldReference.column(candidate.projectedField.name), + new LookupParameter(candidate.parameter.dataType))) + }.toArray + // A copy, so a connector that reorders the array in place cannot break the identity mapping. + val (selectedPredicates, unselectedPredicates) = selectedCandidates(tableName, + offeredPredicates, offeredCandidates, + lookupBuilder.selectLookupPredicates(offeredPredicates.clone())) + residual ++= unselectedPredicates.map(_.condition) + + val scan = scanBuilder.build() + val resolver = SQLConf.get.resolver + // Fields match the way ordinary V2 scans match them (`PushDownUtils.toOutputAttrs`): CHAR and + // VARCHAR are read as STRING, and nullability, including nested nullability, does not count. + val readSchema = CharVarcharUtils.replaceCharVarcharWithStringInSchema(scan.readSchema()) + val lookupInput = readSchema.fields.map { field => + relation.output.find { attr => Review Comment: > [!NOTE] > **DISCLAIMER:** This comment was written by an LLM, Claude Opus 5.5 (`claude-opus-5-5`, 1M context), posting on behalf of @HeartSaVioR. Fixed in #59909 ([73b62b7c559](https://github.com/apache/spark/commit/73b62b7c559b90caf5ac9088417679591118c5d5)) and [01060c7f72e](https://github.com/apache/spark/commit/01060c7f72e4e7256c177939e0e2306e463dfae0). In #59909, `lookupInput` takes the read schema's types and nullability and keeps the relation's exprIds. `lookupOutput` keeps the analyzed attributes in analyzed order, and a read schema that misses one of them fails with `MISSING_SCAN_FIELD`. In this PR, `ExternalLookupJoinExec` reads the connector's value rows through `lookupInput`. Its lookup columns keep their analyzed order and take the read schema's types and nullability, and both the residual and the output projection are bound to the read-schema fields. New tests: - A read schema in another column order returns the right columns. - For the `nested` table (`s.a INT NOT NULL`, read as nullable), the null that the connector returns comes back as null: `to_json(s)` is `{}`. With the table's types it came back as `{"a":0}`. ########## sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/v2/PrepareExternalLookupJoin.scala: ########## @@ -0,0 +1,233 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.spark.sql.execution.datasources.v2 + +import java.util.{Collections, IdentityHashMap} + +import scala.collection.mutable + +import org.apache.spark.sql.catalyst.expressions.{Alias, And, Attribute, EqualTo, Expression, + Literal, PredicateHelper} +import org.apache.spark.sql.catalyst.plans.LeftOuter +import org.apache.spark.sql.catalyst.plans.logical.{ExternalLookupJoin, Filter, Join, LocalRelation, + LogicalPlan, PreparedExternalLookupJoin, Project} +import org.apache.spark.sql.catalyst.rules.Rule +import org.apache.spark.sql.catalyst.trees.TreePattern.EXTERNAL_LOOKUP_JOIN +import org.apache.spark.sql.catalyst.util.CharVarcharUtils +import org.apache.spark.sql.connector.catalog.{SupportsRead, TableCapability} +import org.apache.spark.sql.connector.expressions.{Expression => V2Expression, FieldReference} +import org.apache.spark.sql.connector.expressions.filter.{Predicate => V2Predicate} +import org.apache.spark.sql.connector.read.{LookupParameter, SupportsExternalLookup} +import org.apache.spark.sql.errors.{QueryCompilationErrors, QueryErrorsBase} +import org.apache.spark.sql.internal.SQLConf +import org.apache.spark.sql.types.{BooleanType, DataType} + +/** Finalizes an optimized external lookup side immediately before ordinary V2 scan preparation. */ +object PrepareExternalLookupJoin extends Rule[LogicalPlan] with PredicateHelper + with QueryErrorsBase { + override def apply(plan: LogicalPlan): LogicalPlan = { + val prepared = plan.transformUpWithPruning(_.containsPattern(EXTERNAL_LOOKUP_JOIN)) { + case join: ExternalLookupJoin if provablyEmpty(join) => emptyJoinResult(join) + case join: ExternalLookupJoin => prepare(join) + } + // Prepared lookup joins no longer contain their lookup relation, so a lookup-only table that + // is still read here cannot run. Pre-order visits a join before the relation it reads. + prepared.foreach { + // Classic Dataset.join(right, usingColumns) first analyzes Join(left, right, joinType, None), + // including CheckAnalysis, to disambiguate a self-join, and only then builds the USING join. + // That plan cannot be told apart from a join without a condition, so analysis keeps it, and + // a missing condition is reported here, on the final plan. + case Join(_, right, _, None, _) if readsLookupOnlyTable(right) => + throw QueryCompilationErrors.unsupportedExternalLookupJoinError("MISSING_JOIN_CONDITION") + case node => + ExternalLookupRelations.lookupOnlyTable(node).foreach { table => + throw QueryCompilationErrors.unsupportedBatchReadError(table) + } + } + prepared + } + + private def readsLookupOnlyTable(plan: LogicalPlan): Boolean = plan match { + case node @ (_: Project | _: Filter) => readsLookupOnlyTable(node.children.head) + case node => ExternalLookupRelations.lookupOnlyTable(node).isDefined + } + + // The optimizer folds `WHERE false` on the lookup side into an empty relation, and a condition + // such as `s.id = r.id AND false` into `false`. Such a join needs no connector call. + private def provablyEmpty(join: ExternalLookupJoin): Boolean = + join.right.maxRows.contains(0L) || (join.condition match { + case Some(Literal(value, BooleanType)) => value == null || value == false + case _ => false + }) + + private def emptyJoinResult(join: ExternalLookupJoin): LogicalPlan = join.joinType match { + case LeftOuter => + val nullLookupColumns = join.right.output.map { attr => + Alias(Literal(null, attr.dataType), attr.name)(attr.exprId) + } + Project(join.left.output ++ nullLookupColumns, join.left) + case _ => LocalRelation(join.output, Seq.empty, join.isStreaming) + } + + private def prepare(join: ExternalLookupJoin): PreparedExternalLookupJoin = { + val relation = extractLookupPlan(join.right).getOrElse { + throw QueryCompilationErrors.unsupportedExternalLookupJoinError("LOOKUP_PLAN") + } + val tableName = relation.table.name() + val supportsRead = relation.table match { + case table: SupportsRead + if table.capabilities().contains(TableCapability.EXTERNAL_LOOKUP) => table + case _ => + throw QueryCompilationErrors.unsupportedExternalLookupJoinError( + "TABLE", Map("table" -> toSQLId(tableName))) + } + val scanBuilder = supportsRead.newScanBuilder(relation.options) + val lookupBuilder = scanBuilder match { + case builder: SupportsExternalLookup => builder + case _ => + throw QueryCompilationErrors.unsupportedExternalLookupJoinError( + "SCAN_BUILDER", Map("table" -> toSQLId(tableName))) + } + + val conjuncts = join.condition.toSeq.flatMap(splitConjunctivePredicates) + val offeredCandidates = mutable.ArrayBuffer.empty[OfferedLookupPredicate] + val residual = mutable.ArrayBuffer.empty[Expression] + conjuncts.foreach { + case equality @ EqualTo(leftExpr, rightAttr: Attribute) + if leftExpr.references.subsetOf(join.left.outputSet) && + join.right.outputSet.contains(rightAttr) => + offeredCandidates += OfferedLookupPredicate(leftExpr, rightAttr, equality, "=", false) + case equality @ EqualTo(rightAttr: Attribute, leftExpr) + if leftExpr.references.subsetOf(join.left.outputSet) && + join.right.outputSet.contains(rightAttr) => + offeredCandidates += OfferedLookupPredicate(leftExpr, rightAttr, equality, "=", false) + case other => residual += other Review Comment: > [!NOTE] > **DISCLAIMER:** This comment was written by an LLM, Claude Opus 5.5 (`claude-opus-5-5`, 1M context), posting on behalf of @HeartSaVioR. Fixed in [01060c7f72e](https://github.com/apache/spark/commit/01060c7f72e4e7256c177939e0e2306e463dfae0), as suggested. A conjunct with a scalar Python UDF that references the lookup side never goes into the residual. In an inner join, it is evaluated in a `Filter` above the prepared join, where `ExtractPythonUDFs` extracts it. A left outer join fails during planning with `UNSUPPORTED_FEATURE.PYTHON_UDF_IN_ON_CLAUSE`, as `ExtractPythonUDFFromJoinCondition` does for other joins. A Python UDF over only the stream side stays in the lookup join, where `ExtractPythonUDFs` already extracts it. The new test uses the integrated Python UDF test utilities and covers both join types. ########## sql/core/src/main/scala/org/apache/spark/sql/execution/joins/ExternalLookupJoinEvaluatorFactory.scala: ########## @@ -0,0 +1,410 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.spark.sql.execution.joins + +import java.util.concurrent.ConcurrentHashMap +import java.util.concurrent.atomic.AtomicBoolean + +import scala.collection.mutable + +import org.apache.spark.{PartitionEvaluator, PartitionEvaluatorFactory, SparkIllegalStateException, + TaskContext} +import org.apache.spark.sql.catalyst.InternalRow +import org.apache.spark.sql.catalyst.expressions.{Attribute, CollationKey, Expression, + GenericInternalRow, JoinedRow, Predicate, UnsafeProjection, UnsafeRow} +import org.apache.spark.sql.catalyst.optimizer.NormalizeFloatingNumbers +import org.apache.spark.sql.catalyst.plans.{Inner, JoinType, LeftOuter} +import org.apache.spark.sql.connector.metric.CustomTaskMetric +import org.apache.spark.sql.connector.read.{ExternalLookupReader, ExternalLookupReaderFactory, + ExternalLookupResultCursor} +import org.apache.spark.sql.execution.metric.{CustomMetrics, SQLMetric} +import org.apache.spark.util.Utils + +/** + * Creates the partition evaluators of an [[ExternalLookupJoinExec]]. The factory is sent to the + * tasks, so it holds only what they need; the plan node and its scan stay on the driver. + * + * An evaluator joins its partition one batch of up to `maxParameterRowsPerRequest` stream rows at + * a time. For each batch it sends one request with the distinct parameters that can match, and + * streams the connector's result rows into output rows. + */ +class ExternalLookupJoinEvaluatorFactory( + lookupParameters: Seq[Expression], + nullSafeParameters: Seq[Boolean], + streamOutput: Seq[Attribute], + lookupOutput: Seq[Attribute], + output: Seq[Attribute], + joinType: JoinType, + residualCondition: Option[Expression], + readerFactory: ExternalLookupReaderFactory, + maxParameterRowsPerRequest: Int, + metrics: ExternalLookupJoinMetrics, + readerMetrics: ExternalLookupReaderMetrics) + extends PartitionEvaluatorFactory[InternalRow, InternalRow] { + + require(lookupParameters.length == nullSafeParameters.length, + "Every external lookup parameter must define its null semantics") + + override def createEvaluator(): PartitionEvaluator[InternalRow, InternalRow] = + new ExternalLookupJoinEvaluator + + private class ExternalLookupJoinEvaluator extends PartitionEvaluator[InternalRow, InternalRow] { + override def eval( + partitionIndex: Int, + inputs: Iterator[InternalRow]*): Iterator[InternalRow] = { + assert(inputs.length == 1) + new PartitionJoinIterator(inputs.head, partitionIndex) + } + } + + /** Joins one partition batch by batch, and closes its reader after the last batch. */ + private class PartitionJoinIterator(input: Iterator[InternalRow], partitionIndex: Int) + extends Iterator[InternalRow] { + private val reader = new LookupReader + private val parameters = new LookupParameters(partitionIndex) + private val joinOutput = new JoinOutput(partitionIndex) + private var batch: Iterator[InternalRow] = Iterator.empty + private var inputExhausted = false + + override def hasNext: Boolean = { + tryAdvance() + batch.hasNext + } + + override def next(): InternalRow = { + tryAdvance() + if (!batch.hasNext) { + throw new SparkIllegalStateException( + errorClass = "INTERNAL_ERROR", + messageParameters = Map("message" -> "External lookup join exhausted")) + } + batch.next() + } + + /** Reads more only when nothing is buffered; safe to call repeatedly. */ + private def tryAdvance(): Unit = { + while (!batch.hasNext && !inputExhausted) { + val rows = readBatch() + if (rows.nonEmpty) batch = new BatchJoinIterator(rows, parameters, joinOutput, reader) + inputExhausted = !input.hasNext + } + if (inputExhausted && !batch.hasNext) reader.close() + } + + private def readBatch(): Seq[InternalRow] = { + val rows = mutable.ArrayBuffer.empty[InternalRow] + while (rows.length < maxParameterRowsPerRequest && input.hasNext) { + rows += input.next().copy() + } + rows.toSeq Review Comment: > [!NOTE] > **DISCLAIMER:** This comment was written by an LLM, Claude Opus 5.5 (`claude-opus-5-5`, 1M context), posting on behalf of @HeartSaVioR. Fixed in [01060c7f72e](https://github.com/apache/spark/commit/01060c7f72e4e7256c177939e0e2306e463dfae0). `readBatch` returns an `Array[InternalRow]`, and `BatchJoinIterator` takes the array, so reading a row by position is constant time. Results and request sizes are unchanged. -- 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]
