HeartSaVioR commented on code in PR #59910: URL: https://github.com/apache/spark/pull/59910#discussion_r4452568496
########## 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. A correction to my earlier reply about common expressions such as `BETWEEN`. It said that `RewriteWithExpression` itself is unchanged and that `FormExternalLookupJoin` inlined the definitions. In [b17bb5d62c8](https://github.com/apache/spark/commit/b17bb5d62c8db5efc2bf84d7f46d9025129d3bb8), that inlining moved into `RewriteWithExpression`, as a case for `ExternalLookupJoin` that applies the rule's own inliner. The inliner substitutes every deterministic definition at its references, whatever it costs, and a join condition is deterministic, so no definition is ever evaluated in a projection below the lookup join. `FormExternalLookupJoin` no longer has its own copy. The `Rewrite With expression` batch runs right after the conversion and Finish Analysis, before any rule that could add an operator below the lookup side. The `BETWEEN` tests over a lookup column and a stream column pass unchanged, and `RewriteWithExpressionSuite` passes. -- 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]
