Skip to content

Enable listing SQL function with session property#13949

Merged
rongrong merged 1 commit intoprestodb:masterfrom
caithagoras:s4
Jan 14, 2020
Merged

Enable listing SQL function with session property#13949
rongrong merged 1 commit intoprestodb:masterfrom
caithagoras:s4

Conversation

@caithagoras
Copy link
Contributor

@caithagoras caithagoras commented Jan 10, 2020

Resolves #13864

== RELEASE NOTES ==
General Changes
* Support hiding user-defined SQL functions in ``SHOW FUNCTIONS`` with session property ``list_built_in_functions_only``.
  This can also be achieved by configuration property ``list-built-in-functions-only``, which is repurposed from ``list-non-built-in-functions``.

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 call it list_all_functions?

Copy link
Contributor Author

@caithagoras caithagoras Jan 10, 2020

Choose a reason for hiding this comment

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

What about list_sql_functions, list_user_defined_functions, or list_udfs if you think the current name is cumbersome.

For people who don't have context, it's not clear from reading the name what the difference is between "all functions" and "not all functions", and any of those 3 names at least gives some hint.

Copy link
Contributor

Choose a reason for hiding this comment

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

But is this only going to list sql functions? it will list all functions right? Unless you are planning to make it an enum and allow only listing certain functions. Another way to configure this is to have it as list_static_functions_only.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Since we're using "built in function" as the language in our code, I'm updating the PR to use list_built_in_functions_only.

@caithagoras caithagoras force-pushed the s4 branch 2 times, most recently from f4c8d53 to 0e1e906 Compare January 13, 2020 21:23
@caithagoras caithagoras changed the title Control listing non built-in function with session properties Control listing SQL function with session properties Jan 13, 2020
@caithagoras caithagoras changed the title Control listing SQL function with session properties Control listing SQL function with session property Jan 13, 2020
@caithagoras caithagoras changed the title Control listing SQL function with session property Support enabling listing SQL function with session property Jan 13, 2020
@caithagoras caithagoras changed the title Support enabling listing SQL function with session property Enable listing SQL function with session property Jan 13, 2020
@caithagoras
Copy link
Contributor Author

@rongrong Comments addressed.

@caithagoras caithagoras force-pushed the s4 branch 4 times, most recently from 04803e5 to 9aa93f3 Compare January 13, 2020 21:35
Copy link
Contributor

@ajaygeorge ajaygeorge left a comment

Choose a reason for hiding this comment

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

LGTM.
Try squashing the commits before final merge.

@caithagoras
Copy link
Contributor Author

Commit squashed. Release notes updated.

Also, rename configuration property list-non-built-in-functions to
list-sql-functions.
@caithagoras
Copy link
Contributor Author

@rongrong comments addressed

@rongrong rongrong merged commit 479e655 into prestodb:master Jan 14, 2020
@caithagoras caithagoras deleted the s4 branch January 14, 2020 04:27
@caithagoras caithagoras mentioned this pull request Feb 20, 2020
8 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Make list non-builtin functions a session property

4 participants