You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by "Ppei-Wang (via GitHub)" <gi...@apache.org> on 2024/12/11 13:49:53 UTC
[PR] Spark3.4,3.5: In describe extended view command: fix wrong view catal… [iceberg]
Ppei-Wang opened a new pull request, #11751:
URL: https://github.com/apache/iceberg/pull/11751
Spark3.4,3.5: In describe extended view command: fix wrong view catalog and namespace, and add view text.
Before this fix,the view catalog and namespace in describe extended view command shows the current catalog and namespace when it was create.Not the catalog and namespace of the view itself.
Add view text is Compatible with other engines' usage habits, such as the widely used hive view.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spark3.4,3.5: In describe extended view command: fix wrong view catal… [iceberg]
Posted by "nastra (via GitHub)" <gi...@apache.org>.
nastra commented on code in PR #11751:
URL: https://github.com/apache/iceberg/pull/11751#discussion_r1881661391
##########
spark/v3.4/spark-extensions/src/main/scala/org/apache/spark/sql/execution/datasources/v2/DescribeV2ViewExec.scala:
##########
@@ -55,18 +55,20 @@ case class DescribeV2ViewExec(
private def describeExtended: Seq[InternalRow] = {
val outputColumns = view.queryColumnNames.mkString("[", ", ", "]")
val properties: Map[String, String] = view.properties.asScala.toMap -- ViewCatalog.RESERVED_PROPERTIES.asScala
- val viewCatalogAndNamespace: Seq[String] = view.currentCatalog +: view.currentNamespace.toSeq
+ val viewCatalogAndNamespace: Seq[String] = view.name.split("\\.").take(2)
val viewProperties = properties.toSeq.sortBy(_._1).map {
case (key, value) =>
s"'${escapeSingleQuotedString(key)}' = '${escapeSingleQuotedString(value)}'"
}.mkString("[", ", ", "]")
+ val viewText = view.query
toCatalystRow("# Detailed View Information", "", "") ::
toCatalystRow("Comment", view.properties.getOrDefault(ViewCatalog.PROP_COMMENT, ""), "") ::
toCatalystRow("View Catalog and Namespace", viewCatalogAndNamespace.quoted, "") ::
toCatalystRow("View Query Output Columns", outputColumns, "") ::
toCatalystRow("View Properties", viewProperties, "") ::
+ toCatalystRow("View Text", viewText, "") ::
Review Comment:
we omitted the view text on purpose. See also https://github.com/apache/iceberg/pull/9513#discussion_r1471671329 for some context
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spark3.4,3.5: In describe extended view command: fix wrong view catal… [iceberg]
Posted by "Ppei-Wang (via GitHub)" <gi...@apache.org>.
Ppei-Wang commented on code in PR #11751:
URL: https://github.com/apache/iceberg/pull/11751#discussion_r1882185303
##########
spark/v3.5/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestViews.java:
##########
@@ -1414,7 +1414,42 @@ public void describeExtendedView() {
String.format(
"['format-version' = '1', 'location' = '%s', 'provider' = 'iceberg']",
location),
- ""));
+ ""),
+ row("View Text", sql, ""));
+ }
+
+ @TestTemplate
+ public void describeExtendedViewWithoutCurrentNamespace() {
Review Comment:
@nastra but at org/apache/spark/sql/connector/catalog/View.java is defined that
```
public interface View {
...
/**
* The current catalog when the view is created.
*/
String currentCatalog();
/**
* The current namespace when the view is created.
*/
String[] currentNamespace();
...
}
```
---------------------------
i think above changes have a bit conflicting？
and my suggestion is that added catalog and namespace of view itself at spark/v3.5/spark/src/main/java/org/apache/iceberg/spark/source/SparkView.java. Also change initialization accordingly in create view comand and desc extended view command. what about this idea？
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spark3.4,3.5: In describe extended view command: fix wrong view catal… [iceberg]
Posted by "nastra (via GitHub)" <gi...@apache.org>.
nastra commented on code in PR #11751:
URL: https://github.com/apache/iceberg/pull/11751#discussion_r1888529359
##########
spark/v3.4/spark-extensions/src/main/scala/org/apache/spark/sql/execution/datasources/v2/DescribeV2ViewExec.scala:
##########
@@ -55,13 +55,14 @@ case class DescribeV2ViewExec(
private def describeExtended: Seq[InternalRow] = {
val outputColumns = view.queryColumnNames.mkString("[", ", ", "]")
val properties: Map[String, String] = view.properties.asScala.toMap -- ViewCatalog.RESERVED_PROPERTIES.asScala
- val viewCatalogAndNamespace: Seq[String] = view.currentCatalog +: view.currentNamespace.toSeq
+ val viewCatalogAndNamespace: Seq[String] = view.name.split("\\.").take(2)
val viewProperties = properties.toSeq.sortBy(_._1).map {
case (key, value) =>
s"'${escapeSingleQuotedString(key)}' = '${escapeSingleQuotedString(value)}'"
}.mkString("[", ", ", "]")
+ //omiited view text, because it can show in SHOW CREATE.
Review Comment:
```suggestion
// omitting view text here because it is shown as part of SHOW CREATE TABLE and can result in weird formatting in the DESCRIBE output
```
##########
gradle.properties:
##########
@@ -20,7 +20,7 @@ systemProp.defaultFlinkVersions=1.20
systemProp.knownFlinkVersions=1.18,1.19,1.20
systemProp.defaultHiveVersions=2
systemProp.knownHiveVersions=2,3
-systemProp.defaultSparkVersions=3.5
+systemProp.defaultSparkVersions=3.5,3.4
Review Comment:
```suggestion
systemProp.defaultSparkVersions=3.5
```
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spark3.4,3.5: In describe extended view command: fix wrong view catal… [iceberg]
Posted by "ebyhr (via GitHub)" <gi...@apache.org>.
ebyhr commented on code in PR #11751:
URL: https://github.com/apache/iceberg/pull/11751#discussion_r1883190569
##########
spark/v3.4/spark-extensions/src/main/scala/org/apache/spark/sql/execution/datasources/v2/DescribeV2ViewExec.scala:
##########
@@ -55,18 +55,20 @@ case class DescribeV2ViewExec(
private def describeExtended: Seq[InternalRow] = {
val outputColumns = view.queryColumnNames.mkString("[", ", ", "]")
val properties: Map[String, String] = view.properties.asScala.toMap -- ViewCatalog.RESERVED_PROPERTIES.asScala
- val viewCatalogAndNamespace: Seq[String] = view.currentCatalog +: view.currentNamespace.toSeq
+ val viewCatalogAndNamespace: Seq[String] = view.name.split("\\.").take(2)
val viewProperties = properties.toSeq.sortBy(_._1).map {
case (key, value) =>
s"'${escapeSingleQuotedString(key)}' = '${escapeSingleQuotedString(value)}'"
}.mkString("[", ", ", "]")
+ val viewText = view.query
toCatalystRow("# Detailed View Information", "", "") ::
toCatalystRow("Comment", view.properties.getOrDefault(ViewCatalog.PROP_COMMENT, ""), "") ::
toCatalystRow("View Catalog and Namespace", viewCatalogAndNamespace.quoted, "") ::
toCatalystRow("View Query Output Columns", outputColumns, "") ::
toCatalystRow("View Properties", viewProperties, "") ::
+ toCatalystRow("View Text", viewText, "") ::
Review Comment:
I think it worth leaving a code comment so other developers can avoid adding the view text in the future.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spark3.4,3.5: In describe extended view command: fix wrong view catal… [iceberg]
Posted by "Ppei-Wang (via GitHub)" <gi...@apache.org>.
Ppei-Wang commented on code in PR #11751:
URL: https://github.com/apache/iceberg/pull/11751#discussion_r1888470576
##########
spark/v3.5/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestViews.java:
##########
@@ -1414,7 +1414,42 @@ public void describeExtendedView() {
String.format(
"['format-version' = '1', 'location' = '%s', 'provider' = 'iceberg']",
location),
- ""));
+ ""),
+ row("View Text", sql, ""));
+ }
+
+ @TestTemplate
+ public void describeExtendedViewWithoutCurrentNamespace() {
Review Comment:
@nastra thanks for your suggestions, i added above tests. And do you think it is necessary to modify spark/v3.5/spark/src/main/java/org/apache/iceberg/spark/source/SparkView.java, create view comand and desc extended view command?
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spark3.4,3.5: In describe extended view command: fix wrong view catal… [iceberg]
Posted by "nastra (via GitHub)" <gi...@apache.org>.
nastra merged PR #11751:
URL: https://github.com/apache/iceberg/pull/11751
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spark3.4,3.5: In describe extended view command: fix wrong view catal… [iceberg]
Posted by "Ppei-Wang (via GitHub)" <gi...@apache.org>.
Ppei-Wang commented on code in PR #11751:
URL: https://github.com/apache/iceberg/pull/11751#discussion_r1889529327
##########
spark/v3.5/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestViews.java:
##########
@@ -1414,7 +1414,42 @@ public void describeExtendedView() {
String.format(
"['format-version' = '1', 'location' = '%s', 'provider' = 'iceberg']",
location),
- ""));
+ ""),
+ row("View Text", sql, ""));
+ }
+
+ @TestTemplate
+ public void describeExtendedViewWithoutCurrentNamespace() {
Review Comment:
@nastra ok，I got it. sorry, a bit influenced by the previous comment. There is no problem following my original idea.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spark3.4,3.5: In describe extended view command: fix wrong view catal… [iceberg]
Posted by "nastra (via GitHub)" <gi...@apache.org>.
nastra commented on code in PR #11751:
URL: https://github.com/apache/iceberg/pull/11751#discussion_r1888531567
##########
spark/v3.5/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestViews.java:
##########
@@ -1414,7 +1414,42 @@ public void describeExtendedView() {
String.format(
"['format-version' = '1', 'location' = '%s', 'provider' = 'iceberg']",
location),
- ""));
+ ""),
+ row("View Text", sql, ""));
+ }
+
+ @TestTemplate
+ public void describeExtendedViewWithoutCurrentNamespace() {
Review Comment:
> @nastra thanks for your suggestions, i added above tests. And do you think it is necessary to modify spark/v3.5/spark/src/main/java/org/apache/iceberg/spark/source/SparkView.java, create view comand and desc extended view command?
@Ppei-Wang what exactly did you want to modify in the other places? All the other places should be correct (because they refer to the default catalog that was set when the view was created)
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spark3.4,3.5: In describe extended view command: fix wrong view catal… [iceberg]
Posted by "nastra (via GitHub)" <gi...@apache.org>.
nastra commented on code in PR #11751:
URL: https://github.com/apache/iceberg/pull/11751#discussion_r1881747660
##########
spark/v3.5/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestViews.java:
##########
@@ -1414,7 +1414,42 @@ public void describeExtendedView() {
String.format(
"['format-version' = '1', 'location' = '%s', 'provider' = 'iceberg']",
location),
- ""));
+ ""),
+ row("View Text", sql, ""));
+ }
+
+ @TestTemplate
+ public void describeExtendedViewWithoutCurrentNamespace() {
Review Comment:
I took a look at the issue and I think the issue is less with `DESCRIBE` itself, but rather the namespace being used during view creation:
These 2 tests should be all we need
```
@TestTemplate
public void createViewInDefaultNamespace() {
String viewName = viewName("createViewInDefaultNamespace");
String sql = String.format("SELECT id, data FROM %s WHERE id <= 3", tableName);
sql("CREATE VIEW %s (id, data) AS %s", viewName, sql);
View view = viewCatalog().loadView(TableIdentifier.of(NAMESPACE, viewName));
assertThat(view.currentVersion().defaultCatalog()).isNull();
assertThat(view.currentVersion().defaultNamespace()).isEqualTo(NAMESPACE);
}
@TestTemplate
public void createViewWithoutCurrentNamespace() {
String viewName = viewName("createViewWithoutCurrentNamespace");
Namespace namespace = Namespace.of("test_namespace");
String sql = String.format("SELECT id, data FROM %s WHERE id <= 3", tableName);
sql("CREATE NAMESPACE IF NOT EXISTS %s", namespace);
sql("CREATE VIEW %s.%s (id, data) AS %s", namespace, viewName, sql);
View view = viewCatalog().loadView(TableIdentifier.of(namespace, viewName));
assertThat(view.currentVersion().defaultCatalog()).isNull();
assertThat(view.currentVersion().defaultNamespace()).isEqualTo(namespace);
}
```
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spark3.4,3.5: In describe extended view command: fix wrong view catal… [iceberg]
Posted by "Ppei-Wang (via GitHub)" <gi...@apache.org>.
Ppei-Wang commented on code in PR #11751:
URL: https://github.com/apache/iceberg/pull/11751#discussion_r1882185303
##########
spark/v3.5/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestViews.java:
##########
@@ -1414,7 +1414,42 @@ public void describeExtendedView() {
String.format(
"['format-version' = '1', 'location' = '%s', 'provider' = 'iceberg']",
location),
- ""));
+ ""),
+ row("View Text", sql, ""));
+ }
+
+ @TestTemplate
+ public void describeExtendedViewWithoutCurrentNamespace() {
Review Comment:
but at org/apache/spark/sql/connector/catalog/View.java is defined that
```
public interface View {
...
/**
* The current catalog when the view is created.
*/
String currentCatalog();
/**
* The current namespace when the view is created.
*/
String[] currentNamespace();
...
}
```
---------------------------
i think above changes have a bit conflicting？
and my suggestion is that added catalog and namespace of view itself at spark/v3.5/spark/src/main/java/org/apache/iceberg/spark/source/SparkView.java. Also change initialization accordingly in create view comand and desc extended view command. like this?
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spark3.4,3.5: In describe extended view command: fix wrong view catal… [iceberg]
Posted by "nastra (via GitHub)" <gi...@apache.org>.
nastra commented on code in PR #11751:
URL: https://github.com/apache/iceberg/pull/11751#discussion_r1881747660
##########
spark/v3.5/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestViews.java:
##########
@@ -1414,7 +1414,42 @@ public void describeExtendedView() {
String.format(
"['format-version' = '1', 'location' = '%s', 'provider' = 'iceberg']",
location),
- ""));
+ ""),
+ row("View Text", sql, ""));
+ }
+
+ @TestTemplate
+ public void describeExtendedViewWithoutCurrentNamespace() {
Review Comment:
I took a look at the issue and I think the issue is less with `DESCRIBE` itself, but rather the namespace being used during view creation:
These 2 tests should be all we need
```
@TestTemplate
public void createViewInDefaultNamespace() {
String viewName = viewName("createViewInDefaultNamespace");
String sql = String.format("SELECT id, data FROM %s WHERE id <= 3", tableName);
sql("CREATE VIEW %s (id, data) AS %s", viewName, sql);
View view = viewCatalog().loadView(TableIdentifier.of(NAMESPACE, viewName));
assertThat(view.currentVersion().defaultCatalog()).isNull();
assertThat(view.currentVersion().defaultNamespace()).isEqualTo(NAMESPACE);
}
@TestTemplate
public void createViewWithoutCurrentNamespace() {
String viewName = viewName("createViewWithoutCurrentNamespace");
Namespace namespace = Namespace.of("test_namespace");
String sql = String.format("SELECT id, data FROM %s WHERE id <= 3", tableName);
sql("CREATE NAMESPACE IF NOT EXISTS %s", namespace);
sql("CREATE VIEW %s.%s (id, data) AS %s", namespace, viewName, sql);
View view = viewCatalog().loadView(TableIdentifier.of(namespace, viewName));
assertThat(view.currentVersion().defaultCatalog()).isNull();
assertThat(view.currentVersion().defaultNamespace()).isEqualTo(namespace);
}
```
I still need to double-check some things, but most likely the fix we need is this:
```
--- a/spark/v3.5/spark-extensions/src/main/scala/org/apache/spark/sql/execution/datasources/v2/CreateV2ViewExec.scala
+++ b/spark/v3.5/spark-extensions/src/main/scala/org/apache/spark/sql/execution/datasources/v2/CreateV2ViewExec.scala
@@ -48,7 +48,7 @@ case class CreateV2ViewExec(
override protected def run(): Seq[InternalRow] = {
val currentCatalogName = session.sessionState.catalogManager.currentCatalog.name
val currentCatalog = if (!catalog.name().equals(currentCatalogName)) currentCatalogName else null
- val currentNamespace = session.sessionState.catalogManager.currentNamespace
+ val currentNamespace = ident.namespace()
```
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spark3.4,3.5: In describe extended view command: fix wrong view catal… [iceberg]
Posted by "Ppei-Wang (via GitHub)" <gi...@apache.org>.
Ppei-Wang commented on PR #11751:
URL: https://github.com/apache/iceberg/pull/11751#issuecomment-2536045776
@rdblue hello，Could you please help approve workflow runs and assign a Reviewer?
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spark3.4,3.5: In describe extended view command: fix wrong view catal… [iceberg]
Posted by "Ppei-Wang (via GitHub)" <gi...@apache.org>.
Ppei-Wang commented on code in PR #11751:
URL: https://github.com/apache/iceberg/pull/11751#discussion_r1889532977
##########
gradle.properties:
##########
@@ -20,7 +20,7 @@ systemProp.defaultFlinkVersions=1.20
systemProp.knownFlinkVersions=1.18,1.19,1.20
systemProp.defaultHiveVersions=2
systemProp.knownHiveVersions=2,3
-systemProp.defaultSparkVersions=3.5
+systemProp.defaultSparkVersions=3.5,3.4
Review Comment:
@nastra thanks for your suggestions, modified.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spark3.4,3.5: In describe extended view command: fix wrong view catal… [iceberg]
Posted by "Ppei-Wang (via GitHub)" <gi...@apache.org>.
Ppei-Wang commented on code in PR #11751:
URL: https://github.com/apache/iceberg/pull/11751#discussion_r1888473053
##########
spark/v3.4/spark-extensions/src/main/scala/org/apache/spark/sql/execution/datasources/v2/DescribeV2ViewExec.scala:
##########
@@ -55,18 +55,20 @@ case class DescribeV2ViewExec(
private def describeExtended: Seq[InternalRow] = {
val outputColumns = view.queryColumnNames.mkString("[", ", ", "]")
val properties: Map[String, String] = view.properties.asScala.toMap -- ViewCatalog.RESERVED_PROPERTIES.asScala
- val viewCatalogAndNamespace: Seq[String] = view.currentCatalog +: view.currentNamespace.toSeq
+ val viewCatalogAndNamespace: Seq[String] = view.name.split("\\.").take(2)
val viewProperties = properties.toSeq.sortBy(_._1).map {
case (key, value) =>
s"'${escapeSingleQuotedString(key)}' = '${escapeSingleQuotedString(value)}'"
}.mkString("[", ", ", "]")
+ val viewText = view.query
toCatalystRow("# Detailed View Information", "", "") ::
toCatalystRow("Comment", view.properties.getOrDefault(ViewCatalog.PROP_COMMENT, ""), "") ::
toCatalystRow("View Catalog and Namespace", viewCatalogAndNamespace.quoted, "") ::
toCatalystRow("View Query Output Columns", outputColumns, "") ::
toCatalystRow("View Properties", viewProperties, "") ::
+ toCatalystRow("View Text", viewText, "") ::
Review Comment:
@ebyhr thanks for your suggestion, I think you are right，I added a code comment.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spark3.4,3.5: In describe extended view command: fix wrong view catal… [iceberg]
Posted by "nastra (via GitHub)" <gi...@apache.org>.
nastra commented on code in PR #11751:
URL: https://github.com/apache/iceberg/pull/11751#discussion_r1886968667
##########
spark/v3.5/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestViews.java:
##########
@@ -1414,7 +1414,42 @@ public void describeExtendedView() {
String.format(
"['format-version' = '1', 'location' = '%s', 'provider' = 'iceberg']",
location),
- ""));
+ ""),
+ row("View Text", sql, ""));
+ }
+
+ @TestTemplate
+ public void describeExtendedViewWithoutCurrentNamespace() {
Review Comment:
Sorry, I actually confused myself and that's exactly the part I had to double-check. So you're right about how the current namespace is being used.
Can you please update the tests to this?
```
+
+ @TestTemplate
+ public void createAndDescribeViewInDefaultNamespace() {
+ String viewName = viewName("createViewInDefaultNamespace");
+ String sql = String.format("SELECT id, data FROM %s WHERE id <= 3", tableName);
+
+ sql("CREATE VIEW %s (id, data) AS %s", viewName, sql);
+ TableIdentifier identifier = TableIdentifier.of(NAMESPACE, viewName);
+ View view = viewCatalog().loadView(identifier);
+ assertThat(view.currentVersion().defaultCatalog()).isNull();
+ assertThat(view.name()).isEqualTo(ViewUtil.fullViewName(catalogName, identifier));
+ assertThat(view.currentVersion().defaultNamespace()).isEqualTo(NAMESPACE);
+
+ String location = viewCatalog().loadView(identifier).location();
+ assertThat(sql("DESCRIBE EXTENDED %s.%s", NAMESPACE, viewName))
+ .contains(
+ row("id", "int", ""),
+ row("data", "string", ""),
+ row("", "", ""),
+ row("# Detailed View Information", "", ""),
+ row("Comment", "", ""),
+ row("View Catalog and Namespace", String.format("%s.%s", catalogName, NAMESPACE), ""),
+ row("View Query Output Columns", "[id, data]", ""),
+ row(
+ "View Properties",
+ String.format(
+ "['format-version' = '1', 'location' = '%s', 'provider' = 'iceberg']",
+ location),
+ ""));
+ }
+
+ @TestTemplate
+ public void createAndDescribeViewWithoutCurrentNamespace() {
+ String viewName = viewName("createViewWithoutCurrentNamespace");
+ Namespace namespace = Namespace.of("test_namespace");
+ String sql = String.format("SELECT id, data FROM %s WHERE id <= 3", tableName);
+
+ sql("CREATE NAMESPACE IF NOT EXISTS %s", namespace);
+ sql("CREATE VIEW %s.%s (id, data) AS %s", namespace, viewName, sql);
+ TableIdentifier identifier = TableIdentifier.of(namespace, viewName);
+ View view = viewCatalog().loadView(identifier);
+ assertThat(view.currentVersion().defaultCatalog()).isNull();
+ assertThat(view.name()).isEqualTo(ViewUtil.fullViewName(catalogName, identifier));
+ assertThat(view.currentVersion().defaultNamespace()).isEqualTo(NAMESPACE);
+
+ String location = viewCatalog().loadView(identifier).location();
+ assertThat(sql("DESCRIBE EXTENDED %s.%s", namespace, viewName))
+ .contains(
+ row("id", "int", ""),
+ row("data", "string", ""),
+ row("", "", ""),
+ row("# Detailed View Information", "", ""),
+ row("Comment", "", ""),
+ row("View Catalog and Namespace", String.format("%s.%s", catalogName, namespace), ""),
+ row("View Query Output Columns", "[id, data]", ""),
+ row(
+ "View Properties",
+ String.format(
+ "['format-version' = '1', 'location' = '%s', 'provider' = 'iceberg']",
+ location),
+ ""));
+ }
```
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spark3.4,3.5: In describe extended view command: fix wrong view catal… [iceberg]
Posted by "Ppei-Wang (via GitHub)" <gi...@apache.org>.
Ppei-Wang commented on code in PR #11751:
URL: https://github.com/apache/iceberg/pull/11751#discussion_r1881750426
##########
spark/v3.4/spark-extensions/src/main/scala/org/apache/spark/sql/execution/datasources/v2/DescribeV2ViewExec.scala:
##########
@@ -55,18 +55,20 @@ case class DescribeV2ViewExec(
private def describeExtended: Seq[InternalRow] = {
val outputColumns = view.queryColumnNames.mkString("[", ", ", "]")
val properties: Map[String, String] = view.properties.asScala.toMap -- ViewCatalog.RESERVED_PROPERTIES.asScala
- val viewCatalogAndNamespace: Seq[String] = view.currentCatalog +: view.currentNamespace.toSeq
+ val viewCatalogAndNamespace: Seq[String] = view.name.split("\\.").take(2)
val viewProperties = properties.toSeq.sortBy(_._1).map {
case (key, value) =>
s"'${escapeSingleQuotedString(key)}' = '${escapeSingleQuotedString(value)}'"
}.mkString("[", ", ", "]")
+ val viewText = view.query
toCatalystRow("# Detailed View Information", "", "") ::
toCatalystRow("Comment", view.properties.getOrDefault(ViewCatalog.PROP_COMMENT, ""), "") ::
toCatalystRow("View Catalog and Namespace", viewCatalogAndNamespace.quoted, "") ::
toCatalystRow("View Query Output Columns", outputColumns, "") ::
toCatalystRow("View Properties", viewProperties, "") ::
+ toCatalystRow("View Text", viewText, "") ::
Review Comment:
ok，i got it.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org