[SPARK-51219][SQL] Fix `ShowTablesExec.isTempView` to work with non-`V2SessionCatalog` catalogs by ostronaut · Pull Request #49959 · apache/spark

ostronaut · 2025-02-14T15:36:49Z

What changes were proposed in this pull request?

When non buildin catalog is configured (for example, org.apache.spark.sql.delta.catalog.DeltaCatalog) and there is a temp table in catalog, running catalog listTable will fail:

spark.conf.set("spark.sql.catalog.spark_catalog", "org.apache.spark.sql.delta.catalog.DeltaCatalog")
spark.range(0,2).createOrReplaceTempView("abc")
spark.catalog.listTables().show()
// org.apache.spark.sql.catalyst.parser.ParseException: // [PARSE_EMPTY_STATEMENT] Syntax error, unexpected empty statement. SQLSTATE: 42617 (line 1, pos 0)//// == SQL ==//// ^^^

If default V2SessionCatalog catalog is in use, or there are no temp tables, the same command run without issues.

This behavior is due to ShowTablesExec. isTempView method, where only for V2SessionCatalog catalogs dedicated isTempView is executed. This PR fixes that, by using session.sessionState.catalog.isTempView instead.

Why are the changes needed?

To avoid unnecessary fails when a non-buildin v2 catalog is in use.

Does this PR introduce any user-facing change?

No.

How was this patch tested?

New unit tests.

Was this patch authored or co-authored using generative AI tooling?

No.

ostronaut · 2025-02-14T15:40:23Z

Related issue in Delta: delta-io/delta#2610
cc: @cloud-fan

cloud-fan · 2025-02-17T03:00:59Z

shall we only do it if the catalog name is spark_catalog? i.e. CatalogV2Utils.isSessionCatalog

good point! refactored

…alogs

ostronaut · 2025-02-18T12:16:23Z

Hey @cloud-fan. Do you think we can merge this PR?

cloud-fan · 2025-02-18T12:45:39Z

thanks, merging ton master/4.0/3.5!

…V2SessionCatalog` catalogs ### What changes were proposed in this pull request? When non buildin catalog is configured (for example, `org.apache.spark.sql.delta.catalog.DeltaCatalog`) and there is a temp table in catalog, running catalog listTable will fail: ```scala spark.conf.set("spark.sql.catalog.spark_catalog", "org.apache.spark.sql.delta.catalog.DeltaCatalog") spark.range(0,2).createOrReplaceTempView("abc") spark.catalog.listTables().show() // org.apache.spark.sql.catalyst.parser.ParseException: // [PARSE_EMPTY_STATEMENT] Syntax error, unexpected empty statement. SQLSTATE: 42617 (line 1, pos 0) // // == SQL == // // ^^^ ``` If default `V2SessionCatalog` catalog is in use, or there are no temp tables, the same command run without issues. This behavior is due to `ShowTablesExec. isTempView ` method, where only for `V2SessionCatalog` catalogs dedicated `isTempView` is executed. This PR fixes that, by using `session.sessionState.catalog.isTempView` instead. ### Why are the changes needed? To avoid unnecessary fails when a non-buildin v2 catalog is in use. ### Does this PR introduce _any_ user-facing change? No. ### How was this patch tested? New unit tests. ### Was this patch authored or co-authored using generative AI tooling? No. Closes#49959 from ostronaut/features/fix-ShowTablesExec-isTempView. Authored-by: Dima <dimanowq@gmail.com> Signed-off-by: Wenchen Fan <wenchen@databricks.com> (cherry picked from commit aa37f89) Signed-off-by: Wenchen Fan <wenchen@databricks.com>

…V2SessionCatalog` catalogs When non buildin catalog is configured (for example, `org.apache.spark.sql.delta.catalog.DeltaCatalog`) and there is a temp table in catalog, running catalog listTable will fail: ```scala spark.conf.set("spark.sql.catalog.spark_catalog", "org.apache.spark.sql.delta.catalog.DeltaCatalog") spark.range(0,2).createOrReplaceTempView("abc") spark.catalog.listTables().show() // org.apache.spark.sql.catalyst.parser.ParseException: // [PARSE_EMPTY_STATEMENT] Syntax error, unexpected empty statement. SQLSTATE: 42617 (line 1, pos 0) // // == SQL == // // ^^^ ``` If default `V2SessionCatalog` catalog is in use, or there are no temp tables, the same command run without issues. This behavior is due to `ShowTablesExec. isTempView ` method, where only for `V2SessionCatalog` catalogs dedicated `isTempView` is executed. This PR fixes that, by using `session.sessionState.catalog.isTempView` instead. To avoid unnecessary fails when a non-buildin v2 catalog is in use. No. New unit tests. No. Closes#49959 from ostronaut/features/fix-ShowTablesExec-isTempView. Authored-by: Dima <dimanowq@gmail.com> Signed-off-by: Wenchen Fan <wenchen@databricks.com> (cherry picked from commit aa37f89) Signed-off-by: Wenchen Fan <wenchen@databricks.com>

melin · 2025-02-19T12:00:36Z

scala 2.12

cloud-fan · 2025-02-19T12:36:23Z

+ private def isTempView(ident: Identifier, catalog: TableCatalog): Boolean = {
+if (CatalogV2Util.isSessionCatalog(catalog)) {
+session.sessionState.catalog
+ .isTempView((ident.namespace() :+ ident.name()).toImmutableArraySeq)


@ostronaut can we just call toArray here?

I've took it from existing implementation in default V2 catalog, let me replace it to toArray?

let's try and see if scala 2.12 is happy with it

You can open a PR against branch 3.5, which use scala 2.12

#50008. Lets see if default seq will be enough.

…V2SessionCatalog` catalogs ### What changes were proposed in this pull request? When non buildin catalog is configured (for example, `org.apache.spark.sql.delta.catalog.DeltaCatalog`) and there is a temp table in catalog, running catalog listTable will fail: ```scala spark.conf.set("spark.sql.catalog.spark_catalog", "org.apache.spark.sql.delta.catalog.DeltaCatalog") spark.range(0,2).createOrReplaceTempView("abc") spark.catalog.listTables().show() // org.apache.spark.sql.catalyst.parser.ParseException: // [PARSE_EMPTY_STATEMENT] Syntax error, unexpected empty statement. SQLSTATE: 42617 (line 1, pos 0) // // == SQL == // // ^^^ ``` If default `V2SessionCatalog` catalog is in use, or there are no temp tables, the same command run without issues. This behavior is due to `ShowTablesExec. isTempView ` method, where only for `V2SessionCatalog` catalogs dedicated `isTempView` is executed. This PR fixes that, by using `session.sessionState.catalog.isTempView` instead. ### Why are the changes needed? To avoid unnecessary fails when a non-buildin v2 catalog is in use. ### Does this PR introduce _any_ user-facing change? No. ### How was this patch tested? New unit tests. ### Was this patch authored or co-authored using generative AI tooling? No. Closesapache#49959 from ostronaut/features/fix-ShowTablesExec-isTempView. Authored-by: Dima <dimanowq@gmail.com> Signed-off-by: Wenchen Fan <wenchen@databricks.com> (cherry picked from commit b300b84) Signed-off-by: Wenchen Fan <wenchen@databricks.com>

github-actionsBot added the SQL label Feb 14, 2025

ostronaut mentioned this pull request Feb 14, 2025
[BUG][SPARK] listTables() fails after createOrReplaceTempView('abc') called with PARSE_SYNTAX_ERROR delta-io/delta#2610
Closed
3 tasks

cloud-fan reviewed Feb 17, 2025
View reviewed changes

Fix issue with ShowTablesExec.isTempView for non-V2SessionCatalog cat…
c1014b1
…alogs

ostronaut force-pushed the features/fix-ShowTablesExec-isTempView branch from 1f95347 to c1014b1Compare February 17, 2025 08:58

cloud-fan approved these changes Feb 17, 2025
View reviewed changes

cloud-fan closed this in aa37f89Feb 18, 2025

ostronaut deleted the features/fix-ShowTablesExec-isTempView branch February 18, 2025 14:05

cloud-fan reviewed Feb 19, 2025
View reviewed changes

cloud-fan mentioned this pull request May 19, 2025
[SPARK-45854][SQL] listTables should not fail with a temp view and DelegatingCatalogExtension #47753
Closed

Navigation Menu

Uh oh!

[SPARK-51219][SQL] Fix `ShowTablesExec.isTempView` to work with non-`V2SessionCatalog` catalogs - #49959

[SPARK-51219][SQL] Fix `ShowTablesExec.isTempView` to work with non-`V2SessionCatalog` catalogs#49959
ostronaut wants to merge 1 commit into
apache:masterfrom
ostronaut:features/fix-ShowTablesExec-isTempView

ostronaut commented Feb 14, 2025

Uh oh!

ostronaut commented Feb 14, 2025

Uh oh!

cloud-fanFeb 17, 2025

Uh oh!

ostronautFeb 17, 2025

Uh oh!

ostronaut commented Feb 18, 2025

Uh oh!

cloud-fan commented Feb 18, 2025

Uh oh!

melin commented Feb 19, 2025

Uh oh!

cloud-fanFeb 19, 2025

Uh oh!

ostronautFeb 19, 2025

Uh oh!

cloud-fanFeb 19, 2025

Uh oh!

cloud-fanFeb 19, 2025

Uh oh!

ostronautFeb 19, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

ostronaut commented Feb 14, 2025

What changes were proposed in this pull request?

Why are the changes needed?

Does this PR introduce any user-facing change?

How was this patch tested?

Was this patch authored or co-authored using generative AI tooling?

Uh oh!

ostronaut commented Feb 14, 2025

Uh oh!

cloud-fanFeb 17, 2025

Choose a reason for hiding this comment

Uh oh!

ostronautFeb 17, 2025

Choose a reason for hiding this comment

Uh oh!

ostronaut commented Feb 18, 2025

Uh oh!

cloud-fan commented Feb 18, 2025

Uh oh!

melin commented Feb 19, 2025

Uh oh!

cloud-fanFeb 19, 2025

Choose a reason for hiding this comment

Uh oh!

ostronautFeb 19, 2025

Choose a reason for hiding this comment

Uh oh!

cloud-fanFeb 19, 2025

Choose a reason for hiding this comment

Uh oh!

cloud-fanFeb 19, 2025

Choose a reason for hiding this comment

Uh oh!

ostronautFeb 19, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants