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

Remove hard-coded function aliasing in ANTLR visitor #1459

Open
alancai98 opened this issue May 8, 2024 · 1 comment
Open

Remove hard-coded function aliasing in ANTLR visitor #1459

alancai98 opened this issue May 8, 2024 · 1 comment
Labels
enhancement New feature or request public API

Comments

@alancai98
Copy link
Member

Relevant Issue/Bug

None

Requested Solution/Feature

Currently in the ANTLR parser visitor, we convert function names matching CHARACTER_LENGTH and CHAR_LENGTH to the function name char_length:

GeneratedParser.CHARACTER_LENGTH, GeneratedParser.CHAR_LENGTH ->
identifierSymbol("char_length", Identifier.CaseSensitivity.INSENSITIVE)

This conversion should not be hard-coded within the ANTLR visitor. Realistically, there should be some alias field within the FnSignature that can specify any name alternatives:

public data class FnSignature(
@JvmField public val name: String,
@JvmField public val returns: PartiQLValueType,
@JvmField public val parameters: List<FnParameter>,
@JvmField public val description: String? = null,
@JvmField public val isDeterministic: Boolean = true,
@JvmField public val isNullable: Boolean = true,
@JvmField public val isNullCall: Boolean = false,
@JvmField public val isMissable: Boolean = true,
@JvmField public val isMissingCall: Boolean = true,
) {

We could follow what Trino does w/ their ScalarFunction definition (i.e. define an alias string array):

https://github.com/trinodb/trino/blob/master/core/trino-spi/src/main/java/io/trino/spi/function/ScalarFunction.java#L32C14-L32C19

With this change, the function resolution would occur at a later stage (rather than during parsing) such as during planning using the specified connector metadata.

Describe Alternatives

  1. Leave as it is? <- Can be problematic for future UDFs that may have aliases.
  2. Some other way to pass along aliases (e.g. previous way to pass along a map of function names to signatures). <- I think adding an alias field to the existing APIs is less confusing for the end user.

Additional Context

Add any other context about the feature request here.

DoD (Definition of Done)

  • Hard-coded mapping of function names in ANTLR visitor is removed
@alancai98 alancai98 added enhancement New feature or request public API labels May 8, 2024
@alancai98
Copy link
Member Author

Also applicable to aggregation functions (e.g. SOME and ANY): https://ronsavage.github.io/SQL/sql-99.bnf.html#some

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request public API
Projects
None yet
Development

No branches or pull requests

1 participant