Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[SPARK-37777][SQL] Update the SQL syntax of SHOW FUNCTIONS #35056

Closed
wants to merge 2 commits into from

Conversation

cloud-fan
Copy link
Contributor

What changes were proposed in this pull request?

The SQL syntax of SHOW FUNCTIONS is very weird today. If you want to specify the database to list functions, you need to write SHOW FUNCTIONS LIKE db.f1. This is inconsistent with SHOW TABLES and SHOW VIEWS.

This PR proposes to follow SHOW TABLES and SHOW VIEWS, and support SHOW FUNCTIONS FROM/IN db LIKE pattern. To keep backward compatibility, the legacy syntax is still supported, but it can't be used together with the new FROM/IN db clause.

Why are the changes needed?

Be consistent with SHOW TABLES and SHOW VIEWS

Does this PR introduce any user-facing change?

Yea, as it extends the SHOW FUNCTIONS SQL syntax.

How was this patch tested?

updated tests

@cloud-fan
Copy link
Contributor Author

cc @MaxGekk @imback82 @viirya

@@ -30,7 +30,7 @@ clause is optional and supported only for compatibility with other systems.
### Syntax

```sql
SHOW [ function_kind ] FUNCTIONS [ [ LIKE ] { function_name | regex_pattern } ]
SHOW [ function_kind ] FUNCTIONS [ { FROM | IN } database_name ] [ LIKE regex_pattern ]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I removed the document for the legacy syntax as a soft deprecation.

Copy link
Contributor

@imback82 imback82 left a comment

Choose a reason for hiding this comment

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

LGTM, thanks @cloud-fan!

ShowFunctions(nsPlan, true, true, Some("funct*")))
comparePlans(
parsePlan("SHOW FUNCTIONS IN db LIKE 'funct*'"),
ShowFunctions(UnresolvedNamespace(Seq("db")), true, true, Some("funct*")))
comparePlans(
parsePlan("SHOW FUNCTIONS LIKE a.b.c"),
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you add a test SHOW FUNCTIONS LIKE a so that namespace becomes nsPlan?

val sql = "SHOW other FUNCTIONS"
intercept(sql, s"$sql not supported")
intercept("SHOW FUNCTIONS IN db f1", "Invalid pattern")
Copy link
Contributor

@imback82 imback82 Dec 29, 2021

Choose a reason for hiding this comment

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

nit: add full error message (helpful when searching for a corresponding test case)?

Copy link
Member

@viirya viirya left a comment

Choose a reason for hiding this comment

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

Looks good. Do we need to add new syntax to migration guide?

@cloud-fan
Copy link
Contributor Author

Do we need to add new syntax to migration guide?

IIUC migration guide is for breaking changes, not new features. Since this PR doesn't break anything, we don't need to update migration guide.

@cloud-fan
Copy link
Contributor Author

thanks for the review, merging to master!

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.

3 participants