Native built in functions demo 1 #25661

kevintang2022 · 2025-07-30T22:48:27Z

This PR includes changes for Built in functions for different use cases

Register SQL invoked function in built in namespace when they come from plugins (Introduce getSqlInvokedFunctions SPI and BuiltInPluginFunctionNamespaceManager for registering sql invoked functions #25597)
Register Native functions in built in namespace when they come from sidecar (Add rfc for native built in functions rfcs#41)

Since these use cases have a lot of overlap, we can introduce an abstract class in order to reduce duplicate logic.

Description

Motivation and Context

Impact

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
* ... 
* ... 

Hive Connector Changes
* ... 
* ...

If release note is NOT required, use:

== NO RELEASE NOTE ==

facebook-github-bot · 2025-07-30T22:49:13Z

@kevintang2022 has imported this pull request. If you are a Meta employee, you can view this in D79301760.

Summary: …ceManager for registering sql invoked functions ## Description ## Motivation and Context ## Impact Test Plan:  ## Contributor checklist - [ ] Please make sure your submission complies with our [contributing guide](https://github.com/prestodb/presto/blob/master/CONTRIBUTING.md), in particular [code style](https://github.com/prestodb/presto/blob/master/CONTRIBUTING.md#code-style) and [commit standards](https://github.com/prestodb/presto/blob/master/CONTRIBUTING.md#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](https://github.com/prestodb/presto/wiki/Release-Notes-Guidelines). - [ ] Adequate tests were added if applicable. - [ ] CI passed. ## Release Notes Please follow [release notes guidelines](https://github.com/prestodb/presto/wiki/Release-Notes-Guidelines) and fill in the release notes below. ``` == RELEASE NOTES == General Changes * ... * ... Hive Connector Changes * ... * ... ``` If release note is NOT required, use: ``` == NO RELEASE NOTE == ``` Rollback Plan: Differential Revision: D79301760 Pulled By: kevintang2022

facebook-github-bot · 2025-08-08T21:52:23Z

This pull request was exported from Phabricator. Differential Revision: D79301760

Summary: …ceManager for registering sql invoked functions ## Description ## Motivation and Context ## Impact Test Plan:  ## Contributor checklist - [ ] Please make sure your submission complies with our [contributing guide](https://github.com/prestodb/presto/blob/master/CONTRIBUTING.md), in particular [code style](https://github.com/prestodb/presto/blob/master/CONTRIBUTING.md#code-style) and [commit standards](https://github.com/prestodb/presto/blob/master/CONTRIBUTING.md#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](https://github.com/prestodb/presto/wiki/Release-Notes-Guidelines). - [ ] Adequate tests were added if applicable. - [ ] CI passed. ## Release Notes Please follow [release notes guidelines](https://github.com/prestodb/presto/wiki/Release-Notes-Guidelines) and fill in the release notes below. ``` == RELEASE NOTES == General Changes * ... * ... Hive Connector Changes * ... * ... ``` If release note is NOT required, use: ``` == NO RELEASE NOTE == ``` Rollback Plan: Differential Revision: D79301760 Pulled By: kevintang2022

facebook-github-bot · 2025-08-08T22:42:27Z

This pull request was exported from Phabricator. Differential Revision: D79301760

facebook-github-bot · 2025-08-11T03:13:03Z

This pull request was exported from Phabricator. Differential Revision: D79301760

kevintang2022 · 2025-08-11T21:37:43Z

presto-main-base/src/main/java/com/facebook/presto/metadata/FunctionAndTypeManager.java

+            functions.addAll(builtInNativeFunctionNamespaceManager.listFunctions().stream().collect(toImmutableList()));
+            functions.addAll(builtInPluginFunctionNamespaceManager.listFunctions().stream().collect(toImmutableList()));
        }
        else {
            functions.addAll(SessionFunctionUtils.listFunctions(session.getSessionFunctions()));
            functions.addAll(functionNamespaceManagers.values().stream()
                    .flatMap(manager -> manager.listFunctions(likePattern, escape).stream())
                    .collect(toImmutableList()));
+            functions.addAll(builtInNativeFunctionNamespaceManager.listFunctions().stream().collect(toImmutableList()));
+            functions.addAll(builtInPluginFunctionNamespaceManager.listFunctions().stream().collect(toImmutableList()));


TODO: list functions may not work properly. Need to figure out the deduplication logic for this functions list

facebook-github-bot · 2025-08-11T21:41:02Z

@kevintang2022 has imported this pull request. If you are a Meta employee, you can view this in D79301760.

facebook-github-bot · 2025-08-11T22:35:43Z

This pull request was exported from Phabricator. Differential Revision: D79301760

Summary: This PR includes changes for Built in functions for different use cases - Register SQL invoked function in built in namespace when they come from plugins (prestodb#25597) - Register Native functions in built in namespace when they come from sidecar (prestodb/rfcs#41) Since these use cases have a lot of overlap, we can introduce an abstract class in order to reduce duplicate logic. ## Description ## Motivation and Context ## Impact Pull Request resolved: prestodb#25661 Test Plan:  ## Contributor checklist - [ ] Please make sure your submission complies with our [contributing guide](https://github.com/prestodb/presto/blob/master/CONTRIBUTING.md), in particular [code style](https://github.com/prestodb/presto/blob/master/CONTRIBUTING.md#code-style) and [commit standards](https://github.com/prestodb/presto/blob/master/CONTRIBUTING.md#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](https://github.com/prestodb/presto/wiki/Release-Notes-Guidelines). - [ ] Adequate tests were added if applicable. - [ ] CI passed. ## Release Notes Please follow [release notes guidelines](https://github.com/prestodb/presto/wiki/Release-Notes-Guidelines) and fill in the release notes below. ``` == RELEASE NOTES == General Changes * ... * ... Hive Connector Changes * ... * ... ``` If release note is NOT required, use: ``` == NO RELEASE NOTE == ``` Rollback Plan: Differential Revision: D79301760 Pulled By: kevintang2022

facebook-github-bot · 2025-08-11T22:54:08Z

This pull request was exported from Phabricator. Differential Revision: D79301760

Summary: This PR includes changes for Built in functions for different use cases - Register SQL invoked function in built in namespace when they come from plugins (prestodb#25597) - Register Native functions in built in namespace when they come from sidecar (prestodb/rfcs#41) Since these use cases have a lot of overlap, we can introduce an abstract class in order to reduce duplicate logic. ## Description ## Motivation and Context ## Impact Pull Request resolved: prestodb#25661 Test Plan:  ## Contributor checklist - [ ] Please make sure your submission complies with our [contributing guide](https://github.com/prestodb/presto/blob/master/CONTRIBUTING.md), in particular [code style](https://github.com/prestodb/presto/blob/master/CONTRIBUTING.md#code-style) and [commit standards](https://github.com/prestodb/presto/blob/master/CONTRIBUTING.md#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](https://github.com/prestodb/presto/wiki/Release-Notes-Guidelines). - [ ] Adequate tests were added if applicable. - [ ] CI passed. ## Release Notes Please follow [release notes guidelines](https://github.com/prestodb/presto/wiki/Release-Notes-Guidelines) and fill in the release notes below. ``` == RELEASE NOTES == General Changes * ... * ... Hive Connector Changes * ... * ... ``` If release note is NOT required, use: ``` == NO RELEASE NOTE == ``` Rollback Plan: Differential Revision: D79301760 Pulled By: kevintang2022

kevintang2022 · 2025-08-11T22:58:15Z

presto-main-base/src/main/java/com/facebook/presto/metadata/FunctionAndTypeManager.java

+     *
+     * @throws PrestoException if there are no matches or multiple matches
+     */
+    private FunctionHandle getMatchingFunctionHandle(


Logic here for handling conflicts:

Plugin and Default: throw an error

Plugin but no default: use plugin

native and default: use native if default has a SQL language, but use default in default is a Java language

otherwise, use default or native (whichever one is available)

kevintang2022 · 2025-08-11T23:00:35Z

...-base/src/main/java/com/facebook/presto/metadata/BuiltInSpecialFunctionNamespaceManager.java

This is the abstract class that will be used by the native and plugin BuiltInSpecialFunctionNamespaceManager

I don't think it's required anymore as I changed BuiltInPluginFunctionNamespaceManager in #25597 to inherit from FunctionNamespaceManager.

I think we could still benefit from a helper class though since there's a decent amount of overlap.

Yeah sounds good. I can do that deduplication in my followup PR once you merge yours in

kevintang2022 · 2025-08-11T23:01:27Z

presto-main-base/src/main/java/com/facebook/presto/metadata/NativeFunctionRegistryTool.java

+
+    private URI getSidecarLocationOnStartup()
+    {
+        Node sidecarNode = null;


Retry logic here is needed to ensure that a worker sidecar node is available to read the function registry from

presto-main-base/src/main/java/com/facebook/presto/metadata/NativeFunctionRegistryTool.java

kevintang2022 · 2025-08-11T23:02:42Z

presto-main/src/main/java/com/facebook/presto/server/PrestoServer.java

@@ -197,6 +198,7 @@ public void run()
            NodeInfo nodeInfo = injector.getInstance(NodeInfo.class);
            PluginNodeManager pluginNodeManager = new PluginNodeManager(nodeManager, nodeInfo.getEnvironment());
            planCheckerProviderManager.loadPlanCheckerProviders(pluginNodeManager);
+            injector.getInstance(FunctionAndTypeManager.class).registerNativeFunctions();


This needs to happen after NodeManger is ready, so it is called here

kevintang2022 · 2025-08-11T23:25:06Z

presto-main-base/src/main/java/com/facebook/presto/sql/analyzer/FeaturesConfig.java

@@ -2296,6 +2297,19 @@ public boolean isNativeExecutionEnabled()
        return this.nativeExecutionEnabled;
    }

+    @Config("built-in-sidecar-functions-enabled")


Feature is controlled by server property

kevintang2022 · 2025-08-11T23:37:31Z

presto-spi/src/main/java/com/facebook/presto/spi/function/BuiltInKind.java

Using this enum type instead of isBuiltInPluginFunction and isBuiltInNativeFunction

facebook-github-bot · 2025-08-12T20:21:17Z

This pull request was exported from Phabricator. Differential Revision: D79301760

Summary: This PR includes changes for Built in functions for different use cases - Register SQL invoked function in built in namespace when they come from plugins (prestodb#25597) - Register Native functions in built in namespace when they come from sidecar (prestodb/rfcs#41) Since these use cases have a lot of overlap, we can introduce an abstract class in order to reduce duplicate logic. ## Description ## Motivation and Context ## Impact Pull Request resolved: prestodb#25661 Test Plan:  ## Contributor checklist - [ ] Please make sure your submission complies with our [contributing guide](https://github.com/prestodb/presto/blob/master/CONTRIBUTING.md), in particular [code style](https://github.com/prestodb/presto/blob/master/CONTRIBUTING.md#code-style) and [commit standards](https://github.com/prestodb/presto/blob/master/CONTRIBUTING.md#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](https://github.com/prestodb/presto/wiki/Release-Notes-Guidelines). - [ ] Adequate tests were added if applicable. - [ ] CI passed. ## Release Notes Please follow [release notes guidelines](https://github.com/prestodb/presto/wiki/Release-Notes-Guidelines) and fill in the release notes below. ``` == RELEASE NOTES == General Changes * ... * ... Hive Connector Changes * ... * ... ``` If release note is NOT required, use: ``` == NO RELEASE NOTE == ``` Rollback Plan: Differential Revision: D79301760 Pulled By: kevintang2022

Summary: Demo 1 Differential Revision: D79301760

prestodb-ci added the from:Meta PR from Meta label Jul 30, 2025

kevintang2022 force-pushed the kevin-sql-invoked-functions-SPI branch from 8825888 to 9cf57f9 Compare August 8, 2025 21:52

facebook-github-bot added the fb-exported label Aug 8, 2025

kevintang2022 force-pushed the kevin-sql-invoked-functions-SPI branch from 9cf57f9 to 1bdb396 Compare August 8, 2025 22:42

kevintang2022 changed the title ~~Introduce getSqlInvokedFunctions SPI and BuiltInPluginFunctionNamespa…~~ Native built in functions demo 1 Aug 11, 2025

kevintang2022 force-pushed the kevin-sql-invoked-functions-SPI branch from 1bdb396 to 464830f Compare August 11, 2025 03:13

kevintang2022 commented Aug 11, 2025

View reviewed changes

kevintang2022 force-pushed the kevin-sql-invoked-functions-SPI branch from c61f5b8 to b4d7062 Compare August 11, 2025 22:35

kevintang2022 force-pushed the kevin-sql-invoked-functions-SPI branch from b4d7062 to df7df56 Compare August 11, 2025 22:54

kevintang2022 commented Aug 11, 2025

View reviewed changes

presto-main-base/src/main/java/com/facebook/presto/metadata/NativeFunctionRegistryTool.java Show resolved Hide resolved

kevintang2022 commented Aug 11, 2025

View reviewed changes

kevintang2022 mentioned this pull request Aug 12, 2025

Introduce getSqlInvokedFunctions SPI and BuiltInPluginFunctionNamespaceManager for registering sql invoked functions #25597

Merged

6 tasks

kevintang2022 force-pushed the kevin-sql-invoked-functions-SPI branch from df7df56 to 5a2be22 Compare August 12, 2025 20:21

kevintang2022 added 3 commits August 12, 2025 19:23

Sidecar native built in demo 1

af7425b

Summary: Demo 1 Differential Revision: D79301760

Add unit test

4f0c792

Refactor built in function

f31b05d

kevintang2022 force-pushed the kevin-sql-invoked-functions-SPI branch from 5a2be22 to f31b05d Compare August 13, 2025 02:25

kevintang2022 closed this Aug 13, 2025

Native built in functions demo 1 #25661

Native built in functions demo 1 #25661

Uh oh!

Conversation

kevintang2022 commented Jul 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Motivation and Context

Impact

Test Plan

Contributor checklist

Release Notes

Uh oh!

facebook-github-bot commented Jul 30, 2025

Uh oh!

facebook-github-bot commented Aug 8, 2025

Uh oh!

facebook-github-bot commented Aug 8, 2025

Uh oh!

facebook-github-bot commented Aug 11, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

facebook-github-bot commented Aug 11, 2025

Uh oh!

facebook-github-bot commented Aug 11, 2025

Uh oh!

facebook-github-bot commented Aug 11, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pdabre12 Aug 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

facebook-github-bot commented Aug 12, 2025

Uh oh!

Uh oh!

kevintang2022 commented Jul 30, 2025 •

edited

Loading

pdabre12 Aug 12, 2025 •

edited

Loading