Identifying companion functions

[pramodsatya]

Aggregate functions in Velox register different companion functions with following suffixes: _merge, _merge_extract, extract and _partial. For some aggregates companion functions such as extract, the type is a part of the suffix, whereas for others it is not (eg: approx_percentile_extract_array_smallint vs array_agg_extract). Aggregate companion functions like extract are registered as vector functions whereas other companion functions such as merge_extract are registered as aggregate functions. While working on the presto PR prestodb/presto#22332 to retrieve metadata for presto functions from velox function registries, it was noticed that companion aggregate functions were also returned by the function map. Companion functions are internal to Velox and not applicable in Presto, however there isn't an easier way to identify them in Velox.

Companion functions are currently identified by searching the registered function name for these four suffixes used by companion functions. This approach is used in CoverageUtil.cpp to generate the Velox function coverage map, companion functions are not identified anywhere else in the Velox codebase and this seems to be the only approach currently used.

Starting this discussion to brainstorm approaches to identify companion functions in Velox. Some approaches that have been considered are:

1. Move the existing function isCompanionFunctionName from CoverageUtil.cpp to FunctionRegistry.cpp, and expose this function for external use in FunctionRegistry.h.
2. Instead of searching for the companion function suffixes in the registered function name, this information can be included in the function metadata (currently prototyped in feat: Add companionFunction to function metadata #9250).
3. The suffix used to register companion function can be modified to include a common substring such as velox_companion, as suggested here.

cc: @aditi-pandit @czentgr

[czentgr]

How is option 3 different than what exists today other than the substring is different and more consistent naming?
Other than consistency, it would change the way such a companion function is identified.

Any API exposing the functions and signatures provided by Velox would exclude the companion functions.
Is the option2 function metadata exposed in the API (that Prestissimo or other consumers) would use?

[pramodsatya]

Thanks for the feedback @czentgr. Option 3 would make it simpler to identify companion functions, instead of looking for the multiple suffixes as in Option 1, a single substring search would be enough.
Currently the APIs used to get function signature and function entry, such as getAggregateFunctionEntry, do not exclude companion functions. The function metadata can be obtained from these APIs, once the companion function information is added to the metadata. This approach to retrieve the companion function info from the function's metadata is tested in the linked PR.
Please let me know if any further changes are needed.

[czentgr]

Ok, so I think for the Velox PR. Perhaps exposing the companion flag in the function metadata is not that idea after all. Considering what @aditi-pandit suggested in the Prestissimo PR.

I believe the main proiblem is that Velox as of now itself doesn't have an API to inquire about functions. The prestissimo PR uses two different APIs

FunctionSignatureMap getFunctionSignatures()

from the FunctionRegistry and

getAggregateFunctionSignatures

from Aggregate.cpp in velox::exec.

The respective returns are both maps to different types of signatures

using AggregateFunctionSignatureMap =
    std::unordered_map<std::string, std::vector<AggregateFunctionSignaturePtr>>

using FunctionSignatureMap = std::
    unordered_map<std::string, std::vector<const exec::FunctionSignature*>>;

Windows functions are a subset of aggregate functions.

FunctionSignature and AggregateFunctionSignature are separate entities.

It would be nice to unify this into an API that someone outside of Velox can consume. Currently, it is cobbled together from the various internal usages.

But perhaps only slight modification are needed around the current function maps. Notably, for the aggregate functions there is a new function wrapping the current function to exclude the internally used functions (e.g. the companion functions) based on a flags as was suggested. That way, no filtering needs to be done at the caller (nor does it need an external call to determine an internal function). And the method of exclusion is internal.

[aditi-pandit]

+1 @czentgr.

It would be great if Velox exposed a single API for scalar, aggregate, window functions returning a single style of signature and metadata indicating if its a scalar, aggregate or window. That would make it much simpler in the external service to get this information.

[aditi-pandit]

@kagamiori @mbasmanova : I also wanted some clarifications around companion functions. My understanding was that this is a method of code generation for aggregate functions where all the aggregate function APIs initialize partials, partial -> partial, partial -> final, retract API if one is available are exposed.

But are these queryable in SQL ? We would only want the final aggregate to be queryable.

#4493 leaves me confused.