Add support for SHOW CREATE SCHEMA #24356

agrawalreetika · 2025-01-13T19:57:21Z

Description

Add support for SHOW CREATE SCHEMA

Add SQL Support for SHOW CREATE SCHEMA
Add support for SHOW CREATE SCHEMA in hive & iceberg connector

Motivation and Context

There was no way to check schema properties. So this SQL support will provide a way to check properties like location for schema

Impact

SQL support for SHOW CREATE SCHEMA from a schema :

presto> SHOW CREATE SCHEMA hive.sf1;
                  Create Schema
    -----------------------------------------
     CREATE SCHEMA hive.sf1
     WITH (
        location = 'hdfs://localhost:9000/user/hive/warehouse/sf1.db'
     )
    (1 row)

Test Plan

Contributor checklist

Please make sure your submission complies with our contributing guide, in particular code style and commit standards.
PR description addresses the issue accurately and concisely. If the change is non-trivial, a GitHub Issue is referenced.
Documented new properties (with its default value), SQL syntax, functions, or other functionality.
If release notes are required, they follow the release notes guidelines.
Adequate tests were added if applicable.
CI passed.

Release Notes

Please follow release notes guidelines and fill in the release notes below.

== RELEASE NOTES ==

General Changes
* Add DDL SQL support for ``SHOW CREATE SCHEMA``. :pr:`24356`

github-actions · 2025-01-13T19:58:20Z

Codenotify: Notifying subscribers in CODENOTIFY files for diff b511e7a...dd9494c.

Notify	File(s)
@aditi-pandit	presto-parser/src/main/antlr4/com/facebook/presto/sql/parser/SqlBase.g4
@elharo	presto-parser/src/main/antlr4/com/facebook/presto/sql/parser/SqlBase.g4
@kaikalur	presto-parser/src/main/antlr4/com/facebook/presto/sql/parser/SqlBase.g4
@rschlussel	presto-parser/src/main/antlr4/com/facebook/presto/sql/parser/SqlBase.g4

steveburnett

Thanks! Pull branch, local doc build, everything looks good.

steveburnett · 2025-01-13T21:22:18Z

Thanks for the release note! A couple of nits.

== RELEASE NOTES ==

General Changes
* Add DDL SQL support for ``SHOW CREATE SCHEMA``. :pr:`24356`

ZacBlanco

one minor nit. Otherwise, could we add some negative tests for the case a user specifies only a catalog or a table directly and verify that the query fails?

ZacBlanco · 2025-01-15T19:06:51Z

presto-main/src/main/java/com/facebook/presto/sql/rewrite/ShowQueriesRewrite.java

+                }
+
+                Map<String, Object> properties = metadata.getSchemaProperties(session, catalogSchemaName);
+                Map<String, PropertyMetadata<?>> allTableProperties = metadata.getSchemaPropertyManager().getAllProperties().get(getConnectorIdOrThrow(session, metadata, catalogSchemaName.getCatalogName()));


nit: allSchemaProperties?

hantangwangd

Thanks for this change. One little problem and one point for discussing, otherwise looks good to me.

hantangwangd · 2025-01-20T08:57:39Z

presto-main/src/main/java/com/facebook/presto/sql/rewrite/ShowQueriesRewrite.java

+            if (node.getType() == SCHEMA) {
+                CatalogSchemaName catalogSchemaName = createCatalogSchemaName(session, node, Optional.of(node.getName()));
+                if (!metadataResolver.schemaExists(catalogSchemaName)) {
+                    throw new SemanticException(MISSING_SCHEMA, node, "Schema '%s' does not exist", catalogSchemaName);
+                }
+
+                Map<String, Object> properties = metadata.getSchemaProperties(session, catalogSchemaName);
+                Map<String, PropertyMetadata<?>> allSchemaProperties = metadata.getSchemaPropertyManager().getAllProperties().get(getConnectorIdOrThrow(session, metadata, catalogSchemaName.getCatalogName()));
+                List<Property> propertyNodes = buildProperties(objectName, Optional.empty(), INVALID_SCHEMA_PROPERTY, properties, allSchemaProperties);
+                CreateSchema createSchema = new CreateSchema(
+                        node.getName(),
+                        false,
+                        propertyNodes);
+                return singleValueQuery("Create Schema", formatSql(createSchema, Optional.of(parameters)).trim());
+            }


Is it better to put this code at the beginning of the method? As far as I can see, createQualifiedObjectName(session, node, node.getName()) will fail when we just provide catalogname.schemaname in a session which do not set default catalog and schema. So, if we connector the database without set a default schema, then executing the following statements:

show create schema hive.default;

We will fail with the following message: Catalog must be specified when session catalog is not set.

Moreover, when executing show create schema, there is no need to execute getView(...) and getMaterializedView(...) firstly.

hantangwangd · 2025-01-20T09:03:41Z

presto-hive/src/main/java/com/facebook/presto/hive/HiveSchemaProperties.java

+
+    public static Map<String, Object> getDatabaseProperties(Database database)
+    {
+        ImmutableMap.Builder<String, Object> result = ImmutableMap.builder();
+        database.getLocation().ifPresent(location -> result.put(LOCATION_PROPERTY, location));
+        return result.build();
+    }


Do you think it makes sense to move this class to presto-hive-metastore? Seems it's more like a concept of HMS.

I think that's a good idea. Let me refactor it in different PR and then I can rebase this one
#24401

agrawalreetika self-assigned this Jan 13, 2025

agrawalreetika requested review from steveburnett, elharo, a team, hantangwangd, ZacBlanco, jaystarshot, feilong-liu and ClarenceThreepwood as code owners January 13, 2025 19:57

agrawalreetika requested a review from presto-oss January 13, 2025 19:57

prestodb-ci added the from:IBM PR from IBM label Jan 13, 2025

prestodb-ci requested review from a team, bibith4 and ShahimSharafudeen and removed request for a team January 13, 2025 19:57

steveburnett previously approved these changes Jan 13, 2025

View reviewed changes

agrawalreetika force-pushed the show-create-schema branch from 68ef2bb to 8794ad6 Compare January 14, 2025 05:01

ZacBlanco requested changes Jan 15, 2025

View reviewed changes

Add support for SHOW CREATE SCHEMA

dd9494c

agrawalreetika dismissed steveburnett’s stale review via dd9494c January 16, 2025 07:37

agrawalreetika force-pushed the show-create-schema branch from 8794ad6 to dd9494c Compare January 16, 2025 07:37

hantangwangd reviewed Jan 20, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for SHOW CREATE SCHEMA #24356

Add support for SHOW CREATE SCHEMA #24356

agrawalreetika commented Jan 13, 2025 •

edited

Loading

github-actions bot commented Jan 13, 2025 •

edited

Loading

steveburnett left a comment

steveburnett commented Jan 13, 2025

ZacBlanco left a comment

ZacBlanco Jan 15, 2025

hantangwangd left a comment

hantangwangd Jan 20, 2025

hantangwangd Jan 20, 2025

agrawalreetika Jan 20, 2025 •

edited

Loading

Add support for SHOW CREATE SCHEMA #24356

Are you sure you want to change the base?

Add support for SHOW CREATE SCHEMA #24356

Conversation

agrawalreetika commented Jan 13, 2025 • edited Loading

Description

Motivation and Context

Impact

Test Plan

Contributor checklist

Release Notes

github-actions bot commented Jan 13, 2025 • edited Loading

steveburnett left a comment

Choose a reason for hiding this comment

steveburnett commented Jan 13, 2025

ZacBlanco left a comment

Choose a reason for hiding this comment

ZacBlanco Jan 15, 2025

Choose a reason for hiding this comment

hantangwangd left a comment

Choose a reason for hiding this comment

hantangwangd Jan 20, 2025

Choose a reason for hiding this comment

hantangwangd Jan 20, 2025

Choose a reason for hiding this comment

agrawalreetika Jan 20, 2025 • edited Loading

Choose a reason for hiding this comment

agrawalreetika commented Jan 13, 2025 •

edited

Loading

github-actions bot commented Jan 13, 2025 •

edited

Loading

agrawalreetika Jan 20, 2025 •

edited

Loading