srielau commented on code in PR #55866:
URL: https://github.com/apache/spark/pull/55866#discussion_r3242931940
##########
sql/core/src/test/scala/org/apache/spark/sql/execution/SQLViewSuite.scala:
##########
@@ -1453,6 +1453,106 @@ abstract class SQLViewSuite extends QueryTest {
}
}
+ test("stored view path is ignored when PATH is disabled at read time") {
+ // A view created with PATH enabled persists two things in metadata: the
frozen
+ // resolution path AND the creator session's current catalog+namespace at
CREATE
+ // VIEW time (the view's `viewCatalogAndNamespace` property). If the
reader's
+ // session has `spark.sql.path.enabled=false`, the pinned entries are
intentionally
+ // dropped (`CatalogManager.resolutionPathEntriesForAnalysis`); the view
body's
+ // unqualified references fall back to that captured catalog+namespace,
which is
+ // the creator's USE state at CREATE time -- NOT the schema the view
physically
+ // lives in (the two coincide below only because the test runs
+ // `USE spark_catalog.compat_view_b` before CREATE VIEW). Verify both
directions:
+ // - fully-qualified bodies keep working (qualification doesn't depend
on PATH),
+ // - unqualified bodies that relied on the frozen path now resolve via
the
+ // captured viewCatalogAndNamespace.
+ withDatabase("compat_view_a", "compat_view_b") {
+ sql("CREATE DATABASE compat_view_a")
+ sql("CREATE DATABASE compat_view_b")
+ withTable(
+ "compat_view_a.compat_t",
+ "compat_view_b.compat_t") {
+ sql("CREATE TABLE compat_view_a.compat_t USING parquet AS SELECT 1 AS
id")
+ sql("CREATE TABLE compat_view_b.compat_t USING parquet AS SELECT 2 AS
id")
+ withView(
+ "compat_view_b.v_unq_path",
+ "compat_view_b.v_fq_path") {
+ // Create both views with USE compat_view_b in effect so the stored
+ // viewCatalogAndNamespace points at compat_view_b, then SET PATH=a
so the
+ // frozen path pins compat_view_a.
+ withSQLConf(PATH_ENABLED.key -> "true") {
+ try {
+ sql("USE spark_catalog.compat_view_b")
+ sql("SET PATH = spark_catalog.compat_view_a, system.builtin")
+ sql(
+ """
+ |CREATE VIEW compat_view_b.v_unq_path AS
+ |SELECT id FROM compat_t
+ |""".stripMargin)
+ sql(
+ """
+ |CREATE VIEW compat_view_b.v_fq_path AS
+ |SELECT id FROM spark_catalog.compat_view_a.compat_t
+ |""".stripMargin)
+ } finally {
+ sql("SET PATH = DEFAULT_PATH")
+ sql("USE spark_catalog.default")
+ }
+ }
+
+ // Now read with PATH disabled. The fully-qualified view body is
independent of
+ // PATH and must keep returning rows from compat_view_a. The
unqualified-body view
+ // drops its frozen-path pin and falls back to
viewCatalogAndNamespace
+ // (compat_view_b), so unqualified `compat_t` resolves to
compat_view_b.compat_t.
+ withSQLConf(PATH_ENABLED.key -> "false") {
+ checkAnswer(sql("SELECT id FROM compat_view_b.v_fq_path"), Row(1))
+ checkAnswer(sql("SELECT id FROM compat_view_b.v_unq_path"), Row(2))
+ }
+ }
+ }
+ }
+ }
+
+ test("stored view path with no fallback target fails clearly when PATH is
off") {
Review Comment:
Done in 542eee8.
##########
sql/catalyst/src/test/scala/org/apache/spark/sql/connector/catalog/CatalogManagerSuite.scala:
##########
@@ -150,6 +152,115 @@ class CatalogManagerSuite extends SparkFunSuite with
SQLHelper {
assert(CatalogManager.deserializePathEntries(payload).isEmpty,
s"payload=$payload")
}
}
+
+ test("serializePathEntries round-trips through deserialize for typical
inputs") {
+ val cases = Seq(
+ Seq(Seq("spark_catalog", "default"), Seq("system", "builtin")),
+ Seq(Seq("system", "session")),
+ Seq.empty[Seq[String]])
+ cases.foreach { entries =>
+ val payload = CatalogManager.serializePathEntries(entries)
+ val parsed = CatalogManager.deserializePathEntries(payload)
+ .getOrElse(fail(s"Expected payload to round-trip: $payload"))
+ assert(parsed === entries, s"Round-trip mismatch for $entries; got
$parsed")
+ }
+ }
+
+ test("serializePathEntries round-trips multi-level and quoted identifiers") {
+ val entries = Seq(
+ Seq("cat", "ns1", "ns2"),
+ Seq("spark_catalog", "sch.with.dots"),
+ Seq("spark_catalog", "schema with spaces"))
+ val payload = CatalogManager.serializePathEntries(entries)
+ val parsed = CatalogManager.deserializePathEntries(payload)
+ .getOrElse(fail(s"Expected payload to round-trip: $payload"))
+ assert(parsed === entries)
+ }
+
+ test("deserializePathEntriesOrFail raises a clear AnalysisException for bad
payloads") {
+ val e = intercept[AnalysisException] {
+ CatalogManager.deserializePathEntriesOrFail(
+ storedPathStr = "{bad-json",
+ objectType = "view",
+ objectName = "default.v_broken")
+ }
+ assert(e.getMessage.contains("Invalid stored SQL path metadata for view"))
+ assert(e.getMessage.contains("default.v_broken"))
+ }
+
+ //
---------------------------------------------------------------------------
+ // Direct unit tests for [[PathElement.validateNoStaticDuplicates]]. The
end-to-end
Review Comment:
Choosing the "alternatively, just update the PR description" option -- the
description has been edited to accurately say "additional `CatalogManagerSuite`
cases ... covering `PathElement.validateNoStaticDuplicates`". `PathElement` is
a small, single-purpose helper whose only consumer is `CatalogManager`, so
keeping the seven pure-data tests in the sibling suite (where the existing
`serializePathEntries` / `deserializePathEntries` cases already live) felt more
natural than spinning up a dedicated 7-test file. Happy to re-extract if you
prefer the strict source-mirror pattern -- the tests are self-contained and
easy to move.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
