[DISCUSS][Gandiva] External function registry proposal

[Yue Ni <ni...@gmail.com>]

Hi there,

I'd like to initiate a discussion regarding the proposal to introduce
external function registry support in Gandiva. I've provided a concise
description of the proposal in the following issue:
https://github.com/apache/arrow/issues/37753. I welcome any feedback or
comments on this topic. Please feel free to share your thoughts either here
on the mailing list or directly within the issue. Thank you for your
attention and help.

*Background:*
Our team has been leveraging Gandiva in our projects, and its performance
and capabilities have been commendable. However, we've identified a
constraint concerning the registration of functions. At present, Gandiva
necessitates that functions be registered directly within its codebase.
This method, while functional, is not the most user-friendly and presents
hurdles for those aiming to incorporate third-party functions. Direct
modifications to Gandiva's source code for such integrations can
inadvertently introduce maintenance challenges and potential versioning
conflicts down the line.

*Proposal:*
To address this limitation, I propose the introduction of an external
function registry mechanism in Gandiva. This would allow users and
developers to register and integrate custom functions without directly
modifying Gandiva's core source code. You can find more details in the
issue [1] and the PR [2].

Any feedback is appreciated. Thanks.

*References:*
[1] https://github.com/apache/arrow/issues/37753
[2] https://github.com/apache/arrow/pull/37787

Regards,
Yue Ni

Re: [DISCUSS][Gandiva] External function registry proposal

[Yue Ni <ni...@gmail.com>]

> I think the key idea is to let users call Gandiva functions to register
functions and pass necessary info explicitly to Gandiva, rather than
letting Gandiva discover them by itself.

That makes sense. Thanks Jin and Antonie for your valuable feedback. I will
revise the proposal accordingly later.

Regards,
Yue

On Wed, Sep 27, 2023 at 4:29 AM Jin Shang <sh...@gmail.com> wrote:

> I agree with Antoine that we don't need to define a JSON format or a
> directory structure for Gandiva.
> To support external functions, we essentially need two things:
> 1. Gandiva's function registry needs to be aware of the function metadata:
> We can achieve this by having a
> `FunctionRegistry::AddFunction(NativeFunction* func)` function. The
> `NativeFunction` can come from whatever source the user wants or even hard
> coded, not necessarily from JSON files. The function registry is a
> singleton so this should be easy.
> 2. The LLVM engine needs access to the function IR definition: Users should
> be able register a string representation of IR bytecode similar to what the
> current `Engine::LoadPreCompiledIR` does. So something like
> `LoadExternalIR(std::string_view)` is enough. Although `Engine` is not a
> singleton, we can create a global object holding external IRs and Engines
> can link them on construction.
> I think the key idea is to let users call Gandiva functions to register
> functions and pass necessary info explicitly to Gandiva, rather than
> letting Gandiva discover them by itself.
>
> On Tue, Sep 26, 2023 at 2:14 AM Yue Ni <ni...@gmail.com> wrote:
>
> > > The definition of an external function registry can certainly belong in
> > Gandiva, but how it's populated should be left to third-party projects
> >
> > Are you proposing a more general approach, like incorporating the
> following
> > APIs into Gandiva? (Please note that the function names/signatures are
> > tentative and just meant for illustrative purposes.)
> > 1) AddExternalFunctionRegistry(ExternalFunctionRegistry
> function_registry)
> > 2) AddFunctionBitcodeLoader(FunctionBitcodeLoader bitcode_loader)
> > Where `ExternalFunctionRegistry` can return a list of function
> definitions
> > and `FunctionBitcodeLoader` can return a list of bitcode buffers, so that
> > the specific metadata/bitcode data population logic can be moved out of
> > Gandiva? Thanks.
> >
> > Regards,
> > Yue
> >
> > On Tue, Sep 26, 2023 at 12:25 AM Antoine Pitrou <an...@python.org>
> > wrote:
> >
> > >
> > > Hi Yue,
> > >
> > > Le 25/09/2023 à 18:15, Yue Ni a écrit :
> > > >
> > > >> a CMake entrypoint (for example a function) making it easy for
> > > > third-party projects to compile their own functions
> > > > I can come up with a minimum CMake template so that users can compile
> > C++
> > > > based functions, and I think if the integration happens at the LLVM
> IR
> > > > level, it is possible to author the functions beyond C++ languages,
> > such
> > > as
> > > > Rust/Zig as long as the compiler can generate LLVM IR (there are
> other
> > > > issues that need to be addressed from the Rust experiment I made, but
> > > that
> > > > can be another proposal/PR). If we make that work, CMake is probably
> > not
> > > so
> > > > important either since other languages can use their own build tools
> > such
> > > > as Cargo/zig build, and we just need some documentation to describe
> how
> > > it
> > > > should be interfaced typically.
> > >
> > > As long as there's a well-known and supported way to generate the code
> > > for external functions, then it's fine to me.
> > >
> > > (also the required signature for these functions should be documented
> > > somewhere)
> > >
> > > >> The rest of the proposal (a specific JSON file format, a bunch of
> > > functions
> > > > to iterate directory entries in a specific layout) is IMHO off-topic
> > for
> > > > Gandiva, and each third-party project can implement their own idioms
> > for
> > > > the discovery of external functions
> > >  >
> > > > Could you give some more guidance on how this should work without an
> > > > external function registry containing metadata? As far as I know, for
> > > each
> > > > pre-compiled function used in an expression, Gandiva needs to lookup
> > its
> > > > signature from the function registry, which currently is a C++ class
> > that
> > > > is hard coded to contain 6 categories of built-in functions
> > > > (arithmetic/datetime/hash/mathops/string/datetime arithmetic). If a
> > third
> > > > party function cannot be found in the registry, it cannot be used in
> > the
> > > > expression. If we don't load the pre-compiled function metadata from
> > > > external files, how do we avoid Gandiva rejecting the expression
> when a
> > > > third party function cannot be found in the function registry?
> Thanks.
> > >
> > > What I'm saying is that code to load function metadata from JSON and
> > > walk directories of .bc files does not belong in Gandiva. The
> definition
> > > of an external function registry can certainly belong in Gandiva, but
> > > how it's populated should be left to third-party projects (which then
> > > don't have to use JSON or a given directory layout).
> > >
> > > Regards
> > >
> > > Antoine.
> > >
> >
>

Re: [DISCUSS][Gandiva] External function registry proposal

[Jin Shang <sh...@gmail.com>]

I agree with Antoine that we don't need to define a JSON format or a
directory structure for Gandiva.
To support external functions, we essentially need two things:
1. Gandiva's function registry needs to be aware of the function metadata:
We can achieve this by having a
`FunctionRegistry::AddFunction(NativeFunction* func)` function. The
`NativeFunction` can come from whatever source the user wants or even hard
coded, not necessarily from JSON files. The function registry is a
singleton so this should be easy.
2. The LLVM engine needs access to the function IR definition: Users should
be able register a string representation of IR bytecode similar to what the
current `Engine::LoadPreCompiledIR` does. So something like
`LoadExternalIR(std::string_view)` is enough. Although `Engine` is not a
singleton, we can create a global object holding external IRs and Engines
can link them on construction.
I think the key idea is to let users call Gandiva functions to register
functions and pass necessary info explicitly to Gandiva, rather than
letting Gandiva discover them by itself.

On Tue, Sep 26, 2023 at 2:14 AM Yue Ni <ni...@gmail.com> wrote:

> > The definition of an external function registry can certainly belong in
> Gandiva, but how it's populated should be left to third-party projects
>
> Are you proposing a more general approach, like incorporating the following
> APIs into Gandiva? (Please note that the function names/signatures are
> tentative and just meant for illustrative purposes.)
> 1) AddExternalFunctionRegistry(ExternalFunctionRegistry function_registry)
> 2) AddFunctionBitcodeLoader(FunctionBitcodeLoader bitcode_loader)
> Where `ExternalFunctionRegistry` can return a list of function definitions
> and `FunctionBitcodeLoader` can return a list of bitcode buffers, so that
> the specific metadata/bitcode data population logic can be moved out of
> Gandiva? Thanks.
>
> Regards,
> Yue
>
> On Tue, Sep 26, 2023 at 12:25 AM Antoine Pitrou <an...@python.org>
> wrote:
>
> >
> > Hi Yue,
> >
> > Le 25/09/2023 à 18:15, Yue Ni a écrit :
> > >
> > >> a CMake entrypoint (for example a function) making it easy for
> > > third-party projects to compile their own functions
> > > I can come up with a minimum CMake template so that users can compile
> C++
> > > based functions, and I think if the integration happens at the LLVM IR
> > > level, it is possible to author the functions beyond C++ languages,
> such
> > as
> > > Rust/Zig as long as the compiler can generate LLVM IR (there are other
> > > issues that need to be addressed from the Rust experiment I made, but
> > that
> > > can be another proposal/PR). If we make that work, CMake is probably
> not
> > so
> > > important either since other languages can use their own build tools
> such
> > > as Cargo/zig build, and we just need some documentation to describe how
> > it
> > > should be interfaced typically.
> >
> > As long as there's a well-known and supported way to generate the code
> > for external functions, then it's fine to me.
> >
> > (also the required signature for these functions should be documented
> > somewhere)
> >
> > >> The rest of the proposal (a specific JSON file format, a bunch of
> > functions
> > > to iterate directory entries in a specific layout) is IMHO off-topic
> for
> > > Gandiva, and each third-party project can implement their own idioms
> for
> > > the discovery of external functions
> >  >
> > > Could you give some more guidance on how this should work without an
> > > external function registry containing metadata? As far as I know, for
> > each
> > > pre-compiled function used in an expression, Gandiva needs to lookup
> its
> > > signature from the function registry, which currently is a C++ class
> that
> > > is hard coded to contain 6 categories of built-in functions
> > > (arithmetic/datetime/hash/mathops/string/datetime arithmetic). If a
> third
> > > party function cannot be found in the registry, it cannot be used in
> the
> > > expression. If we don't load the pre-compiled function metadata from
> > > external files, how do we avoid Gandiva rejecting the expression when a
> > > third party function cannot be found in the function registry? Thanks.
> >
> > What I'm saying is that code to load function metadata from JSON and
> > walk directories of .bc files does not belong in Gandiva. The definition
> > of an external function registry can certainly belong in Gandiva, but
> > how it's populated should be left to third-party projects (which then
> > don't have to use JSON or a given directory layout).
> >
> > Regards
> >
> > Antoine.
> >
>

Re: [DISCUSS][Gandiva] External function registry proposal

[Yue Ni <ni...@gmail.com>]

> The definition of an external function registry can certainly belong in
Gandiva, but how it's populated should be left to third-party projects

Are you proposing a more general approach, like incorporating the following
APIs into Gandiva? (Please note that the function names/signatures are
tentative and just meant for illustrative purposes.)
1) AddExternalFunctionRegistry(ExternalFunctionRegistry function_registry)
2) AddFunctionBitcodeLoader(FunctionBitcodeLoader bitcode_loader)
Where `ExternalFunctionRegistry` can return a list of function definitions
and `FunctionBitcodeLoader` can return a list of bitcode buffers, so that
the specific metadata/bitcode data population logic can be moved out of
Gandiva? Thanks.

Regards,
Yue

On Tue, Sep 26, 2023 at 12:25 AM Antoine Pitrou <an...@python.org> wrote:

>
> Hi Yue,
>
> Le 25/09/2023 à 18:15, Yue Ni a écrit :
> >
> >> a CMake entrypoint (for example a function) making it easy for
> > third-party projects to compile their own functions
> > I can come up with a minimum CMake template so that users can compile C++
> > based functions, and I think if the integration happens at the LLVM IR
> > level, it is possible to author the functions beyond C++ languages, such
> as
> > Rust/Zig as long as the compiler can generate LLVM IR (there are other
> > issues that need to be addressed from the Rust experiment I made, but
> that
> > can be another proposal/PR). If we make that work, CMake is probably not
> so
> > important either since other languages can use their own build tools such
> > as Cargo/zig build, and we just need some documentation to describe how
> it
> > should be interfaced typically.
>
> As long as there's a well-known and supported way to generate the code
> for external functions, then it's fine to me.
>
> (also the required signature for these functions should be documented
> somewhere)
>
> >> The rest of the proposal (a specific JSON file format, a bunch of
> functions
> > to iterate directory entries in a specific layout) is IMHO off-topic for
> > Gandiva, and each third-party project can implement their own idioms for
> > the discovery of external functions
>  >
> > Could you give some more guidance on how this should work without an
> > external function registry containing metadata? As far as I know, for
> each
> > pre-compiled function used in an expression, Gandiva needs to lookup its
> > signature from the function registry, which currently is a C++ class that
> > is hard coded to contain 6 categories of built-in functions
> > (arithmetic/datetime/hash/mathops/string/datetime arithmetic). If a third
> > party function cannot be found in the registry, it cannot be used in the
> > expression. If we don't load the pre-compiled function metadata from
> > external files, how do we avoid Gandiva rejecting the expression when a
> > third party function cannot be found in the function registry? Thanks.
>
> What I'm saying is that code to load function metadata from JSON and
> walk directories of .bc files does not belong in Gandiva. The definition
> of an external function registry can certainly belong in Gandiva, but
> how it's populated should be left to third-party projects (which then
> don't have to use JSON or a given directory layout).
>
> Regards
>
> Antoine.
>

Re: [DISCUSS][Gandiva] External function registry proposal

[Antoine Pitrou <an...@python.org>]

Hi Yue,

Le 25/09/2023 à 18:15, Yue Ni a écrit :
> 
>> a CMake entrypoint (for example a function) making it easy for
> third-party projects to compile their own functions
> I can come up with a minimum CMake template so that users can compile C++
> based functions, and I think if the integration happens at the LLVM IR
> level, it is possible to author the functions beyond C++ languages, such as
> Rust/Zig as long as the compiler can generate LLVM IR (there are other
> issues that need to be addressed from the Rust experiment I made, but that
> can be another proposal/PR). If we make that work, CMake is probably not so
> important either since other languages can use their own build tools such
> as Cargo/zig build, and we just need some documentation to describe how it
> should be interfaced typically.

As long as there's a well-known and supported way to generate the code 
for external functions, then it's fine to me.

(also the required signature for these functions should be documented 
somewhere)

>> The rest of the proposal (a specific JSON file format, a bunch of functions
> to iterate directory entries in a specific layout) is IMHO off-topic for
> Gandiva, and each third-party project can implement their own idioms for
> the discovery of external functions
 >
> Could you give some more guidance on how this should work without an
> external function registry containing metadata? As far as I know, for each
> pre-compiled function used in an expression, Gandiva needs to lookup its
> signature from the function registry, which currently is a C++ class that
> is hard coded to contain 6 categories of built-in functions
> (arithmetic/datetime/hash/mathops/string/datetime arithmetic). If a third
> party function cannot be found in the registry, it cannot be used in the
> expression. If we don't load the pre-compiled function metadata from
> external files, how do we avoid Gandiva rejecting the expression when a
> third party function cannot be found in the function registry? Thanks.

What I'm saying is that code to load function metadata from JSON and 
walk directories of .bc files does not belong in Gandiva. The definition 
of an external function registry can certainly belong in Gandiva, but 
how it's populated should be left to third-party projects (which then 
don't have to use JSON or a given directory layout).

Regards

Antoine.

Re: [DISCUSS][Gandiva] External function registry proposal

[Yue Ni <ni...@gmail.com>]

Hi Antoine,

Thanks for the feedback.

> a CMake entrypoint (for example a function) making it easy for
third-party projects to compile their own functions
I can come up with a minimum CMake template so that users can compile C++
based functions, and I think if the integration happens at the LLVM IR
level, it is possible to author the functions beyond C++ languages, such as
Rust/Zig as long as the compiler can generate LLVM IR (there are other
issues that need to be addressed from the Rust experiment I made, but that
can be another proposal/PR). If we make that work, CMake is probably not so
important either since other languages can use their own build tools such
as Cargo/zig build, and we just need some documentation to describe how it
should be interfaced typically.

> The rest of the proposal (a specific JSON file format, a bunch of functions
to iterate directory entries in a specific layout) is IMHO off-topic for
Gandiva, and each third-party project can implement their own idioms for
the discovery of external functions
Could you give some more guidance on how this should work without an
external function registry containing metadata? As far as I know, for each
pre-compiled function used in an expression, Gandiva needs to lookup its
signature from the function registry, which currently is a C++ class that
is hard coded to contain 6 categories of built-in functions
(arithmetic/datetime/hash/mathops/string/datetime arithmetic). If a third
party function cannot be found in the registry, it cannot be used in the
expression. If we don't load the pre-compiled function metadata from
external files, how do we avoid Gandiva rejecting the expression when a
third party function cannot be found in the function registry? Thanks.

Regards,
Yue

On Mon, Sep 25, 2023 at 10:39 PM Antoine Pitrou <an...@python.org> wrote:

>
> Hello,
>
> Being making Gandiva more extensible sounds like a worthwhile improvement.
>
> However, I'm not sure why we would need to choose a JSON-based format
> for this. Instead, I think Gandiva could simply provide the two
> following basic-blocks:
>
> 1. a CMake entrypoint (for example a function) making it easy for
> third-party projects to compile their own functions
>
> 2. a C++ entrypoint to load a bitcode file with the corresponding
> function definition(s)
>
> The rest of the proposal (a specific JSON file format, a bunch of
> functions to iterate directory entries in a specific layout) is IMHO
> off-topic for Gandiva, and each third-party project can implement their
> own idioms for the discovery of external functions.
>
> I'd add that this should be documented somewhere so that it is generally
> useful, not only for the contributors of the feature.
>
> Also, I hope that this will get more people interested in Gandiva
> maintenance.
>
> Regards
>
> Antoine.
>
>
> Le 25/09/2023 à 16:17, Yue Ni a écrit :
> > Hi there,
> >
> > I'd like to initiate a discussion regarding the proposal to introduce
> > external function registry support in Gandiva. I've provided a concise
> > description of the proposal in the following issue:
> > https://github.com/apache/arrow/issues/37753. I welcome any feedback or
> > comments on this topic. Please feel free to share your thoughts either
> here
> > on the mailing list or directly within the issue. Thank you for your
> > attention and help.
> >
> > *Background:*
> > Our team has been leveraging Gandiva in our projects, and its performance
> > and capabilities have been commendable. However, we've identified a
> > constraint concerning the registration of functions. At present, Gandiva
> > necessitates that functions be registered directly within its codebase.
> > This method, while functional, is not the most user-friendly and presents
> > hurdles for those aiming to incorporate third-party functions. Direct
> > modifications to Gandiva's source code for such integrations can
> > inadvertently introduce maintenance challenges and potential versioning
> > conflicts down the line.
> >
> > *Proposal:*
> > To address this limitation, I propose the introduction of an external
> > function registry mechanism in Gandiva. This would allow users and
> > developers to register and integrate custom functions without directly
> > modifying Gandiva's core source code. You can find more details in the
> > issue [1] and the PR [2].
> >
> > Any feedback is appreciated. Thanks.
> >
> > *References:*
> > [1] https://github.com/apache/arrow/issues/37753
> > [2] https://github.com/apache/arrow/pull/37787
> >
> > Regards,
> > Yue Ni
> >
>

Re: [DISCUSS][Gandiva] External function registry proposal

[Antoine Pitrou <an...@python.org>]

Hello,

Being making Gandiva more extensible sounds like a worthwhile improvement.

However, I'm not sure why we would need to choose a JSON-based format 
for this. Instead, I think Gandiva could simply provide the two 
following basic-blocks:

1. a CMake entrypoint (for example a function) making it easy for 
third-party projects to compile their own functions

2. a C++ entrypoint to load a bitcode file with the corresponding 
function definition(s)

The rest of the proposal (a specific JSON file format, a bunch of 
functions to iterate directory entries in a specific layout) is IMHO 
off-topic for Gandiva, and each third-party project can implement their 
own idioms for the discovery of external functions.

I'd add that this should be documented somewhere so that it is generally 
useful, not only for the contributors of the feature.

Also, I hope that this will get more people interested in Gandiva 
maintenance.

Regards

Antoine.


Le 25/09/2023 à 16:17, Yue Ni a écrit :
> Hi there,
> 
> I'd like to initiate a discussion regarding the proposal to introduce
> external function registry support in Gandiva. I've provided a concise
> description of the proposal in the following issue:
> https://github.com/apache/arrow/issues/37753. I welcome any feedback or
> comments on this topic. Please feel free to share your thoughts either here
> on the mailing list or directly within the issue. Thank you for your
> attention and help.
> 
> *Background:*
> Our team has been leveraging Gandiva in our projects, and its performance
> and capabilities have been commendable. However, we've identified a
> constraint concerning the registration of functions. At present, Gandiva
> necessitates that functions be registered directly within its codebase.
> This method, while functional, is not the most user-friendly and presents
> hurdles for those aiming to incorporate third-party functions. Direct
> modifications to Gandiva's source code for such integrations can
> inadvertently introduce maintenance challenges and potential versioning
> conflicts down the line.
> 
> *Proposal:*
> To address this limitation, I propose the introduction of an external
> function registry mechanism in Gandiva. This would allow users and
> developers to register and integrate custom functions without directly
> modifying Gandiva's core source code. You can find more details in the
> issue [1] and the PR [2].
> 
> Any feedback is appreciated. Thanks.
> 
> *References:*
> [1] https://github.com/apache/arrow/issues/37753
> [2] https://github.com/apache/arrow/pull/37787
> 
> Regards,
> Yue Ni
>