From 7d5b8a86f2e2407bd985d223363dd1dd029b5dc2 Mon Sep 17 00:00:00 2001 From: Prashant Singh Date: Tue, 8 Sep 2026 10:59:22 -0700 Subject: [PATCH 01/11] Spark: Send the referenced-by view chain through SparkCatalog Resolve the chain of views a relation was reached through and pass it to the catalog as a LoadContext, so a REST catalog can forward it to the server. SparkSupportsLoadContext extends the loading contract with context-carrying overloads of loadTable and loadView, including the time-travel forms, and SparkCatalog and SparkSessionCatalog implement it. ResolveViews records the chain while resolving nested views, with UnResolvedRelationFromView carrying it through analysis. Cross-catalog view references fail rather than silently dropping the chain. Gated behind spark.sql.iceberg.referenced-by-enabled, off by default. --- .../sql/catalyst/analysis/ResolveViews.scala | 67 ++++- .../sql/catalyst/analysis/ViewUtil.scala | 142 ++++++++++ .../views/UnResolvedRelationFromView.scala | 72 +++++ .../extensions/TestReferencedByViewChain.java | 263 ++++++++++++++++++ .../apache/iceberg/spark/SparkCatalog.java | 53 +++- .../iceberg/spark/SparkSessionCatalog.java | 64 ++++- .../spark/SparkSupportsLoadContext.java | 78 ++++++ 7 files changed, 719 insertions(+), 20 deletions(-) create mode 100644 spark/v4.1/spark-extensions/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/views/UnResolvedRelationFromView.scala create mode 100644 spark/v4.1/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestReferencedByViewChain.java create mode 100644 spark/v4.1/spark/src/main/java/org/apache/iceberg/spark/SparkSupportsLoadContext.java diff --git a/spark/v4.1/spark-extensions/src/main/scala/org/apache/spark/sql/catalyst/analysis/ResolveViews.scala b/spark/v4.1/spark-extensions/src/main/scala/org/apache/spark/sql/catalyst/analysis/ResolveViews.scala index 83e501257ced..ad4f3101bfc0 100644 --- a/spark/v4.1/spark-extensions/src/main/scala/org/apache/spark/sql/catalyst/analysis/ResolveViews.scala +++ b/spark/v4.1/spark-extensions/src/main/scala/org/apache/spark/sql/catalyst/analysis/ResolveViews.scala @@ -18,6 +18,7 @@ */ package org.apache.spark.sql.catalyst.analysis +import org.apache.iceberg.catalog.LoadContext import org.apache.iceberg.spark.SparkSQLProperties import org.apache.spark.sql.SparkSession import org.apache.spark.sql.catalyst.FunctionIdentifier @@ -32,6 +33,7 @@ import org.apache.spark.sql.catalyst.plans.logical.Project import org.apache.spark.sql.catalyst.plans.logical.SubqueryAlias import org.apache.spark.sql.catalyst.plans.logical.views.CreateIcebergView import org.apache.spark.sql.catalyst.plans.logical.views.ResolvedV2View +import org.apache.spark.sql.catalyst.plans.logical.views.UnResolvedRelationFromView import org.apache.spark.sql.catalyst.rules.Rule import org.apache.spark.sql.catalyst.trees.CurrentOrigin import org.apache.spark.sql.catalyst.trees.Origin @@ -39,6 +41,7 @@ import org.apache.spark.sql.connector.catalog.CatalogManager import org.apache.spark.sql.connector.catalog.LookupCatalog import org.apache.spark.sql.connector.catalog.View import org.apache.spark.sql.errors.QueryCompilationErrors +import org.apache.spark.sql.execution.datasources.v2.DataSourceV2Relation import org.apache.spark.sql.types.MetadataBuilder case class ResolveViews(spark: SparkSession) extends Rule[LogicalPlan] with LookupCatalog { @@ -64,6 +67,27 @@ case class ResolveViews(spark: SparkSession) extends Rule[LogicalPlan] with Look .map(_ => ResolvedV2View(catalog.asViewCatalog, ident)) .getOrElse(u) + case u @ UnResolvedRelationFromView( + tableParts @ CatalogAndIdentifier(catalog, tableIdent), + viewChain, + options, + isStreaming, + timeTravelVersion, + timeTravelTimestamp) => + val referencedBy = ViewUtil.buildReferencedByChain(viewChain, catalog.name()) + val context = LoadContext.builder().referencedBy(referencedBy).build() + try { + val table = + ViewUtil.loadTable(catalog, tableIdent, context, timeTravelVersion, timeTravelTimestamp) + DataSourceV2Relation.create(table, Some(catalog), Some(tableIdent), options) + } catch { + case _: NoSuchTableException => + ViewUtil + .loadView(catalog, tableIdent, context) + .map(view => createViewRelation(tableParts, view, viewChain)) + .getOrElse(UnresolvedRelation(tableParts, options, isStreaming)) + } + case c @ CreateIcebergView( ResolvedIdentifier(_, _), _, @@ -104,12 +128,19 @@ case class ResolveViews(spark: SparkSession) extends Rule[LogicalPlan] with Look } } - private def createViewRelation(nameParts: Seq[String], view: View): LogicalPlan = { + private def createViewRelation( + nameParts: Seq[String], + view: View, + existingChain: Seq[Seq[String]] = Seq.empty): LogicalPlan = { val parsed = parseViewText(nameParts.quoted, view.query) // Apply any necessary rewrites to preserve correct resolution val viewCatalogAndNamespace: Seq[String] = view.currentCatalog +: view.currentNamespace.toSeq - val rewritten = rewriteIdentifiers(parsed, viewCatalogAndNamespace); + + val viewChain = + ViewUtil.buildViewChain(nameParts, viewCatalogAndNamespace, existingChain, isCatalog) + + val rewritten = rewriteIdentifiers(parsed, viewCatalogAndNamespace, viewChain) // Apply the field aliases and column comments // This logic differs from how Spark handles views in SessionCatalog.fromCatalogTable. @@ -164,11 +195,13 @@ case class ResolveViews(spark: SparkSession) extends Rule[LogicalPlan] with Look private def rewriteIdentifiers( plan: LogicalPlan, - catalogAndNamespace: Seq[String]): LogicalPlan = { + catalogAndNamespace: Seq[String], + viewChain: Seq[Seq[String]] = Seq.empty): LogicalPlan = { // Rewrite unresolved functions and relations qualifyTableIdentifiers( qualifyFunctionIdentifiers(CTESubstitution.apply(plan), catalogAndNamespace), - catalogAndNamespace) + catalogAndNamespace, + viewChain) } private def qualifyFunctionIdentifiers( @@ -189,17 +222,31 @@ case class ResolveViews(spark: SparkSession) extends Rule[LogicalPlan] with Look */ private def qualifyTableIdentifiers( child: LogicalPlan, - catalogAndNamespace: Seq[String]): LogicalPlan = + catalogAndNamespace: Seq[String], + viewChain: Seq[Seq[String]]): LogicalPlan = { child transform { - case u @ UnresolvedRelation(Seq(table), _, _) => - u.copy(multipartIdentifier = catalogAndNamespace :+ table) - case u @ UnresolvedRelation(parts, _, _) if !isCatalog(parts.head) => - u.copy(multipartIdentifier = catalogAndNamespace.head +: parts) + case UnresolvedRelation(parts, options, isStreaming) => + val qualifiedTableId = ViewUtil.qualifyParts(parts, catalogAndNamespace, isCatalog) + UnResolvedRelationFromView(qualifiedTableId, viewChain, options, isStreaming) + case RelationTimeTravel( + UnresolvedRelation(parts, options, isStreaming), + timestampOpt, + versionOpt) => + val qualifiedTableId = ViewUtil.qualifyParts(parts, catalogAndNamespace, isCatalog) + UnResolvedRelationFromView( + qualifiedTableId, + viewChain, + options, + isStreaming, + timeTravelVersion = versionOpt, + timeTravelTimestamp = timestampOpt) case other => other.transformExpressions { case subquery: SubqueryExpression => - subquery.withNewPlan(qualifyTableIdentifiers(subquery.plan, catalogAndNamespace)) + subquery.withNewPlan( + qualifyTableIdentifiers(subquery.plan, catalogAndNamespace, viewChain)) } } + } private def isCatalog(name: String): Boolean = { catalogManager.isCatalogRegistered(name) diff --git a/spark/v4.1/spark-extensions/src/main/scala/org/apache/spark/sql/catalyst/analysis/ViewUtil.scala b/spark/v4.1/spark-extensions/src/main/scala/org/apache/spark/sql/catalyst/analysis/ViewUtil.scala index c27cb140347e..14a7f99662fc 100644 --- a/spark/v4.1/spark-extensions/src/main/scala/org/apache/spark/sql/catalyst/analysis/ViewUtil.scala +++ b/spark/v4.1/spark-extensions/src/main/scala/org/apache/spark/sql/catalyst/analysis/ViewUtil.scala @@ -18,13 +18,24 @@ */ package org.apache.spark.sql.catalyst.analysis +import java.util +import org.apache.iceberg.catalog.LoadContext +import org.apache.iceberg.catalog.Namespace +import org.apache.iceberg.catalog.TableIdentifier +import org.apache.iceberg.spark.SparkSupportsLoadContext +import org.apache.spark.sql.catalyst.expressions.Expression import org.apache.spark.sql.connector.catalog.CatalogPlugin import org.apache.spark.sql.connector.catalog.Identifier +import org.apache.spark.sql.connector.catalog.Table import org.apache.spark.sql.connector.catalog.View import org.apache.spark.sql.connector.catalog.ViewCatalog import org.apache.spark.sql.errors.QueryCompilationErrors +import scala.jdk.CollectionConverters._ object ViewUtil { + + import org.apache.spark.sql.connector.catalog.CatalogV2Implicits._ + def loadView(catalog: CatalogPlugin, ident: Identifier): Option[View] = catalog match { case viewCatalog: ViewCatalog => try { @@ -35,6 +46,137 @@ object ViewUtil { case _ => None } + def loadView(catalog: CatalogPlugin, ident: Identifier, context: LoadContext): Option[View] = + catalog match { + case supportsLoadContext: SparkSupportsLoadContext => + try { + Option(supportsLoadContext.loadView(ident, context)) + } catch { + case _: NoSuchViewException => None + } + case viewCatalog: ViewCatalog => + try { + Option(viewCatalog.loadView(ident)) + } catch { + case _: NoSuchViewException => None + } + case _ => None + } + + /** + * Build the referenced-by view chain from a sequence of fully-qualified + * view identifier parts. Validates that all entries are fully qualified and belong to the + * same catalog as the target. + */ + def buildReferencedByChain( + viewChain: Seq[Seq[String]], + targetCatalogName: String): java.util.List[TableIdentifier] = { + viewChain.foreach { parts => + require( + parts.size >= 3, + s"View chain entry must be fully qualified [catalog, namespace..., name], got: ${parts.mkString(".")}") + } + val crossCatalogViews = + viewChain.filter(parts => !parts.headOption.contains(targetCatalogName)) + if (crossCatalogViews.nonEmpty) { + throw new IllegalStateException( + s"Cross-catalog view references are not supported with referenced-by enabled. " + + s"Views from catalogs [${crossCatalogViews.map(_.head).distinct.mkString(", ")}] " + + s"cannot reference entities in catalog [$targetCatalogName]") + } + val viewIdentifiers = viewChain.map { parts => + val nsParts = parts.drop(1).init + val ns = Namespace.of(nsParts: _*) + TableIdentifier.of(ns, parts.last) + } + new util.ArrayList[TableIdentifier](viewIdentifiers.asJava) + } + + /** + * Qualify identifier parts against a catalog/namespace prefix. A single-part identifier becomes + * `catalogAndNamespace :+ part`; a multi-part identifier whose head isn't a registered catalog + * is prefixed with the catalog; otherwise it's returned as-is. + */ + def qualifyParts( + parts: Seq[String], + catalogAndNamespace: Seq[String], + isCatalog: String => Boolean): Seq[String] = { + parts match { + case Seq(name) => catalogAndNamespace :+ name + case _ if !isCatalog(parts.head) => catalogAndNamespace.head +: parts + case _ => parts + } + } + + /** + * Build or extend the view chain by appending the current view's fully qualified identifier. + * + * @param nameParts the current view's identifier parts (may be 1, 2, or 3+ parts) + * @param viewCatalogAndNamespace the view's catalog and namespace prefix + * @param existingChain the existing view chain from outer views (empty for the outermost view) + * @param isCatalog function to check if a name is a registered catalog + */ + def buildViewChain( + nameParts: Seq[String], + viewCatalogAndNamespace: Seq[String], + existingChain: Seq[Seq[String]], + isCatalog: String => Boolean): Seq[Seq[String]] = { + existingChain :+ qualifyParts(nameParts, viewCatalogAndNamespace, isCatalog) + } + + /** + * Load a table with contextual load support, dispatching to the appropriate loadTable overload + * based on whether the catalog supports contextual load and whether time travel is requested. + */ + def loadTable( + catalog: CatalogPlugin, + ident: Identifier, + context: LoadContext, + timeTravelVersion: Option[String] = None, + timeTravelTimestamp: Option[Expression] = None): Table = { + catalog match { + case supportsLoadContext: SparkSupportsLoadContext => + loadTableWithTimeTravel( + supportsLoadContext, + ident, + context, + timeTravelVersion, + timeTravelTimestamp) + case c if c.asTableCatalog.isInstanceOf[SparkSupportsLoadContext] => + loadTableWithTimeTravel( + c.asTableCatalog.asInstanceOf[SparkSupportsLoadContext], + ident, + context, + timeTravelVersion, + timeTravelTimestamp) + case _ => + (timeTravelVersion, timeTravelTimestamp) match { + case (Some(version), _) => + catalog.asTableCatalog.loadTable(ident, version) + case (_, Some(timestamp)) => + catalog.asTableCatalog.loadTable(ident, timestamp.eval().asInstanceOf[Long]) + case _ => + catalog.asTableCatalog.loadTable(ident) + } + } + } + + private def loadTableWithTimeTravel( + supportsLoadContext: SparkSupportsLoadContext, + ident: Identifier, + context: LoadContext, + timeTravelVersion: Option[String], + timeTravelTimestamp: Option[Expression]): Table = { + (timeTravelVersion, timeTravelTimestamp) match { + case (Some(version), _) => + supportsLoadContext.loadTable(ident, version, context) + case (_, Some(timestamp)) => + supportsLoadContext.loadTable(ident, timestamp.eval().asInstanceOf[Long], context) + case _ => + supportsLoadContext.loadTable(ident, context) + } + } + def isViewCatalog(catalog: CatalogPlugin): Boolean = { catalog.isInstanceOf[ViewCatalog] } diff --git a/spark/v4.1/spark-extensions/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/views/UnResolvedRelationFromView.scala b/spark/v4.1/spark-extensions/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/views/UnResolvedRelationFromView.scala new file mode 100644 index 000000000000..edfdad6e506b --- /dev/null +++ b/spark/v4.1/spark-extensions/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/views/UnResolvedRelationFromView.scala @@ -0,0 +1,72 @@ +/* + * 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.plans.logical.views + +import org.apache.spark.sql.catalyst.expressions.Attribute +import org.apache.spark.sql.catalyst.expressions.Expression +import org.apache.spark.sql.catalyst.plans.logical.LeafNode +import org.apache.spark.sql.catalyst.util.quoteIfNeeded +import org.apache.spark.sql.util.CaseInsensitiveStringMap + +/** + * Represents an unresolved table/relation that was referenced from within a view. + * This node carries both the table identifier and the chain of view identifiers that + * referenced it, enabling view-aware authorization and security mechanisms. + * + * This allows catalogs to implement different security models such as: + * - Definer rights: Use view creator's permissions to access the underlying table + * - Invoker rights: Use current user's permissions to access the underlying table + * + * @param tableMultipartIdentifier The multipart identifier for the target table + * @param viewChain The chain of view identifiers that reference this table, ordered from + * outermost view first to innermost view last. Each element is a multipart + * identifier (Seq[String]). + * @param options Options passed to the table (similar to UnresolvedRelation) + * @param isStreaming Whether this is a streaming relation + * @param timeTravelVersion Optional version for time travel (e.g., snapshot ID or reference name) + * @param timeTravelTimestamp Optional timestamp expression for time travel + */ +case class UnResolvedRelationFromView( + tableMultipartIdentifier: Seq[String], + viewChain: Seq[Seq[String]], + options: CaseInsensitiveStringMap = CaseInsensitiveStringMap.empty(), + override val isStreaming: Boolean = false, + timeTravelVersion: Option[String] = None, + timeTravelTimestamp: Option[Expression] = None) + extends LeafNode { + + override def output: Seq[Attribute] = Nil + + override lazy val resolved = false + + def tableName: String = tableMultipartIdentifier.map(quoteIfNeeded).mkString(".") + + def viewName: String = viewChain.map(_.map(quoteIfNeeded).mkString(".")).mkString(" -> ") + + override def simpleString(maxFields: Int): String = { + s"'UnResolvedRelationFromView [table=$tableName, views=$viewName, " + + s"${if (isStreaming) "streaming=true, " else ""}options=$options]" + } + + override def toString: String = { + val chainStr = viewChain.map(parts => s"[${parts.mkString(", ")}]").mkString(", ") + s"UnResolvedRelationFromView([${tableMultipartIdentifier.mkString(", ")}], " + + s"chain=[$chainStr], $isStreaming)" + } +} diff --git a/spark/v4.1/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestReferencedByViewChain.java b/spark/v4.1/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestReferencedByViewChain.java new file mode 100644 index 000000000000..ba2fb7f0bc69 --- /dev/null +++ b/spark/v4.1/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestReferencedByViewChain.java @@ -0,0 +1,263 @@ +/* + * 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.iceberg.spark.extensions; + +import static org.assertj.core.api.Assertions.assertThat; + +import java.net.InetAddress; +import java.util.List; +import java.util.concurrent.CopyOnWriteArrayList; +import java.util.stream.Collectors; +import java.util.stream.IntStream; +import org.apache.iceberg.CatalogProperties; +import org.apache.iceberg.Table; +import org.apache.iceberg.catalog.Catalog; +import org.apache.iceberg.catalog.LoadContext; +import org.apache.iceberg.catalog.Namespace; +import org.apache.iceberg.catalog.TableIdentifier; +import org.apache.iceberg.catalog.ViewCatalog; +import org.apache.iceberg.inmemory.InMemoryCatalog; +import org.apache.iceberg.spark.Spark3Util; +import org.apache.iceberg.spark.SparkCatalog; +import org.apache.iceberg.spark.SparkSchemaUtil; +import org.apache.iceberg.spark.SparkTestHelperBase; +import org.apache.iceberg.spark.source.SimpleRecord; +import org.apache.iceberg.view.View; +import org.apache.spark.sql.Dataset; +import org.apache.spark.sql.Row; +import org.apache.spark.sql.SparkSession; +import org.junit.jupiter.api.AfterAll; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +/** + * Tests that the referenced-by view chain is correctly constructed and passed to {@link + * org.apache.iceberg.catalog.Catalog#loadTable(TableIdentifier, LoadContext)} during view + * resolution. + * + *

This verifies: + * + *

+ */ +public class TestReferencedByViewChain extends SparkTestHelperBase { + + private static final String CATALOG_NAME = "ref_test_catalog"; + private static final Namespace NAMESPACE = Namespace.of("default"); + private static final String TABLE_NAME = "test_table"; + + private static SparkSession spark; + + /** + * An InMemoryCatalog that records all contextual loadTable/loadView calls for later assertion. + */ + public static class ContextTrackingCatalog extends InMemoryCatalog { + + /** Records of (tableIdentifier, referencedBy) captured from loadTable calls. */ + public static final List CAPTURED = new CopyOnWriteArrayList<>(); + + /** Records of (viewIdentifier, referencedBy) captured from loadView calls. */ + public static final List CAPTURED_VIEWS = new CopyOnWriteArrayList<>(); + + public static class CapturedContext { + public final TableIdentifier tableIdentifier; + public final List referencedBy; + + CapturedContext(TableIdentifier tableIdentifier, List referencedBy) { + this.tableIdentifier = tableIdentifier; + this.referencedBy = referencedBy; + } + } + + public static void clearCaptured() { + CAPTURED.clear(); + CAPTURED_VIEWS.clear(); + } + + @Override + public Table loadTable(TableIdentifier identifier, LoadContext context) { + CAPTURED.add(new CapturedContext(identifier, context.referencedBy())); + return super.loadTable(identifier); + } + + @Override + public View loadView(TableIdentifier identifier, LoadContext context) { + CAPTURED_VIEWS.add(new CapturedContext(identifier, context.referencedBy())); + return super.loadView(identifier); + } + } + + @BeforeAll + public static void startSpark() { + spark = + SparkSession.builder() + .master("local[2]") + .config("spark.driver.host", InetAddress.getLoopbackAddress().getHostAddress()) + .config("spark.ui.enabled", "false") + .config( + "spark.metrics.conf.*.sink.servlet.class", + "org.apache.iceberg.spark.DummyMetricsServlet") + .config("spark.sql.extensions", IcebergSparkSessionExtensions.class.getName()) + .config("spark.sql.catalog." + CATALOG_NAME, SparkCatalog.class.getName()) + .config( + "spark.sql.catalog." + CATALOG_NAME + "." + CatalogProperties.CATALOG_IMPL, + ContextTrackingCatalog.class.getName()) + .config("spark.sql.catalog." + CATALOG_NAME + ".default-namespace", "default") + .config("spark.sql.catalog." + CATALOG_NAME + ".cache-enabled", "false") + .config("spark.sql.defaultCatalog", CATALOG_NAME) + .getOrCreate(); + + spark.sql(String.format("CREATE NAMESPACE IF NOT EXISTS %s", NAMESPACE)); + } + + @AfterAll + public static void stopSpark() { + if (spark != null) { + spark.stop(); + spark = null; + } + } + + @BeforeEach + public void before() { + ContextTrackingCatalog.clearCaptured(); + + spark.sql(String.format("USE %s.%s", CATALOG_NAME, NAMESPACE)); + spark.sql(String.format("CREATE TABLE IF NOT EXISTS %s (id INT, data STRING)", TABLE_NAME)); + + try { + List records = + IntStream.rangeClosed(1, 5) + .mapToObj(i -> new SimpleRecord(i, String.valueOf(i))) + .collect(Collectors.toList()); + Dataset df = spark.createDataFrame(records, SimpleRecord.class); + df.writeTo(TABLE_NAME).append(); + } catch (org.apache.spark.sql.catalyst.analysis.NoSuchTableException e) { + throw new RuntimeException(e); + } + + ContextTrackingCatalog.clearCaptured(); + } + + @AfterEach + public void after() { + spark.sql(String.format("USE %s.%s", CATALOG_NAME, NAMESPACE)); + spark.sql("DROP VIEW IF EXISTS simple_view"); + spark.sql("DROP VIEW IF EXISTS view_a"); + spark.sql("DROP VIEW IF EXISTS view_b"); + spark.sql("DROP VIEW IF EXISTS view_c"); + spark.sql(String.format("DROP TABLE IF EXISTS %s", TABLE_NAME)); + ContextTrackingCatalog.clearCaptured(); + } + + @Test + public void testDirectTableAccessHasNoContext() { + ContextTrackingCatalog.clearCaptured(); + + List rows = spark.sql(String.format("SELECT * FROM %s", TABLE_NAME)).collectAsList(); + assertThat(rows).hasSize(5); + + // direct table access is loaded with a context that carries no view chain + assertThat(ContextTrackingCatalog.CAPTURED) + .isNotEmpty() + .allSatisfy(captured -> assertThat(captured.referencedBy).isEmpty()); + } + + @Test + public void testSingleViewPassesViewIdentifierInContext() { + createView("simple_view", String.format("SELECT id FROM %s", TABLE_NAME)); + ContextTrackingCatalog.clearCaptured(); + + List result = spark.sql("SELECT * FROM simple_view").collectAsList(); + assertThat(result).hasSize(5); + + // exactly one load carries a view chain; the rest are plain loads with an empty context + assertThat(ContextTrackingCatalog.CAPTURED) + .filteredOn(captured -> !captured.referencedBy.isEmpty()) + .hasSize(1); + assertCapturedTableChain(ContextTrackingCatalog.CAPTURED, TABLE_NAME, "simple_view"); + } + + @Test + public void testNestedViewChainAccumulatesContext() { + // view_c -> view_b -> view_a -> table + // When querying view_c: + // - loading view_a should receive context with [view_c, view_b] + // - loading table should receive context with [view_c, view_b, view_a] + createView("view_a", String.format("SELECT id, data FROM %s", TABLE_NAME)); + createView("view_b", "SELECT id FROM view_a WHERE id <= 3"); + createView("view_c", "SELECT id FROM view_b WHERE id > 1"); + ContextTrackingCatalog.clearCaptured(); + + List result = rowsToJava(spark.sql("SELECT * FROM view_c").collectAsList()); + assertThat(result).hasSize(2).containsExactlyInAnyOrder(new Object[] {2}, new Object[] {3}); + + // Verify the table load has the full chain [view_c, view_b, view_a] + assertCapturedTableChain( + ContextTrackingCatalog.CAPTURED, TABLE_NAME, "view_c", "view_b", "view_a"); + + // Verify view_a was loaded with context containing [view_c, view_b] + assertCapturedTableChain(ContextTrackingCatalog.CAPTURED_VIEWS, "view_a", "view_c", "view_b"); + } + + private void createView(String viewName, String sql) { + ViewCatalog catalog = viewCatalog(); + catalog + .buildView(TableIdentifier.of(NAMESPACE, viewName)) + .withQuery("spark", sql) + .withDefaultNamespace(NAMESPACE) + .withDefaultCatalog(CATALOG_NAME) + .withSchema(SparkSchemaUtil.convert(spark.sql(sql).schema())) + .create(); + } + + /** + * Asserts that a captured context list contains an entry for the given target with the expected + * view chain. + */ + private void assertCapturedTableChain( + List captures, + String targetName, + String... expectedViewNames) { + List matching = + captures.stream() + .filter(c -> c.tableIdentifier.equals(TableIdentifier.of(NAMESPACE, targetName))) + .filter(c -> c.referencedBy != null && !c.referencedBy.isEmpty()) + .collect(Collectors.toList()); + + assertThat(matching).isNotEmpty(); + + List viewChain = matching.get(0).referencedBy; + assertThat(viewChain).hasSize(expectedViewNames.length); + for (int i = 0; i < expectedViewNames.length; i++) { + assertThat(viewChain.get(i)).isEqualTo(TableIdentifier.of(NAMESPACE, expectedViewNames[i])); + } + } + + private ViewCatalog viewCatalog() { + Catalog icebergCatalog = Spark3Util.loadIcebergCatalog(spark, CATALOG_NAME); + assertThat(icebergCatalog).isInstanceOf(ViewCatalog.class); + return (ViewCatalog) icebergCatalog; + } +} diff --git a/spark/v4.1/spark/src/main/java/org/apache/iceberg/spark/SparkCatalog.java b/spark/v4.1/spark/src/main/java/org/apache/iceberg/spark/SparkCatalog.java index 40db152076c8..da03c35e3fe5 100644 --- a/spark/v4.1/spark/src/main/java/org/apache/iceberg/spark/SparkCatalog.java +++ b/spark/v4.1/spark/src/main/java/org/apache/iceberg/spark/SparkCatalog.java @@ -38,6 +38,7 @@ import org.apache.iceberg.Schema; import org.apache.iceberg.Transaction; import org.apache.iceberg.catalog.Catalog; +import org.apache.iceberg.catalog.LoadContext; import org.apache.iceberg.catalog.Namespace; import org.apache.iceberg.catalog.SupportsNamespaces; import org.apache.iceberg.catalog.TableIdentifier; @@ -120,8 +121,7 @@ * *

*/ -public class SparkCatalog extends BaseCatalog { - +public class SparkCatalog extends BaseCatalog implements SparkSupportsLoadContext { private static final Logger LOG = LoggerFactory.getLogger(SparkCatalog.class); private static final Set DEFAULT_NS_KEYS = ImmutableSet.of(TableCatalog.PROP_OWNER); private static final Splitter COMMA = Splitter.on(","); @@ -168,17 +168,46 @@ protected TableIdentifier buildIdentifier(Identifier identifier) { @Override public Table loadTable(Identifier ident) throws NoSuchTableException { - return load(ident, null /* no time travel */); + return loadTable(ident, LoadContext.empty()); + } + + @Override + public Table loadTable(Identifier ident, LoadContext context) throws NoSuchTableException { + try { + return load(ident, null /* no time travel */, context); + } catch (org.apache.iceberg.exceptions.NoSuchTableException e) { + throw new NoSuchTableException(ident); + } } @Override public Table loadTable(Identifier ident, String version) throws NoSuchTableException { - return load(ident, TimeTravel.version(version)); + return loadTable(ident, version, LoadContext.empty()); + } + + @Override + public Table loadTable(Identifier ident, String version, LoadContext context) + throws NoSuchTableException { + try { + return load(ident, TimeTravel.version(version), context); + } catch (org.apache.iceberg.exceptions.NoSuchTableException e) { + throw new NoSuchTableException(ident); + } } @Override public Table loadTable(Identifier ident, long timestampMicros) throws NoSuchTableException { - return load(ident, TimeTravel.timestampMicros(timestampMicros)); + return loadTable(ident, timestampMicros, LoadContext.empty()); + } + + @Override + public Table loadTable(Identifier ident, long timestampMicros, LoadContext context) + throws NoSuchTableException { + try { + return load(ident, TimeTravel.timestampMicros(timestampMicros), context); + } catch (org.apache.iceberg.exceptions.NoSuchTableException e) { + throw new NoSuchTableException(ident); + } } @Override @@ -595,9 +624,14 @@ public boolean viewExists(Identifier ident) { @Override public View loadView(Identifier ident) throws NoSuchViewException { + return loadView(ident, LoadContext.empty()); + } + + @Override + public View loadView(Identifier ident, LoadContext context) throws NoSuchViewException { if (null != asViewCatalog) { try { - org.apache.iceberg.view.View view = asViewCatalog.loadView(buildIdentifier(ident)); + org.apache.iceberg.view.View view = asViewCatalog.loadView(buildIdentifier(ident), context); return new SparkView(catalogName, view); } catch (org.apache.iceberg.exceptions.NoSuchViewException e) { throw new NoSuchViewException(ident); @@ -892,13 +926,14 @@ private static void checkNotPathIdentifier(Identifier identifier, String method) } } - private Table load(Identifier ident, TimeTravel timeTravel) throws NoSuchTableException { + private Table load(Identifier ident, TimeTravel timeTravel, LoadContext context) + throws NoSuchTableException { if (isPathIdentifier(ident)) { return loadPath((PathIdentifier) ident, timeTravel); } try { - org.apache.iceberg.Table table = icebergCatalog.loadTable(buildIdentifier(ident)); + org.apache.iceberg.Table table = icebergCatalog.loadTable(buildIdentifier(ident), context); return SparkTable.create(table, timeTravel); } catch (org.apache.iceberg.exceptions.NoSuchTableException e) { @@ -911,7 +946,7 @@ private Table load(Identifier ident, TimeTravel timeTravel) throws NoSuchTableEx TableIdentifier namespaceAsIdent = buildIdentifier(namespaceToIdentifier(ident.namespace())); org.apache.iceberg.Table table; try { - table = icebergCatalog.loadTable(namespaceAsIdent); + table = icebergCatalog.loadTable(namespaceAsIdent, context); } catch (Exception ignored) { // the namespace does not identify a table, so it cannot be a table with a snapshot selector // throw an exception for the original identifier diff --git a/spark/v4.1/spark/src/main/java/org/apache/iceberg/spark/SparkSessionCatalog.java b/spark/v4.1/spark/src/main/java/org/apache/iceberg/spark/SparkSessionCatalog.java index ec37435ba119..cad143ba33a3 100644 --- a/spark/v4.1/spark/src/main/java/org/apache/iceberg/spark/SparkSessionCatalog.java +++ b/spark/v4.1/spark/src/main/java/org/apache/iceberg/spark/SparkSessionCatalog.java @@ -24,6 +24,7 @@ import org.apache.iceberg.CatalogProperties; import org.apache.iceberg.CatalogUtil; import org.apache.iceberg.catalog.Catalog; +import org.apache.iceberg.catalog.LoadContext; import org.apache.iceberg.relocated.com.google.common.base.Preconditions; import org.apache.iceberg.spark.source.HasIcebergCatalog; import org.apache.spark.sql.SparkSession; @@ -55,6 +56,8 @@ import org.apache.spark.sql.connector.expressions.Transform; import org.apache.spark.sql.types.StructType; import org.apache.spark.sql.util.CaseInsensitiveStringMap; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; /** * A Spark catalog that can also load non-Iceberg tables. @@ -64,7 +67,8 @@ */ public class SparkSessionCatalog< T extends TableCatalog & FunctionCatalog & SupportsNamespaces & ViewCatalog> - extends BaseCatalog implements CatalogExtension { + extends BaseCatalog implements CatalogExtension, SparkSupportsLoadContext { + private static final Logger LOG = LoggerFactory.getLogger(SparkSessionCatalog.class); private static final String[] DEFAULT_NAMESPACE = new String[] {"default"}; private String catalogName = null; @@ -144,7 +148,21 @@ public Identifier[] listTables(String[] namespace) throws NoSuchNamespaceExcepti @Override public Table loadTable(Identifier ident) throws NoSuchTableException { + return loadTable(ident, LoadContext.empty()); + } + + @Override + public Table loadTable(Identifier ident, LoadContext context) throws NoSuchTableException { try { + if (!context.referencedBy().isEmpty() && icebergCatalog instanceof SparkSupportsLoadContext) { + return ((SparkSupportsLoadContext) icebergCatalog).loadTable(ident, context); + } + if (!context.referencedBy().isEmpty()) { + LOG.warn( + "Catalog {} does not support contextual load, ignoring context for table {}", + icebergCatalog.name(), + ident); + } return icebergCatalog.loadTable(ident); } catch (NoSuchTableException e) { return getSessionCatalog().loadTable(ident); @@ -153,7 +171,22 @@ public Table loadTable(Identifier ident) throws NoSuchTableException { @Override public Table loadTable(Identifier ident, String version) throws NoSuchTableException { + return loadTable(ident, version, LoadContext.empty()); + } + + @Override + public Table loadTable(Identifier ident, String version, LoadContext context) + throws NoSuchTableException { try { + if (!context.referencedBy().isEmpty() && icebergCatalog instanceof SparkSupportsLoadContext) { + return ((SparkSupportsLoadContext) icebergCatalog).loadTable(ident, version, context); + } + if (!context.referencedBy().isEmpty()) { + LOG.warn( + "Catalog {} does not support contextual load, ignoring context for table {}", + icebergCatalog.name(), + ident); + } return icebergCatalog.loadTable(ident, version); } catch (NoSuchTableException e) { return getSessionCatalog().loadTable(ident, version); @@ -162,7 +195,22 @@ public Table loadTable(Identifier ident, String version) throws NoSuchTableExcep @Override public Table loadTable(Identifier ident, long timestamp) throws NoSuchTableException { + return loadTable(ident, timestamp, LoadContext.empty()); + } + + @Override + public Table loadTable(Identifier ident, long timestamp, LoadContext context) + throws NoSuchTableException { try { + if (!context.referencedBy().isEmpty() && icebergCatalog instanceof SparkSupportsLoadContext) { + return ((SparkSupportsLoadContext) icebergCatalog).loadTable(ident, timestamp, context); + } + if (!context.referencedBy().isEmpty()) { + LOG.warn( + "Catalog {} does not support contextual load, ignoring context for table {}", + icebergCatalog.name(), + ident); + } return icebergCatalog.loadTable(ident, timestamp); } catch (NoSuchTableException e) { return getSessionCatalog().loadTable(ident, timestamp); @@ -488,7 +536,21 @@ public boolean viewExists(Identifier ident) { @Override public View loadView(Identifier ident) throws NoSuchViewException { + return loadView(ident, LoadContext.empty()); + } + + @Override + public View loadView(Identifier ident, LoadContext context) throws NoSuchViewException { if (null != asViewCatalog && asViewCatalog.viewExists(ident)) { + if (!context.referencedBy().isEmpty() && asViewCatalog instanceof SparkSupportsLoadContext) { + return ((SparkSupportsLoadContext) asViewCatalog).loadView(ident, context); + } + if (!context.referencedBy().isEmpty()) { + LOG.warn( + "Catalog {} does not support contextual load, ignoring context for view {}", + asViewCatalog.name(), + ident); + } return asViewCatalog.loadView(ident); } else if (isViewCatalog() && getSessionCatalog().viewExists(ident)) { return getSessionCatalog().loadView(ident); diff --git a/spark/v4.1/spark/src/main/java/org/apache/iceberg/spark/SparkSupportsLoadContext.java b/spark/v4.1/spark/src/main/java/org/apache/iceberg/spark/SparkSupportsLoadContext.java new file mode 100644 index 000000000000..76f2a12487b7 --- /dev/null +++ b/spark/v4.1/spark/src/main/java/org/apache/iceberg/spark/SparkSupportsLoadContext.java @@ -0,0 +1,78 @@ +/* + * 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.iceberg.spark; + +import org.apache.iceberg.catalog.LoadContext; +import org.apache.spark.sql.catalyst.analysis.NoSuchTableException; +import org.apache.spark.sql.catalyst.analysis.NoSuchViewException; +import org.apache.spark.sql.connector.catalog.Identifier; +import org.apache.spark.sql.connector.catalog.Table; +import org.apache.spark.sql.connector.catalog.View; + +/** + * A Spark Catalog API extension for loading tables and views with additional load context. Extends + * the loading contract to accept a {@link LoadContext} that carries the view reference chain and + * other contextual information. + */ +public interface SparkSupportsLoadContext { + + /** + * Load a table with additional load context. + * + * @param identifier a table identifier + * @param context the load context + * @return instance of {@link Table} referred by the identifier + * @throws NoSuchTableException if the table does not exist + */ + Table loadTable(Identifier identifier, LoadContext context) throws NoSuchTableException; + + /** + * Load a table at a specific version with additional load context. + * + * @param identifier a table identifier + * @param version the version string + * @param context the load context + * @return instance of {@link Table} referred by the identifier at the specified version + * @throws NoSuchTableException if the table does not exist + */ + Table loadTable(Identifier identifier, String version, LoadContext context) + throws NoSuchTableException; + + /** + * Load a table at a specific timestamp with additional load context. + * + * @param identifier a table identifier + * @param timestamp the timestamp in microseconds + * @param context the load context + * @return instance of {@link Table} referred by the identifier as of the specified timestamp + * @throws NoSuchTableException if the table does not exist + */ + Table loadTable(Identifier identifier, long timestamp, LoadContext context) + throws NoSuchTableException; + + /** + * Load a view with additional load context. + * + * @param identifier a view identifier + * @param context the load context + * @return instance of {@link View} referred by the identifier + * @throws NoSuchViewException if the view does not exist + */ + View loadView(Identifier identifier, LoadContext context) throws NoSuchViewException; +} From c0a391f794d6f3cb552ae69b44f9e0c559b5d496 Mon Sep 17 00:00:00 2001 From: Prashant Singh Date: Wed, 16 Sep 2026 23:48:36 -0700 Subject: [PATCH 02/11] Spark: Move referenced-by context to Spark 4.2 Generated-by: Codex --- .../sql/catalyst/analysis/ResolveViews.scala | 67 ++------- .../sql/catalyst/analysis/ViewUtil.scala | 142 ------------------ .../views/UnResolvedRelationFromView.scala | 72 --------- .../apache/iceberg/spark/SparkCatalog.java | 53 ++----- .../iceberg/spark/SparkSessionCatalog.java | 64 +------- .../extensions/TestReferencedByViewChain.java | 62 +++----- .../apache/iceberg/spark/SparkCatalog.java | 57 ++++++- .../iceberg/spark/SparkSessionCatalog.java | 75 ++++++++- .../spark/SparkSupportsLoadContext.java | 0 .../sql/catalyst/analysis/ViewUtil.scala | 105 +++++++++++++ 10 files changed, 266 insertions(+), 431 deletions(-) delete mode 100644 spark/v4.1/spark-extensions/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/views/UnResolvedRelationFromView.scala rename spark/{v4.1 => v4.2}/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestReferencedByViewChain.java (79%) rename spark/{v4.1 => v4.2}/spark/src/main/java/org/apache/iceberg/spark/SparkSupportsLoadContext.java (100%) diff --git a/spark/v4.1/spark-extensions/src/main/scala/org/apache/spark/sql/catalyst/analysis/ResolveViews.scala b/spark/v4.1/spark-extensions/src/main/scala/org/apache/spark/sql/catalyst/analysis/ResolveViews.scala index ad4f3101bfc0..83e501257ced 100644 --- a/spark/v4.1/spark-extensions/src/main/scala/org/apache/spark/sql/catalyst/analysis/ResolveViews.scala +++ b/spark/v4.1/spark-extensions/src/main/scala/org/apache/spark/sql/catalyst/analysis/ResolveViews.scala @@ -18,7 +18,6 @@ */ package org.apache.spark.sql.catalyst.analysis -import org.apache.iceberg.catalog.LoadContext import org.apache.iceberg.spark.SparkSQLProperties import org.apache.spark.sql.SparkSession import org.apache.spark.sql.catalyst.FunctionIdentifier @@ -33,7 +32,6 @@ import org.apache.spark.sql.catalyst.plans.logical.Project import org.apache.spark.sql.catalyst.plans.logical.SubqueryAlias import org.apache.spark.sql.catalyst.plans.logical.views.CreateIcebergView import org.apache.spark.sql.catalyst.plans.logical.views.ResolvedV2View -import org.apache.spark.sql.catalyst.plans.logical.views.UnResolvedRelationFromView import org.apache.spark.sql.catalyst.rules.Rule import org.apache.spark.sql.catalyst.trees.CurrentOrigin import org.apache.spark.sql.catalyst.trees.Origin @@ -41,7 +39,6 @@ import org.apache.spark.sql.connector.catalog.CatalogManager import org.apache.spark.sql.connector.catalog.LookupCatalog import org.apache.spark.sql.connector.catalog.View import org.apache.spark.sql.errors.QueryCompilationErrors -import org.apache.spark.sql.execution.datasources.v2.DataSourceV2Relation import org.apache.spark.sql.types.MetadataBuilder case class ResolveViews(spark: SparkSession) extends Rule[LogicalPlan] with LookupCatalog { @@ -67,27 +64,6 @@ case class ResolveViews(spark: SparkSession) extends Rule[LogicalPlan] with Look .map(_ => ResolvedV2View(catalog.asViewCatalog, ident)) .getOrElse(u) - case u @ UnResolvedRelationFromView( - tableParts @ CatalogAndIdentifier(catalog, tableIdent), - viewChain, - options, - isStreaming, - timeTravelVersion, - timeTravelTimestamp) => - val referencedBy = ViewUtil.buildReferencedByChain(viewChain, catalog.name()) - val context = LoadContext.builder().referencedBy(referencedBy).build() - try { - val table = - ViewUtil.loadTable(catalog, tableIdent, context, timeTravelVersion, timeTravelTimestamp) - DataSourceV2Relation.create(table, Some(catalog), Some(tableIdent), options) - } catch { - case _: NoSuchTableException => - ViewUtil - .loadView(catalog, tableIdent, context) - .map(view => createViewRelation(tableParts, view, viewChain)) - .getOrElse(UnresolvedRelation(tableParts, options, isStreaming)) - } - case c @ CreateIcebergView( ResolvedIdentifier(_, _), _, @@ -128,19 +104,12 @@ case class ResolveViews(spark: SparkSession) extends Rule[LogicalPlan] with Look } } - private def createViewRelation( - nameParts: Seq[String], - view: View, - existingChain: Seq[Seq[String]] = Seq.empty): LogicalPlan = { + private def createViewRelation(nameParts: Seq[String], view: View): LogicalPlan = { val parsed = parseViewText(nameParts.quoted, view.query) // Apply any necessary rewrites to preserve correct resolution val viewCatalogAndNamespace: Seq[String] = view.currentCatalog +: view.currentNamespace.toSeq - - val viewChain = - ViewUtil.buildViewChain(nameParts, viewCatalogAndNamespace, existingChain, isCatalog) - - val rewritten = rewriteIdentifiers(parsed, viewCatalogAndNamespace, viewChain) + val rewritten = rewriteIdentifiers(parsed, viewCatalogAndNamespace); // Apply the field aliases and column comments // This logic differs from how Spark handles views in SessionCatalog.fromCatalogTable. @@ -195,13 +164,11 @@ case class ResolveViews(spark: SparkSession) extends Rule[LogicalPlan] with Look private def rewriteIdentifiers( plan: LogicalPlan, - catalogAndNamespace: Seq[String], - viewChain: Seq[Seq[String]] = Seq.empty): LogicalPlan = { + catalogAndNamespace: Seq[String]): LogicalPlan = { // Rewrite unresolved functions and relations qualifyTableIdentifiers( qualifyFunctionIdentifiers(CTESubstitution.apply(plan), catalogAndNamespace), - catalogAndNamespace, - viewChain) + catalogAndNamespace) } private def qualifyFunctionIdentifiers( @@ -222,31 +189,17 @@ case class ResolveViews(spark: SparkSession) extends Rule[LogicalPlan] with Look */ private def qualifyTableIdentifiers( child: LogicalPlan, - catalogAndNamespace: Seq[String], - viewChain: Seq[Seq[String]]): LogicalPlan = { + catalogAndNamespace: Seq[String]): LogicalPlan = child transform { - case UnresolvedRelation(parts, options, isStreaming) => - val qualifiedTableId = ViewUtil.qualifyParts(parts, catalogAndNamespace, isCatalog) - UnResolvedRelationFromView(qualifiedTableId, viewChain, options, isStreaming) - case RelationTimeTravel( - UnresolvedRelation(parts, options, isStreaming), - timestampOpt, - versionOpt) => - val qualifiedTableId = ViewUtil.qualifyParts(parts, catalogAndNamespace, isCatalog) - UnResolvedRelationFromView( - qualifiedTableId, - viewChain, - options, - isStreaming, - timeTravelVersion = versionOpt, - timeTravelTimestamp = timestampOpt) + case u @ UnresolvedRelation(Seq(table), _, _) => + u.copy(multipartIdentifier = catalogAndNamespace :+ table) + case u @ UnresolvedRelation(parts, _, _) if !isCatalog(parts.head) => + u.copy(multipartIdentifier = catalogAndNamespace.head +: parts) case other => other.transformExpressions { case subquery: SubqueryExpression => - subquery.withNewPlan( - qualifyTableIdentifiers(subquery.plan, catalogAndNamespace, viewChain)) + subquery.withNewPlan(qualifyTableIdentifiers(subquery.plan, catalogAndNamespace)) } } - } private def isCatalog(name: String): Boolean = { catalogManager.isCatalogRegistered(name) diff --git a/spark/v4.1/spark-extensions/src/main/scala/org/apache/spark/sql/catalyst/analysis/ViewUtil.scala b/spark/v4.1/spark-extensions/src/main/scala/org/apache/spark/sql/catalyst/analysis/ViewUtil.scala index 14a7f99662fc..c27cb140347e 100644 --- a/spark/v4.1/spark-extensions/src/main/scala/org/apache/spark/sql/catalyst/analysis/ViewUtil.scala +++ b/spark/v4.1/spark-extensions/src/main/scala/org/apache/spark/sql/catalyst/analysis/ViewUtil.scala @@ -18,24 +18,13 @@ */ package org.apache.spark.sql.catalyst.analysis -import java.util -import org.apache.iceberg.catalog.LoadContext -import org.apache.iceberg.catalog.Namespace -import org.apache.iceberg.catalog.TableIdentifier -import org.apache.iceberg.spark.SparkSupportsLoadContext -import org.apache.spark.sql.catalyst.expressions.Expression import org.apache.spark.sql.connector.catalog.CatalogPlugin import org.apache.spark.sql.connector.catalog.Identifier -import org.apache.spark.sql.connector.catalog.Table import org.apache.spark.sql.connector.catalog.View import org.apache.spark.sql.connector.catalog.ViewCatalog import org.apache.spark.sql.errors.QueryCompilationErrors -import scala.jdk.CollectionConverters._ object ViewUtil { - - import org.apache.spark.sql.connector.catalog.CatalogV2Implicits._ - def loadView(catalog: CatalogPlugin, ident: Identifier): Option[View] = catalog match { case viewCatalog: ViewCatalog => try { @@ -46,137 +35,6 @@ object ViewUtil { case _ => None } - def loadView(catalog: CatalogPlugin, ident: Identifier, context: LoadContext): Option[View] = - catalog match { - case supportsLoadContext: SparkSupportsLoadContext => - try { - Option(supportsLoadContext.loadView(ident, context)) - } catch { - case _: NoSuchViewException => None - } - case viewCatalog: ViewCatalog => - try { - Option(viewCatalog.loadView(ident)) - } catch { - case _: NoSuchViewException => None - } - case _ => None - } - - /** - * Build the referenced-by view chain from a sequence of fully-qualified - * view identifier parts. Validates that all entries are fully qualified and belong to the - * same catalog as the target. - */ - def buildReferencedByChain( - viewChain: Seq[Seq[String]], - targetCatalogName: String): java.util.List[TableIdentifier] = { - viewChain.foreach { parts => - require( - parts.size >= 3, - s"View chain entry must be fully qualified [catalog, namespace..., name], got: ${parts.mkString(".")}") - } - val crossCatalogViews = - viewChain.filter(parts => !parts.headOption.contains(targetCatalogName)) - if (crossCatalogViews.nonEmpty) { - throw new IllegalStateException( - s"Cross-catalog view references are not supported with referenced-by enabled. " + - s"Views from catalogs [${crossCatalogViews.map(_.head).distinct.mkString(", ")}] " + - s"cannot reference entities in catalog [$targetCatalogName]") - } - val viewIdentifiers = viewChain.map { parts => - val nsParts = parts.drop(1).init - val ns = Namespace.of(nsParts: _*) - TableIdentifier.of(ns, parts.last) - } - new util.ArrayList[TableIdentifier](viewIdentifiers.asJava) - } - - /** - * Qualify identifier parts against a catalog/namespace prefix. A single-part identifier becomes - * `catalogAndNamespace :+ part`; a multi-part identifier whose head isn't a registered catalog - * is prefixed with the catalog; otherwise it's returned as-is. - */ - def qualifyParts( - parts: Seq[String], - catalogAndNamespace: Seq[String], - isCatalog: String => Boolean): Seq[String] = { - parts match { - case Seq(name) => catalogAndNamespace :+ name - case _ if !isCatalog(parts.head) => catalogAndNamespace.head +: parts - case _ => parts - } - } - - /** - * Build or extend the view chain by appending the current view's fully qualified identifier. - * - * @param nameParts the current view's identifier parts (may be 1, 2, or 3+ parts) - * @param viewCatalogAndNamespace the view's catalog and namespace prefix - * @param existingChain the existing view chain from outer views (empty for the outermost view) - * @param isCatalog function to check if a name is a registered catalog - */ - def buildViewChain( - nameParts: Seq[String], - viewCatalogAndNamespace: Seq[String], - existingChain: Seq[Seq[String]], - isCatalog: String => Boolean): Seq[Seq[String]] = { - existingChain :+ qualifyParts(nameParts, viewCatalogAndNamespace, isCatalog) - } - - /** - * Load a table with contextual load support, dispatching to the appropriate loadTable overload - * based on whether the catalog supports contextual load and whether time travel is requested. - */ - def loadTable( - catalog: CatalogPlugin, - ident: Identifier, - context: LoadContext, - timeTravelVersion: Option[String] = None, - timeTravelTimestamp: Option[Expression] = None): Table = { - catalog match { - case supportsLoadContext: SparkSupportsLoadContext => - loadTableWithTimeTravel( - supportsLoadContext, - ident, - context, - timeTravelVersion, - timeTravelTimestamp) - case c if c.asTableCatalog.isInstanceOf[SparkSupportsLoadContext] => - loadTableWithTimeTravel( - c.asTableCatalog.asInstanceOf[SparkSupportsLoadContext], - ident, - context, - timeTravelVersion, - timeTravelTimestamp) - case _ => - (timeTravelVersion, timeTravelTimestamp) match { - case (Some(version), _) => - catalog.asTableCatalog.loadTable(ident, version) - case (_, Some(timestamp)) => - catalog.asTableCatalog.loadTable(ident, timestamp.eval().asInstanceOf[Long]) - case _ => - catalog.asTableCatalog.loadTable(ident) - } - } - } - - private def loadTableWithTimeTravel( - supportsLoadContext: SparkSupportsLoadContext, - ident: Identifier, - context: LoadContext, - timeTravelVersion: Option[String], - timeTravelTimestamp: Option[Expression]): Table = { - (timeTravelVersion, timeTravelTimestamp) match { - case (Some(version), _) => - supportsLoadContext.loadTable(ident, version, context) - case (_, Some(timestamp)) => - supportsLoadContext.loadTable(ident, timestamp.eval().asInstanceOf[Long], context) - case _ => - supportsLoadContext.loadTable(ident, context) - } - } - def isViewCatalog(catalog: CatalogPlugin): Boolean = { catalog.isInstanceOf[ViewCatalog] } diff --git a/spark/v4.1/spark-extensions/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/views/UnResolvedRelationFromView.scala b/spark/v4.1/spark-extensions/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/views/UnResolvedRelationFromView.scala deleted file mode 100644 index edfdad6e506b..000000000000 --- a/spark/v4.1/spark-extensions/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/views/UnResolvedRelationFromView.scala +++ /dev/null @@ -1,72 +0,0 @@ -/* - * 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.plans.logical.views - -import org.apache.spark.sql.catalyst.expressions.Attribute -import org.apache.spark.sql.catalyst.expressions.Expression -import org.apache.spark.sql.catalyst.plans.logical.LeafNode -import org.apache.spark.sql.catalyst.util.quoteIfNeeded -import org.apache.spark.sql.util.CaseInsensitiveStringMap - -/** - * Represents an unresolved table/relation that was referenced from within a view. - * This node carries both the table identifier and the chain of view identifiers that - * referenced it, enabling view-aware authorization and security mechanisms. - * - * This allows catalogs to implement different security models such as: - * - Definer rights: Use view creator's permissions to access the underlying table - * - Invoker rights: Use current user's permissions to access the underlying table - * - * @param tableMultipartIdentifier The multipart identifier for the target table - * @param viewChain The chain of view identifiers that reference this table, ordered from - * outermost view first to innermost view last. Each element is a multipart - * identifier (Seq[String]). - * @param options Options passed to the table (similar to UnresolvedRelation) - * @param isStreaming Whether this is a streaming relation - * @param timeTravelVersion Optional version for time travel (e.g., snapshot ID or reference name) - * @param timeTravelTimestamp Optional timestamp expression for time travel - */ -case class UnResolvedRelationFromView( - tableMultipartIdentifier: Seq[String], - viewChain: Seq[Seq[String]], - options: CaseInsensitiveStringMap = CaseInsensitiveStringMap.empty(), - override val isStreaming: Boolean = false, - timeTravelVersion: Option[String] = None, - timeTravelTimestamp: Option[Expression] = None) - extends LeafNode { - - override def output: Seq[Attribute] = Nil - - override lazy val resolved = false - - def tableName: String = tableMultipartIdentifier.map(quoteIfNeeded).mkString(".") - - def viewName: String = viewChain.map(_.map(quoteIfNeeded).mkString(".")).mkString(" -> ") - - override def simpleString(maxFields: Int): String = { - s"'UnResolvedRelationFromView [table=$tableName, views=$viewName, " + - s"${if (isStreaming) "streaming=true, " else ""}options=$options]" - } - - override def toString: String = { - val chainStr = viewChain.map(parts => s"[${parts.mkString(", ")}]").mkString(", ") - s"UnResolvedRelationFromView([${tableMultipartIdentifier.mkString(", ")}], " + - s"chain=[$chainStr], $isStreaming)" - } -} diff --git a/spark/v4.1/spark/src/main/java/org/apache/iceberg/spark/SparkCatalog.java b/spark/v4.1/spark/src/main/java/org/apache/iceberg/spark/SparkCatalog.java index da03c35e3fe5..40db152076c8 100644 --- a/spark/v4.1/spark/src/main/java/org/apache/iceberg/spark/SparkCatalog.java +++ b/spark/v4.1/spark/src/main/java/org/apache/iceberg/spark/SparkCatalog.java @@ -38,7 +38,6 @@ import org.apache.iceberg.Schema; import org.apache.iceberg.Transaction; import org.apache.iceberg.catalog.Catalog; -import org.apache.iceberg.catalog.LoadContext; import org.apache.iceberg.catalog.Namespace; import org.apache.iceberg.catalog.SupportsNamespaces; import org.apache.iceberg.catalog.TableIdentifier; @@ -121,7 +120,8 @@ * *

*/ -public class SparkCatalog extends BaseCatalog implements SparkSupportsLoadContext { +public class SparkCatalog extends BaseCatalog { + private static final Logger LOG = LoggerFactory.getLogger(SparkCatalog.class); private static final Set DEFAULT_NS_KEYS = ImmutableSet.of(TableCatalog.PROP_OWNER); private static final Splitter COMMA = Splitter.on(","); @@ -168,46 +168,17 @@ protected TableIdentifier buildIdentifier(Identifier identifier) { @Override public Table loadTable(Identifier ident) throws NoSuchTableException { - return loadTable(ident, LoadContext.empty()); - } - - @Override - public Table loadTable(Identifier ident, LoadContext context) throws NoSuchTableException { - try { - return load(ident, null /* no time travel */, context); - } catch (org.apache.iceberg.exceptions.NoSuchTableException e) { - throw new NoSuchTableException(ident); - } + return load(ident, null /* no time travel */); } @Override public Table loadTable(Identifier ident, String version) throws NoSuchTableException { - return loadTable(ident, version, LoadContext.empty()); - } - - @Override - public Table loadTable(Identifier ident, String version, LoadContext context) - throws NoSuchTableException { - try { - return load(ident, TimeTravel.version(version), context); - } catch (org.apache.iceberg.exceptions.NoSuchTableException e) { - throw new NoSuchTableException(ident); - } + return load(ident, TimeTravel.version(version)); } @Override public Table loadTable(Identifier ident, long timestampMicros) throws NoSuchTableException { - return loadTable(ident, timestampMicros, LoadContext.empty()); - } - - @Override - public Table loadTable(Identifier ident, long timestampMicros, LoadContext context) - throws NoSuchTableException { - try { - return load(ident, TimeTravel.timestampMicros(timestampMicros), context); - } catch (org.apache.iceberg.exceptions.NoSuchTableException e) { - throw new NoSuchTableException(ident); - } + return load(ident, TimeTravel.timestampMicros(timestampMicros)); } @Override @@ -624,14 +595,9 @@ public boolean viewExists(Identifier ident) { @Override public View loadView(Identifier ident) throws NoSuchViewException { - return loadView(ident, LoadContext.empty()); - } - - @Override - public View loadView(Identifier ident, LoadContext context) throws NoSuchViewException { if (null != asViewCatalog) { try { - org.apache.iceberg.view.View view = asViewCatalog.loadView(buildIdentifier(ident), context); + org.apache.iceberg.view.View view = asViewCatalog.loadView(buildIdentifier(ident)); return new SparkView(catalogName, view); } catch (org.apache.iceberg.exceptions.NoSuchViewException e) { throw new NoSuchViewException(ident); @@ -926,14 +892,13 @@ private static void checkNotPathIdentifier(Identifier identifier, String method) } } - private Table load(Identifier ident, TimeTravel timeTravel, LoadContext context) - throws NoSuchTableException { + private Table load(Identifier ident, TimeTravel timeTravel) throws NoSuchTableException { if (isPathIdentifier(ident)) { return loadPath((PathIdentifier) ident, timeTravel); } try { - org.apache.iceberg.Table table = icebergCatalog.loadTable(buildIdentifier(ident), context); + org.apache.iceberg.Table table = icebergCatalog.loadTable(buildIdentifier(ident)); return SparkTable.create(table, timeTravel); } catch (org.apache.iceberg.exceptions.NoSuchTableException e) { @@ -946,7 +911,7 @@ private Table load(Identifier ident, TimeTravel timeTravel, LoadContext context) TableIdentifier namespaceAsIdent = buildIdentifier(namespaceToIdentifier(ident.namespace())); org.apache.iceberg.Table table; try { - table = icebergCatalog.loadTable(namespaceAsIdent, context); + table = icebergCatalog.loadTable(namespaceAsIdent); } catch (Exception ignored) { // the namespace does not identify a table, so it cannot be a table with a snapshot selector // throw an exception for the original identifier diff --git a/spark/v4.1/spark/src/main/java/org/apache/iceberg/spark/SparkSessionCatalog.java b/spark/v4.1/spark/src/main/java/org/apache/iceberg/spark/SparkSessionCatalog.java index cad143ba33a3..ec37435ba119 100644 --- a/spark/v4.1/spark/src/main/java/org/apache/iceberg/spark/SparkSessionCatalog.java +++ b/spark/v4.1/spark/src/main/java/org/apache/iceberg/spark/SparkSessionCatalog.java @@ -24,7 +24,6 @@ import org.apache.iceberg.CatalogProperties; import org.apache.iceberg.CatalogUtil; import org.apache.iceberg.catalog.Catalog; -import org.apache.iceberg.catalog.LoadContext; import org.apache.iceberg.relocated.com.google.common.base.Preconditions; import org.apache.iceberg.spark.source.HasIcebergCatalog; import org.apache.spark.sql.SparkSession; @@ -56,8 +55,6 @@ import org.apache.spark.sql.connector.expressions.Transform; import org.apache.spark.sql.types.StructType; import org.apache.spark.sql.util.CaseInsensitiveStringMap; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; /** * A Spark catalog that can also load non-Iceberg tables. @@ -67,8 +64,7 @@ */ public class SparkSessionCatalog< T extends TableCatalog & FunctionCatalog & SupportsNamespaces & ViewCatalog> - extends BaseCatalog implements CatalogExtension, SparkSupportsLoadContext { - private static final Logger LOG = LoggerFactory.getLogger(SparkSessionCatalog.class); + extends BaseCatalog implements CatalogExtension { private static final String[] DEFAULT_NAMESPACE = new String[] {"default"}; private String catalogName = null; @@ -148,21 +144,7 @@ public Identifier[] listTables(String[] namespace) throws NoSuchNamespaceExcepti @Override public Table loadTable(Identifier ident) throws NoSuchTableException { - return loadTable(ident, LoadContext.empty()); - } - - @Override - public Table loadTable(Identifier ident, LoadContext context) throws NoSuchTableException { try { - if (!context.referencedBy().isEmpty() && icebergCatalog instanceof SparkSupportsLoadContext) { - return ((SparkSupportsLoadContext) icebergCatalog).loadTable(ident, context); - } - if (!context.referencedBy().isEmpty()) { - LOG.warn( - "Catalog {} does not support contextual load, ignoring context for table {}", - icebergCatalog.name(), - ident); - } return icebergCatalog.loadTable(ident); } catch (NoSuchTableException e) { return getSessionCatalog().loadTable(ident); @@ -171,22 +153,7 @@ public Table loadTable(Identifier ident, LoadContext context) throws NoSuchTable @Override public Table loadTable(Identifier ident, String version) throws NoSuchTableException { - return loadTable(ident, version, LoadContext.empty()); - } - - @Override - public Table loadTable(Identifier ident, String version, LoadContext context) - throws NoSuchTableException { try { - if (!context.referencedBy().isEmpty() && icebergCatalog instanceof SparkSupportsLoadContext) { - return ((SparkSupportsLoadContext) icebergCatalog).loadTable(ident, version, context); - } - if (!context.referencedBy().isEmpty()) { - LOG.warn( - "Catalog {} does not support contextual load, ignoring context for table {}", - icebergCatalog.name(), - ident); - } return icebergCatalog.loadTable(ident, version); } catch (NoSuchTableException e) { return getSessionCatalog().loadTable(ident, version); @@ -195,22 +162,7 @@ public Table loadTable(Identifier ident, String version, LoadContext context) @Override public Table loadTable(Identifier ident, long timestamp) throws NoSuchTableException { - return loadTable(ident, timestamp, LoadContext.empty()); - } - - @Override - public Table loadTable(Identifier ident, long timestamp, LoadContext context) - throws NoSuchTableException { try { - if (!context.referencedBy().isEmpty() && icebergCatalog instanceof SparkSupportsLoadContext) { - return ((SparkSupportsLoadContext) icebergCatalog).loadTable(ident, timestamp, context); - } - if (!context.referencedBy().isEmpty()) { - LOG.warn( - "Catalog {} does not support contextual load, ignoring context for table {}", - icebergCatalog.name(), - ident); - } return icebergCatalog.loadTable(ident, timestamp); } catch (NoSuchTableException e) { return getSessionCatalog().loadTable(ident, timestamp); @@ -536,21 +488,7 @@ public boolean viewExists(Identifier ident) { @Override public View loadView(Identifier ident) throws NoSuchViewException { - return loadView(ident, LoadContext.empty()); - } - - @Override - public View loadView(Identifier ident, LoadContext context) throws NoSuchViewException { if (null != asViewCatalog && asViewCatalog.viewExists(ident)) { - if (!context.referencedBy().isEmpty() && asViewCatalog instanceof SparkSupportsLoadContext) { - return ((SparkSupportsLoadContext) asViewCatalog).loadView(ident, context); - } - if (!context.referencedBy().isEmpty()) { - LOG.warn( - "Catalog {} does not support contextual load, ignoring context for view {}", - asViewCatalog.name(), - ident); - } return asViewCatalog.loadView(ident); } else if (isViewCatalog() && getSessionCatalog().viewExists(ident)) { return getSessionCatalog().loadView(ident); diff --git a/spark/v4.1/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestReferencedByViewChain.java b/spark/v4.2/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestReferencedByViewChain.java similarity index 79% rename from spark/v4.1/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestReferencedByViewChain.java rename to spark/v4.2/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestReferencedByViewChain.java index ba2fb7f0bc69..c979e26ed510 100644 --- a/spark/v4.1/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestReferencedByViewChain.java +++ b/spark/v4.2/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestReferencedByViewChain.java @@ -21,6 +21,7 @@ import static org.assertj.core.api.Assertions.assertThat; import java.net.InetAddress; +import java.util.Collections; import java.util.List; import java.util.concurrent.CopyOnWriteArrayList; import java.util.stream.Collectors; @@ -48,19 +49,6 @@ import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; -/** - * Tests that the referenced-by view chain is correctly constructed and passed to {@link - * org.apache.iceberg.catalog.Catalog#loadTable(TableIdentifier, LoadContext)} during view - * resolution. - * - *

This verifies: - * - *

    - *
  • Single view to table: context contains one view identifier - *
  • Nested view to view to table: context contains the full chain (outer first, inner last) - *
  • Direct table access: no context is passed - *
- */ public class TestReferencedByViewChain extends SparkTestHelperBase { private static final String CATALOG_NAME = "ref_test_catalog"; @@ -69,15 +57,8 @@ public class TestReferencedByViewChain extends SparkTestHelperBase { private static SparkSession spark; - /** - * An InMemoryCatalog that records all contextual loadTable/loadView calls for later assertion. - */ public static class ContextTrackingCatalog extends InMemoryCatalog { - - /** Records of (tableIdentifier, referencedBy) captured from loadTable calls. */ public static final List CAPTURED = new CopyOnWriteArrayList<>(); - - /** Records of (viewIdentifier, referencedBy) captured from loadView calls. */ public static final List CAPTURED_VIEWS = new CopyOnWriteArrayList<>(); public static class CapturedContext { @@ -97,15 +78,19 @@ public static void clearCaptured() { @Override public Table loadTable(TableIdentifier identifier, LoadContext context) { - CAPTURED.add(new CapturedContext(identifier, context.referencedBy())); + CAPTURED.add(new CapturedContext(identifier, referencedBy(context))); return super.loadTable(identifier); } @Override public View loadView(TableIdentifier identifier, LoadContext context) { - CAPTURED_VIEWS.add(new CapturedContext(identifier, context.referencedBy())); + CAPTURED_VIEWS.add(new CapturedContext(identifier, referencedBy(context))); return super.loadView(identifier); } + + private static List referencedBy(LoadContext context) { + return context != null ? context.referencedBy() : Collections.emptyList(); + } } @BeforeAll @@ -172,27 +157,25 @@ public void after() { } @Test - public void testDirectTableAccessHasNoContext() { + public void directTableAccessHasEmptyContext() { ContextTrackingCatalog.clearCaptured(); List rows = spark.sql(String.format("SELECT * FROM %s", TABLE_NAME)).collectAsList(); assertThat(rows).hasSize(5); - // direct table access is loaded with a context that carries no view chain assertThat(ContextTrackingCatalog.CAPTURED) .isNotEmpty() .allSatisfy(captured -> assertThat(captured.referencedBy).isEmpty()); } @Test - public void testSingleViewPassesViewIdentifierInContext() { + public void singleViewPassesViewIdentifierInContext() { createView("simple_view", String.format("SELECT id FROM %s", TABLE_NAME)); ContextTrackingCatalog.clearCaptured(); List result = spark.sql("SELECT * FROM simple_view").collectAsList(); assertThat(result).hasSize(5); - // exactly one load carries a view chain; the rest are plain loads with an empty context assertThat(ContextTrackingCatalog.CAPTURED) .filteredOn(captured -> !captured.referencedBy.isEmpty()) .hasSize(1); @@ -200,11 +183,7 @@ public void testSingleViewPassesViewIdentifierInContext() { } @Test - public void testNestedViewChainAccumulatesContext() { - // view_c -> view_b -> view_a -> table - // When querying view_c: - // - loading view_a should receive context with [view_c, view_b] - // - loading table should receive context with [view_c, view_b, view_a] + public void nestedViewChainAccumulatesContext() { createView("view_a", String.format("SELECT id, data FROM %s", TABLE_NAME)); createView("view_b", "SELECT id FROM view_a WHERE id <= 3"); createView("view_c", "SELECT id FROM view_b WHERE id > 1"); @@ -213,17 +192,13 @@ public void testNestedViewChainAccumulatesContext() { List result = rowsToJava(spark.sql("SELECT * FROM view_c").collectAsList()); assertThat(result).hasSize(2).containsExactlyInAnyOrder(new Object[] {2}, new Object[] {3}); - // Verify the table load has the full chain [view_c, view_b, view_a] assertCapturedTableChain( ContextTrackingCatalog.CAPTURED, TABLE_NAME, "view_c", "view_b", "view_a"); - - // Verify view_a was loaded with context containing [view_c, view_b] assertCapturedTableChain(ContextTrackingCatalog.CAPTURED_VIEWS, "view_a", "view_c", "view_b"); } private void createView(String viewName, String sql) { - ViewCatalog catalog = viewCatalog(); - catalog + viewCatalog() .buildView(TableIdentifier.of(NAMESPACE, viewName)) .withQuery("spark", sql) .withDefaultNamespace(NAMESPACE) @@ -232,26 +207,25 @@ private void createView(String viewName, String sql) { .create(); } - /** - * Asserts that a captured context list contains an entry for the given target with the expected - * view chain. - */ private void assertCapturedTableChain( List captures, String targetName, String... expectedViewNames) { List matching = captures.stream() - .filter(c -> c.tableIdentifier.equals(TableIdentifier.of(NAMESPACE, targetName))) - .filter(c -> c.referencedBy != null && !c.referencedBy.isEmpty()) + .filter( + captured -> + captured.tableIdentifier.equals(TableIdentifier.of(NAMESPACE, targetName))) + .filter(captured -> captured.referencedBy != null && !captured.referencedBy.isEmpty()) .collect(Collectors.toList()); assertThat(matching).isNotEmpty(); List viewChain = matching.get(0).referencedBy; assertThat(viewChain).hasSize(expectedViewNames.length); - for (int i = 0; i < expectedViewNames.length; i++) { - assertThat(viewChain.get(i)).isEqualTo(TableIdentifier.of(NAMESPACE, expectedViewNames[i])); + for (int index = 0; index < expectedViewNames.length; index += 1) { + assertThat(viewChain.get(index)) + .isEqualTo(TableIdentifier.of(NAMESPACE, expectedViewNames[index])); } } diff --git a/spark/v4.2/spark/src/main/java/org/apache/iceberg/spark/SparkCatalog.java b/spark/v4.2/spark/src/main/java/org/apache/iceberg/spark/SparkCatalog.java index a0bdfea817fc..faf92654e9bf 100644 --- a/spark/v4.2/spark/src/main/java/org/apache/iceberg/spark/SparkCatalog.java +++ b/spark/v4.2/spark/src/main/java/org/apache/iceberg/spark/SparkCatalog.java @@ -44,6 +44,7 @@ import org.apache.iceberg.Transaction; import org.apache.iceberg.UnboundPartitionSpec; import org.apache.iceberg.catalog.Catalog; +import org.apache.iceberg.catalog.LoadContext; import org.apache.iceberg.catalog.Namespace; import org.apache.iceberg.catalog.SupportsNamespaces; import org.apache.iceberg.catalog.TableIdentifier; @@ -83,6 +84,7 @@ import org.apache.spark.sql.catalyst.analysis.ViewUtil; import org.apache.spark.sql.connector.catalog.Identifier; import org.apache.spark.sql.connector.catalog.NamespaceChange; +import org.apache.spark.sql.connector.catalog.Relation; import org.apache.spark.sql.connector.catalog.StagedTable; import org.apache.spark.sql.connector.catalog.Table; import org.apache.spark.sql.connector.catalog.TableCatalog; @@ -126,7 +128,7 @@ * *

*/ -public class SparkCatalog extends BaseCatalog { +public class SparkCatalog extends BaseCatalog implements SparkSupportsLoadContext { private static final Logger LOG = LoggerFactory.getLogger(SparkCatalog.class); private static final Set DEFAULT_NS_KEYS = ImmutableSet.of(TableCatalog.PROP_OWNER); @@ -194,17 +196,49 @@ protected TableIdentifier buildIdentifier(Identifier identifier) { @Override public Table loadTable(Identifier ident) throws NoSuchTableException { - return load(ident, null /* no time travel */); + return loadTable(ident, LoadContext.empty()); + } + + @Override + public Table loadTable(Identifier ident, LoadContext context) throws NoSuchTableException { + return load(ident, null /* no time travel */, context); } @Override public Table loadTable(Identifier ident, String version) throws NoSuchTableException { - return load(ident, TimeTravel.version(version)); + return loadTable(ident, version, LoadContext.empty()); + } + + @Override + public Table loadTable(Identifier ident, String version, LoadContext context) + throws NoSuchTableException { + return load(ident, TimeTravel.version(version), context); } @Override public Table loadTable(Identifier ident, long timestampMicros) throws NoSuchTableException { - return load(ident, TimeTravel.timestampMicros(timestampMicros)); + return loadTable(ident, timestampMicros, LoadContext.empty()); + } + + @Override + public Table loadTable(Identifier ident, long timestampMicros, LoadContext context) + throws NoSuchTableException { + return load(ident, TimeTravel.timestampMicros(timestampMicros), context); + } + + @Override + public Relation loadRelation(Identifier ident) throws NoSuchTableException { + LoadContext context = ViewUtil.loadContext(catalogName); + try { + return loadTable(ident, context); + } catch (NoSuchTableException e) { + try { + return ViewUtil.withReferencedByContext(loadView(ident, context), catalogName, ident); + } catch (NoSuchViewException viewException) { + e.addSuppressed(viewException); + throw e; + } + } } @Override @@ -691,9 +725,15 @@ public boolean viewExists(Identifier ident) { @Override public View loadView(Identifier ident) throws NoSuchViewException { + return loadView(ident, LoadContext.empty()); + } + + @Override + public View loadView(Identifier ident, LoadContext context) throws NoSuchViewException { if (null != asViewCatalog) { try { - org.apache.iceberg.view.View icebergView = asViewCatalog.loadView(buildIdentifier(ident)); + org.apache.iceberg.view.View icebergView = + asViewCatalog.loadView(buildIdentifier(ident), context); return SparkView.toView(catalogName, icebergView); } catch (org.apache.iceberg.exceptions.NoSuchViewException e) { throw new NoSuchViewException(ident); @@ -1055,13 +1095,14 @@ private static void checkNotPathIdentifier(Identifier identifier, String method) } } - private Table load(Identifier ident, TimeTravel timeTravel) throws NoSuchTableException { + private Table load(Identifier ident, TimeTravel timeTravel, LoadContext context) + throws NoSuchTableException { if (isPathIdentifier(ident)) { return loadPath((PathIdentifier) ident, timeTravel); } try { - org.apache.iceberg.Table table = icebergCatalog.loadTable(buildIdentifier(ident)); + org.apache.iceberg.Table table = icebergCatalog.loadTable(buildIdentifier(ident), context); return SparkTable.create(table, timeTravel); } catch (org.apache.iceberg.exceptions.NoSuchTableException e) { @@ -1074,7 +1115,7 @@ private Table load(Identifier ident, TimeTravel timeTravel) throws NoSuchTableEx TableIdentifier namespaceAsIdent = buildIdentifier(namespaceToIdentifier(ident.namespace())); org.apache.iceberg.Table table; try { - table = icebergCatalog.loadTable(namespaceAsIdent); + table = icebergCatalog.loadTable(namespaceAsIdent, context); } catch (Exception ignored) { // the namespace does not identify a table, so it cannot be a table with a snapshot selector // throw an exception for the original identifier diff --git a/spark/v4.2/spark/src/main/java/org/apache/iceberg/spark/SparkSessionCatalog.java b/spark/v4.2/spark/src/main/java/org/apache/iceberg/spark/SparkSessionCatalog.java index 457c46edb6ee..fa59dffb2fa6 100644 --- a/spark/v4.2/spark/src/main/java/org/apache/iceberg/spark/SparkSessionCatalog.java +++ b/spark/v4.2/spark/src/main/java/org/apache/iceberg/spark/SparkSessionCatalog.java @@ -27,6 +27,7 @@ import org.apache.iceberg.CatalogProperties; import org.apache.iceberg.CatalogUtil; import org.apache.iceberg.catalog.Catalog; +import org.apache.iceberg.catalog.LoadContext; import org.apache.iceberg.catalog.TableIdentifier; import org.apache.iceberg.exceptions.AlreadyExistsException; import org.apache.iceberg.relocated.com.google.common.base.Preconditions; @@ -42,11 +43,13 @@ import org.apache.spark.sql.catalyst.analysis.NonEmptyNamespaceException; import org.apache.spark.sql.catalyst.analysis.TableAlreadyExistsException; import org.apache.spark.sql.catalyst.analysis.ViewAlreadyExistsException; +import org.apache.spark.sql.catalyst.analysis.ViewUtil; import org.apache.spark.sql.connector.catalog.CatalogExtension; import org.apache.spark.sql.connector.catalog.CatalogPlugin; import org.apache.spark.sql.connector.catalog.FunctionCatalog; import org.apache.spark.sql.connector.catalog.Identifier; import org.apache.spark.sql.connector.catalog.NamespaceChange; +import org.apache.spark.sql.connector.catalog.Relation; import org.apache.spark.sql.connector.catalog.StagedTable; import org.apache.spark.sql.connector.catalog.StagingTableCatalog; import org.apache.spark.sql.connector.catalog.SupportsNamespaces; @@ -59,6 +62,8 @@ import org.apache.spark.sql.connector.catalog.ViewCatalog; import org.apache.spark.sql.connector.catalog.functions.UnboundFunction; import org.apache.spark.sql.util.CaseInsensitiveStringMap; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; /** * A Spark catalog that can also load non-Iceberg tables. @@ -68,7 +73,8 @@ */ public class SparkSessionCatalog< T extends TableCatalog & FunctionCatalog & SupportsNamespaces & ViewCatalog> - extends BaseCatalog implements CatalogExtension { + extends BaseCatalog implements CatalogExtension, SparkSupportsLoadContext { + private static final Logger LOG = LoggerFactory.getLogger(SparkSessionCatalog.class); private static final String[] DEFAULT_NAMESPACE = new String[] {"default"}; private String catalogName = null; @@ -199,7 +205,17 @@ private static boolean isViewType(String tableType) { @Override public Table loadTable(Identifier ident) throws NoSuchTableException { + return loadTable(ident, LoadContext.empty()); + } + + @Override + public Table loadTable(Identifier ident, LoadContext context) throws NoSuchTableException { try { + if (icebergCatalog instanceof SparkSupportsLoadContext) { + return ((SparkSupportsLoadContext) icebergCatalog).loadTable(ident, context); + } + + logUnsupportedContext("table", ident, context); return icebergCatalog.loadTable(ident); } catch (NoSuchTableException e) { return getSessionCatalog().loadTable(ident); @@ -208,7 +224,18 @@ public Table loadTable(Identifier ident) throws NoSuchTableException { @Override public Table loadTable(Identifier ident, String version) throws NoSuchTableException { + return loadTable(ident, version, LoadContext.empty()); + } + + @Override + public Table loadTable(Identifier ident, String version, LoadContext context) + throws NoSuchTableException { try { + if (icebergCatalog instanceof SparkSupportsLoadContext) { + return ((SparkSupportsLoadContext) icebergCatalog).loadTable(ident, version, context); + } + + logUnsupportedContext("table", ident, context); return icebergCatalog.loadTable(ident, version); } catch (NoSuchTableException e) { return getSessionCatalog().loadTable(ident, version); @@ -217,13 +244,39 @@ public Table loadTable(Identifier ident, String version) throws NoSuchTableExcep @Override public Table loadTable(Identifier ident, long timestamp) throws NoSuchTableException { + return loadTable(ident, timestamp, LoadContext.empty()); + } + + @Override + public Table loadTable(Identifier ident, long timestamp, LoadContext context) + throws NoSuchTableException { try { + if (icebergCatalog instanceof SparkSupportsLoadContext) { + return ((SparkSupportsLoadContext) icebergCatalog).loadTable(ident, timestamp, context); + } + + logUnsupportedContext("table", ident, context); return icebergCatalog.loadTable(ident, timestamp); } catch (NoSuchTableException e) { return getSessionCatalog().loadTable(ident, timestamp); } } + @Override + public Relation loadRelation(Identifier ident) throws NoSuchTableException { + LoadContext context = ViewUtil.loadContext(catalogName); + try { + return loadTable(ident, context); + } catch (NoSuchTableException e) { + try { + return ViewUtil.withReferencedByContext(loadView(ident, context), catalogName, ident); + } catch (NoSuchViewException viewException) { + e.addSuppressed(viewException); + throw e; + } + } + } + @Override public void invalidateTable(Identifier ident) { // We do not need to check whether the table exists and whether @@ -595,7 +648,17 @@ public boolean viewExists(Identifier ident) { @Override public View loadView(Identifier ident) throws NoSuchViewException { + return loadView(ident, LoadContext.empty()); + } + + @Override + public View loadView(Identifier ident, LoadContext context) throws NoSuchViewException { if (null != asViewCatalog && asViewCatalog.viewExists(ident)) { + if (asViewCatalog instanceof SparkSupportsLoadContext) { + return ((SparkSupportsLoadContext) asViewCatalog).loadView(ident, context); + } + + logUnsupportedContext("view", ident, context); return asViewCatalog.loadView(ident); } else if (isViewCatalog() && getSessionCatalog().viewExists(ident)) { return getSessionCatalog().loadView(ident); @@ -604,6 +667,16 @@ public View loadView(Identifier ident) throws NoSuchViewException { throw new NoSuchViewException(ident); } + private void logUnsupportedContext(String relationType, Identifier ident, LoadContext context) { + if (!context.referencedBy().isEmpty()) { + LOG.warn( + "Catalog {} does not support contextual load, ignoring context for {} {}", + catalogName, + relationType, + ident); + } + } + @Override public View createView(Identifier ident, View view) throws ViewAlreadyExistsException, NoSuchNamespaceException { diff --git a/spark/v4.1/spark/src/main/java/org/apache/iceberg/spark/SparkSupportsLoadContext.java b/spark/v4.2/spark/src/main/java/org/apache/iceberg/spark/SparkSupportsLoadContext.java similarity index 100% rename from spark/v4.1/spark/src/main/java/org/apache/iceberg/spark/SparkSupportsLoadContext.java rename to spark/v4.2/spark/src/main/java/org/apache/iceberg/spark/SparkSupportsLoadContext.java diff --git a/spark/v4.2/spark/src/main/scala/org/apache/spark/sql/catalyst/analysis/ViewUtil.scala b/spark/v4.2/spark/src/main/scala/org/apache/spark/sql/catalyst/analysis/ViewUtil.scala index e7a7060097e9..8056acb3f159 100644 --- a/spark/v4.2/spark/src/main/scala/org/apache/spark/sql/catalyst/analysis/ViewUtil.scala +++ b/spark/v4.2/spark/src/main/scala/org/apache/spark/sql/catalyst/analysis/ViewUtil.scala @@ -18,6 +18,13 @@ */ package org.apache.spark.sql.catalyst.analysis +import java.nio.charset.StandardCharsets +import java.util +import java.util.Base64 + +import org.apache.iceberg.catalog.LoadContext +import org.apache.iceberg.catalog.Namespace +import org.apache.iceberg.catalog.TableIdentifier import org.apache.iceberg.spark.Spark3Util import org.apache.iceberg.spark.source.HasIcebergCatalog import org.apache.iceberg.spark.source.SparkView @@ -29,9 +36,14 @@ import org.apache.spark.sql.connector.catalog.V1View import org.apache.spark.sql.connector.catalog.View import org.apache.spark.sql.connector.catalog.ViewCatalog import org.apache.spark.sql.errors.QueryCompilationErrors +import org.apache.spark.sql.internal.SQLConf import scala.jdk.CollectionConverters._ object ViewUtil { + private val REFERENCED_BY_CONTEXT = "spark.sql.iceberg.referenced-by-context" + private val ENTRY_SEPARATOR = "\u001e" + private val PART_SEPARATOR = "\u001f" + val RESERVED_PROPERTIES: Seq[String] = Seq( TableCatalog.PROP_COMMENT, @@ -73,6 +85,68 @@ object ViewUtil { None } + def loadContext(targetCatalogName: String): LoadContext = { + LoadContext + .builder() + .referencedBy(buildReferencedByChain(currentViewChain, targetCatalogName)) + .build() + } + + def withReferencedByContext(view: View, catalogName: String, ident: Identifier): View = { + if (view == null || view.queryText() == null || view.columns() == null) { + view + } else { + val viewChain = currentViewChain :+ qualifiedView(catalogName, ident) + val sqlConfigs = + Option(view.sqlConfigs()).map(_.asScala.toMap).getOrElse(Map.empty) + + (REFERENCED_BY_CONTEXT -> encodeViewChain(viewChain)) + + SparkView + .applyOptionalFields( + new View.Builder() + .withColumns(view.columns()) + .withProperties(view.properties()) + .withQueryText(view.queryText()) + .withCurrentCatalog(view.currentCatalog()) + .withCurrentNamespace(view.currentNamespace()) + .withSqlConfigs(sqlConfigs.asJava) + .withQueryColumnNames(view.queryColumnNames()), + view.schemaMode(), + sqlConfigs.asJava, + view.viewDependencies()) + .build() + } + } + + /** + * Build the referenced-by view chain from fully qualified view identifier parts. + * Entries must belong to the same catalog as the loaded target. + */ + def buildReferencedByChain( + viewChain: Seq[Seq[String]], + targetCatalogName: String): java.util.List[TableIdentifier] = { + viewChain.foreach { parts => + require( + parts.size >= 3, + s"View chain entry must be fully qualified [catalog, namespace..., name], got: " + + parts.mkString(".")) + } + + val crossCatalogViews = viewChain.filter(parts => !parts.headOption.contains(targetCatalogName)) + if (crossCatalogViews.nonEmpty) { + throw new IllegalStateException( + s"Cross-catalog view references are not supported with referenced-by enabled. " + + s"Views from catalogs [${crossCatalogViews.map(_.head).distinct.mkString(", ")}] " + + s"cannot reference entities in catalog [$targetCatalogName]") + } + + val viewIdentifiers = viewChain.map { parts => + TableIdentifier.of(Namespace.of(parts.drop(1).init: _*), parts.last) + } + + new util.ArrayList[TableIdentifier](viewIdentifiers.asJava) + } + def isIcebergViewCatalog(catalog: CatalogPlugin): Boolean = { catalog.isInstanceOf[ViewCatalog] && catalog.isInstanceOf[HasIcebergCatalog] } @@ -90,4 +164,35 @@ object ViewUtil { throw QueryCompilationErrors.missingCatalogViewsAbilityError(plugin) } } + + private def currentViewChain: Seq[Seq[String]] = { + val encoded = SQLConf.get.getConfString(REFERENCED_BY_CONTEXT, null) + if (encoded == null || encoded.isEmpty) { + Seq.empty + } else { + encoded + .split(ENTRY_SEPARATOR, -1) + .toIndexedSeq + .filter(_.nonEmpty) + .map(_.split(PART_SEPARATOR, -1).toIndexedSeq.map(decodePart)) + } + } + + private def qualifiedView(catalogName: String, ident: Identifier): Seq[String] = { + catalogName +: ident.namespace().toIndexedSeq :+ ident.name() + } + + private def encodeViewChain(viewChain: Seq[Seq[String]]): String = { + viewChain.map(_.map(encodePart).mkString(PART_SEPARATOR)).mkString(ENTRY_SEPARATOR) + } + + private def encodePart(part: String): String = { + Base64.getUrlEncoder + .withoutPadding() + .encodeToString(part.getBytes(StandardCharsets.UTF_8)) + } + + private def decodePart(part: String): String = { + new String(Base64.getUrlDecoder.decode(part), StandardCharsets.UTF_8) + } } From 759a96ae0b084c731a93401dc3479bcd86b29e06 Mon Sep 17 00:00:00 2001 From: Prashant Singh Date: Thu, 17 Sep 2026 00:16:54 -0700 Subject: [PATCH 03/11] Spark: Fix ViewUtil Scala import grouping Generated-by: Codex --- .../scala/org/apache/spark/sql/catalyst/analysis/ViewUtil.scala | 1 - 1 file changed, 1 deletion(-) diff --git a/spark/v4.2/spark/src/main/scala/org/apache/spark/sql/catalyst/analysis/ViewUtil.scala b/spark/v4.2/spark/src/main/scala/org/apache/spark/sql/catalyst/analysis/ViewUtil.scala index 8056acb3f159..0853a876c520 100644 --- a/spark/v4.2/spark/src/main/scala/org/apache/spark/sql/catalyst/analysis/ViewUtil.scala +++ b/spark/v4.2/spark/src/main/scala/org/apache/spark/sql/catalyst/analysis/ViewUtil.scala @@ -21,7 +21,6 @@ package org.apache.spark.sql.catalyst.analysis import java.nio.charset.StandardCharsets import java.util import java.util.Base64 - import org.apache.iceberg.catalog.LoadContext import org.apache.iceberg.catalog.Namespace import org.apache.iceberg.catalog.TableIdentifier From 8eeaffa14f58308d9059f4fb8906c3dd9cdc9935 Mon Sep 17 00:00:00 2001 From: Prashant Singh Date: Thu, 17 Sep 2026 11:04:20 -0700 Subject: [PATCH 04/11] Spark 4.2: Implement contextual loadTable in TestSparkCatalog TestSparkCatalog only overrode loadTable(Identifier), so once the read path started resolving through loadRelation -> loadTable(Identifier, LoadContext), the double was no longer consulted and lookups fell through to the real catalog. Override the contextual variant to delegate to the existing one. The double ignores the context; TestReferencedByViewChain covers the referenced-by behaviour against real catalogs. Co-Authored-By: Claude Opus 5 (1M context) --- .../org/apache/iceberg/spark/source/TestSparkCatalog.java | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/spark/v4.2/spark/src/test/java/org/apache/iceberg/spark/source/TestSparkCatalog.java b/spark/v4.2/spark/src/test/java/org/apache/iceberg/spark/source/TestSparkCatalog.java index ce24e173c67b..9416535b025f 100644 --- a/spark/v4.2/spark/src/test/java/org/apache/iceberg/spark/source/TestSparkCatalog.java +++ b/spark/v4.2/spark/src/test/java/org/apache/iceberg/spark/source/TestSparkCatalog.java @@ -19,6 +19,7 @@ package org.apache.iceberg.spark.source; import java.util.Map; +import org.apache.iceberg.catalog.LoadContext; import org.apache.iceberg.catalog.Namespace; import org.apache.iceberg.catalog.TableIdentifier; import org.apache.iceberg.relocated.com.google.common.base.Preconditions; @@ -66,6 +67,11 @@ public Table loadTable(Identifier ident) throws NoSuchTableException { return new SparkTable(table); } + @Override + public Table loadTable(Identifier ident, LoadContext context) throws NoSuchTableException { + return loadTable(ident); + } + public static void clearTables() { TABLE_MAP.clear(); } From 48024a954f150bbf74d5746fe3d9fd1684706ebe Mon Sep 17 00:00:00 2001 From: Prashant Singh Date: Fri, 18 Sep 2026 08:35:35 -0700 Subject: [PATCH 05/11] Spark 4.2: Restore explicit referenced-by view resolution Restore the Spark 4.1-style analyzer path for referenced-by propagation in Spark 4.2. ResolveViews now expands Iceberg views, carries the view chain through UnResolvedRelationFromView, and loads nested relations with an explicit LoadContext instead of using SQLConf-backed view metadata. Generated-by: Codex --- .../IcebergSparkSessionExtensions.scala | 2 + .../sql/catalyst/analysis/ResolveViews.scala | 172 +++++++++++++++++- .../views/UnResolvedRelationFromView.scala | 48 +++++ .../apache/iceberg/spark/SparkCatalog.java | 5 +- .../iceberg/spark/SparkSessionCatalog.java | 6 +- .../sql/catalyst/analysis/ViewUtil.scala | 160 +++++++++------- 6 files changed, 309 insertions(+), 84 deletions(-) create mode 100644 spark/v4.2/spark-extensions/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/views/UnResolvedRelationFromView.scala diff --git a/spark/v4.2/spark-extensions/src/main/scala/org/apache/iceberg/spark/extensions/IcebergSparkSessionExtensions.scala b/spark/v4.2/spark-extensions/src/main/scala/org/apache/iceberg/spark/extensions/IcebergSparkSessionExtensions.scala index 81824e05e92d..6cb5b594c0ea 100644 --- a/spark/v4.2/spark-extensions/src/main/scala/org/apache/iceberg/spark/extensions/IcebergSparkSessionExtensions.scala +++ b/spark/v4.2/spark-extensions/src/main/scala/org/apache/iceberg/spark/extensions/IcebergSparkSessionExtensions.scala @@ -33,6 +33,8 @@ class IcebergSparkSessionExtensions extends (SparkSessionExtensions => Unit) { extensions.injectParser { case (_, parser) => new IcebergSparkSqlExtensionsParser(parser) } // analyzer extensions + // Run before Spark's ResolveRelations so Iceberg can rewrite view children with load context. + extensions.injectHintResolutionRule { spark => ResolveViews(spark) } extensions.injectResolutionRule { spark => ResolveViews(spark) } extensions.injectPostHocResolutionRule { spark => ResolveBranch(spark) } extensions.injectCheckRule(_ => CheckViews) diff --git a/spark/v4.2/spark-extensions/src/main/scala/org/apache/spark/sql/catalyst/analysis/ResolveViews.scala b/spark/v4.2/spark-extensions/src/main/scala/org/apache/spark/sql/catalyst/analysis/ResolveViews.scala index 4945c7ea56ce..d7be8d714c51 100644 --- a/spark/v4.2/spark-extensions/src/main/scala/org/apache/spark/sql/catalyst/analysis/ResolveViews.scala +++ b/spark/v4.2/spark-extensions/src/main/scala/org/apache/spark/sql/catalyst/analysis/ResolveViews.scala @@ -18,29 +18,187 @@ */ package org.apache.spark.sql.catalyst.analysis +import org.apache.iceberg.catalog.LoadContext import org.apache.spark.sql.SparkSession import org.apache.spark.sql.catalyst.analysis.ViewUtil.IcebergViewHelper +import org.apache.spark.sql.catalyst.expressions.Alias +import org.apache.spark.sql.catalyst.expressions.Cast +import org.apache.spark.sql.catalyst.expressions.SubqueryExpression +import org.apache.spark.sql.catalyst.expressions.UpCast +import org.apache.spark.sql.catalyst.parser.ParseException import org.apache.spark.sql.catalyst.plans.logical.LogicalPlan +import org.apache.spark.sql.catalyst.plans.logical.Project +import org.apache.spark.sql.catalyst.plans.logical.SubqueryAlias import org.apache.spark.sql.catalyst.plans.logical.views.ResolvedV2View +import org.apache.spark.sql.catalyst.plans.logical.views.UnResolvedRelationFromView import org.apache.spark.sql.catalyst.rules.Rule +import org.apache.spark.sql.catalyst.trees.CurrentOrigin +import org.apache.spark.sql.catalyst.trees.Origin import org.apache.spark.sql.connector.catalog.CatalogManager +import org.apache.spark.sql.connector.catalog.CatalogPlugin +import org.apache.spark.sql.connector.catalog.Identifier import org.apache.spark.sql.connector.catalog.LookupCatalog +import org.apache.spark.sql.connector.catalog.View +import org.apache.spark.sql.errors.QueryCompilationErrors +import org.apache.spark.sql.execution.datasources.v2.DataSourceV2Relation -/** - * Resolves Iceberg views referenced by commands that use `UnresolvedTableOrView`. - * - * In Spark 4.2, view relations are expanded through `RelationCatalog.loadRelation`, so Spark owns - * query parsing, identifier resolution, and schema-mode application. This rule only converts the - * remaining command targets to `ResolvedV2View` for Iceberg command planning. - */ case class ResolveViews(spark: SparkSession) extends Rule[LogicalPlan] with LookupCatalog { + + import org.apache.spark.sql.connector.catalog.CatalogV2Implicits._ + protected lazy val catalogManager: CatalogManager = spark.sessionState.catalogManager override def apply(plan: LogicalPlan): LogicalPlan = plan resolveOperators { + case u @ UnresolvedRelation(nameParts, _, _) + if catalogManager.v1SessionCatalog.isTempView(nameParts) => + u + + case u @ UnresolvedRelation(parts @ CatalogAndIdentifier(catalog, ident), _, _) => + ViewUtil + .loadView(catalog, ident) + .map(createViewRelation(parts, catalog, ident, _)) + .getOrElse(u) + case u @ UnresolvedTableOrView(CatalogAndIdentifier(catalog, ident), _, _, _) => ViewUtil .loadView(catalog, ident) .map(view => ResolvedV2View(catalog.asViewCatalog, ident, view)) .getOrElse(u) + + case u @ UnResolvedRelationFromView( + tableParts @ CatalogAndIdentifier(catalog, tableIdent), + viewChain, + options, + isStreaming, + timeTravelVersion, + timeTravelTimestamp) => + val referencedBy = ViewUtil.buildReferencedByChain(viewChain, catalog.name()) + val context = LoadContext.builder().referencedBy(referencedBy).build() + try { + val table = + ViewUtil.loadTable(catalog, tableIdent, context, timeTravelVersion, timeTravelTimestamp) + DataSourceV2Relation.create(table, Some(catalog), Some(tableIdent), options) + } catch { + case _: NoSuchTableException => + ViewUtil + .loadView(catalog, tableIdent, context) + .map(view => createViewRelation(tableParts, catalog, tableIdent, view, viewChain)) + .getOrElse(UnresolvedRelation(tableParts, options, isStreaming)) + } + } + + private def createViewRelation( + nameParts: Seq[String], + catalog: CatalogPlugin, + ident: Identifier, + view: View, + existingChain: Seq[Seq[String]] = Seq.empty): LogicalPlan = { + val parsed = parseViewText(nameParts.quoted, view.queryText()) + val viewCatalogAndNamespace = catalogAndNamespace(catalog, ident, view) + val viewChain = + ViewUtil.buildViewChain(nameParts, viewCatalogAndNamespace, existingChain, isCatalog) + val rewritten = rewriteIdentifiers(parsed, viewCatalogAndNamespace, viewChain) + val aliases = view + .schema() + .fields + .zipWithIndex + .map { case (expected, pos) => + val attr = GetColumnByOrdinal(pos, expected.dataType) + val cast = if (isSchemaCompensation(view.schemaMode())) { + Cast(attr, expected.dataType, ansiEnabled = true) + } else { + UpCast(attr, expected.dataType) + } + Alias(cast, expected.name)(explicitMetadata = Some(expected.metadata)) + } + .toIndexedSeq + + SubqueryAlias(nameParts, Project(aliases, rewritten)) + } + + private def catalogAndNamespace( + catalog: CatalogPlugin, + ident: Identifier, + view: View): Seq[String] = { + val currentCatalog = Option(view.currentCatalog()).filter(_.nonEmpty).getOrElse(catalog.name()) + val currentNamespace = Option(view.currentNamespace()).map(_.toIndexedSeq).getOrElse { + ident.namespace().toIndexedSeq + } + currentCatalog +: currentNamespace + } + + private def parseViewText(name: String, viewText: String): LogicalPlan = { + val origin = Origin(objectType = Some("VIEW"), objectName = Some(name)) + try { + CurrentOrigin.withOrigin(origin) { + spark.sessionState.sqlParser.parseQuery(viewText) + } + } catch { + case _: ParseException => + throw QueryCompilationErrors.invalidViewNameError(name) + } + } + + private def rewriteIdentifiers( + plan: LogicalPlan, + catalogAndNamespace: Seq[String], + viewChain: Seq[Seq[String]]): LogicalPlan = { + qualifyTableIdentifiers( + qualifyFunctionIdentifiers(CTESubstitution.apply(plan), catalogAndNamespace), + catalogAndNamespace, + viewChain) + } + + private def qualifyFunctionIdentifiers( + plan: LogicalPlan, + catalogAndNamespace: Seq[String]): LogicalPlan = plan transformExpressions { + case u @ UnresolvedFunction(Seq(name), _, _, _, _, _, _) => + if (!isBuiltinFunction(name)) { + u.copy(nameParts = catalogAndNamespace :+ name) + } else { + u + } + case u @ UnresolvedFunction(parts, _, _, _, _, _, _) if !isCatalog(parts.head) => + u.copy(nameParts = catalogAndNamespace.head +: parts) + } + + private def qualifyTableIdentifiers( + child: LogicalPlan, + catalogAndNamespace: Seq[String], + viewChain: Seq[Seq[String]]): LogicalPlan = { + child transform { + case UnresolvedRelation(parts, options, isStreaming) => + val qualifiedTableId = ViewUtil.qualifyParts(parts, catalogAndNamespace, isCatalog) + UnResolvedRelationFromView(qualifiedTableId, viewChain, options, isStreaming) + case RelationTimeTravel( + UnresolvedRelation(parts, options, isStreaming), + timestampOpt, + versionOpt) => + val qualifiedTableId = ViewUtil.qualifyParts(parts, catalogAndNamespace, isCatalog) + UnResolvedRelationFromView( + qualifiedTableId, + viewChain, + options, + isStreaming, + timeTravelVersion = versionOpt, + timeTravelTimestamp = timestampOpt) + case other => + other.transformExpressions { case subquery: SubqueryExpression => + subquery.withNewPlan( + qualifyTableIdentifiers(subquery.plan, catalogAndNamespace, viewChain)) + } + } + } + + private def isSchemaCompensation(mode: String): Boolean = { + mode != null && mode.equalsIgnoreCase(SchemaCompensation.toString) + } + + private def isCatalog(name: String): Boolean = { + catalogManager.isCatalogRegistered(name) + } + + private def isBuiltinFunction(name: String): Boolean = { + catalogManager.v1SessionCatalog.isBuiltinFunction(name) } } diff --git a/spark/v4.2/spark-extensions/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/views/UnResolvedRelationFromView.scala b/spark/v4.2/spark-extensions/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/views/UnResolvedRelationFromView.scala new file mode 100644 index 000000000000..6ee0dccd90c6 --- /dev/null +++ b/spark/v4.2/spark-extensions/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/views/UnResolvedRelationFromView.scala @@ -0,0 +1,48 @@ +/* + * 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.plans.logical.views + +import org.apache.spark.sql.catalyst.expressions.Attribute +import org.apache.spark.sql.catalyst.expressions.Expression +import org.apache.spark.sql.catalyst.plans.logical.LeafNode +import org.apache.spark.sql.catalyst.util.quoteIfNeeded +import org.apache.spark.sql.util.CaseInsensitiveStringMap + +case class UnResolvedRelationFromView( + tableMultipartIdentifier: Seq[String], + viewChain: Seq[Seq[String]], + options: CaseInsensitiveStringMap = CaseInsensitiveStringMap.empty(), + override val isStreaming: Boolean = false, + timeTravelVersion: Option[String] = None, + timeTravelTimestamp: Option[Expression] = None) + extends LeafNode { + + override def output: Seq[Attribute] = Nil + + override lazy val resolved: Boolean = false + + def tableName: String = tableMultipartIdentifier.map(quoteIfNeeded).mkString(".") + + def viewName: String = viewChain.map(_.map(quoteIfNeeded).mkString(".")).mkString(" -> ") + + override def simpleString(maxFields: Int): String = { + s"'UnResolvedRelationFromView [table=$tableName, views=$viewName, " + + s"${if (isStreaming) "streaming=true, " else ""}options=$options]" + } +} diff --git a/spark/v4.2/spark/src/main/java/org/apache/iceberg/spark/SparkCatalog.java b/spark/v4.2/spark/src/main/java/org/apache/iceberg/spark/SparkCatalog.java index faf92654e9bf..6effd3762603 100644 --- a/spark/v4.2/spark/src/main/java/org/apache/iceberg/spark/SparkCatalog.java +++ b/spark/v4.2/spark/src/main/java/org/apache/iceberg/spark/SparkCatalog.java @@ -228,12 +228,11 @@ public Table loadTable(Identifier ident, long timestampMicros, LoadContext conte @Override public Relation loadRelation(Identifier ident) throws NoSuchTableException { - LoadContext context = ViewUtil.loadContext(catalogName); try { - return loadTable(ident, context); + return loadTable(ident); } catch (NoSuchTableException e) { try { - return ViewUtil.withReferencedByContext(loadView(ident, context), catalogName, ident); + return loadView(ident); } catch (NoSuchViewException viewException) { e.addSuppressed(viewException); throw e; diff --git a/spark/v4.2/spark/src/main/java/org/apache/iceberg/spark/SparkSessionCatalog.java b/spark/v4.2/spark/src/main/java/org/apache/iceberg/spark/SparkSessionCatalog.java index fa59dffb2fa6..a2f434611c7b 100644 --- a/spark/v4.2/spark/src/main/java/org/apache/iceberg/spark/SparkSessionCatalog.java +++ b/spark/v4.2/spark/src/main/java/org/apache/iceberg/spark/SparkSessionCatalog.java @@ -43,7 +43,6 @@ import org.apache.spark.sql.catalyst.analysis.NonEmptyNamespaceException; import org.apache.spark.sql.catalyst.analysis.TableAlreadyExistsException; import org.apache.spark.sql.catalyst.analysis.ViewAlreadyExistsException; -import org.apache.spark.sql.catalyst.analysis.ViewUtil; import org.apache.spark.sql.connector.catalog.CatalogExtension; import org.apache.spark.sql.connector.catalog.CatalogPlugin; import org.apache.spark.sql.connector.catalog.FunctionCatalog; @@ -264,12 +263,11 @@ public Table loadTable(Identifier ident, long timestamp, LoadContext context) @Override public Relation loadRelation(Identifier ident) throws NoSuchTableException { - LoadContext context = ViewUtil.loadContext(catalogName); try { - return loadTable(ident, context); + return loadTable(ident); } catch (NoSuchTableException e) { try { - return ViewUtil.withReferencedByContext(loadView(ident, context), catalogName, ident); + return loadView(ident); } catch (NoSuchViewException viewException) { e.addSuppressed(viewException); throw e; diff --git a/spark/v4.2/spark/src/main/scala/org/apache/spark/sql/catalyst/analysis/ViewUtil.scala b/spark/v4.2/spark/src/main/scala/org/apache/spark/sql/catalyst/analysis/ViewUtil.scala index 0853a876c520..340ba67ed808 100644 --- a/spark/v4.2/spark/src/main/scala/org/apache/spark/sql/catalyst/analysis/ViewUtil.scala +++ b/spark/v4.2/spark/src/main/scala/org/apache/spark/sql/catalyst/analysis/ViewUtil.scala @@ -18,30 +18,29 @@ */ package org.apache.spark.sql.catalyst.analysis -import java.nio.charset.StandardCharsets import java.util -import java.util.Base64 import org.apache.iceberg.catalog.LoadContext import org.apache.iceberg.catalog.Namespace import org.apache.iceberg.catalog.TableIdentifier import org.apache.iceberg.spark.Spark3Util +import org.apache.iceberg.spark.SparkSupportsLoadContext import org.apache.iceberg.spark.source.HasIcebergCatalog import org.apache.iceberg.spark.source.SparkView import org.apache.iceberg.view.{View => IcebergView} +import org.apache.spark.sql.catalyst.expressions.Expression import org.apache.spark.sql.connector.catalog.CatalogPlugin import org.apache.spark.sql.connector.catalog.Identifier +import org.apache.spark.sql.connector.catalog.Table import org.apache.spark.sql.connector.catalog.TableCatalog import org.apache.spark.sql.connector.catalog.V1View import org.apache.spark.sql.connector.catalog.View import org.apache.spark.sql.connector.catalog.ViewCatalog import org.apache.spark.sql.errors.QueryCompilationErrors -import org.apache.spark.sql.internal.SQLConf import scala.jdk.CollectionConverters._ object ViewUtil { - private val REFERENCED_BY_CONTEXT = "spark.sql.iceberg.referenced-by-context" - private val ENTRY_SEPARATOR = "\u001e" - private val PART_SEPARATOR = "\u001f" + + import org.apache.spark.sql.connector.catalog.CatalogV2Implicits._ val RESERVED_PROPERTIES: Seq[String] = Seq( @@ -73,6 +72,23 @@ object ViewUtil { case _ => None } + def loadView(catalog: CatalogPlugin, ident: Identifier, context: LoadContext): Option[View] = + catalog match { + case supportsLoadContext: SparkSupportsLoadContext => + try { + Option(supportsLoadContext.loadView(ident, context)).filter(isIcebergView) + } catch { + case _: NoSuchViewException => None + } + case viewCatalog: ViewCatalog if catalog.isInstanceOf[HasIcebergCatalog] => + try { + Option(viewCatalog.loadView(ident)).filter(isIcebergView) + } catch { + case _: NoSuchViewException => None + } + case _ => None + } + def loadIcebergView(catalog: CatalogPlugin, ident: Identifier): Option[IcebergView] = catalog match { case catalogWithIceberg: HasIcebergCatalog => @@ -84,39 +100,6 @@ object ViewUtil { None } - def loadContext(targetCatalogName: String): LoadContext = { - LoadContext - .builder() - .referencedBy(buildReferencedByChain(currentViewChain, targetCatalogName)) - .build() - } - - def withReferencedByContext(view: View, catalogName: String, ident: Identifier): View = { - if (view == null || view.queryText() == null || view.columns() == null) { - view - } else { - val viewChain = currentViewChain :+ qualifiedView(catalogName, ident) - val sqlConfigs = - Option(view.sqlConfigs()).map(_.asScala.toMap).getOrElse(Map.empty) + - (REFERENCED_BY_CONTEXT -> encodeViewChain(viewChain)) - - SparkView - .applyOptionalFields( - new View.Builder() - .withColumns(view.columns()) - .withProperties(view.properties()) - .withQueryText(view.queryText()) - .withCurrentCatalog(view.currentCatalog()) - .withCurrentNamespace(view.currentNamespace()) - .withSqlConfigs(sqlConfigs.asJava) - .withQueryColumnNames(view.queryColumnNames()), - view.schemaMode(), - sqlConfigs.asJava, - view.viewDependencies()) - .build() - } - } - /** * Build the referenced-by view chain from fully qualified view identifier parts. * Entries must belong to the same catalog as the loaded target. @@ -146,10 +129,78 @@ object ViewUtil { new util.ArrayList[TableIdentifier](viewIdentifiers.asJava) } + def qualifyParts( + parts: Seq[String], + catalogAndNamespace: Seq[String], + isCatalog: String => Boolean): Seq[String] = { + parts match { + case Seq(name) => catalogAndNamespace :+ name + case _ if !isCatalog(parts.head) => catalogAndNamespace.head +: parts + case _ => parts + } + } + + def buildViewChain( + nameParts: Seq[String], + viewCatalogAndNamespace: Seq[String], + existingChain: Seq[Seq[String]], + isCatalog: String => Boolean): Seq[Seq[String]] = { + existingChain :+ qualifyParts(nameParts, viewCatalogAndNamespace, isCatalog) + } + + def loadTable( + catalog: CatalogPlugin, + ident: Identifier, + context: LoadContext, + timeTravelVersion: Option[String] = None, + timeTravelTimestamp: Option[Expression] = None): Table = { + catalog match { + case supportsLoadContext: SparkSupportsLoadContext => + loadTableWithTimeTravel( + supportsLoadContext, + ident, + context, + timeTravelVersion, + timeTravelTimestamp) + case c if c.asTableCatalog.isInstanceOf[SparkSupportsLoadContext] => + loadTableWithTimeTravel( + c.asTableCatalog.asInstanceOf[SparkSupportsLoadContext], + ident, + context, + timeTravelVersion, + timeTravelTimestamp) + case _ => + (timeTravelVersion, timeTravelTimestamp) match { + case (Some(version), _) => + catalog.asTableCatalog.loadTable(ident, version) + case (_, Some(timestamp)) => + catalog.asTableCatalog.loadTable(ident, timestamp.eval().asInstanceOf[Long]) + case _ => + catalog.asTableCatalog.loadTable(ident) + } + } + } + def isIcebergViewCatalog(catalog: CatalogPlugin): Boolean = { catalog.isInstanceOf[ViewCatalog] && catalog.isInstanceOf[HasIcebergCatalog] } + private def loadTableWithTimeTravel( + supportsLoadContext: SparkSupportsLoadContext, + ident: Identifier, + context: LoadContext, + timeTravelVersion: Option[String], + timeTravelTimestamp: Option[Expression]): Table = { + (timeTravelVersion, timeTravelTimestamp) match { + case (Some(version), _) => + supportsLoadContext.loadTable(ident, version, context) + case (_, Some(timestamp)) => + supportsLoadContext.loadTable(ident, timestamp.eval().asInstanceOf[Long], context) + case _ => + supportsLoadContext.loadTable(ident, context) + } + } + private def isIcebergView(view: View): Boolean = { !view.isInstanceOf[V1View] && "iceberg".equalsIgnoreCase(view.properties.get(TableCatalog.PROP_PROVIDER)) @@ -163,35 +214,4 @@ object ViewUtil { throw QueryCompilationErrors.missingCatalogViewsAbilityError(plugin) } } - - private def currentViewChain: Seq[Seq[String]] = { - val encoded = SQLConf.get.getConfString(REFERENCED_BY_CONTEXT, null) - if (encoded == null || encoded.isEmpty) { - Seq.empty - } else { - encoded - .split(ENTRY_SEPARATOR, -1) - .toIndexedSeq - .filter(_.nonEmpty) - .map(_.split(PART_SEPARATOR, -1).toIndexedSeq.map(decodePart)) - } - } - - private def qualifiedView(catalogName: String, ident: Identifier): Seq[String] = { - catalogName +: ident.namespace().toIndexedSeq :+ ident.name() - } - - private def encodeViewChain(viewChain: Seq[Seq[String]]): String = { - viewChain.map(_.map(encodePart).mkString(PART_SEPARATOR)).mkString(ENTRY_SEPARATOR) - } - - private def encodePart(part: String): String = { - Base64.getUrlEncoder - .withoutPadding() - .encodeToString(part.getBytes(StandardCharsets.UTF_8)) - } - - private def decodePart(part: String): String = { - new String(Base64.getUrlDecoder.decode(part), StandardCharsets.UTF_8) - } } From 55bfb199c445289d78947723a3ae1389c44c9da8 Mon Sep 17 00:00:00 2001 From: Prashant Singh Date: Fri, 18 Sep 2026 11:32:37 -0700 Subject: [PATCH 06/11] Core, Spark: Address referenced-by review feedback Preserve existing cross-catalog view behavior by dropping catalog-relative referenced-by entries that cannot be represented for the target catalog. Bypass CachingCatalog for non-empty LoadContext loads so referenced-by reaches catalogs such as REST, and add regression coverage for cache-enabled Spark reads, time-travel views, and cross-catalog view references. Generated-by: Codex --- .../org/apache/iceberg/CachingCatalog.java | 10 ++ .../iceberg/hadoop/TestCachingCatalog.java | 43 +++++++ .../extensions/TestReferencedByViewChain.java | 105 +++++++++++++++--- .../sql/catalyst/analysis/ViewUtil.scala | 12 +- 4 files changed, 143 insertions(+), 27 deletions(-) diff --git a/core/src/main/java/org/apache/iceberg/CachingCatalog.java b/core/src/main/java/org/apache/iceberg/CachingCatalog.java index 4ee6d61c0910..7a8492f73da8 100644 --- a/core/src/main/java/org/apache/iceberg/CachingCatalog.java +++ b/core/src/main/java/org/apache/iceberg/CachingCatalog.java @@ -29,6 +29,7 @@ import java.util.Map; import java.util.concurrent.atomic.AtomicBoolean; import org.apache.iceberg.catalog.Catalog; +import org.apache.iceberg.catalog.LoadContext; import org.apache.iceberg.catalog.Namespace; import org.apache.iceberg.catalog.TableIdentifier; import org.apache.iceberg.exceptions.AlreadyExistsException; @@ -168,6 +169,15 @@ public Table loadTable(TableIdentifier ident) { return table; } + @Override + public Table loadTable(TableIdentifier ident, LoadContext context) { + if (context == null || context.referencedBy().isEmpty()) { + return loadTable(ident); + } + + return catalog.loadTable(ident, context); + } + @Override public boolean dropTable(TableIdentifier ident, boolean purge) { boolean dropped = catalog.dropTable(ident, purge); diff --git a/core/src/test/java/org/apache/iceberg/hadoop/TestCachingCatalog.java b/core/src/test/java/org/apache/iceberg/hadoop/TestCachingCatalog.java index 1c36c1acea2e..876800e98d66 100644 --- a/core/src/test/java/org/apache/iceberg/hadoop/TestCachingCatalog.java +++ b/core/src/test/java/org/apache/iceberg/hadoop/TestCachingCatalog.java @@ -31,6 +31,7 @@ import java.util.concurrent.Executors; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicInteger; +import org.apache.hadoop.conf.Configuration; import org.apache.iceberg.BaseMetadataTable; import org.apache.iceberg.CachingCatalog; import org.apache.iceberg.CatalogProperties; @@ -40,6 +41,7 @@ import org.apache.iceberg.Table; import org.apache.iceberg.TestableCachingCatalog; import org.apache.iceberg.catalog.Catalog; +import org.apache.iceberg.catalog.LoadContext; import org.apache.iceberg.catalog.Namespace; import org.apache.iceberg.catalog.TableIdentifier; import org.apache.iceberg.exceptions.NoSuchTableException; @@ -275,6 +277,25 @@ public void testCatalogExpirationTtlRefreshesAfterAccessViaCatalog() throws IOEx assertThat(catalog.remainingAgeFor(tableIdent)).get().isEqualTo(HALF_OF_EXPIRATION); } + @Test + public void contextualLoadBypassesTableCache() throws IOException { + ContextTrackingHadoopCatalog underlyingCatalog = contextTrackingHadoopCatalog(); + TestableCachingCatalog catalog = + TestableCachingCatalog.wrap(underlyingCatalog, EXPIRATION_TTL, ticker); + Namespace namespace = Namespace.of("db", "ns1", "ns2"); + TableIdentifier tableIdent = TableIdentifier.of(namespace, "tbl"); + TableIdentifier viewIdent = TableIdentifier.of(namespace, "view"); + Table cachedTable = catalog.createTable(tableIdent, SCHEMA, SPEC, ImmutableMap.of()); + + LoadContext context = LoadContext.builder().referencedBy(Lists.newArrayList(viewIdent)).build(); + Table contextualTable = catalog.loadTable(tableIdent, context); + + assertThat(contextualTable).isNotSameAs(cachedTable); + assertThat(catalog.cache().asMap()).containsEntry(tableIdent, cachedTable); + assertThat(underlyingCatalog.loadContexts()).hasSize(1); + assertThat(underlyingCatalog.loadContexts().get(0).referencedBy()).containsExactly(viewIdent); + } + @Test public void testCacheExpirationEagerlyRemovesMetadataTables() throws IOException { TestableCachingCatalog catalog = @@ -441,4 +462,26 @@ public static TableIdentifier[] metadataTables(TableIdentifier tableIdent) { .map(type -> TableIdentifier.parse(tableIdent + "." + type.name().toLowerCase(Locale.ROOT))) .toArray(TableIdentifier[]::new); } + + private ContextTrackingHadoopCatalog contextTrackingHadoopCatalog() { + ContextTrackingHadoopCatalog catalog = new ContextTrackingHadoopCatalog(); + catalog.setConf(new Configuration()); + catalog.initialize( + "hadoop", ImmutableMap.of(CatalogProperties.WAREHOUSE_LOCATION, tempDir.getAbsolutePath())); + return catalog; + } + + private static class ContextTrackingHadoopCatalog extends HadoopCatalog { + private final List loadContexts = Lists.newArrayList(); + + @Override + public Table loadTable(TableIdentifier identifier, LoadContext context) { + loadContexts.add(context); + return super.loadTable(identifier); + } + + private List loadContexts() { + return loadContexts; + } + } } diff --git a/spark/v4.2/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestReferencedByViewChain.java b/spark/v4.2/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestReferencedByViewChain.java index c979e26ed510..eab9adb8a1fd 100644 --- a/spark/v4.2/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestReferencedByViewChain.java +++ b/spark/v4.2/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestReferencedByViewChain.java @@ -28,7 +28,6 @@ import java.util.stream.IntStream; import org.apache.iceberg.CatalogProperties; import org.apache.iceberg.Table; -import org.apache.iceberg.catalog.Catalog; import org.apache.iceberg.catalog.LoadContext; import org.apache.iceberg.catalog.Namespace; import org.apache.iceberg.catalog.TableIdentifier; @@ -38,11 +37,13 @@ import org.apache.iceberg.spark.SparkCatalog; import org.apache.iceberg.spark.SparkSchemaUtil; import org.apache.iceberg.spark.SparkTestHelperBase; +import org.apache.iceberg.spark.source.HasIcebergCatalog; import org.apache.iceberg.spark.source.SimpleRecord; import org.apache.iceberg.view.View; import org.apache.spark.sql.Dataset; import org.apache.spark.sql.Row; import org.apache.spark.sql.SparkSession; +import org.apache.spark.sql.connector.catalog.CatalogPlugin; import org.junit.jupiter.api.AfterAll; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeAll; @@ -52,8 +53,10 @@ public class TestReferencedByViewChain extends SparkTestHelperBase { private static final String CATALOG_NAME = "ref_test_catalog"; + private static final String OTHER_CATALOG_NAME = "other_ref_test_catalog"; private static final Namespace NAMESPACE = Namespace.of("default"); private static final String TABLE_NAME = "test_table"; + private static final String OTHER_TABLE_NAME = "other_table"; private static SparkSession spark; @@ -76,12 +79,24 @@ public static void clearCaptured() { CAPTURED_VIEWS.clear(); } + @Override + public Table loadTable(TableIdentifier identifier) { + CAPTURED.add(new CapturedContext(identifier, Collections.emptyList())); + return super.loadTable(identifier); + } + @Override public Table loadTable(TableIdentifier identifier, LoadContext context) { CAPTURED.add(new CapturedContext(identifier, referencedBy(context))); return super.loadTable(identifier); } + @Override + public View loadView(TableIdentifier identifier) { + CAPTURED_VIEWS.add(new CapturedContext(identifier, Collections.emptyList())); + return super.loadView(identifier); + } + @Override public View loadView(TableIdentifier identifier, LoadContext context) { CAPTURED_VIEWS.add(new CapturedContext(identifier, referencedBy(context))); @@ -109,11 +124,18 @@ public static void startSpark() { "spark.sql.catalog." + CATALOG_NAME + "." + CatalogProperties.CATALOG_IMPL, ContextTrackingCatalog.class.getName()) .config("spark.sql.catalog." + CATALOG_NAME + ".default-namespace", "default") - .config("spark.sql.catalog." + CATALOG_NAME + ".cache-enabled", "false") + .config("spark.sql.catalog." + CATALOG_NAME + ".cache-enabled", "true") + .config("spark.sql.catalog." + OTHER_CATALOG_NAME, SparkCatalog.class.getName()) + .config( + "spark.sql.catalog." + OTHER_CATALOG_NAME + "." + CatalogProperties.CATALOG_IMPL, + ContextTrackingCatalog.class.getName()) + .config("spark.sql.catalog." + OTHER_CATALOG_NAME + ".default-namespace", "default") + .config("spark.sql.catalog." + OTHER_CATALOG_NAME + ".cache-enabled", "true") .config("spark.sql.defaultCatalog", CATALOG_NAME) .getOrCreate(); - spark.sql(String.format("CREATE NAMESPACE IF NOT EXISTS %s", NAMESPACE)); + spark.sql(String.format("CREATE NAMESPACE IF NOT EXISTS %s.%s", CATALOG_NAME, NAMESPACE)); + spark.sql(String.format("CREATE NAMESPACE IF NOT EXISTS %s.%s", OTHER_CATALOG_NAME, NAMESPACE)); } @AfterAll @@ -130,17 +152,8 @@ public void before() { spark.sql(String.format("USE %s.%s", CATALOG_NAME, NAMESPACE)); spark.sql(String.format("CREATE TABLE IF NOT EXISTS %s (id INT, data STRING)", TABLE_NAME)); - - try { - List records = - IntStream.rangeClosed(1, 5) - .mapToObj(i -> new SimpleRecord(i, String.valueOf(i))) - .collect(Collectors.toList()); - Dataset df = spark.createDataFrame(records, SimpleRecord.class); - df.writeTo(TABLE_NAME).append(); - } catch (org.apache.spark.sql.catalyst.analysis.NoSuchTableException e) { - throw new RuntimeException(e); - } + appendRecords(TABLE_NAME); + spark.sql(String.format("REFRESH TABLE %s", TABLE_NAME)); ContextTrackingCatalog.clearCaptured(); } @@ -152,7 +165,12 @@ public void after() { spark.sql("DROP VIEW IF EXISTS view_a"); spark.sql("DROP VIEW IF EXISTS view_b"); spark.sql("DROP VIEW IF EXISTS view_c"); + spark.sql("DROP VIEW IF EXISTS time_travel_view"); + spark.sql("DROP VIEW IF EXISTS cross_view"); spark.sql(String.format("DROP TABLE IF EXISTS %s", TABLE_NAME)); + spark.sql( + String.format( + "DROP TABLE IF EXISTS %s.%s.%s", OTHER_CATALOG_NAME, NAMESPACE, OTHER_TABLE_NAME)); ContextTrackingCatalog.clearCaptured(); } @@ -197,6 +215,44 @@ public void nestedViewChainAccumulatesContext() { assertCapturedTableChain(ContextTrackingCatalog.CAPTURED_VIEWS, "view_a", "view_c", "view_b"); } + @Test + public void timeTravelViewPassesViewIdentifierInContext() throws Exception { + long snapshotId = Spark3Util.loadIcebergTable(spark, TABLE_NAME).currentSnapshot().snapshotId(); + createView( + "time_travel_view", + String.format("SELECT id FROM %s VERSION AS OF %d", TABLE_NAME, snapshotId)); + ContextTrackingCatalog.clearCaptured(); + + List result = spark.sql("SELECT * FROM time_travel_view").collectAsList(); + assertThat(result).hasSize(5); + + assertCapturedTableChain(ContextTrackingCatalog.CAPTURED, TABLE_NAME, "time_travel_view"); + } + + @Test + public void crossCatalogViewAccessDoesNotFail() { + String otherTable = String.format("%s.%s.%s", OTHER_CATALOG_NAME, NAMESPACE, OTHER_TABLE_NAME); + spark.sql(String.format("CREATE TABLE %s (id INT, data STRING)", otherTable)); + appendRecords(otherTable); + spark.sql(String.format("REFRESH TABLE %s", otherTable)); + createView("cross_view", String.format("SELECT id FROM %s", otherTable)); + spark.sql(String.format("REFRESH TABLE %s", otherTable)); + ContextTrackingCatalog.clearCaptured(); + + List result = spark.sql("SELECT * FROM cross_view").collectAsList(); + assertThat(result).hasSize(5); + + assertThat( + ContextTrackingCatalog.CAPTURED.stream() + .filter( + captured -> + captured.tableIdentifier.equals( + TableIdentifier.of(NAMESPACE, OTHER_TABLE_NAME))) + .collect(Collectors.toList())) + .isNotEmpty() + .allSatisfy(captured -> assertThat(captured.referencedBy).isEmpty()); + } + private void createView(String viewName, String sql) { viewCatalog() .buildView(TableIdentifier.of(NAMESPACE, viewName)) @@ -207,6 +263,19 @@ private void createView(String viewName, String sql) { .create(); } + private void appendRecords(String tableName) { + try { + List records = + IntStream.rangeClosed(1, 5) + .mapToObj(i -> new SimpleRecord(i, String.valueOf(i))) + .collect(Collectors.toList()); + Dataset df = spark.createDataFrame(records, SimpleRecord.class); + df.writeTo(tableName).append(); + } catch (org.apache.spark.sql.catalyst.analysis.NoSuchTableException e) { + throw new RuntimeException(e); + } + } + private void assertCapturedTableChain( List captures, String targetName, @@ -230,8 +299,10 @@ private void assertCapturedTableChain( } private ViewCatalog viewCatalog() { - Catalog icebergCatalog = Spark3Util.loadIcebergCatalog(spark, CATALOG_NAME); - assertThat(icebergCatalog).isInstanceOf(ViewCatalog.class); - return (ViewCatalog) icebergCatalog; + CatalogPlugin catalogPlugin = spark.sessionState().catalogManager().catalog(CATALOG_NAME); + assertThat(catalogPlugin).isInstanceOf(HasIcebergCatalog.class); + ViewCatalog viewCatalog = ((HasIcebergCatalog) catalogPlugin).icebergViewCatalog(); + assertThat(viewCatalog).isNotNull(); + return viewCatalog; } } diff --git a/spark/v4.2/spark/src/main/scala/org/apache/spark/sql/catalyst/analysis/ViewUtil.scala b/spark/v4.2/spark/src/main/scala/org/apache/spark/sql/catalyst/analysis/ViewUtil.scala index 340ba67ed808..5b8bfcddd840 100644 --- a/spark/v4.2/spark/src/main/scala/org/apache/spark/sql/catalyst/analysis/ViewUtil.scala +++ b/spark/v4.2/spark/src/main/scala/org/apache/spark/sql/catalyst/analysis/ViewUtil.scala @@ -102,7 +102,7 @@ object ViewUtil { /** * Build the referenced-by view chain from fully qualified view identifier parts. - * Entries must belong to the same catalog as the loaded target. + * Cross-catalog entries are omitted because LoadContext uses catalog-relative identifiers. */ def buildReferencedByChain( viewChain: Seq[Seq[String]], @@ -114,15 +114,7 @@ object ViewUtil { parts.mkString(".")) } - val crossCatalogViews = viewChain.filter(parts => !parts.headOption.contains(targetCatalogName)) - if (crossCatalogViews.nonEmpty) { - throw new IllegalStateException( - s"Cross-catalog view references are not supported with referenced-by enabled. " + - s"Views from catalogs [${crossCatalogViews.map(_.head).distinct.mkString(", ")}] " + - s"cannot reference entities in catalog [$targetCatalogName]") - } - - val viewIdentifiers = viewChain.map { parts => + val viewIdentifiers = viewChain.filter(_.headOption.contains(targetCatalogName)).map { parts => TableIdentifier.of(Namespace.of(parts.drop(1).init: _*), parts.last) } From 3dc7bd86cc474a257ca5dd184e2a9e2d71bd0c68 Mon Sep 17 00:00:00 2001 From: Prashant Singh Date: Fri, 18 Sep 2026 12:00:03 -0700 Subject: [PATCH 07/11] Spark 4.2: Gate referenced-by view resolution Add a default-off Spark SQL flag for referenced-by view-chain propagation. When disabled, Spark keeps its native view resolution behavior, including cross-catalog views. When enabled, Iceberg uses the explicit analyzer path and rejects cross-catalog chains because LoadContext carries catalog-relative identifiers. Generated-by: Codex --- .../sql/catalyst/analysis/ResolveViews.scala | 38 ++++++++++++--- .../extensions/TestReferencedByViewChain.java | 47 ++++++++++++++++--- .../iceberg/spark/SparkSQLProperties.java | 4 ++ .../sql/catalyst/analysis/ViewUtil.scala | 12 ++++- 4 files changed, 85 insertions(+), 16 deletions(-) diff --git a/spark/v4.2/spark-extensions/src/main/scala/org/apache/spark/sql/catalyst/analysis/ResolveViews.scala b/spark/v4.2/spark-extensions/src/main/scala/org/apache/spark/sql/catalyst/analysis/ResolveViews.scala index d7be8d714c51..f3d1d7c593fe 100644 --- a/spark/v4.2/spark-extensions/src/main/scala/org/apache/spark/sql/catalyst/analysis/ResolveViews.scala +++ b/spark/v4.2/spark-extensions/src/main/scala/org/apache/spark/sql/catalyst/analysis/ResolveViews.scala @@ -19,6 +19,7 @@ package org.apache.spark.sql.catalyst.analysis import org.apache.iceberg.catalog.LoadContext +import org.apache.iceberg.spark.SparkSQLProperties import org.apache.spark.sql.SparkSession import org.apache.spark.sql.catalyst.analysis.ViewUtil.IcebergViewHelper import org.apache.spark.sql.catalyst.expressions.Alias @@ -48,7 +49,15 @@ case class ResolveViews(spark: SparkSession) extends Rule[LogicalPlan] with Look protected lazy val catalogManager: CatalogManager = spark.sessionState.catalogManager - override def apply(plan: LogicalPlan): LogicalPlan = plan resolveOperators { + override def apply(plan: LogicalPlan): LogicalPlan = { + if (referencedByEnabled) { + resolveWithReferencedBy(plan) + } else { + resolveViewCommands(plan) + } + } + + private def resolveWithReferencedBy(plan: LogicalPlan): LogicalPlan = plan resolveOperators { case u @ UnresolvedRelation(nameParts, _, _) if catalogManager.v1SessionCatalog.isTempView(nameParts) => u @@ -59,12 +68,6 @@ case class ResolveViews(spark: SparkSession) extends Rule[LogicalPlan] with Look .map(createViewRelation(parts, catalog, ident, _)) .getOrElse(u) - case u @ UnresolvedTableOrView(CatalogAndIdentifier(catalog, ident), _, _, _) => - ViewUtil - .loadView(catalog, ident) - .map(view => ResolvedV2View(catalog.asViewCatalog, ident, view)) - .getOrElse(u) - case u @ UnResolvedRelationFromView( tableParts @ CatalogAndIdentifier(catalog, tableIdent), viewChain, @@ -85,6 +88,20 @@ case class ResolveViews(spark: SparkSession) extends Rule[LogicalPlan] with Look .map(view => createViewRelation(tableParts, catalog, tableIdent, view, viewChain)) .getOrElse(UnresolvedRelation(tableParts, options, isStreaming)) } + + case u @ UnresolvedTableOrView(CatalogAndIdentifier(catalog, ident), _, _, _) => + ViewUtil + .loadView(catalog, ident) + .map(view => ResolvedV2View(catalog.asViewCatalog, ident, view)) + .getOrElse(u) + } + + private def resolveViewCommands(plan: LogicalPlan): LogicalPlan = plan resolveOperators { + case u @ UnresolvedTableOrView(CatalogAndIdentifier(catalog, ident), _, _, _) => + ViewUtil + .loadView(catalog, ident) + .map(view => ResolvedV2View(catalog.asViewCatalog, ident, view)) + .getOrElse(u) } private def createViewRelation( @@ -201,4 +218,11 @@ case class ResolveViews(spark: SparkSession) extends Rule[LogicalPlan] with Look private def isBuiltinFunction(name: String): Boolean = { catalogManager.v1SessionCatalog.isBuiltinFunction(name) } + + private def referencedByEnabled: Boolean = { + java.lang.Boolean.parseBoolean( + conf.getConfString( + SparkSQLProperties.REFERENCED_BY_ENABLED, + SparkSQLProperties.REFERENCED_BY_ENABLED_DEFAULT.toString)) + } } diff --git a/spark/v4.2/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestReferencedByViewChain.java b/spark/v4.2/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestReferencedByViewChain.java index eab9adb8a1fd..8427d1e43309 100644 --- a/spark/v4.2/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestReferencedByViewChain.java +++ b/spark/v4.2/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestReferencedByViewChain.java @@ -19,6 +19,7 @@ package org.apache.iceberg.spark.extensions; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; import java.net.InetAddress; import java.util.Collections; @@ -35,6 +36,7 @@ import org.apache.iceberg.inmemory.InMemoryCatalog; import org.apache.iceberg.spark.Spark3Util; import org.apache.iceberg.spark.SparkCatalog; +import org.apache.iceberg.spark.SparkSQLProperties; import org.apache.iceberg.spark.SparkSchemaUtil; import org.apache.iceberg.spark.SparkTestHelperBase; import org.apache.iceberg.spark.source.HasIcebergCatalog; @@ -150,6 +152,7 @@ public static void stopSpark() { public void before() { ContextTrackingCatalog.clearCaptured(); + spark.conf().set(SparkSQLProperties.REFERENCED_BY_ENABLED, "true"); spark.sql(String.format("USE %s.%s", CATALOG_NAME, NAMESPACE)); spark.sql(String.format("CREATE TABLE IF NOT EXISTS %s (id INT, data STRING)", TABLE_NAME)); appendRecords(TABLE_NAME); @@ -174,6 +177,21 @@ public void after() { ContextTrackingCatalog.clearCaptured(); } + @Test + public void referencedByIsDisabledByDefault() { + createView("simple_view", String.format("SELECT id FROM %s", TABLE_NAME)); + spark.conf().unset(SparkSQLProperties.REFERENCED_BY_ENABLED); + ContextTrackingCatalog.clearCaptured(); + + List result = spark.sql("SELECT * FROM simple_view").collectAsList(); + assertThat(result).hasSize(5); + + assertThat(ContextTrackingCatalog.CAPTURED) + .allSatisfy(captured -> assertThat(captured.referencedBy).isEmpty()); + assertThat(ContextTrackingCatalog.CAPTURED_VIEWS) + .allSatisfy(captured -> assertThat(captured.referencedBy).isEmpty()); + } + @Test public void directTableAccessHasEmptyContext() { ContextTrackingCatalog.clearCaptured(); @@ -230,13 +248,9 @@ public void timeTravelViewPassesViewIdentifierInContext() throws Exception { } @Test - public void crossCatalogViewAccessDoesNotFail() { - String otherTable = String.format("%s.%s.%s", OTHER_CATALOG_NAME, NAMESPACE, OTHER_TABLE_NAME); - spark.sql(String.format("CREATE TABLE %s (id INT, data STRING)", otherTable)); - appendRecords(otherTable); - spark.sql(String.format("REFRESH TABLE %s", otherTable)); - createView("cross_view", String.format("SELECT id FROM %s", otherTable)); - spark.sql(String.format("REFRESH TABLE %s", otherTable)); + public void crossCatalogViewAccessDoesNotFailWhenReferencedByIsDisabled() { + createCrossCatalogView(); + spark.conf().unset(SparkSQLProperties.REFERENCED_BY_ENABLED); ContextTrackingCatalog.clearCaptured(); List result = spark.sql("SELECT * FROM cross_view").collectAsList(); @@ -253,6 +267,16 @@ public void crossCatalogViewAccessDoesNotFail() { .allSatisfy(captured -> assertThat(captured.referencedBy).isEmpty()); } + @Test + public void crossCatalogViewAccessFailsWhenReferencedByIsEnabled() { + createCrossCatalogView(); + ContextTrackingCatalog.clearCaptured(); + + assertThatThrownBy(() -> spark.sql("SELECT * FROM cross_view").collectAsList()) + .hasMessageContaining("Cross-catalog view references are not supported") + .hasMessageContaining(OTHER_CATALOG_NAME); + } + private void createView(String viewName, String sql) { viewCatalog() .buildView(TableIdentifier.of(NAMESPACE, viewName)) @@ -263,6 +287,15 @@ private void createView(String viewName, String sql) { .create(); } + private void createCrossCatalogView() { + String otherTable = String.format("%s.%s.%s", OTHER_CATALOG_NAME, NAMESPACE, OTHER_TABLE_NAME); + spark.sql(String.format("CREATE TABLE %s (id INT, data STRING)", otherTable)); + appendRecords(otherTable); + spark.sql(String.format("REFRESH TABLE %s", otherTable)); + createView("cross_view", String.format("SELECT id FROM %s", otherTable)); + spark.sql(String.format("REFRESH TABLE %s", otherTable)); + } + private void appendRecords(String tableName) { try { List records = diff --git a/spark/v4.2/spark/src/main/java/org/apache/iceberg/spark/SparkSQLProperties.java b/spark/v4.2/spark/src/main/java/org/apache/iceberg/spark/SparkSQLProperties.java index 428644ecfc91..6c631edaef06 100644 --- a/spark/v4.2/spark/src/main/java/org/apache/iceberg/spark/SparkSQLProperties.java +++ b/spark/v4.2/spark/src/main/java/org/apache/iceberg/spark/SparkSQLProperties.java @@ -107,6 +107,10 @@ private SparkSQLProperties() {} // Rely on identifier fields as a PRIMARY KEY constraint for query optimization (not enforced) public static final String IDENTIFIER_FIELDS_RELY = "spark.sql.iceberg.identifier-fields-rely"; + // Controls whether to send referenced-by view chains while loading relations + public static final String REFERENCED_BY_ENABLED = "spark.sql.iceberg.referenced-by.enabled"; + public static final boolean REFERENCED_BY_ENABLED_DEFAULT = false; + // Prefix for custom snapshot properties public static final String SNAPSHOT_PROPERTY_PREFIX = "spark.sql.iceberg.snapshot-property."; diff --git a/spark/v4.2/spark/src/main/scala/org/apache/spark/sql/catalyst/analysis/ViewUtil.scala b/spark/v4.2/spark/src/main/scala/org/apache/spark/sql/catalyst/analysis/ViewUtil.scala index 5b8bfcddd840..340ba67ed808 100644 --- a/spark/v4.2/spark/src/main/scala/org/apache/spark/sql/catalyst/analysis/ViewUtil.scala +++ b/spark/v4.2/spark/src/main/scala/org/apache/spark/sql/catalyst/analysis/ViewUtil.scala @@ -102,7 +102,7 @@ object ViewUtil { /** * Build the referenced-by view chain from fully qualified view identifier parts. - * Cross-catalog entries are omitted because LoadContext uses catalog-relative identifiers. + * Entries must belong to the same catalog as the loaded target. */ def buildReferencedByChain( viewChain: Seq[Seq[String]], @@ -114,7 +114,15 @@ object ViewUtil { parts.mkString(".")) } - val viewIdentifiers = viewChain.filter(_.headOption.contains(targetCatalogName)).map { parts => + val crossCatalogViews = viewChain.filter(parts => !parts.headOption.contains(targetCatalogName)) + if (crossCatalogViews.nonEmpty) { + throw new IllegalStateException( + s"Cross-catalog view references are not supported with referenced-by enabled. " + + s"Views from catalogs [${crossCatalogViews.map(_.head).distinct.mkString(", ")}] " + + s"cannot reference entities in catalog [$targetCatalogName]") + } + + val viewIdentifiers = viewChain.map { parts => TableIdentifier.of(Namespace.of(parts.drop(1).init: _*), parts.last) } From 0311044cacceab058a375020c9e41518027fee06 Mon Sep 17 00:00:00 2001 From: Prashant Singh Date: Mon, 21 Sep 2026 22:09:34 -0700 Subject: [PATCH 08/11] Spark 4.2: Use resolved view identity for referenced-by Build the referenced-by view chain from the catalog and identifier Spark resolved when loading the view. Keep view metadata current catalog and namespace only for resolving unqualified references inside the view SQL. Generated-by: Codex --- .../sql/catalyst/analysis/ResolveViews.scala | 18 +++++-- .../extensions/TestReferencedByViewChain.java | 54 ++++++++++++++++++- 2 files changed, 67 insertions(+), 5 deletions(-) diff --git a/spark/v4.2/spark-extensions/src/main/scala/org/apache/spark/sql/catalyst/analysis/ResolveViews.scala b/spark/v4.2/spark-extensions/src/main/scala/org/apache/spark/sql/catalyst/analysis/ResolveViews.scala index f3d1d7c593fe..ca65a9ec853d 100644 --- a/spark/v4.2/spark-extensions/src/main/scala/org/apache/spark/sql/catalyst/analysis/ResolveViews.scala +++ b/spark/v4.2/spark-extensions/src/main/scala/org/apache/spark/sql/catalyst/analysis/ResolveViews.scala @@ -111,10 +111,14 @@ case class ResolveViews(spark: SparkSession) extends Rule[LogicalPlan] with Look view: View, existingChain: Seq[Seq[String]] = Seq.empty): LogicalPlan = { val parsed = parseViewText(nameParts.quoted, view.queryText()) - val viewCatalogAndNamespace = catalogAndNamespace(catalog, ident, view) + val viewQueryCatalogAndNamespace = queryCatalogAndNamespace(catalog, ident, view) val viewChain = - ViewUtil.buildViewChain(nameParts, viewCatalogAndNamespace, existingChain, isCatalog) - val rewritten = rewriteIdentifiers(parsed, viewCatalogAndNamespace, viewChain) + ViewUtil.buildViewChain( + Seq(ident.name()), + resolvedCatalogAndNamespace(catalog, ident), + existingChain, + isCatalog) + val rewritten = rewriteIdentifiers(parsed, viewQueryCatalogAndNamespace, viewChain) val aliases = view .schema() .fields @@ -133,7 +137,7 @@ case class ResolveViews(spark: SparkSession) extends Rule[LogicalPlan] with Look SubqueryAlias(nameParts, Project(aliases, rewritten)) } - private def catalogAndNamespace( + private def queryCatalogAndNamespace( catalog: CatalogPlugin, ident: Identifier, view: View): Seq[String] = { @@ -144,6 +148,12 @@ case class ResolveViews(spark: SparkSession) extends Rule[LogicalPlan] with Look currentCatalog +: currentNamespace } + private def resolvedCatalogAndNamespace( + catalog: CatalogPlugin, + ident: Identifier): Seq[String] = { + catalog.name() +: ident.namespace().toIndexedSeq + } + private def parseViewText(name: String, viewText: String): LogicalPlan = { val origin = Origin(objectType = Some("VIEW"), objectName = Some(name)) try { diff --git a/spark/v4.2/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestReferencedByViewChain.java b/spark/v4.2/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestReferencedByViewChain.java index 8427d1e43309..785da3e04234 100644 --- a/spark/v4.2/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestReferencedByViewChain.java +++ b/spark/v4.2/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestReferencedByViewChain.java @@ -57,8 +57,11 @@ public class TestReferencedByViewChain extends SparkTestHelperBase { private static final String CATALOG_NAME = "ref_test_catalog"; private static final String OTHER_CATALOG_NAME = "other_ref_test_catalog"; private static final Namespace NAMESPACE = Namespace.of("default"); + private static final Namespace VIEW_DEFAULT_NAMESPACE = Namespace.of("view_defaults"); private static final String TABLE_NAME = "test_table"; private static final String OTHER_TABLE_NAME = "other_table"; + private static final String VIEW_DEFAULT_TABLE_NAME = "view_default_table"; + private static final String VIEW_DEFAULT_VIEW_NAME = "view_default_view"; private static SparkSession spark; @@ -137,6 +140,9 @@ public static void startSpark() { .getOrCreate(); spark.sql(String.format("CREATE NAMESPACE IF NOT EXISTS %s.%s", CATALOG_NAME, NAMESPACE)); + spark.sql( + String.format( + "CREATE NAMESPACE IF NOT EXISTS %s.%s", CATALOG_NAME, VIEW_DEFAULT_NAMESPACE)); spark.sql(String.format("CREATE NAMESPACE IF NOT EXISTS %s.%s", OTHER_CATALOG_NAME, NAMESPACE)); } @@ -170,7 +176,11 @@ public void after() { spark.sql("DROP VIEW IF EXISTS view_c"); spark.sql("DROP VIEW IF EXISTS time_travel_view"); spark.sql("DROP VIEW IF EXISTS cross_view"); + spark.sql(String.format("DROP VIEW IF EXISTS %s", VIEW_DEFAULT_VIEW_NAME)); spark.sql(String.format("DROP TABLE IF EXISTS %s", TABLE_NAME)); + spark.sql( + String.format( + "DROP TABLE IF EXISTS %s.%s", VIEW_DEFAULT_NAMESPACE, VIEW_DEFAULT_TABLE_NAME)); spark.sql( String.format( "DROP TABLE IF EXISTS %s.%s.%s", OTHER_CATALOG_NAME, NAMESPACE, OTHER_TABLE_NAME)); @@ -218,6 +228,39 @@ public void singleViewPassesViewIdentifierInContext() { assertCapturedTableChain(ContextTrackingCatalog.CAPTURED, TABLE_NAME, "simple_view"); } + @Test + public void referencedByUsesResolvedViewIdentifier() { + spark.sql( + String.format( + "CREATE TABLE %s.%s (id INT, data STRING)", + VIEW_DEFAULT_NAMESPACE, VIEW_DEFAULT_TABLE_NAME)); + appendRecords(String.format("%s.%s", VIEW_DEFAULT_NAMESPACE, VIEW_DEFAULT_TABLE_NAME)); + spark.sql( + String.format("REFRESH TABLE %s.%s", VIEW_DEFAULT_NAMESPACE, VIEW_DEFAULT_TABLE_NAME)); + + String viewSql = String.format("SELECT id FROM %s", VIEW_DEFAULT_TABLE_NAME); + String schemaSql = + String.format("SELECT id FROM %s.%s", VIEW_DEFAULT_NAMESPACE, VIEW_DEFAULT_TABLE_NAME); + viewCatalog() + .buildView(TableIdentifier.of(NAMESPACE, VIEW_DEFAULT_VIEW_NAME)) + .withQuery("spark", viewSql) + .withDefaultNamespace(VIEW_DEFAULT_NAMESPACE) + .withDefaultCatalog(CATALOG_NAME) + .withSchema(SparkSchemaUtil.convert(spark.sql(schemaSql).schema())) + .create(); + ContextTrackingCatalog.clearCaptured(); + + List result = + spark.sql(String.format("SELECT * FROM %s", VIEW_DEFAULT_VIEW_NAME)).collectAsList(); + assertThat(result).hasSize(5); + + assertCapturedTableChain( + ContextTrackingCatalog.CAPTURED, + VIEW_DEFAULT_NAMESPACE, + VIEW_DEFAULT_TABLE_NAME, + VIEW_DEFAULT_VIEW_NAME); + } + @Test public void nestedViewChainAccumulatesContext() { createView("view_a", String.format("SELECT id, data FROM %s", TABLE_NAME)); @@ -313,11 +356,20 @@ private void assertCapturedTableChain( List captures, String targetName, String... expectedViewNames) { + assertCapturedTableChain(captures, NAMESPACE, targetName, expectedViewNames); + } + + private void assertCapturedTableChain( + List captures, + Namespace targetNamespace, + String targetName, + String... expectedViewNames) { List matching = captures.stream() .filter( captured -> - captured.tableIdentifier.equals(TableIdentifier.of(NAMESPACE, targetName))) + captured.tableIdentifier.equals( + TableIdentifier.of(targetNamespace, targetName))) .filter(captured -> captured.referencedBy != null && !captured.referencedBy.isEmpty()) .collect(Collectors.toList()); From 464f64255d23496282492177afd264790df4068d Mon Sep 17 00:00:00 2001 From: Prashant Singh Date: Thu, 8 Oct 2026 16:13:12 -0700 Subject: [PATCH 09/11] Spark 4.2: Address referenced-by review feedback Keep the view chain on Spark's pre-execution table refresh by binding it to the catalog of relations resolved through a view, so a table read through a view is never reloaded as direct access. Convert view time travel with Spark's TimeTravelSpec so string and integer timestamps work and pinned snapshots are not refreshed. Leave V1 and delegating tables read through a view to Spark, and alias view-resolved tables so table-qualified columns resolve. Remove loadRelation overrides that duplicate BaseCatalog. --- .../analysis/LoadContextTableCatalog.scala | 61 +++++++ .../sql/catalyst/analysis/ResolveViews.scala | 47 +++++- .../extensions/TestReferencedByViewChain.java | 151 +++++++++++++++++- .../apache/iceberg/spark/SparkCatalog.java | 15 -- .../iceberg/spark/SparkSessionCatalog.java | 15 -- .../sql/catalyst/analysis/ViewUtil.scala | 49 ++---- 6 files changed, 258 insertions(+), 80 deletions(-) create mode 100644 spark/v4.2/spark-extensions/src/main/scala/org/apache/spark/sql/catalyst/analysis/LoadContextTableCatalog.scala diff --git a/spark/v4.2/spark-extensions/src/main/scala/org/apache/spark/sql/catalyst/analysis/LoadContextTableCatalog.scala b/spark/v4.2/spark-extensions/src/main/scala/org/apache/spark/sql/catalyst/analysis/LoadContextTableCatalog.scala new file mode 100644 index 000000000000..76b877cab8dc --- /dev/null +++ b/spark/v4.2/spark-extensions/src/main/scala/org/apache/spark/sql/catalyst/analysis/LoadContextTableCatalog.scala @@ -0,0 +1,61 @@ +/* + * 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.iceberg.catalog.LoadContext +import org.apache.iceberg.spark.SparkSupportsLoadContext +import org.apache.spark.sql.connector.catalog.Identifier +import org.apache.spark.sql.connector.catalog.Table +import org.apache.spark.sql.connector.catalog.TableCatalog +import org.apache.spark.sql.connector.catalog.TableChange +import org.apache.spark.sql.util.CaseInsensitiveStringMap + +/** + * Catalog for a table read through a view, so that reloads keep the view chain. Spark's table + * version refresh before execution reloads tables by identifier only, using the relation's + * catalog. Only read operations are supported. + */ +private[analysis] case class LoadContextTableCatalog( + delegate: TableCatalog with SparkSupportsLoadContext, + context: LoadContext) + extends TableCatalog { + + override def name(): String = delegate.name() + + override def initialize(name: String, options: CaseInsensitiveStringMap): Unit = {} + + override def loadTable(ident: Identifier): Table = delegate.loadTable(ident, context) + + override def tableExists(ident: Identifier): Boolean = delegate.tableExists(ident) + + override def invalidateTable(ident: Identifier): Unit = delegate.invalidateTable(ident) + + override def listTables(namespace: Array[String]): Array[Identifier] = + delegate.listTables(namespace) + + override def alterTable(ident: Identifier, changes: TableChange*): Table = + throw new UnsupportedOperationException(s"Cannot alter table $ident through a view relation") + + override def dropTable(ident: Identifier): Boolean = + throw new UnsupportedOperationException(s"Cannot drop table $ident through a view relation") + + override def renameTable(oldIdent: Identifier, newIdent: Identifier): Unit = + throw new UnsupportedOperationException( + s"Cannot rename table $oldIdent through a view relation") +} diff --git a/spark/v4.2/spark-extensions/src/main/scala/org/apache/spark/sql/catalyst/analysis/ResolveViews.scala b/spark/v4.2/spark-extensions/src/main/scala/org/apache/spark/sql/catalyst/analysis/ResolveViews.scala index ca65a9ec853d..1517b0fed5f2 100644 --- a/spark/v4.2/spark-extensions/src/main/scala/org/apache/spark/sql/catalyst/analysis/ResolveViews.scala +++ b/spark/v4.2/spark-extensions/src/main/scala/org/apache/spark/sql/catalyst/analysis/ResolveViews.scala @@ -20,6 +20,7 @@ package org.apache.spark.sql.catalyst.analysis import org.apache.iceberg.catalog.LoadContext import org.apache.iceberg.spark.SparkSQLProperties +import org.apache.iceberg.spark.SparkSupportsLoadContext import org.apache.spark.sql.SparkSession import org.apache.spark.sql.catalyst.analysis.ViewUtil.IcebergViewHelper import org.apache.spark.sql.catalyst.expressions.Alias @@ -37,12 +38,25 @@ import org.apache.spark.sql.catalyst.trees.CurrentOrigin import org.apache.spark.sql.catalyst.trees.Origin import org.apache.spark.sql.connector.catalog.CatalogManager import org.apache.spark.sql.connector.catalog.CatalogPlugin +import org.apache.spark.sql.connector.catalog.DelegatingTable import org.apache.spark.sql.connector.catalog.Identifier import org.apache.spark.sql.connector.catalog.LookupCatalog +import org.apache.spark.sql.connector.catalog.TableCatalog +import org.apache.spark.sql.connector.catalog.V1Table import org.apache.spark.sql.connector.catalog.View import org.apache.spark.sql.errors.QueryCompilationErrors import org.apache.spark.sql.execution.datasources.v2.DataSourceV2Relation +/** + * Resolves Iceberg views referenced by commands that use `UnresolvedTableOrView`. + * + * In Spark 4.2, view relations are expanded through `RelationCatalog.loadRelation`, so Spark owns + * query parsing, identifier resolution, and schema-mode application. This rule only converts the + * remaining command targets to `ResolvedV2View` for Iceberg command planning. + * + * When referenced-by is enabled, this rule also expands Iceberg views itself so that tables they + * read are loaded with the referencing view chain. + */ case class ResolveViews(spark: SparkSession) extends Rule[LogicalPlan] with LookupCatalog { import org.apache.spark.sql.connector.catalog.CatalogV2Implicits._ @@ -74,13 +88,38 @@ case class ResolveViews(spark: SparkSession) extends Rule[LogicalPlan] with Look options, isStreaming, timeTravelVersion, - timeTravelTimestamp) => + timeTravelTimestamp) if timeTravelTimestamp.forall(_.resolved) => val referencedBy = ViewUtil.buildReferencedByChain(viewChain, catalog.name()) val context = LoadContext.builder().referencedBy(referencedBy).build() + val timeTravelSpec = + TimeTravelSpec.create(timeTravelTimestamp, timeTravelVersion, conf.sessionLocalTimeZone) try { - val table = - ViewUtil.loadTable(catalog, tableIdent, context, timeTravelVersion, timeTravelTimestamp) - DataSourceV2Relation.create(table, Some(catalog), Some(tableIdent), options) + val table = ViewUtil.loadTable(catalog, tableIdent, context, timeTravelSpec) + table match { + case _: V1Table | _: DelegatingTable => + val relation = UnresolvedRelation(tableParts, options, isStreaming) + if (timeTravelSpec.isEmpty) { + relation + } else { + RelationTimeTravel(relation, timeTravelTimestamp, timeTravelVersion) + } + + case _ => + val relationCatalog = catalog match { + case contextual: TableCatalog with SparkSupportsLoadContext + if timeTravelSpec.isEmpty => + LoadContextTableCatalog(contextual, context) + case _ => catalog + } + SubqueryAlias( + tableIdent.toQualifiedNameParts(catalog), + DataSourceV2Relation.create( + table, + Some(relationCatalog), + Some(tableIdent), + options, + timeTravelSpec)) + } } catch { case _: NoSuchTableException => ViewUtil diff --git a/spark/v4.2/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestReferencedByViewChain.java b/spark/v4.2/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestReferencedByViewChain.java index 785da3e04234..f21205d30215 100644 --- a/spark/v4.2/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestReferencedByViewChain.java +++ b/spark/v4.2/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestReferencedByViewChain.java @@ -18,19 +18,24 @@ */ package org.apache.iceberg.spark.extensions; +import static org.apache.iceberg.TestHelpers.waitUntilAfter; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; import java.net.InetAddress; +import java.nio.file.Path; +import java.sql.Timestamp; import java.util.Collections; import java.util.List; import java.util.concurrent.CopyOnWriteArrayList; +import java.util.concurrent.TimeUnit; import java.util.stream.Collectors; import java.util.stream.IntStream; import org.apache.iceberg.CatalogProperties; import org.apache.iceberg.Table; import org.apache.iceberg.catalog.LoadContext; import org.apache.iceberg.catalog.Namespace; +import org.apache.iceberg.catalog.SupportsNamespaces; import org.apache.iceberg.catalog.TableIdentifier; import org.apache.iceberg.catalog.ViewCatalog; import org.apache.iceberg.inmemory.InMemoryCatalog; @@ -38,6 +43,7 @@ import org.apache.iceberg.spark.SparkCatalog; import org.apache.iceberg.spark.SparkSQLProperties; import org.apache.iceberg.spark.SparkSchemaUtil; +import org.apache.iceberg.spark.SparkSessionCatalog; import org.apache.iceberg.spark.SparkTestHelperBase; import org.apache.iceberg.spark.source.HasIcebergCatalog; import org.apache.iceberg.spark.source.SimpleRecord; @@ -51,6 +57,7 @@ import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; public class TestReferencedByViewChain extends SparkTestHelperBase { @@ -62,6 +69,11 @@ public class TestReferencedByViewChain extends SparkTestHelperBase { private static final String OTHER_TABLE_NAME = "other_table"; private static final String VIEW_DEFAULT_TABLE_NAME = "view_default_table"; private static final String VIEW_DEFAULT_VIEW_NAME = "view_default_view"; + private static final String SESSION_CATALOG_NAME = "spark_catalog"; + private static final String PARQUET_TABLE_NAME = "parquet_table"; + private static final String PARQUET_VIEW_NAME = "parquet_view"; + + @TempDir private static Path warehouse; private static SparkSession spark; @@ -136,6 +148,13 @@ public static void startSpark() { ContextTrackingCatalog.class.getName()) .config("spark.sql.catalog." + OTHER_CATALOG_NAME + ".default-namespace", "default") .config("spark.sql.catalog." + OTHER_CATALOG_NAME + ".cache-enabled", "true") + .config( + "spark.sql.catalog." + SESSION_CATALOG_NAME, SparkSessionCatalog.class.getName()) + .config( + "spark.sql.catalog." + SESSION_CATALOG_NAME + "." + CatalogProperties.CATALOG_IMPL, + InMemoryCatalog.class.getName()) + .config("spark.sql.catalog." + SESSION_CATALOG_NAME + ".cache-enabled", "false") + .config("spark.sql.warehouse.dir", warehouse.toString()) .config("spark.sql.defaultCatalog", CATALOG_NAME) .getOrCreate(); @@ -144,6 +163,8 @@ public static void startSpark() { String.format( "CREATE NAMESPACE IF NOT EXISTS %s.%s", CATALOG_NAME, VIEW_DEFAULT_NAMESPACE)); spark.sql(String.format("CREATE NAMESPACE IF NOT EXISTS %s.%s", OTHER_CATALOG_NAME, NAMESPACE)); + ((SupportsNamespaces) Spark3Util.loadIcebergCatalog(spark, SESSION_CATALOG_NAME)) + .createNamespace(NAMESPACE); } @AfterAll @@ -184,6 +205,10 @@ public void after() { spark.sql( String.format( "DROP TABLE IF EXISTS %s.%s.%s", OTHER_CATALOG_NAME, NAMESPACE, OTHER_TABLE_NAME)); + viewCatalog(SESSION_CATALOG_NAME).dropView(TableIdentifier.of(NAMESPACE, PARQUET_VIEW_NAME)); + spark.sql( + String.format( + "DROP TABLE IF EXISTS %s.%s.%s", SESSION_CATALOG_NAME, NAMESPACE, PARQUET_TABLE_NAME)); ContextTrackingCatalog.clearCaptured(); } @@ -217,15 +242,32 @@ public void directTableAccessHasEmptyContext() { @Test public void singleViewPassesViewIdentifierInContext() { createView("simple_view", String.format("SELECT id FROM %s", TABLE_NAME)); + spark.sql(String.format("REFRESH TABLE %s", TABLE_NAME)); ContextTrackingCatalog.clearCaptured(); List result = spark.sql("SELECT * FROM simple_view").collectAsList(); assertThat(result).hasSize(5); - assertThat(ContextTrackingCatalog.CAPTURED) - .filteredOn(captured -> !captured.referencedBy.isEmpty()) - .hasSize(1); - assertCapturedTableChain(ContextTrackingCatalog.CAPTURED, TABLE_NAME, "simple_view"); + assertThat(capturedLoadsOf(TableIdentifier.of(NAMESPACE, TABLE_NAME))) + .isNotEmpty() + .allSatisfy( + captured -> + assertThat(captured.referencedBy) + .containsExactly(TableIdentifier.of(NAMESPACE, "simple_view"))); + } + + @Test + public void viewBodyResolvesTableQualifiedColumns() { + createView("view_a", String.format("SELECT %s.id FROM %s", TABLE_NAME, TABLE_NAME)); + createView( + "view_b", + String.format( + "SELECT %s.%s.%s.id FROM %s.%s.%s", + CATALOG_NAME, NAMESPACE, TABLE_NAME, CATALOG_NAME, NAMESPACE, TABLE_NAME)); + ContextTrackingCatalog.clearCaptured(); + + assertThat(spark.sql("SELECT * FROM view_a").collectAsList()).hasSize(5); + assertThat(spark.sql("SELECT * FROM view_b").collectAsList()).hasSize(5); } @Test @@ -320,12 +362,103 @@ public void crossCatalogViewAccessFailsWhenReferencedByIsEnabled() { .hasMessageContaining(OTHER_CATALOG_NAME); } + @Test + public void versionTimeTravelViewReadsPinnedSnapshot() throws Exception { + long snapshotId = Spark3Util.loadIcebergTable(spark, TABLE_NAME).currentSnapshot().snapshotId(); + createView( + "time_travel_view", + String.format("SELECT id FROM %s VERSION AS OF %d", TABLE_NAME, snapshotId)); + appendRecords(TABLE_NAME); + ContextTrackingCatalog.clearCaptured(); + + List result = spark.sql("SELECT * FROM time_travel_view").collectAsList(); + assertThat(result).hasSize(5); + } + + @Test + public void timestampStringTimeTravelView() throws Exception { + long asOfMillis = waitUntilAfter(currentSnapshotMillis()); + String asOf = new Timestamp(asOfMillis).toString(); + createView( + "time_travel_view", + String.format("SELECT id FROM %s TIMESTAMP AS OF '%s'", TABLE_NAME, asOf)); + waitUntilAfter(asOfMillis); + appendRecords(TABLE_NAME); + ContextTrackingCatalog.clearCaptured(); + + List result = spark.sql("SELECT * FROM time_travel_view").collectAsList(); + assertThat(result).hasSize(5); + + assertCapturedTableChain(ContextTrackingCatalog.CAPTURED, TABLE_NAME, "time_travel_view"); + } + + @Test + public void timestampSecondsTimeTravelView() throws Exception { + long asOfSeconds = TimeUnit.MILLISECONDS.toSeconds(currentSnapshotMillis()) + 1; + createView( + "time_travel_view", + String.format("SELECT id FROM %s TIMESTAMP AS OF %d", TABLE_NAME, asOfSeconds)); + waitUntilAfter(TimeUnit.SECONDS.toMillis(asOfSeconds)); + appendRecords(TABLE_NAME); + ContextTrackingCatalog.clearCaptured(); + + List result = spark.sql("SELECT * FROM time_travel_view").collectAsList(); + assertThat(result).hasSize(5); + + assertCapturedTableChain(ContextTrackingCatalog.CAPTURED, TABLE_NAME, "time_travel_view"); + } + + @Test + public void directReadOfNonIcebergTableInSessionCatalog() { + String parquetTable = createSessionCatalogParquetTable(); + + assertThat(spark.sql(String.format("SELECT * FROM %s", parquetTable)).count()).isEqualTo(3); + } + + @Test + public void viewReadsNonIcebergTableInSessionCatalog() { + String parquetTable = createSessionCatalogParquetTable(); + createView( + SESSION_CATALOG_NAME, PARQUET_VIEW_NAME, String.format("SELECT id FROM %s", parquetTable)); + + assertThat( + spark + .sql( + String.format( + "SELECT * FROM %s.%s.%s", + SESSION_CATALOG_NAME, NAMESPACE, PARQUET_VIEW_NAME)) + .count()) + .isEqualTo(3); + } + + private String createSessionCatalogParquetTable() { + String parquetTable = + String.format("%s.%s.%s", SESSION_CATALOG_NAME, NAMESPACE, PARQUET_TABLE_NAME); + spark.sql(String.format("CREATE TABLE %s (id INT) USING parquet", parquetTable)); + spark.sql(String.format("INSERT INTO %s VALUES (1), (2), (3)", parquetTable)); + return parquetTable; + } + + private long currentSnapshotMillis() throws Exception { + return Spark3Util.loadIcebergTable(spark, TABLE_NAME).currentSnapshot().timestampMillis(); + } + + private List capturedLoadsOf(TableIdentifier ident) { + return ContextTrackingCatalog.CAPTURED.stream() + .filter(captured -> captured.tableIdentifier.equals(ident)) + .collect(Collectors.toList()); + } + private void createView(String viewName, String sql) { - viewCatalog() + createView(CATALOG_NAME, viewName, sql); + } + + private void createView(String catalogName, String viewName, String sql) { + viewCatalog(catalogName) .buildView(TableIdentifier.of(NAMESPACE, viewName)) .withQuery("spark", sql) .withDefaultNamespace(NAMESPACE) - .withDefaultCatalog(CATALOG_NAME) + .withDefaultCatalog(catalogName) .withSchema(SparkSchemaUtil.convert(spark.sql(sql).schema())) .create(); } @@ -384,7 +517,11 @@ private void assertCapturedTableChain( } private ViewCatalog viewCatalog() { - CatalogPlugin catalogPlugin = spark.sessionState().catalogManager().catalog(CATALOG_NAME); + return viewCatalog(CATALOG_NAME); + } + + private ViewCatalog viewCatalog(String catalogName) { + CatalogPlugin catalogPlugin = spark.sessionState().catalogManager().catalog(catalogName); assertThat(catalogPlugin).isInstanceOf(HasIcebergCatalog.class); ViewCatalog viewCatalog = ((HasIcebergCatalog) catalogPlugin).icebergViewCatalog(); assertThat(viewCatalog).isNotNull(); diff --git a/spark/v4.2/spark/src/main/java/org/apache/iceberg/spark/SparkCatalog.java b/spark/v4.2/spark/src/main/java/org/apache/iceberg/spark/SparkCatalog.java index 6effd3762603..45f44fa8e9c5 100644 --- a/spark/v4.2/spark/src/main/java/org/apache/iceberg/spark/SparkCatalog.java +++ b/spark/v4.2/spark/src/main/java/org/apache/iceberg/spark/SparkCatalog.java @@ -84,7 +84,6 @@ import org.apache.spark.sql.catalyst.analysis.ViewUtil; import org.apache.spark.sql.connector.catalog.Identifier; import org.apache.spark.sql.connector.catalog.NamespaceChange; -import org.apache.spark.sql.connector.catalog.Relation; import org.apache.spark.sql.connector.catalog.StagedTable; import org.apache.spark.sql.connector.catalog.Table; import org.apache.spark.sql.connector.catalog.TableCatalog; @@ -226,20 +225,6 @@ public Table loadTable(Identifier ident, long timestampMicros, LoadContext conte return load(ident, TimeTravel.timestampMicros(timestampMicros), context); } - @Override - public Relation loadRelation(Identifier ident) throws NoSuchTableException { - try { - return loadTable(ident); - } catch (NoSuchTableException e) { - try { - return loadView(ident); - } catch (NoSuchViewException viewException) { - e.addSuppressed(viewException); - throw e; - } - } - } - @Override public boolean tableExists(Identifier ident) { if (isPathIdentifier(ident)) { diff --git a/spark/v4.2/spark/src/main/java/org/apache/iceberg/spark/SparkSessionCatalog.java b/spark/v4.2/spark/src/main/java/org/apache/iceberg/spark/SparkSessionCatalog.java index a2f434611c7b..5cb2c033e56b 100644 --- a/spark/v4.2/spark/src/main/java/org/apache/iceberg/spark/SparkSessionCatalog.java +++ b/spark/v4.2/spark/src/main/java/org/apache/iceberg/spark/SparkSessionCatalog.java @@ -48,7 +48,6 @@ import org.apache.spark.sql.connector.catalog.FunctionCatalog; import org.apache.spark.sql.connector.catalog.Identifier; import org.apache.spark.sql.connector.catalog.NamespaceChange; -import org.apache.spark.sql.connector.catalog.Relation; import org.apache.spark.sql.connector.catalog.StagedTable; import org.apache.spark.sql.connector.catalog.StagingTableCatalog; import org.apache.spark.sql.connector.catalog.SupportsNamespaces; @@ -261,20 +260,6 @@ public Table loadTable(Identifier ident, long timestamp, LoadContext context) } } - @Override - public Relation loadRelation(Identifier ident) throws NoSuchTableException { - try { - return loadTable(ident); - } catch (NoSuchTableException e) { - try { - return loadView(ident); - } catch (NoSuchViewException viewException) { - e.addSuppressed(viewException); - throw e; - } - } - } - @Override public void invalidateTable(Identifier ident) { // We do not need to check whether the table exists and whether diff --git a/spark/v4.2/spark/src/main/scala/org/apache/spark/sql/catalyst/analysis/ViewUtil.scala b/spark/v4.2/spark/src/main/scala/org/apache/spark/sql/catalyst/analysis/ViewUtil.scala index 340ba67ed808..638c76a9d916 100644 --- a/spark/v4.2/spark/src/main/scala/org/apache/spark/sql/catalyst/analysis/ViewUtil.scala +++ b/spark/v4.2/spark/src/main/scala/org/apache/spark/sql/catalyst/analysis/ViewUtil.scala @@ -27,7 +27,6 @@ import org.apache.iceberg.spark.SparkSupportsLoadContext import org.apache.iceberg.spark.source.HasIcebergCatalog import org.apache.iceberg.spark.source.SparkView import org.apache.iceberg.view.{View => IcebergView} -import org.apache.spark.sql.catalyst.expressions.Expression import org.apache.spark.sql.connector.catalog.CatalogPlugin import org.apache.spark.sql.connector.catalog.Identifier import org.apache.spark.sql.connector.catalog.Table @@ -152,31 +151,19 @@ object ViewUtil { catalog: CatalogPlugin, ident: Identifier, context: LoadContext, - timeTravelVersion: Option[String] = None, - timeTravelTimestamp: Option[Expression] = None): Table = { + timeTravelSpec: Option[TimeTravelSpec] = None): Table = { catalog match { case supportsLoadContext: SparkSupportsLoadContext => - loadTableWithTimeTravel( - supportsLoadContext, - ident, - context, - timeTravelVersion, - timeTravelTimestamp) - case c if c.asTableCatalog.isInstanceOf[SparkSupportsLoadContext] => - loadTableWithTimeTravel( - c.asTableCatalog.asInstanceOf[SparkSupportsLoadContext], - ident, - context, - timeTravelVersion, - timeTravelTimestamp) + timeTravelSpec match { + case Some(AsOfVersion(version)) => supportsLoadContext.loadTable(ident, version, context) + case Some(AsOfTimestamp(micros)) => supportsLoadContext.loadTable(ident, micros, context) + case None => supportsLoadContext.loadTable(ident, context) + } case _ => - (timeTravelVersion, timeTravelTimestamp) match { - case (Some(version), _) => - catalog.asTableCatalog.loadTable(ident, version) - case (_, Some(timestamp)) => - catalog.asTableCatalog.loadTable(ident, timestamp.eval().asInstanceOf[Long]) - case _ => - catalog.asTableCatalog.loadTable(ident) + timeTravelSpec match { + case Some(AsOfVersion(version)) => catalog.asTableCatalog.loadTable(ident, version) + case Some(AsOfTimestamp(micros)) => catalog.asTableCatalog.loadTable(ident, micros) + case None => catalog.asTableCatalog.loadTable(ident) } } } @@ -185,22 +172,6 @@ object ViewUtil { catalog.isInstanceOf[ViewCatalog] && catalog.isInstanceOf[HasIcebergCatalog] } - private def loadTableWithTimeTravel( - supportsLoadContext: SparkSupportsLoadContext, - ident: Identifier, - context: LoadContext, - timeTravelVersion: Option[String], - timeTravelTimestamp: Option[Expression]): Table = { - (timeTravelVersion, timeTravelTimestamp) match { - case (Some(version), _) => - supportsLoadContext.loadTable(ident, version, context) - case (_, Some(timestamp)) => - supportsLoadContext.loadTable(ident, timestamp.eval().asInstanceOf[Long], context) - case _ => - supportsLoadContext.loadTable(ident, context) - } - } - private def isIcebergView(view: View): Boolean = { !view.isInstanceOf[V1View] && "iceberg".equalsIgnoreCase(view.properties.get(TableCatalog.PROP_PROVIDER)) From 187c990291bdf62b5eac86225c56d9e726b67551 Mon Sep 17 00:00:00 2001 From: Prashant Singh Date: Fri, 16 Oct 2026 10:39:51 -0700 Subject: [PATCH 10/11] Spark 4.2: Resolve referenced-by views in a dedicated rule Move referenced-by view expansion out of ResolveViews into ResolveReferencedByViews, which does nothing when referenced-by is disabled. ResolveViews is back to its main behavior. With referenced-by enabled, BaseCatalog.loadRelation returns tables only, so Spark resolves tables itself and leaves Iceberg views unresolved for the new rule. This removes the hint-batch registration, which Spark's Hive session state ignores, and keeps a table read to a single load. Expanded views are wrapped in Spark's View node so cycle detection and cache invalidation keep working, and parse, streaming and not-found errors match Spark's. LoadContext gains equals and hashCode so cached plans over a view are reused by later queries. SparkSessionCatalog fails a load with a referenced-by chain when its Iceberg catalog cannot pass the context, instead of dropping it. Generated-by: Claude Code --- .../apache/iceberg/catalog/LoadContext.java | 19 ++ .../iceberg/hadoop/TestCachingCatalog.java | 2 +- .../IcebergSparkSessionExtensions.scala | 4 +- .../analysis/LoadContextTableCatalog.scala | 2 - .../analysis/ResolveReferencedByViews.scala | 281 ++++++++++++++++ .../sql/catalyst/analysis/ResolveViews.scala | 236 +------------- .../views/UnResolvedRelationFromView.scala | 8 +- .../extensions/TestReferencedByViewChain.java | 303 ++++++++++++++---- .../org/apache/iceberg/spark/BaseCatalog.java | 7 + .../iceberg/spark/SparkSQLProperties.java | 3 +- .../iceberg/spark/SparkSessionCatalog.java | 54 ++-- .../sql/catalyst/analysis/ViewUtil.scala | 43 +-- .../spark/TestSparkSessionCatalog.java | 45 +++ 13 files changed, 653 insertions(+), 354 deletions(-) create mode 100644 spark/v4.2/spark-extensions/src/main/scala/org/apache/spark/sql/catalyst/analysis/ResolveReferencedByViews.scala diff --git a/api/src/main/java/org/apache/iceberg/catalog/LoadContext.java b/api/src/main/java/org/apache/iceberg/catalog/LoadContext.java index af31a37282fe..dce80095b8ae 100644 --- a/api/src/main/java/org/apache/iceberg/catalog/LoadContext.java +++ b/api/src/main/java/org/apache/iceberg/catalog/LoadContext.java @@ -60,6 +60,25 @@ public static LoadContext empty() { return EMPTY; } + @Override + public boolean equals(Object other) { + if (this == other) { + return true; + } + + if (other == null || getClass() != other.getClass()) { + return false; + } + + LoadContext that = (LoadContext) other; + return referencedBy.equals(that.referencedBy); + } + + @Override + public int hashCode() { + return referencedBy.hashCode(); + } + public static final class Builder { private List referencedBy; diff --git a/core/src/test/java/org/apache/iceberg/hadoop/TestCachingCatalog.java b/core/src/test/java/org/apache/iceberg/hadoop/TestCachingCatalog.java index 876800e98d66..21365e2afd0d 100644 --- a/core/src/test/java/org/apache/iceberg/hadoop/TestCachingCatalog.java +++ b/core/src/test/java/org/apache/iceberg/hadoop/TestCachingCatalog.java @@ -278,7 +278,7 @@ public void testCatalogExpirationTtlRefreshesAfterAccessViaCatalog() throws IOEx } @Test - public void contextualLoadBypassesTableCache() throws IOException { + void contextualLoadBypassesTableCache() throws IOException { ContextTrackingHadoopCatalog underlyingCatalog = contextTrackingHadoopCatalog(); TestableCachingCatalog catalog = TestableCachingCatalog.wrap(underlyingCatalog, EXPIRATION_TTL, ticker); diff --git a/spark/v4.2/spark-extensions/src/main/scala/org/apache/iceberg/spark/extensions/IcebergSparkSessionExtensions.scala b/spark/v4.2/spark-extensions/src/main/scala/org/apache/iceberg/spark/extensions/IcebergSparkSessionExtensions.scala index 6cb5b594c0ea..f8532b12963f 100644 --- a/spark/v4.2/spark-extensions/src/main/scala/org/apache/iceberg/spark/extensions/IcebergSparkSessionExtensions.scala +++ b/spark/v4.2/spark-extensions/src/main/scala/org/apache/iceberg/spark/extensions/IcebergSparkSessionExtensions.scala @@ -21,6 +21,7 @@ package org.apache.iceberg.spark.extensions import org.apache.spark.sql.SparkSessionExtensions import org.apache.spark.sql.catalyst.analysis.CheckViews import org.apache.spark.sql.catalyst.analysis.ResolveBranch +import org.apache.spark.sql.catalyst.analysis.ResolveReferencedByViews import org.apache.spark.sql.catalyst.analysis.ResolveViews import org.apache.spark.sql.catalyst.optimizer.ReplaceStaticInvoke import org.apache.spark.sql.catalyst.parser.extensions.IcebergSparkSqlExtensionsParser @@ -33,8 +34,7 @@ class IcebergSparkSessionExtensions extends (SparkSessionExtensions => Unit) { extensions.injectParser { case (_, parser) => new IcebergSparkSqlExtensionsParser(parser) } // analyzer extensions - // Run before Spark's ResolveRelations so Iceberg can rewrite view children with load context. - extensions.injectHintResolutionRule { spark => ResolveViews(spark) } + extensions.injectResolutionRule { spark => ResolveReferencedByViews(spark) } extensions.injectResolutionRule { spark => ResolveViews(spark) } extensions.injectPostHocResolutionRule { spark => ResolveBranch(spark) } extensions.injectCheckRule(_ => CheckViews) diff --git a/spark/v4.2/spark-extensions/src/main/scala/org/apache/spark/sql/catalyst/analysis/LoadContextTableCatalog.scala b/spark/v4.2/spark-extensions/src/main/scala/org/apache/spark/sql/catalyst/analysis/LoadContextTableCatalog.scala index 76b877cab8dc..4c5d4bcaf7c4 100644 --- a/spark/v4.2/spark-extensions/src/main/scala/org/apache/spark/sql/catalyst/analysis/LoadContextTableCatalog.scala +++ b/spark/v4.2/spark-extensions/src/main/scala/org/apache/spark/sql/catalyst/analysis/LoadContextTableCatalog.scala @@ -42,8 +42,6 @@ private[analysis] case class LoadContextTableCatalog( override def loadTable(ident: Identifier): Table = delegate.loadTable(ident, context) - override def tableExists(ident: Identifier): Boolean = delegate.tableExists(ident) - override def invalidateTable(ident: Identifier): Unit = delegate.invalidateTable(ident) override def listTables(namespace: Array[String]): Array[Identifier] = diff --git a/spark/v4.2/spark-extensions/src/main/scala/org/apache/spark/sql/catalyst/analysis/ResolveReferencedByViews.scala b/spark/v4.2/spark-extensions/src/main/scala/org/apache/spark/sql/catalyst/analysis/ResolveReferencedByViews.scala new file mode 100644 index 000000000000..eed5455dabf8 --- /dev/null +++ b/spark/v4.2/spark-extensions/src/main/scala/org/apache/spark/sql/catalyst/analysis/ResolveReferencedByViews.scala @@ -0,0 +1,281 @@ +/* + * 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.iceberg.catalog.LoadContext +import org.apache.iceberg.spark.SparkSupportsLoadContext +import org.apache.spark.sql.SparkSession +import org.apache.spark.sql.catalyst.expressions.Alias +import org.apache.spark.sql.catalyst.expressions.Cast +import org.apache.spark.sql.catalyst.expressions.SubqueryExpression +import org.apache.spark.sql.catalyst.expressions.UpCast +import org.apache.spark.sql.catalyst.plans.logical.{View => ViewNode} +import org.apache.spark.sql.catalyst.plans.logical.LogicalPlan +import org.apache.spark.sql.catalyst.plans.logical.Project +import org.apache.spark.sql.catalyst.plans.logical.SubqueryAlias +import org.apache.spark.sql.catalyst.plans.logical.views.UnResolvedRelationFromView +import org.apache.spark.sql.catalyst.rules.Rule +import org.apache.spark.sql.catalyst.trees.CurrentOrigin +import org.apache.spark.sql.catalyst.trees.TreeNodeTag +import org.apache.spark.sql.connector.catalog.CatalogManager +import org.apache.spark.sql.connector.catalog.CatalogPlugin +import org.apache.spark.sql.connector.catalog.DelegatingTable +import org.apache.spark.sql.connector.catalog.Identifier +import org.apache.spark.sql.connector.catalog.LookupCatalog +import org.apache.spark.sql.connector.catalog.TableCatalog +import org.apache.spark.sql.connector.catalog.V1Table +import org.apache.spark.sql.connector.catalog.View +import org.apache.spark.sql.errors.QueryCompilationErrors +import org.apache.spark.sql.execution.datasources.v2.DataSourceV2Relation + +/** + * Expands Iceberg views when referenced-by is enabled, so that the tables and views they read are + * loaded with the referencing view chain. + * + * Spark 4.2 expands views itself without passing the view identity to the catalog, so with + * referenced-by enabled Iceberg catalogs leave views unresolved for this rule. + */ +case class ResolveReferencedByViews(spark: SparkSession) + extends Rule[LogicalPlan] + with LookupCatalog { + + import ResolveReferencedByViews.CheckedForView + import org.apache.spark.sql.connector.catalog.CatalogV2Implicits._ + + protected lazy val catalogManager: CatalogManager = spark.sessionState.catalogManager + + override def apply(plan: LogicalPlan): LogicalPlan = { + if (ViewUtil.referencedByEnabled) { + resolveWithReferencedBy(plan) + } else { + plan + } + } + + private def resolveWithReferencedBy(plan: LogicalPlan): LogicalPlan = plan resolveOperators { + case u @ UnresolvedRelation(nameParts, _, _) if isTempView(nameParts) => + u + + case u @ UnresolvedRelation(CatalogAndIdentifier(catalog, ident), _, true) + if u.getTagValue(CheckedForView).isEmpty => + ViewUtil.loadView(catalog, ident).foreach { _ => + throw QueryCompilationErrors.permanentViewNotSupportedByStreamingReadingAPIError( + ident.quoted) + } + markCheckedForView(u) + + case u @ UnresolvedRelation(parts @ CatalogAndIdentifier(catalog, ident), options, _) + if !options.containsKey(UnresolvedRelation.REQUIRED_WRITE_PRIVILEGES) && + u.getTagValue(CheckedForView).isEmpty => + ViewUtil + .loadView(catalog, ident) + .map(createViewRelation(parts, catalog, ident, _)) + .getOrElse(markCheckedForView(u)) + + case u @ UnResolvedRelationFromView( + tableParts @ CatalogAndIdentifier(catalog, tableIdent), + _, + _, + viewChain, + options, + isStreaming, + timeTravelVersion, + timeTravelTimestamp) if timeTravelTimestamp.forall(_.resolved) => + val referencedBy = ViewUtil.buildReferencedByChain(viewChain, catalog.name()) + val context = LoadContext.builder().referencedBy(referencedBy).build() + val timeTravelSpec = + TimeTravelSpec.create(timeTravelTimestamp, timeTravelVersion, conf.sessionLocalTimeZone) + try { + val table = ViewUtil.loadTable(catalog, tableIdent, context, timeTravelSpec) + table match { + case _: V1Table | _: DelegatingTable => + val relation = markCheckedForView(UnresolvedRelation(tableParts, options, isStreaming)) + if (timeTravelSpec.isEmpty) { + relation + } else { + RelationTimeTravel(relation, timeTravelTimestamp, timeTravelVersion) + } + + case _ => + val relationCatalog = catalog match { + case contextual: TableCatalog with SparkSupportsLoadContext + if timeTravelSpec.isEmpty => + LoadContextTableCatalog(contextual, context) + case _ => catalog + } + SubqueryAlias( + tableIdent.toQualifiedNameParts(catalog), + DataSourceV2Relation.create( + table, + Some(relationCatalog), + Some(tableIdent), + options, + timeTravelSpec)) + } + } catch { + case _: NoSuchTableException => + ViewUtil + .loadView(catalog, tableIdent, context) + .map(view => createViewRelation(tableParts, catalog, tableIdent, view, viewChain)) + .getOrElse(throw tableOrViewNotFound(u)) + } + } + + private def createViewRelation( + nameParts: Seq[String], + catalog: CatalogPlugin, + ident: Identifier, + view: View, + existingChain: Seq[Seq[String]] = Seq.empty): LogicalPlan = { + val desc = V1Table.toCatalogTable(catalog, ident, view) + val parsed = parseViewText(desc.qualifiedName, view.queryText()) + val viewQueryCatalogAndNamespace = queryCatalogAndNamespace(catalog, ident, view) + val viewChain = existingChain :+ ident.toQualifiedNameParts(catalog) + val rewritten = rewriteIdentifiers(parsed, viewQueryCatalogAndNamespace, viewChain) + val aliases = view + .schema() + .fields + .zipWithIndex + .map { case (expected, pos) => + val attr = GetColumnByOrdinal(pos, expected.dataType) + val cast = if (isSchemaCompensation(view.schemaMode())) { + Cast(attr, expected.dataType, ansiEnabled = true) + } else { + UpCast(attr, expected.dataType) + } + Alias(cast, expected.name)(explicitMetadata = Some(expected.metadata)) + } + .toIndexedSeq + + SubqueryAlias(nameParts, ViewNode(desc, isTempView = false, Project(aliases, rewritten))) + } + + private def queryCatalogAndNamespace( + catalog: CatalogPlugin, + ident: Identifier, + view: View): Seq[String] = { + val currentCatalog = Option(view.currentCatalog()).filter(_.nonEmpty).getOrElse(catalog.name()) + val currentNamespace = Option(view.currentNamespace()).map(_.toIndexedSeq).getOrElse { + ident.namespace().toIndexedSeq + } + currentCatalog +: currentNamespace + } + + private def parseViewText(name: String, viewText: String): LogicalPlan = { + val origin = CurrentOrigin.get.copy(objectType = Some("VIEW"), objectName = Some(name)) + CurrentOrigin.withOrigin(origin) { + spark.sessionState.sqlParser.parseQuery(viewText) + } + } + + private def rewriteIdentifiers( + plan: LogicalPlan, + catalogAndNamespace: Seq[String], + viewChain: Seq[Seq[String]]): LogicalPlan = { + qualifyTableIdentifiers( + qualifyFunctionIdentifiers(CTESubstitution.apply(plan), catalogAndNamespace), + catalogAndNamespace, + viewChain) + } + + private def qualifyFunctionIdentifiers( + plan: LogicalPlan, + catalogAndNamespace: Seq[String]): LogicalPlan = plan transformExpressions { + case u @ UnresolvedFunction(Seq(name), _, _, _, _, _, _) => + if (!isBuiltinFunction(name)) { + u.copy(nameParts = catalogAndNamespace :+ name) + } else { + u + } + case u @ UnresolvedFunction(parts, _, _, _, _, _, _) if !isCatalog(parts.head) => + u.copy(nameParts = catalogAndNamespace.head +: parts) + } + + private def qualifyTableIdentifiers( + child: LogicalPlan, + catalogAndNamespace: Seq[String], + viewChain: Seq[Seq[String]]): LogicalPlan = { + child transform { + case UnresolvedRelation(parts, options, isStreaming) => + val qualifiedTableId = ViewUtil.qualifyParts(parts, catalogAndNamespace, isCatalog) + UnResolvedRelationFromView( + qualifiedTableId, + parts, + catalogAndNamespace, + viewChain, + options, + isStreaming) + case RelationTimeTravel( + UnresolvedRelation(parts, options, isStreaming), + timestampOpt, + versionOpt) => + val qualifiedTableId = ViewUtil.qualifyParts(parts, catalogAndNamespace, isCatalog) + UnResolvedRelationFromView( + qualifiedTableId, + parts, + catalogAndNamespace, + viewChain, + options, + isStreaming, + timeTravelVersion = versionOpt, + timeTravelTimestamp = timestampOpt) + case other => + other.transformExpressions { case subquery: SubqueryExpression => + subquery.withNewPlan( + qualifyTableIdentifiers(subquery.plan, catalogAndNamespace, viewChain)) + } + } + } + + private def tableOrViewNotFound(relation: UnResolvedRelationFromView): Throwable = { + val catalogAndNamespace = relation.viewCatalogAndNamespace + val searchPath = catalogManager + .sqlResolutionPathEntries(catalogAndNamespace.head, catalogAndNamespace.tail) + .map(QueryCompilationErrors.toSQLId) + QueryCompilationErrors.tableOrViewNotFoundWithSearchPath( + relation.nameParts, + searchPath, + relation.origin) + } + + private def isSchemaCompensation(mode: String): Boolean = { + mode != null && mode.equalsIgnoreCase(SchemaCompensation.toString) + } + + private def markCheckedForView(relation: UnresolvedRelation): UnresolvedRelation = { + relation.setTagValue(CheckedForView, ()) + relation + } + + private def isTempView(nameParts: Seq[String]): Boolean = { + catalogManager.v1SessionCatalog.isTempView(nameParts) + } + + private def isCatalog(name: String): Boolean = { + catalogManager.isCatalogRegistered(name) + } + + private def isBuiltinFunction(name: String): Boolean = { + catalogManager.v1SessionCatalog.isBuiltinFunction(name) + } +} + +object ResolveReferencedByViews { + private val CheckedForView = TreeNodeTag[Unit]("iceberg.referenced-by.checked-for-view") +} diff --git a/spark/v4.2/spark-extensions/src/main/scala/org/apache/spark/sql/catalyst/analysis/ResolveViews.scala b/spark/v4.2/spark-extensions/src/main/scala/org/apache/spark/sql/catalyst/analysis/ResolveViews.scala index 1517b0fed5f2..fac9cb6ec68d 100644 --- a/spark/v4.2/spark-extensions/src/main/scala/org/apache/spark/sql/catalyst/analysis/ResolveViews.scala +++ b/spark/v4.2/spark-extensions/src/main/scala/org/apache/spark/sql/catalyst/analysis/ResolveViews.scala @@ -18,260 +18,30 @@ */ package org.apache.spark.sql.catalyst.analysis -import org.apache.iceberg.catalog.LoadContext -import org.apache.iceberg.spark.SparkSQLProperties -import org.apache.iceberg.spark.SparkSupportsLoadContext import org.apache.spark.sql.SparkSession import org.apache.spark.sql.catalyst.analysis.ViewUtil.IcebergViewHelper -import org.apache.spark.sql.catalyst.expressions.Alias -import org.apache.spark.sql.catalyst.expressions.Cast -import org.apache.spark.sql.catalyst.expressions.SubqueryExpression -import org.apache.spark.sql.catalyst.expressions.UpCast -import org.apache.spark.sql.catalyst.parser.ParseException import org.apache.spark.sql.catalyst.plans.logical.LogicalPlan -import org.apache.spark.sql.catalyst.plans.logical.Project -import org.apache.spark.sql.catalyst.plans.logical.SubqueryAlias import org.apache.spark.sql.catalyst.plans.logical.views.ResolvedV2View -import org.apache.spark.sql.catalyst.plans.logical.views.UnResolvedRelationFromView import org.apache.spark.sql.catalyst.rules.Rule -import org.apache.spark.sql.catalyst.trees.CurrentOrigin -import org.apache.spark.sql.catalyst.trees.Origin import org.apache.spark.sql.connector.catalog.CatalogManager -import org.apache.spark.sql.connector.catalog.CatalogPlugin -import org.apache.spark.sql.connector.catalog.DelegatingTable -import org.apache.spark.sql.connector.catalog.Identifier import org.apache.spark.sql.connector.catalog.LookupCatalog -import org.apache.spark.sql.connector.catalog.TableCatalog -import org.apache.spark.sql.connector.catalog.V1Table -import org.apache.spark.sql.connector.catalog.View -import org.apache.spark.sql.errors.QueryCompilationErrors -import org.apache.spark.sql.execution.datasources.v2.DataSourceV2Relation /** * Resolves Iceberg views referenced by commands that use `UnresolvedTableOrView`. * * In Spark 4.2, view relations are expanded through `RelationCatalog.loadRelation`, so Spark owns * query parsing, identifier resolution, and schema-mode application. This rule only converts the - * remaining command targets to `ResolvedV2View` for Iceberg command planning. - * - * When referenced-by is enabled, this rule also expands Iceberg views itself so that tables they - * read are loaded with the referencing view chain. + * remaining command targets to `ResolvedV2View` for Iceberg command planning. With referenced-by + * enabled, Iceberg views in queries are expanded by `ResolveReferencedByViews` instead. */ case class ResolveViews(spark: SparkSession) extends Rule[LogicalPlan] with LookupCatalog { - - import org.apache.spark.sql.connector.catalog.CatalogV2Implicits._ - protected lazy val catalogManager: CatalogManager = spark.sessionState.catalogManager - override def apply(plan: LogicalPlan): LogicalPlan = { - if (referencedByEnabled) { - resolveWithReferencedBy(plan) - } else { - resolveViewCommands(plan) - } - } - - private def resolveWithReferencedBy(plan: LogicalPlan): LogicalPlan = plan resolveOperators { - case u @ UnresolvedRelation(nameParts, _, _) - if catalogManager.v1SessionCatalog.isTempView(nameParts) => - u - - case u @ UnresolvedRelation(parts @ CatalogAndIdentifier(catalog, ident), _, _) => - ViewUtil - .loadView(catalog, ident) - .map(createViewRelation(parts, catalog, ident, _)) - .getOrElse(u) - - case u @ UnResolvedRelationFromView( - tableParts @ CatalogAndIdentifier(catalog, tableIdent), - viewChain, - options, - isStreaming, - timeTravelVersion, - timeTravelTimestamp) if timeTravelTimestamp.forall(_.resolved) => - val referencedBy = ViewUtil.buildReferencedByChain(viewChain, catalog.name()) - val context = LoadContext.builder().referencedBy(referencedBy).build() - val timeTravelSpec = - TimeTravelSpec.create(timeTravelTimestamp, timeTravelVersion, conf.sessionLocalTimeZone) - try { - val table = ViewUtil.loadTable(catalog, tableIdent, context, timeTravelSpec) - table match { - case _: V1Table | _: DelegatingTable => - val relation = UnresolvedRelation(tableParts, options, isStreaming) - if (timeTravelSpec.isEmpty) { - relation - } else { - RelationTimeTravel(relation, timeTravelTimestamp, timeTravelVersion) - } - - case _ => - val relationCatalog = catalog match { - case contextual: TableCatalog with SparkSupportsLoadContext - if timeTravelSpec.isEmpty => - LoadContextTableCatalog(contextual, context) - case _ => catalog - } - SubqueryAlias( - tableIdent.toQualifiedNameParts(catalog), - DataSourceV2Relation.create( - table, - Some(relationCatalog), - Some(tableIdent), - options, - timeTravelSpec)) - } - } catch { - case _: NoSuchTableException => - ViewUtil - .loadView(catalog, tableIdent, context) - .map(view => createViewRelation(tableParts, catalog, tableIdent, view, viewChain)) - .getOrElse(UnresolvedRelation(tableParts, options, isStreaming)) - } - - case u @ UnresolvedTableOrView(CatalogAndIdentifier(catalog, ident), _, _, _) => - ViewUtil - .loadView(catalog, ident) - .map(view => ResolvedV2View(catalog.asViewCatalog, ident, view)) - .getOrElse(u) - } - - private def resolveViewCommands(plan: LogicalPlan): LogicalPlan = plan resolveOperators { + override def apply(plan: LogicalPlan): LogicalPlan = plan resolveOperators { case u @ UnresolvedTableOrView(CatalogAndIdentifier(catalog, ident), _, _, _) => ViewUtil .loadView(catalog, ident) .map(view => ResolvedV2View(catalog.asViewCatalog, ident, view)) .getOrElse(u) } - - private def createViewRelation( - nameParts: Seq[String], - catalog: CatalogPlugin, - ident: Identifier, - view: View, - existingChain: Seq[Seq[String]] = Seq.empty): LogicalPlan = { - val parsed = parseViewText(nameParts.quoted, view.queryText()) - val viewQueryCatalogAndNamespace = queryCatalogAndNamespace(catalog, ident, view) - val viewChain = - ViewUtil.buildViewChain( - Seq(ident.name()), - resolvedCatalogAndNamespace(catalog, ident), - existingChain, - isCatalog) - val rewritten = rewriteIdentifiers(parsed, viewQueryCatalogAndNamespace, viewChain) - val aliases = view - .schema() - .fields - .zipWithIndex - .map { case (expected, pos) => - val attr = GetColumnByOrdinal(pos, expected.dataType) - val cast = if (isSchemaCompensation(view.schemaMode())) { - Cast(attr, expected.dataType, ansiEnabled = true) - } else { - UpCast(attr, expected.dataType) - } - Alias(cast, expected.name)(explicitMetadata = Some(expected.metadata)) - } - .toIndexedSeq - - SubqueryAlias(nameParts, Project(aliases, rewritten)) - } - - private def queryCatalogAndNamespace( - catalog: CatalogPlugin, - ident: Identifier, - view: View): Seq[String] = { - val currentCatalog = Option(view.currentCatalog()).filter(_.nonEmpty).getOrElse(catalog.name()) - val currentNamespace = Option(view.currentNamespace()).map(_.toIndexedSeq).getOrElse { - ident.namespace().toIndexedSeq - } - currentCatalog +: currentNamespace - } - - private def resolvedCatalogAndNamespace( - catalog: CatalogPlugin, - ident: Identifier): Seq[String] = { - catalog.name() +: ident.namespace().toIndexedSeq - } - - private def parseViewText(name: String, viewText: String): LogicalPlan = { - val origin = Origin(objectType = Some("VIEW"), objectName = Some(name)) - try { - CurrentOrigin.withOrigin(origin) { - spark.sessionState.sqlParser.parseQuery(viewText) - } - } catch { - case _: ParseException => - throw QueryCompilationErrors.invalidViewNameError(name) - } - } - - private def rewriteIdentifiers( - plan: LogicalPlan, - catalogAndNamespace: Seq[String], - viewChain: Seq[Seq[String]]): LogicalPlan = { - qualifyTableIdentifiers( - qualifyFunctionIdentifiers(CTESubstitution.apply(plan), catalogAndNamespace), - catalogAndNamespace, - viewChain) - } - - private def qualifyFunctionIdentifiers( - plan: LogicalPlan, - catalogAndNamespace: Seq[String]): LogicalPlan = plan transformExpressions { - case u @ UnresolvedFunction(Seq(name), _, _, _, _, _, _) => - if (!isBuiltinFunction(name)) { - u.copy(nameParts = catalogAndNamespace :+ name) - } else { - u - } - case u @ UnresolvedFunction(parts, _, _, _, _, _, _) if !isCatalog(parts.head) => - u.copy(nameParts = catalogAndNamespace.head +: parts) - } - - private def qualifyTableIdentifiers( - child: LogicalPlan, - catalogAndNamespace: Seq[String], - viewChain: Seq[Seq[String]]): LogicalPlan = { - child transform { - case UnresolvedRelation(parts, options, isStreaming) => - val qualifiedTableId = ViewUtil.qualifyParts(parts, catalogAndNamespace, isCatalog) - UnResolvedRelationFromView(qualifiedTableId, viewChain, options, isStreaming) - case RelationTimeTravel( - UnresolvedRelation(parts, options, isStreaming), - timestampOpt, - versionOpt) => - val qualifiedTableId = ViewUtil.qualifyParts(parts, catalogAndNamespace, isCatalog) - UnResolvedRelationFromView( - qualifiedTableId, - viewChain, - options, - isStreaming, - timeTravelVersion = versionOpt, - timeTravelTimestamp = timestampOpt) - case other => - other.transformExpressions { case subquery: SubqueryExpression => - subquery.withNewPlan( - qualifyTableIdentifiers(subquery.plan, catalogAndNamespace, viewChain)) - } - } - } - - private def isSchemaCompensation(mode: String): Boolean = { - mode != null && mode.equalsIgnoreCase(SchemaCompensation.toString) - } - - private def isCatalog(name: String): Boolean = { - catalogManager.isCatalogRegistered(name) - } - - private def isBuiltinFunction(name: String): Boolean = { - catalogManager.v1SessionCatalog.isBuiltinFunction(name) - } - - private def referencedByEnabled: Boolean = { - java.lang.Boolean.parseBoolean( - conf.getConfString( - SparkSQLProperties.REFERENCED_BY_ENABLED, - SparkSQLProperties.REFERENCED_BY_ENABLED_DEFAULT.toString)) - } } diff --git a/spark/v4.2/spark-extensions/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/views/UnResolvedRelationFromView.scala b/spark/v4.2/spark-extensions/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/views/UnResolvedRelationFromView.scala index 6ee0dccd90c6..28820a8168a5 100644 --- a/spark/v4.2/spark-extensions/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/views/UnResolvedRelationFromView.scala +++ b/spark/v4.2/spark-extensions/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/views/UnResolvedRelationFromView.scala @@ -21,11 +21,13 @@ package org.apache.spark.sql.catalyst.plans.logical.views import org.apache.spark.sql.catalyst.expressions.Attribute import org.apache.spark.sql.catalyst.expressions.Expression import org.apache.spark.sql.catalyst.plans.logical.LeafNode -import org.apache.spark.sql.catalyst.util.quoteIfNeeded +import org.apache.spark.sql.connector.catalog.CatalogV2Implicits.MultipartIdentifierHelper import org.apache.spark.sql.util.CaseInsensitiveStringMap case class UnResolvedRelationFromView( tableMultipartIdentifier: Seq[String], + nameParts: Seq[String], + viewCatalogAndNamespace: Seq[String], viewChain: Seq[Seq[String]], options: CaseInsensitiveStringMap = CaseInsensitiveStringMap.empty(), override val isStreaming: Boolean = false, @@ -37,9 +39,9 @@ case class UnResolvedRelationFromView( override lazy val resolved: Boolean = false - def tableName: String = tableMultipartIdentifier.map(quoteIfNeeded).mkString(".") + def tableName: String = tableMultipartIdentifier.quoted - def viewName: String = viewChain.map(_.map(quoteIfNeeded).mkString(".")).mkString(" -> ") + def viewName: String = viewChain.map(_.quoted).mkString(" -> ") override def simpleString(maxFields: Int): String = { s"'UnResolvedRelationFromView [table=$tableName, views=$viewName, " + diff --git a/spark/v4.2/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestReferencedByViewChain.java b/spark/v4.2/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestReferencedByViewChain.java index f21205d30215..f595af65fa37 100644 --- a/spark/v4.2/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestReferencedByViewChain.java +++ b/spark/v4.2/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestReferencedByViewChain.java @@ -21,17 +21,21 @@ import static org.apache.iceberg.TestHelpers.waitUntilAfter; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; +import static org.assertj.core.api.Assertions.catchThrowable; import java.net.InetAddress; import java.nio.file.Path; import java.sql.Timestamp; +import java.util.Arrays; import java.util.Collections; import java.util.List; +import java.util.UUID; import java.util.concurrent.CopyOnWriteArrayList; import java.util.concurrent.TimeUnit; import java.util.stream.Collectors; import java.util.stream.IntStream; import org.apache.iceberg.CatalogProperties; +import org.apache.iceberg.Schema; import org.apache.iceberg.Table; import org.apache.iceberg.catalog.LoadContext; import org.apache.iceberg.catalog.Namespace; @@ -45,13 +49,18 @@ import org.apache.iceberg.spark.SparkSchemaUtil; import org.apache.iceberg.spark.SparkSessionCatalog; import org.apache.iceberg.spark.SparkTestHelperBase; +import org.apache.iceberg.spark.TestBase; import org.apache.iceberg.spark.source.HasIcebergCatalog; import org.apache.iceberg.spark.source.SimpleRecord; +import org.apache.iceberg.types.Types; import org.apache.iceberg.view.View; +import org.apache.spark.SparkThrowable; import org.apache.spark.sql.Dataset; import org.apache.spark.sql.Row; import org.apache.spark.sql.SparkSession; +import org.apache.spark.sql.catalyst.analysis.NoSuchTableException; import org.apache.spark.sql.connector.catalog.CatalogPlugin; +import org.assertj.core.api.ThrowableAssert.ThrowingCallable; import org.junit.jupiter.api.AfterAll; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeAll; @@ -126,15 +135,12 @@ private static List referencedBy(LoadContext context) { } @BeforeAll - public static void startSpark() { + static void startSpark() { spark = SparkSession.builder() .master("local[2]") .config("spark.driver.host", InetAddress.getLoopbackAddress().getHostAddress()) - .config("spark.ui.enabled", "false") - .config( - "spark.metrics.conf.*.sink.servlet.class", - "org.apache.iceberg.spark.DummyMetricsServlet") + .config(TestBase.DISABLE_UI) .config("spark.sql.extensions", IcebergSparkSessionExtensions.class.getName()) .config("spark.sql.catalog." + CATALOG_NAME, SparkCatalog.class.getName()) .config( @@ -147,7 +153,7 @@ public static void startSpark() { "spark.sql.catalog." + OTHER_CATALOG_NAME + "." + CatalogProperties.CATALOG_IMPL, ContextTrackingCatalog.class.getName()) .config("spark.sql.catalog." + OTHER_CATALOG_NAME + ".default-namespace", "default") - .config("spark.sql.catalog." + OTHER_CATALOG_NAME + ".cache-enabled", "true") + .config("spark.sql.catalog." + OTHER_CATALOG_NAME + ".cache-enabled", "false") .config( "spark.sql.catalog." + SESSION_CATALOG_NAME, SparkSessionCatalog.class.getName()) .config( @@ -156,6 +162,12 @@ public static void startSpark() { .config("spark.sql.catalog." + SESSION_CATALOG_NAME + ".cache-enabled", "false") .config("spark.sql.warehouse.dir", warehouse.toString()) .config("spark.sql.defaultCatalog", CATALOG_NAME) + // Cover Hive-enabled sessions, whose analyzer is built separately from the default one + .config("spark.hadoop.hive.metastore.uris", "") + .config( + "spark.hadoop.javax.jdo.option.ConnectionURL", + "jdbc:derby:memory:" + UUID.randomUUID() + ";create=true") + .enableHiveSupport() .getOrCreate(); spark.sql(String.format("CREATE NAMESPACE IF NOT EXISTS %s.%s", CATALOG_NAME, NAMESPACE)); @@ -168,7 +180,7 @@ public static void startSpark() { } @AfterAll - public static void stopSpark() { + static void stopSpark() { if (spark != null) { spark.stop(); spark = null; @@ -176,9 +188,7 @@ public static void stopSpark() { } @BeforeEach - public void before() { - ContextTrackingCatalog.clearCaptured(); - + void before() { spark.conf().set(SparkSQLProperties.REFERENCED_BY_ENABLED, "true"); spark.sql(String.format("USE %s.%s", CATALOG_NAME, NAMESPACE)); spark.sql(String.format("CREATE TABLE IF NOT EXISTS %s (id INT, data STRING)", TABLE_NAME)); @@ -189,7 +199,7 @@ public void before() { } @AfterEach - public void after() { + void after() { spark.sql(String.format("USE %s.%s", CATALOG_NAME, NAMESPACE)); spark.sql("DROP VIEW IF EXISTS simple_view"); spark.sql("DROP VIEW IF EXISTS view_a"); @@ -197,6 +207,7 @@ public void after() { spark.sql("DROP VIEW IF EXISTS view_c"); spark.sql("DROP VIEW IF EXISTS time_travel_view"); spark.sql("DROP VIEW IF EXISTS cross_view"); + spark.sql("DROP VIEW IF EXISTS shadow_v"); spark.sql(String.format("DROP VIEW IF EXISTS %s", VIEW_DEFAULT_VIEW_NAME)); spark.sql(String.format("DROP TABLE IF EXISTS %s", TABLE_NAME)); spark.sql( @@ -213,7 +224,7 @@ public void after() { } @Test - public void referencedByIsDisabledByDefault() { + void referencedByIsDisabledByDefault() { createView("simple_view", String.format("SELECT id FROM %s", TABLE_NAME)); spark.conf().unset(SparkSQLProperties.REFERENCED_BY_ENABLED); ContextTrackingCatalog.clearCaptured(); @@ -228,9 +239,7 @@ public void referencedByIsDisabledByDefault() { } @Test - public void directTableAccessHasEmptyContext() { - ContextTrackingCatalog.clearCaptured(); - + void directTableAccessHasEmptyContext() { List rows = spark.sql(String.format("SELECT * FROM %s", TABLE_NAME)).collectAsList(); assertThat(rows).hasSize(5); @@ -240,7 +249,7 @@ public void directTableAccessHasEmptyContext() { } @Test - public void singleViewPassesViewIdentifierInContext() { + void singleViewPassesViewIdentifierInContext() { createView("simple_view", String.format("SELECT id FROM %s", TABLE_NAME)); spark.sql(String.format("REFRESH TABLE %s", TABLE_NAME)); ContextTrackingCatalog.clearCaptured(); @@ -257,21 +266,20 @@ public void singleViewPassesViewIdentifierInContext() { } @Test - public void viewBodyResolvesTableQualifiedColumns() { + void viewBodyResolvesTableQualifiedColumns() { createView("view_a", String.format("SELECT %s.id FROM %s", TABLE_NAME, TABLE_NAME)); createView( "view_b", String.format( "SELECT %s.%s.%s.id FROM %s.%s.%s", CATALOG_NAME, NAMESPACE, TABLE_NAME, CATALOG_NAME, NAMESPACE, TABLE_NAME)); - ContextTrackingCatalog.clearCaptured(); assertThat(spark.sql("SELECT * FROM view_a").collectAsList()).hasSize(5); assertThat(spark.sql("SELECT * FROM view_b").collectAsList()).hasSize(5); } @Test - public void referencedByUsesResolvedViewIdentifier() { + void referencedByUsesResolvedViewIdentifier() { spark.sql( String.format( "CREATE TABLE %s.%s (id INT, data STRING)", @@ -283,7 +291,7 @@ public void referencedByUsesResolvedViewIdentifier() { String viewSql = String.format("SELECT id FROM %s", VIEW_DEFAULT_TABLE_NAME); String schemaSql = String.format("SELECT id FROM %s.%s", VIEW_DEFAULT_NAMESPACE, VIEW_DEFAULT_TABLE_NAME); - viewCatalog() + viewCatalog(CATALOG_NAME) .buildView(TableIdentifier.of(NAMESPACE, VIEW_DEFAULT_VIEW_NAME)) .withQuery("spark", viewSql) .withDefaultNamespace(VIEW_DEFAULT_NAMESPACE) @@ -304,7 +312,7 @@ public void referencedByUsesResolvedViewIdentifier() { } @Test - public void nestedViewChainAccumulatesContext() { + void nestedViewChainAccumulatesContext() { createView("view_a", String.format("SELECT id, data FROM %s", TABLE_NAME)); createView("view_b", "SELECT id FROM view_a WHERE id <= 3"); createView("view_c", "SELECT id FROM view_b WHERE id > 1"); @@ -319,7 +327,7 @@ public void nestedViewChainAccumulatesContext() { } @Test - public void timeTravelViewPassesViewIdentifierInContext() throws Exception { + void timeTravelViewPassesViewIdentifierInContext() throws Exception { long snapshotId = Spark3Util.loadIcebergTable(spark, TABLE_NAME).currentSnapshot().snapshotId(); createView( "time_travel_view", @@ -333,7 +341,7 @@ public void timeTravelViewPassesViewIdentifierInContext() throws Exception { } @Test - public void crossCatalogViewAccessDoesNotFailWhenReferencedByIsDisabled() { + void crossCatalogViewAccessDoesNotFailWhenReferencedByIsDisabled() { createCrossCatalogView(); spark.conf().unset(SparkSQLProperties.REFERENCED_BY_ENABLED); ContextTrackingCatalog.clearCaptured(); @@ -341,42 +349,37 @@ public void crossCatalogViewAccessDoesNotFailWhenReferencedByIsDisabled() { List result = spark.sql("SELECT * FROM cross_view").collectAsList(); assertThat(result).hasSize(5); - assertThat( - ContextTrackingCatalog.CAPTURED.stream() - .filter( - captured -> - captured.tableIdentifier.equals( - TableIdentifier.of(NAMESPACE, OTHER_TABLE_NAME))) - .collect(Collectors.toList())) + assertThat(capturedLoadsOf(TableIdentifier.of(NAMESPACE, OTHER_TABLE_NAME))) .isNotEmpty() .allSatisfy(captured -> assertThat(captured.referencedBy).isEmpty()); } @Test - public void crossCatalogViewAccessFailsWhenReferencedByIsEnabled() { + void crossCatalogViewAccessFailsWhenReferencedByIsEnabled() { createCrossCatalogView(); - ContextTrackingCatalog.clearCaptured(); assertThatThrownBy(() -> spark.sql("SELECT * FROM cross_view").collectAsList()) - .hasMessageContaining("Cross-catalog view references are not supported") - .hasMessageContaining(OTHER_CATALOG_NAME); + .isInstanceOf(IllegalStateException.class) + .hasMessage( + "Cross-catalog view references are not supported with referenced-by enabled. " + + "Views from catalogs [%s] cannot reference entities in catalog [%s]", + CATALOG_NAME, OTHER_CATALOG_NAME); } @Test - public void versionTimeTravelViewReadsPinnedSnapshot() throws Exception { + void versionTimeTravelViewReadsPinnedSnapshot() throws Exception { long snapshotId = Spark3Util.loadIcebergTable(spark, TABLE_NAME).currentSnapshot().snapshotId(); createView( "time_travel_view", String.format("SELECT id FROM %s VERSION AS OF %d", TABLE_NAME, snapshotId)); appendRecords(TABLE_NAME); - ContextTrackingCatalog.clearCaptured(); List result = spark.sql("SELECT * FROM time_travel_view").collectAsList(); assertThat(result).hasSize(5); } @Test - public void timestampStringTimeTravelView() throws Exception { + void timestampStringTimeTravelView() throws Exception { long asOfMillis = waitUntilAfter(currentSnapshotMillis()); String asOf = new Timestamp(asOfMillis).toString(); createView( @@ -393,7 +396,7 @@ public void timestampStringTimeTravelView() throws Exception { } @Test - public void timestampSecondsTimeTravelView() throws Exception { + void timestampSecondsTimeTravelView() throws Exception { long asOfSeconds = TimeUnit.MILLISECONDS.toSeconds(currentSnapshotMillis()) + 1; createView( "time_travel_view", @@ -409,14 +412,27 @@ public void timestampSecondsTimeTravelView() throws Exception { } @Test - public void directReadOfNonIcebergTableInSessionCatalog() { + void timestampExpressionTimeTravelView() { + createView( + "time_travel_view", + String.format("SELECT id FROM %s TIMESTAMP AS OF current_timestamp()", TABLE_NAME)); + ContextTrackingCatalog.clearCaptured(); + + List result = spark.sql("SELECT * FROM time_travel_view").collectAsList(); + assertThat(result).hasSize(5); + + assertCapturedTableChain(ContextTrackingCatalog.CAPTURED, TABLE_NAME, "time_travel_view"); + } + + @Test + void directReadOfNonIcebergTableInSessionCatalog() { String parquetTable = createSessionCatalogParquetTable(); assertThat(spark.sql(String.format("SELECT * FROM %s", parquetTable)).count()).isEqualTo(3); } @Test - public void viewReadsNonIcebergTableInSessionCatalog() { + void viewReadsNonIcebergTableInSessionCatalog() { String parquetTable = createSessionCatalogParquetTable(); createView( SESSION_CATALOG_NAME, PARQUET_VIEW_NAME, String.format("SELECT id FROM %s", parquetTable)); @@ -431,6 +447,166 @@ public void viewReadsNonIcebergTableInSessionCatalog() { .isEqualTo(3); } + @Test + void directTableReadDoesNotLoadView() { + spark.sql(String.format("SELECT * FROM %s", TABLE_NAME)).collectAsList(); + + assertThat(ContextTrackingCatalog.CAPTURED_VIEWS).isEmpty(); + } + + @Test + void directTableReadLoadsTableAsOftenAsWhenReferencedByIsDisabled() { + String otherTable = String.format("%s.%s.%s", OTHER_CATALOG_NAME, NAMESPACE, OTHER_TABLE_NAME); + spark.sql(String.format("CREATE TABLE %s (id INT, data STRING)", otherTable)); + appendRecords(otherTable); + TableIdentifier ident = TableIdentifier.of(NAMESPACE, OTHER_TABLE_NAME); + + spark.conf().unset(SparkSQLProperties.REFERENCED_BY_ENABLED); + ContextTrackingCatalog.clearCaptured(); + spark.sql(String.format("SELECT * FROM %s", otherTable)).collectAsList(); + int expectedLoads = capturedLoadsOf(ident).size(); + + spark.conf().set(SparkSQLProperties.REFERENCED_BY_ENABLED, "true"); + ContextTrackingCatalog.clearCaptured(); + spark.sql(String.format("SELECT * FROM %s", otherTable)).collectAsList(); + + assertThat(expectedLoads).isPositive(); + assertThat(capturedLoadsOf(ident)).hasSize(expectedLoads); + } + + @Test + void tempViewShadowsIcebergViewWhenReferencedByIsDisabled() { + spark.conf().unset(SparkSQLProperties.REFERENCED_BY_ENABLED); + + assertDescribeResolvesTempView(); + } + + @Test + void tempViewShadowsIcebergViewWhenReferencedByIsEnabled() { + assertDescribeResolvesTempView(); + } + + @Test + void cachedViewIsReusedByLaterQuery() { + createView("simple_view", String.format("SELECT id FROM %s", TABLE_NAME)); + Dataset cached = spark.sql("SELECT * FROM simple_view"); + cached.cache(); + try { + cached.count(); + Dataset query = spark.sql("SELECT * FROM simple_view"); + + assertThat(query.queryExecution().withCachedData().toString()).contains("InMemoryRelation"); + } finally { + cached.unpersist(); + } + } + + @Test + void streamingReadOfViewFailsAsWhenReferencedByIsDisabled() { + createView("simple_view", String.format("SELECT id FROM %s", TABLE_NAME)); + + assertSameFailureAsReferencedByDisabled(() -> spark.readStream().table("simple_view")); + } + + @Test + void deleteFromViewFailsAsWhenReferencedByIsDisabled() { + createView("simple_view", String.format("SELECT id FROM %s", TABLE_NAME)); + + assertSameFailureAsReferencedByDisabled( + () -> spark.sql("DELETE FROM simple_view WHERE id = 1")); + } + + @Test + void tableApiReadOfViewPassesViewIdentifierInContext() { + createView("simple_view", String.format("SELECT id FROM %s", TABLE_NAME)); + ContextTrackingCatalog.clearCaptured(); + + assertThat(spark.table("simple_view").collectAsList()).hasSize(5); + + assertCapturedTableChain(ContextTrackingCatalog.CAPTURED, TABLE_NAME, "simple_view"); + } + + @Test + void createOrReplaceViewWithCycleFailsAsWhenReferencedByIsDisabled() { + createView("view_a", String.format("SELECT id FROM %s", TABLE_NAME)); + createView("view_b", "SELECT id FROM view_a"); + + assertSameFailureAsReferencedByDisabled( + () -> spark.sql("CREATE OR REPLACE VIEW view_a AS SELECT id FROM view_b")); + } + + @Test + void alterViewPropertiesUncachesView() { + createView("simple_view", String.format("SELECT id FROM %s", TABLE_NAME)); + spark.catalog().cacheTable("simple_view"); + + spark.sql("ALTER VIEW simple_view SET TBLPROPERTIES ('key' = 'value')"); + + assertThat(spark.catalog().isCached("simple_view")).isFalse(); + } + + @Test + void viewCommandsMatchReferencedByDisabled() { + createView("simple_view", String.format("SELECT id FROM %s", TABLE_NAME)); + + for (String command : List.of("DESCRIBE TABLE simple_view", "SHOW CREATE TABLE simple_view")) { + spark.conf().unset(SparkSQLProperties.REFERENCED_BY_ENABLED); + List expected = spark.sql(command).collectAsList(); + + spark.conf().set(SparkSQLProperties.REFERENCED_BY_ENABLED, "true"); + assertThat(spark.sql(command).collectAsList()).as(command).isEqualTo(expected); + } + } + + @Test + void invalidViewSqlFailsAsWhenReferencedByIsDisabled() { + createViewWithoutAnalysis("simple_view", "invalid SQL"); + + assertSameFailureAsReferencedByDisabled( + () -> spark.sql("SELECT * FROM simple_view").collectAsList()); + } + + @Test + void viewReadingMissingTableFailsAsWhenReferencedByIsDisabled() { + createViewWithoutAnalysis("simple_view", "SELECT id FROM non_existing"); + + assertSameFailureAsReferencedByDisabled( + () -> spark.sql("SELECT * FROM simple_view").collectAsList()); + } + + private void assertDescribeResolvesTempView() { + createView("shadow_v", String.format("SELECT id FROM %s", TABLE_NAME)); + spark.sql("CREATE TEMPORARY VIEW shadow_v AS SELECT 'x' AS temp_col"); + try { + List columns = + spark.sql("DESCRIBE TABLE shadow_v").collectAsList().stream() + .map(row -> row.getString(0)) + .collect(Collectors.toList()); + + assertThat(columns).contains("temp_col").doesNotContain("id"); + } finally { + spark.catalog().dropTempView("shadow_v"); + } + } + + private void assertSameFailureAsReferencedByDisabled(ThrowingCallable action) { + spark.conf().unset(SparkSQLProperties.REFERENCED_BY_ENABLED); + Throwable expected = catchThrowable(action); + assertThat(expected).isNotNull(); + + spark.conf().set(SparkSQLProperties.REFERENCED_BY_ENABLED, "true"); + Throwable actual = catchThrowable(action); + if (expected instanceof SparkThrowable) { + assertThat(actual).isInstanceOf(SparkThrowable.class); + assertThat(((SparkThrowable) actual).getCondition()) + .isEqualTo(((SparkThrowable) expected).getCondition()); + assertThat(((SparkThrowable) actual).getMessageParameters()) + .isEqualTo(((SparkThrowable) expected).getMessageParameters()); + } else { + assertThat(actual).isInstanceOf(expected.getClass()).hasMessage(expected.getMessage()); + } + } + private String createSessionCatalogParquetTable() { String parquetTable = String.format("%s.%s.%s", SESSION_CATALOG_NAME, NAMESPACE, PARQUET_TABLE_NAME); @@ -444,7 +620,12 @@ private long currentSnapshotMillis() throws Exception { } private List capturedLoadsOf(TableIdentifier ident) { - return ContextTrackingCatalog.CAPTURED.stream() + return capturedLoadsOf(ContextTrackingCatalog.CAPTURED, ident); + } + + private List capturedLoadsOf( + List captures, TableIdentifier ident) { + return captures.stream() .filter(captured -> captured.tableIdentifier.equals(ident)) .collect(Collectors.toList()); } @@ -454,12 +635,24 @@ private void createView(String viewName, String sql) { } private void createView(String catalogName, String viewName, String sql) { + createView(catalogName, viewName, sql, SparkSchemaUtil.convert(spark.sql(sql).schema())); + } + + private void createViewWithoutAnalysis(String viewName, String sql) { + createView( + CATALOG_NAME, + viewName, + sql, + new Schema(Types.NestedField.optional(1, "id", Types.IntegerType.get()))); + } + + private void createView(String catalogName, String viewName, String sql, Schema schema) { viewCatalog(catalogName) .buildView(TableIdentifier.of(NAMESPACE, viewName)) .withQuery("spark", sql) .withDefaultNamespace(NAMESPACE) .withDefaultCatalog(catalogName) - .withSchema(SparkSchemaUtil.convert(spark.sql(sql).schema())) + .withSchema(schema) .create(); } @@ -480,7 +673,7 @@ private void appendRecords(String tableName) { .collect(Collectors.toList()); Dataset df = spark.createDataFrame(records, SimpleRecord.class); df.writeTo(tableName).append(); - } catch (org.apache.spark.sql.catalyst.analysis.NoSuchTableException e) { + } catch (NoSuchTableException e) { throw new RuntimeException(e); } } @@ -498,26 +691,16 @@ private void assertCapturedTableChain( String targetName, String... expectedViewNames) { List matching = - captures.stream() - .filter( - captured -> - captured.tableIdentifier.equals( - TableIdentifier.of(targetNamespace, targetName))) - .filter(captured -> captured.referencedBy != null && !captured.referencedBy.isEmpty()) + capturedLoadsOf(captures, TableIdentifier.of(targetNamespace, targetName)).stream() + .filter(captured -> !captured.referencedBy.isEmpty()) .collect(Collectors.toList()); assertThat(matching).isNotEmpty(); - - List viewChain = matching.get(0).referencedBy; - assertThat(viewChain).hasSize(expectedViewNames.length); - for (int index = 0; index < expectedViewNames.length; index += 1) { - assertThat(viewChain.get(index)) - .isEqualTo(TableIdentifier.of(NAMESPACE, expectedViewNames[index])); - } - } - - private ViewCatalog viewCatalog() { - return viewCatalog(CATALOG_NAME); + assertThat(matching.get(0).referencedBy) + .containsExactly( + Arrays.stream(expectedViewNames) + .map(name -> TableIdentifier.of(NAMESPACE, name)) + .toArray(TableIdentifier[]::new)); } private ViewCatalog viewCatalog(String catalogName) { diff --git a/spark/v4.2/spark/src/main/java/org/apache/iceberg/spark/BaseCatalog.java b/spark/v4.2/spark/src/main/java/org/apache/iceberg/spark/BaseCatalog.java index c2a43306ce3e..df9a62e609e4 100644 --- a/spark/v4.2/spark/src/main/java/org/apache/iceberg/spark/BaseCatalog.java +++ b/spark/v4.2/spark/src/main/java/org/apache/iceberg/spark/BaseCatalog.java @@ -25,6 +25,7 @@ import org.apache.iceberg.util.PropertyUtil; import org.apache.spark.sql.catalyst.analysis.NoSuchTableException; import org.apache.spark.sql.catalyst.analysis.NoSuchViewException; +import org.apache.spark.sql.catalyst.analysis.ViewUtil; import org.apache.spark.sql.connector.catalog.Identifier; import org.apache.spark.sql.connector.catalog.ProcedureCatalog; import org.apache.spark.sql.connector.catalog.Relation; @@ -108,6 +109,12 @@ public Relation loadRelation(Identifier ident) throws NoSuchTableException { try { return loadTable(ident); } catch (NoSuchTableException e) { + if (ViewUtil.referencedByEnabled()) { + // Don't return views: Spark's own view expansion can't pass the referenced-by chain to the + // catalog. ResolveReferencedByViews expands Iceberg views that are left unresolved instead. + throw e; + } + try { return loadView(ident); } catch (NoSuchViewException viewException) { diff --git a/spark/v4.2/spark/src/main/java/org/apache/iceberg/spark/SparkSQLProperties.java b/spark/v4.2/spark/src/main/java/org/apache/iceberg/spark/SparkSQLProperties.java index 6c631edaef06..84ef44dca50a 100644 --- a/spark/v4.2/spark/src/main/java/org/apache/iceberg/spark/SparkSQLProperties.java +++ b/spark/v4.2/spark/src/main/java/org/apache/iceberg/spark/SparkSQLProperties.java @@ -107,7 +107,8 @@ private SparkSQLProperties() {} // Rely on identifier fields as a PRIMARY KEY constraint for query optimization (not enforced) public static final String IDENTIFIER_FIELDS_RELY = "spark.sql.iceberg.identifier-fields-rely"; - // Controls whether to send referenced-by view chains while loading relations + // Controls whether to send referenced-by view chains while loading relations; requires the + // Iceberg SQL extensions, which expand Iceberg views when this is enabled public static final String REFERENCED_BY_ENABLED = "spark.sql.iceberg.referenced-by.enabled"; public static final boolean REFERENCED_BY_ENABLED_DEFAULT = false; diff --git a/spark/v4.2/spark/src/main/java/org/apache/iceberg/spark/SparkSessionCatalog.java b/spark/v4.2/spark/src/main/java/org/apache/iceberg/spark/SparkSessionCatalog.java index 5cb2c033e56b..07080a0e38e2 100644 --- a/spark/v4.2/spark/src/main/java/org/apache/iceberg/spark/SparkSessionCatalog.java +++ b/spark/v4.2/spark/src/main/java/org/apache/iceberg/spark/SparkSessionCatalog.java @@ -60,8 +60,6 @@ import org.apache.spark.sql.connector.catalog.ViewCatalog; import org.apache.spark.sql.connector.catalog.functions.UnboundFunction; import org.apache.spark.sql.util.CaseInsensitiveStringMap; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; /** * A Spark catalog that can also load non-Iceberg tables. @@ -72,13 +70,13 @@ public class SparkSessionCatalog< T extends TableCatalog & FunctionCatalog & SupportsNamespaces & ViewCatalog> extends BaseCatalog implements CatalogExtension, SparkSupportsLoadContext { - private static final Logger LOG = LoggerFactory.getLogger(SparkSessionCatalog.class); private static final String[] DEFAULT_NAMESPACE = new String[] {"default"}; private String catalogName = null; private TableCatalog icebergCatalog = null; private StagingTableCatalog asStagingCatalog = null; private ViewCatalog asViewCatalog = null; + private SparkSupportsLoadContext asLoadContextCatalog = null; private T sessionCatalog = null; private boolean createParquetAsIceberg = false; private boolean createAvroAsIceberg = false; @@ -209,11 +207,11 @@ public Table loadTable(Identifier ident) throws NoSuchTableException { @Override public Table loadTable(Identifier ident, LoadContext context) throws NoSuchTableException { try { - if (icebergCatalog instanceof SparkSupportsLoadContext) { - return ((SparkSupportsLoadContext) icebergCatalog).loadTable(ident, context); + if (asLoadContextCatalog != null) { + return asLoadContextCatalog.loadTable(ident, context); } - logUnsupportedContext("table", ident, context); + checkLoadContextSupported("table", ident, context); return icebergCatalog.loadTable(ident); } catch (NoSuchTableException e) { return getSessionCatalog().loadTable(ident); @@ -229,11 +227,11 @@ public Table loadTable(Identifier ident, String version) throws NoSuchTableExcep public Table loadTable(Identifier ident, String version, LoadContext context) throws NoSuchTableException { try { - if (icebergCatalog instanceof SparkSupportsLoadContext) { - return ((SparkSupportsLoadContext) icebergCatalog).loadTable(ident, version, context); + if (asLoadContextCatalog != null) { + return asLoadContextCatalog.loadTable(ident, version, context); } - logUnsupportedContext("table", ident, context); + checkLoadContextSupported("table", ident, context); return icebergCatalog.loadTable(ident, version); } catch (NoSuchTableException e) { return getSessionCatalog().loadTable(ident, version); @@ -249,11 +247,11 @@ public Table loadTable(Identifier ident, long timestamp) throws NoSuchTableExcep public Table loadTable(Identifier ident, long timestamp, LoadContext context) throws NoSuchTableException { try { - if (icebergCatalog instanceof SparkSupportsLoadContext) { - return ((SparkSupportsLoadContext) icebergCatalog).loadTable(ident, timestamp, context); + if (asLoadContextCatalog != null) { + return asLoadContextCatalog.loadTable(ident, timestamp, context); } - logUnsupportedContext("table", ident, context); + checkLoadContextSupported("table", ident, context); return icebergCatalog.loadTable(ident, timestamp); } catch (NoSuchTableException e) { return getSessionCatalog().loadTable(ident, timestamp); @@ -463,6 +461,10 @@ public final void initialize(String name, CaseInsensitiveStringMap options) { this.asViewCatalog = (ViewCatalog) icebergCatalog; } + if (icebergCatalog instanceof SparkSupportsLoadContext) { + this.asLoadContextCatalog = (SparkSupportsLoadContext) icebergCatalog; + } + this.createParquetAsIceberg = options.getBoolean("parquet-enabled", createParquetAsIceberg); this.createAvroAsIceberg = options.getBoolean("avro-enabled", createAvroAsIceberg); this.createOrcAsIceberg = options.getBoolean("orc-enabled", createOrcAsIceberg); @@ -636,27 +638,31 @@ public View loadView(Identifier ident) throws NoSuchViewException { @Override public View loadView(Identifier ident, LoadContext context) throws NoSuchViewException { - if (null != asViewCatalog && asViewCatalog.viewExists(ident)) { - if (asViewCatalog instanceof SparkSupportsLoadContext) { - return ((SparkSupportsLoadContext) asViewCatalog).loadView(ident, context); + if (null != asViewCatalog && null != asLoadContextCatalog) { + try { + return asLoadContextCatalog.loadView(ident, context); + } catch (NoSuchViewException ignored) { + // not an Iceberg view, so check the session catalog below } - - logUnsupportedContext("view", ident, context); + } else if (null != asViewCatalog && asViewCatalog.viewExists(ident)) { + checkLoadContextSupported("view", ident, context); return asViewCatalog.loadView(ident); - } else if (isViewCatalog() && getSessionCatalog().viewExists(ident)) { + } + + if (isViewCatalog() && getSessionCatalog().viewExists(ident)) { return getSessionCatalog().loadView(ident); } throw new NoSuchViewException(ident); } - private void logUnsupportedContext(String relationType, Identifier ident, LoadContext context) { + private void checkLoadContextSupported( + String relationType, Identifier ident, LoadContext context) { if (!context.referencedBy().isEmpty()) { - LOG.warn( - "Catalog {} does not support contextual load, ignoring context for {} {}", - catalogName, - relationType, - ident); + throw new UnsupportedOperationException( + String.format( + "Cannot load %s %s with referenced-by: %s does not support load context", + relationType, ident, icebergCatalog.getClass().getName())); } } diff --git a/spark/v4.2/spark/src/main/scala/org/apache/spark/sql/catalyst/analysis/ViewUtil.scala b/spark/v4.2/spark/src/main/scala/org/apache/spark/sql/catalyst/analysis/ViewUtil.scala index 638c76a9d916..cff270eb4689 100644 --- a/spark/v4.2/spark/src/main/scala/org/apache/spark/sql/catalyst/analysis/ViewUtil.scala +++ b/spark/v4.2/spark/src/main/scala/org/apache/spark/sql/catalyst/analysis/ViewUtil.scala @@ -18,16 +18,16 @@ */ package org.apache.spark.sql.catalyst.analysis -import java.util import org.apache.iceberg.catalog.LoadContext -import org.apache.iceberg.catalog.Namespace import org.apache.iceberg.catalog.TableIdentifier import org.apache.iceberg.spark.Spark3Util +import org.apache.iceberg.spark.SparkSQLProperties import org.apache.iceberg.spark.SparkSupportsLoadContext import org.apache.iceberg.spark.source.HasIcebergCatalog import org.apache.iceberg.spark.source.SparkView import org.apache.iceberg.view.{View => IcebergView} import org.apache.spark.sql.connector.catalog.CatalogPlugin +import org.apache.spark.sql.connector.catalog.CatalogV2Util import org.apache.spark.sql.connector.catalog.Identifier import org.apache.spark.sql.connector.catalog.Table import org.apache.spark.sql.connector.catalog.TableCatalog @@ -35,12 +35,11 @@ import org.apache.spark.sql.connector.catalog.V1View import org.apache.spark.sql.connector.catalog.View import org.apache.spark.sql.connector.catalog.ViewCatalog import org.apache.spark.sql.errors.QueryCompilationErrors +import org.apache.spark.sql.internal.SQLConf import scala.jdk.CollectionConverters._ object ViewUtil { - import org.apache.spark.sql.connector.catalog.CatalogV2Implicits._ - val RESERVED_PROPERTIES: Seq[String] = Seq( TableCatalog.PROP_COMMENT, @@ -79,13 +78,7 @@ object ViewUtil { } catch { case _: NoSuchViewException => None } - case viewCatalog: ViewCatalog if catalog.isInstanceOf[HasIcebergCatalog] => - try { - Option(viewCatalog.loadView(ident)).filter(isIcebergView) - } catch { - case _: NoSuchViewException => None - } - case _ => None + case _ => loadView(catalog, ident) } def loadIcebergView(catalog: CatalogPlugin, ident: Identifier): Option[IcebergView] = @@ -122,10 +115,10 @@ object ViewUtil { } val viewIdentifiers = viewChain.map { parts => - TableIdentifier.of(Namespace.of(parts.drop(1).init: _*), parts.last) + TableIdentifier.of(parts.tail: _*) } - new util.ArrayList[TableIdentifier](viewIdentifiers.asJava) + viewIdentifiers.asJava } def qualifyParts( @@ -139,19 +132,11 @@ object ViewUtil { } } - def buildViewChain( - nameParts: Seq[String], - viewCatalogAndNamespace: Seq[String], - existingChain: Seq[Seq[String]], - isCatalog: String => Boolean): Seq[Seq[String]] = { - existingChain :+ qualifyParts(nameParts, viewCatalogAndNamespace, isCatalog) - } - def loadTable( catalog: CatalogPlugin, ident: Identifier, context: LoadContext, - timeTravelSpec: Option[TimeTravelSpec] = None): Table = { + timeTravelSpec: Option[TimeTravelSpec]): Table = { catalog match { case supportsLoadContext: SparkSupportsLoadContext => timeTravelSpec match { @@ -159,15 +144,17 @@ object ViewUtil { case Some(AsOfTimestamp(micros)) => supportsLoadContext.loadTable(ident, micros, context) case None => supportsLoadContext.loadTable(ident, context) } - case _ => - timeTravelSpec match { - case Some(AsOfVersion(version)) => catalog.asTableCatalog.loadTable(ident, version) - case Some(AsOfTimestamp(micros)) => catalog.asTableCatalog.loadTable(ident, micros) - case None => catalog.asTableCatalog.loadTable(ident) - } + case _ => CatalogV2Util.getTable(catalog, ident, timeTravelSpec) } } + def referencedByEnabled: Boolean = { + java.lang.Boolean.parseBoolean( + SQLConf.get.getConfString( + SparkSQLProperties.REFERENCED_BY_ENABLED, + SparkSQLProperties.REFERENCED_BY_ENABLED_DEFAULT.toString)) + } + def isIcebergViewCatalog(catalog: CatalogPlugin): Boolean = { catalog.isInstanceOf[ViewCatalog] && catalog.isInstanceOf[HasIcebergCatalog] } diff --git a/spark/v4.2/spark/src/test/java/org/apache/iceberg/spark/TestSparkSessionCatalog.java b/spark/v4.2/spark/src/test/java/org/apache/iceberg/spark/TestSparkSessionCatalog.java index 61b0eaa15db7..a7373fcdae12 100644 --- a/spark/v4.2/spark/src/test/java/org/apache/iceberg/spark/TestSparkSessionCatalog.java +++ b/spark/v4.2/spark/src/test/java/org/apache/iceberg/spark/TestSparkSessionCatalog.java @@ -32,6 +32,8 @@ import java.nio.file.Path; import java.util.Collections; +import java.util.List; +import org.apache.iceberg.catalog.LoadContext; import org.apache.iceberg.catalog.TableIdentifier; import org.apache.iceberg.exceptions.AlreadyExistsException; import org.apache.spark.sql.catalyst.analysis.NoSuchNamespaceException; @@ -631,6 +633,49 @@ public void createOrReplaceViewUsesExistingSessionCatalogView() verify(icebergViewCatalog, never()).createOrReplaceView(viewIdent, replacement); } + @Test + void loadTableWithReferencedByFailsWhenCatalogCannotPassContext() { + Identifier ident = Identifier.of(new String[] {"default"}, "table"); + TableCatalog icebergCatalog = icebergCatalogWithViews(); + SparkSessionCatalog catalog = catalogWithViews(icebergCatalog, sessionCatalogWithViews()); + LoadContext context = + LoadContext.builder().referencedBy(List.of(TableIdentifier.of("default", "view"))).build(); + + assertThatThrownBy(() -> catalog.loadTable(ident, context)) + .isInstanceOf(UnsupportedOperationException.class) + .hasMessage( + "Cannot load table default.table with referenced-by: %s does not support load context", + icebergCatalog.getClass().getName()); + } + + @Test + void loadTableWithoutReferencedByUsesCatalogWithoutContext() throws NoSuchTableException { + Identifier ident = Identifier.of(new String[] {"default"}, "table"); + TableCatalog icebergCatalog = icebergCatalogWithViews(); + SparkSessionCatalog catalog = catalogWithViews(icebergCatalog, sessionCatalogWithViews()); + + catalog.loadTable(ident, LoadContext.empty()); + + verify(icebergCatalog).loadTable(ident); + } + + @Test + void loadViewWithReferencedByDoesNotCheckExistenceWithoutContext() throws NoSuchViewException { + Identifier ident = Identifier.of(new String[] {"default"}, "view"); + TableCatalog icebergCatalog = + mock( + TableCatalog.class, + withSettings().extraInterfaces(ViewCatalog.class, SparkSupportsLoadContext.class)); + SparkSessionCatalog catalog = catalogWithViews(icebergCatalog, sessionCatalogWithViews()); + LoadContext context = + LoadContext.builder().referencedBy(List.of(TableIdentifier.of("default", "outer"))).build(); + + catalog.loadView(ident, context); + + verify((SparkSupportsLoadContext) icebergCatalog).loadView(ident, context); + verify((ViewCatalog) icebergCatalog, never()).viewExists(any()); + } + private static TableCatalog sessionCatalogWithViews() { return mock( TableCatalog.class, From 3cc4e49b3358acc7108fec0faddcd2c5505a50bb Mon Sep 17 00:00:00 2001 From: Prashant Singh Date: Thu, 22 Oct 2026 12:22:30 -0700 Subject: [PATCH 11/11] Core, Spark 4.2: Bound referenced-by view depth and canonicalize contextual loads Fail referenced-by view expansion past spark.sql.view.maxNestedViewDepth with the same error Spark raises when referenced-by is disabled, so a view cycle no longer overflows the stack during analysis. CachingCatalog now sends contextual loads the same canonical identifier as cached loads, so a table read through a view and read directly reach the delegate under one name when the cache is case-insensitive. Drop two Spark tests covered by more specific ones. Generated-by: Claude Code --- .../org/apache/iceberg/CachingCatalog.java | 2 +- .../iceberg/hadoop/TestCachingCatalog.java | 23 +++++++++++++++ .../analysis/ResolveReferencedByViews.scala | 15 +++++++--- .../extensions/TestReferencedByViewChain.java | 29 ++++++------------- 4 files changed, 44 insertions(+), 25 deletions(-) diff --git a/core/src/main/java/org/apache/iceberg/CachingCatalog.java b/core/src/main/java/org/apache/iceberg/CachingCatalog.java index 7a8492f73da8..cf0c0836d817 100644 --- a/core/src/main/java/org/apache/iceberg/CachingCatalog.java +++ b/core/src/main/java/org/apache/iceberg/CachingCatalog.java @@ -175,7 +175,7 @@ public Table loadTable(TableIdentifier ident, LoadContext context) { return loadTable(ident); } - return catalog.loadTable(ident, context); + return catalog.loadTable(canonicalizeIdentifier(ident), context); } @Override diff --git a/core/src/test/java/org/apache/iceberg/hadoop/TestCachingCatalog.java b/core/src/test/java/org/apache/iceberg/hadoop/TestCachingCatalog.java index 21365e2afd0d..4a8953b5f976 100644 --- a/core/src/test/java/org/apache/iceberg/hadoop/TestCachingCatalog.java +++ b/core/src/test/java/org/apache/iceberg/hadoop/TestCachingCatalog.java @@ -296,6 +296,23 @@ void contextualLoadBypassesTableCache() throws IOException { assertThat(underlyingCatalog.loadContexts().get(0).referencedBy()).containsExactly(viewIdent); } + @Test + void contextualLoadUsesCanonicalIdentifierWhenCaseInsensitive() throws IOException { + ContextTrackingHadoopCatalog underlyingCatalog = contextTrackingHadoopCatalog(); + Catalog catalog = CachingCatalog.wrap(underlyingCatalog, false, EXPIRATION_TTL.toMillis()); + Namespace namespace = Namespace.of("db"); + TableIdentifier tableIdent = TableIdentifier.of(namespace, "tbl"); + catalog.createTable(tableIdent, SCHEMA, SPEC, ImmutableMap.of()); + + LoadContext context = + LoadContext.builder() + .referencedBy(Lists.newArrayList(TableIdentifier.of(namespace, "view"))) + .build(); + catalog.loadTable(TableIdentifier.of(Namespace.of("DB"), "TBL"), context); + + assertThat(underlyingCatalog.loadIdentifiers()).containsExactly(tableIdent); + } + @Test public void testCacheExpirationEagerlyRemovesMetadataTables() throws IOException { TestableCachingCatalog catalog = @@ -472,14 +489,20 @@ private ContextTrackingHadoopCatalog contextTrackingHadoopCatalog() { } private static class ContextTrackingHadoopCatalog extends HadoopCatalog { + private final List loadIdentifiers = Lists.newArrayList(); private final List loadContexts = Lists.newArrayList(); @Override public Table loadTable(TableIdentifier identifier, LoadContext context) { + loadIdentifiers.add(identifier); loadContexts.add(context); return super.loadTable(identifier); } + private List loadIdentifiers() { + return loadIdentifiers; + } + private List loadContexts() { return loadContexts; } diff --git a/spark/v4.2/spark-extensions/src/main/scala/org/apache/spark/sql/catalyst/analysis/ResolveReferencedByViews.scala b/spark/v4.2/spark-extensions/src/main/scala/org/apache/spark/sql/catalyst/analysis/ResolveReferencedByViews.scala index eed5455dabf8..aa3b9dab20dd 100644 --- a/spark/v4.2/spark-extensions/src/main/scala/org/apache/spark/sql/catalyst/analysis/ResolveReferencedByViews.scala +++ b/spark/v4.2/spark-extensions/src/main/scala/org/apache/spark/sql/catalyst/analysis/ResolveReferencedByViews.scala @@ -21,6 +21,7 @@ package org.apache.spark.sql.catalyst.analysis import org.apache.iceberg.catalog.LoadContext import org.apache.iceberg.spark.SparkSupportsLoadContext import org.apache.spark.sql.SparkSession +import org.apache.spark.sql.catalyst.analysis.ResolveReferencedByViews.CheckedForView import org.apache.spark.sql.catalyst.expressions.Alias import org.apache.spark.sql.catalyst.expressions.Cast import org.apache.spark.sql.catalyst.expressions.SubqueryExpression @@ -35,6 +36,7 @@ import org.apache.spark.sql.catalyst.trees.CurrentOrigin import org.apache.spark.sql.catalyst.trees.TreeNodeTag import org.apache.spark.sql.connector.catalog.CatalogManager import org.apache.spark.sql.connector.catalog.CatalogPlugin +import org.apache.spark.sql.connector.catalog.CatalogV2Implicits._ import org.apache.spark.sql.connector.catalog.DelegatingTable import org.apache.spark.sql.connector.catalog.Identifier import org.apache.spark.sql.connector.catalog.LookupCatalog @@ -55,9 +57,6 @@ case class ResolveReferencedByViews(spark: SparkSession) extends Rule[LogicalPlan] with LookupCatalog { - import ResolveReferencedByViews.CheckedForView - import org.apache.spark.sql.connector.catalog.CatalogV2Implicits._ - protected lazy val catalogManager: CatalogManager = spark.sessionState.catalogManager override def apply(plan: LogicalPlan): LogicalPlan = { @@ -132,7 +131,15 @@ case class ResolveReferencedByViews(spark: SparkSession) case _: NoSuchTableException => ViewUtil .loadView(catalog, tableIdent, context) - .map(view => createViewRelation(tableParts, catalog, tableIdent, view, viewChain)) + .map { view => + if (viewChain.size >= conf.maxNestedViewDepth) { + throw QueryCompilationErrors.viewDepthExceedsMaxResolutionDepthError( + tableIdent.toQualifiedNameParts(catalog), + conf.maxNestedViewDepth, + u) + } + createViewRelation(tableParts, catalog, tableIdent, view, viewChain) + } .getOrElse(throw tableOrViewNotFound(u)) } } diff --git a/spark/v4.2/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestReferencedByViewChain.java b/spark/v4.2/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestReferencedByViewChain.java index f595af65fa37..990b577ce2d3 100644 --- a/spark/v4.2/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestReferencedByViewChain.java +++ b/spark/v4.2/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestReferencedByViewChain.java @@ -238,16 +238,6 @@ void referencedByIsDisabledByDefault() { .allSatisfy(captured -> assertThat(captured.referencedBy).isEmpty()); } - @Test - void directTableAccessHasEmptyContext() { - List rows = spark.sql(String.format("SELECT * FROM %s", TABLE_NAME)).collectAsList(); - assertThat(rows).hasSize(5); - - assertThat(ContextTrackingCatalog.CAPTURED) - .isNotEmpty() - .allSatisfy(captured -> assertThat(captured.referencedBy).isEmpty()); - } - @Test void singleViewPassesViewIdentifierInContext() { createView("simple_view", String.format("SELECT id FROM %s", TABLE_NAME)); @@ -516,16 +506,6 @@ void deleteFromViewFailsAsWhenReferencedByIsDisabled() { () -> spark.sql("DELETE FROM simple_view WHERE id = 1")); } - @Test - void tableApiReadOfViewPassesViewIdentifierInContext() { - createView("simple_view", String.format("SELECT id FROM %s", TABLE_NAME)); - ContextTrackingCatalog.clearCaptured(); - - assertThat(spark.table("simple_view").collectAsList()).hasSize(5); - - assertCapturedTableChain(ContextTrackingCatalog.CAPTURED, TABLE_NAME, "simple_view"); - } - @Test void createOrReplaceViewWithCycleFailsAsWhenReferencedByIsDisabled() { createView("view_a", String.format("SELECT id FROM %s", TABLE_NAME)); @@ -535,6 +515,15 @@ void createOrReplaceViewWithCycleFailsAsWhenReferencedByIsDisabled() { () -> spark.sql("CREATE OR REPLACE VIEW view_a AS SELECT id FROM view_b")); } + @Test + void viewCycleFailsAsWhenReferencedByIsDisabled() { + createViewWithoutAnalysis("view_a", "SELECT id FROM view_b"); + createViewWithoutAnalysis("view_b", "SELECT id FROM view_a"); + + assertSameFailureAsReferencedByDisabled( + () -> spark.sql("SELECT * FROM view_a").collectAsList()); + } + @Test void alterViewPropertiesUncachesView() { createView("simple_view", String.format("SELECT id FROM %s", TABLE_NAME));