Skip to content

SHOW VIEWS failed with AssertionError#10442

Merged
szehon-ho merged 7 commits into
apache:mainfrom
huaxingao:showView
Jun 5, 2024
Merged

SHOW VIEWS failed with AssertionError#10442
szehon-ho merged 7 commits into
apache:mainfrom
huaxingao:showView

Conversation

@huaxingao
Copy link
Copy Markdown
Contributor

sql("CREATE VIEW %s AS SELECT 1 AS id", "test");
sql("SHOW VIEWS")

assertion failed
java.lang.AssertionError: assertion failed
	at scala.Predef$.assert(Predef.scala:208)
	at org.apache.spark.sql.connector.catalog.LookupCatalog$CatalogAndNamespace$.unapply(LookupCatalog.scala:84)
	at org.apache.spark.sql.catalyst.analysis.RewriteViewCommands$$anonfun$apply$1.applyOrElse(RewriteViewCommands.scala:74)
	at org.apache.spark.sql.catalyst.analysis.RewriteViewCommands$$anonfun$apply$1.applyOrElse(RewriteViewCommands.scala:51)

@github-actions github-actions Bot added the spark label Jun 4, 2024
@huaxingao
Copy link
Copy Markdown
Contributor Author

cc @nastra @szehon-ho @flyrain

@szehon-ho
Copy link
Copy Markdown
Member

@nastra fyi

@szehon-ho
Copy link
Copy Markdown
Member

Ill commit this tomorrow if no further comments

@nastra
Copy link
Copy Markdown
Contributor

nastra commented Jun 5, 2024

I'll take a look later today

sql("DROP VIEW IF EXISTS %s", "test");
sql("CREATE VIEW %s AS SELECT 1 AS id", "test");
Object[] expected = row("default", "test", false);
assertThat(sql("SHOW VIEWS")).contains(expected);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

expected can be inlined here:

Suggested change
assertThat(sql("SHOW VIEWS")).contains(expected);
assertThat(sql("SHOW VIEWS")).contains(row("default", "test", false));

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed. Thanks for the suggestion!

sql("DROP VIEW IF EXISTS %s", "test");
sql("CREATE VIEW %s AS SELECT 1 AS id", "test");
Object[] expected = row("default", "test", false);
assertThat(sql("SHOW VIEWS")).contains(expected);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
assertThat(sql("SHOW VIEWS")).contains(expected);
assertThat(sql("SHOW VIEWS")).contains(row("default", "test", false));

Copy link
Copy Markdown
Contributor

@nastra nastra left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM with one suggestion, thanks for catching this

@nastra
Copy link
Copy Markdown
Contributor

nastra commented Jun 5, 2024

@huaxingao could you also please add the below diff to TestViews.showViews()

--- a/spark/v3.5/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestViews.java
+++ b/spark/v3.5/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestViews.java
@@ -1462,6 +1462,11 @@ public class TestViews extends ExtensionsTestBase {
         .contains(
             // spark stores temp views case-insensitive by default
             row("global_temp", "globalviewforlisting", true), tempView);
+
+    sql("USE spark_catalog");
+    assertThat(sql("SHOW VIEWS")).contains(tempView);
+
+    assertThat(sql("SHOW VIEWS IN default")).contains(tempView);
   }

@github-actions github-actions Bot added the build label Jun 5, 2024
@huaxingao
Copy link
Copy Markdown
Contributor Author

could you also please add the below diff to TestViews.showViews()

Added. Thanks for the suggestion! @nastra

Comment thread gradle.properties Outdated
Comment thread gradle.properties Outdated
@szehon-ho szehon-ho merged commit 59e9377 into apache:main Jun 5, 2024
@szehon-ho
Copy link
Copy Markdown
Member

Thanks @huaxingao for the pr, @nastra @viirya for reviews

@huaxingao
Copy link
Copy Markdown
Contributor Author

Thanks, everyone!

@huaxingao huaxingao deleted the showView branch June 5, 2024 17:34
@nastra nastra mentioned this pull request Jun 6, 2024
jasonf20 pushed a commit to jasonf20/iceberg that referenced this pull request Aug 4, 2024
szehon-ho pushed a commit to szehon-ho/iceberg that referenced this pull request Sep 16, 2024
sasankpagolu pushed a commit to sasankpagolu/iceberg that referenced this pull request Oct 27, 2024
sasankpagolu pushed a commit to sasankpagolu/iceberg that referenced this pull request Oct 27, 2024
zachdisc pushed a commit to zachdisc/iceberg that referenced this pull request Dec 23, 2024
zhongyujiang pushed a commit to zhongyujiang/iceberg that referenced this pull request Apr 16, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants