Skip to content

Conversation

@imback82
Copy link
Contributor

@imback82 imback82 commented Nov 3, 2019

What changes were proposed in this pull request?

This PR introduces a new SQL command: SHOW CURRENT NAMESPACE.

Why are the changes needed?

Datasource V2 supports multiple catalogs/namespaces and having SHOW CURRENT NAMESPACE to retrieve the current catalog/namespace info would be useful.

Does this PR introduce any user-facing change?

Yes, the user can perform the following:

scala> spark.sql("SHOW CURRENT NAMESPACE").show
+-------------+---------+
|      catalog|namespace|
+-------------+---------+
|spark_catalog|  default|
+-------------+---------+

scala> spark.sql("USE testcat.ns1.ns2").show
scala> spark.sql("SHOW CURRENT NAMESPACE").show
+-------+---------+
|catalog|namespace|
+-------+---------+
|testcat|  ns1.ns2|
+-------+---------+

How was this patch tested?

Added unit tests.

@imback82
Copy link
Contributor Author

imback82 commented Nov 3, 2019

cc: @cloud-fan @rdblue

@imback82
Copy link
Contributor Author

imback82 commented Nov 3, 2019

I was planning to expose SHOW CURRENT NAMESPACE as a separate command. But, how about just exposing SHOW CURRENT NAMESPACE which prints both current catalog and namespace? In this case, we don't need to expose SHOW CURRENT CATALOG. Please let me know your thoughts. Thanks!

@SparkQA
Copy link

SparkQA commented Nov 3, 2019

Test build #113181 has finished for PR 26379 at commit 7df7574.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds the following public classes (experimental):
  • case class AlterTableRenamePartitionStatement(
  • case class AlterTableDropPartitionStatement(

@cloud-fan
Copy link
Contributor

SGTM. SHOW CURRENT NAMESPACE always put the current catalog name in the first part, which should be good enough.

@imback82 imback82 changed the title [SPARK-29734][SQL] Datasource V2: Support SHOW CURRENT CATALOG [SPARK-29734][SQL] Datasource V2: Support SHOW CURRENT NAMESPACE Nov 4, 2019
@imback82
Copy link
Contributor Author

imback82 commented Nov 4, 2019

Thanks @cloud-fan. I updated it to SHOW CURRENT NAMESPACE and updated title/description of PR.

*/
case class ShowCurrentNamespace(catalogManager: CatalogManager) extends Command {
override val output: Seq[Attribute] = Seq(
AttributeReference("catalogName", StringType, nullable = false)(),
Copy link
Contributor

Choose a reason for hiding this comment

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

let's be consistent. either catalog/namespace or catalogName/namespacName

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated to catalog/namespace

@SparkQA
Copy link

SparkQA commented Nov 4, 2019

Test build #113215 has finished for PR 26379 at commit 506ce2a.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

Copy link
Member

@dongjoon-hyun dongjoon-hyun left a comment

Choose a reason for hiding this comment

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

+1, LGTM (except three minor comments about test cases.)
In general, unit test case should not have a side-effect after the test. Please confirm if the new UT follow that principle.

Thanks, @imback82 !

@SparkQA
Copy link

SparkQA commented Nov 4, 2019

Test build #113218 has finished for PR 26379 at commit aed9bc8.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@dongjoon-hyun
Copy link
Member

Merged to master. The last touched test case already passed~

@imback82
Copy link
Contributor Author

imback82 commented Nov 5, 2019

Thanks everyone for review and help!

@imback82 imback82 deleted the show_current_catalog branch November 5, 2019 02:11
@SparkQA
Copy link

SparkQA commented Nov 5, 2019

Test build #113227 has finished for PR 26379 at commit a667321.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants