[native] Add longVariableConstraints to function metadata

[pdabre12]

Description

Add longVariableConstraints to function metadata.
longVariableConstraints are optional constraints present in the signature which are used to identify the applicable functions in the coordinator.

For eg:
Consider a query like SELECT mod(orderkey, linenumber) FROM lineitem.
While identifying the applicable functions for mod, there is a function signature with decimal args and a decimal return type.

{
        "docString": "presto.default.mod",
        "functionKind": "SCALAR",
        "longVariableConstraints":[
            {
                "expression":"max(i5, i6)",
                "name":"i7"
            },
            {
                "expression":"min(i2 - i6, i1 - i5) + max(i5, i6)",
                "name":"i3"
            }
        ],
        "outputType":"decimal(i3,i7)",
        "paramTypes":["decimal(i1,i5)","decimal(i2,i6)"],
        "routineCharacteristics": {
          "determinism": "DETERMINISTIC",
          "language": "CPP",
          "nullCallClause": "RETURNS_NULL_ON_NULL_INPUT"
        },
        "schema": "default",
        "typeVariableConstraints":[],
        "variableArity":false
      }

 To resolve the variables `i3` and `i7` to valid integers, you use the underlying formula specified in the longVariableConstraints.
 
 To clarify more, 
 For this eg: select mod(decimal(`<i1>` 18,`<i5>` 2), decimal(`<i2>` 38,`<i6>` 2));
 i7 will be resolved to max(i5, i6) = max(2,2) = 2
 i3 will be resolved to min(i2 - i6, i1 - i5) + max(i5, i6) = min(38 - 2, 18 - 2) + max(2,2) = 15 + 2 = 17
 
 The final output type = decimal(i3, i7) = (17,2)

Motivation and Context

When sidecar is enabled, this query fails
"SELECT mod(orderkey, linenumber) FROM lineitem"); with the error: Variable is not bound: i3
The issue arose when identifying applicable functions for the mod operation, as the lack of longVariableConstraints prevented the coordinator from binding variables properly. Without this metadata, the coordinator could not determine how to bind variables to the functions.

This change addresses the issue by adding the necessary longVariableConstraints metadata to the functions, enabling the coordinator to bind the variables correctly using the formulae defined in longVariableConstraints.

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 ==

General Changes

*  (Functions with long constraints) fail in sidecar

[pdabre12]

@ScrapCodes Addressed your comments, PTAL.

[aditi-pandit]

@pdabre12 : Have an overall design question.

[ScrapCodes]

Hi @aditi-pandit and @pdabre12 , I have a basic question - how are these long variable constraint expressed by an end user/ developer?

[aditi-pandit]

@ScrapCodes : The Velox developers add these constraints in function registration. Most of the current usage is for computing precision, scale of the result of arithmetic calculations of decimals

example:
https://github.com/facebookincubator/velox/blob/main/velox/functions/sparksql/DecimalArithmetic.cpp#L608.

[aditi-pandit]

@pdabre12 :This looks good overall. Just wanted you to confirm about the protocol generation.

[aditi-pandit]

Please can you confirm that you built this with the make presto_protocol command, and these are not manual changes.

[pdabre12]

Can confirm that this is built with the make presto_protocol command.

[aditi-pandit]

Thanks @pdabre12

[aditi-pandit]

@jaystarshot : PTAL.

checking tests

[jaystarshot]

I may not have the right context but can you help me understand what the behavior change is (i.e what was happening before and what this fixes) and check which test case to look at to understand that?

[jaystarshot]

What is this testing? Was the query failing before?

[pdabre12]

I was testing function e2e test cases when sidecar is enabled, I ran into this issue there. Draft PR
Before this change, this query was failing
"SELECT mod(orderkey, linenumber) FROM lineitem"); with the error: Variable is not bound: i3.
When identifying the applicable functions from all the available mod functions , because of the lack of the longVariableConstraints, the coordinator did not know how to bind these variables.

This PR adds the necessary longVariableConstraints metadata to functions so coordinator knows how to bind these variables using the formulae defined in longVariableConstraints.

[jaystarshot]

I don't see a @constraint annotation for any mod function? (java implementation)

[pdabre12]

These are functions that are pulled in from the C++ worker.
The constraints are already defined in the C++ function signatures, just adding them to the metadata returned via the \v1\functions endpoint here.

[jaystarshot]

But then why are constraints added in c++ function signature for mod when there is no corresponding definition in java?

[pdabre12]

For java, they are defined here:
https://github.com/prestodb/presto/blob/master/presto-main-base/src/main/java/com/facebook/presto/type/DecimalOperators.java decimalAddOperator() when defining the signatures.

[jaystarshot]

I see so this just passes those constraints to the v1/functions? so this query should still succeed in native coordinator-worker setup but i guess its failing in the sidecar only?

Unrelated but still I am a bit confused by why mod has those constraints in c++ but not java.

[pdabre12]

Yes, it's failing for native execution with sidecar enabled only.
For native execution without sidecar enabled, we still depend on the Java built-in functions for resolution and java Mod function signatures already have the constraints defined, hence we do not run into this issue.

[pdabre12]

Even java mod functions have these constraints , they are added here

presto/presto-main-base/src/main/java/com/facebook/presto/type/DecimalOperators.java

Lines 475 to 485 in 899f740

	TypeSignature decimalRightSignature = parseTypeSignature("decimal(b_precision, b_scale)", ImmutableSet.of("b_precision", "b_scale"));
	TypeSignature decimalResultSignature = parseTypeSignature("decimal(r_precision, r_scale)", ImmutableSet.of("r_precision", "r_scale"));
	return SignatureBuilder.builder()
	.longVariableConstraints(
	longVariableExpression("r_precision", "min(b_precision - b_scale, a_precision - a_scale) + max(a_scale, b_scale)"),
	longVariableExpression("r_scale", "max(a_scale, b_scale)"))
	.argumentTypes(decimalLeftSignature, decimalRightSignature)
	.returnType(decimalResultSignature);
	}

[jaystarshot]

Please add a release note (functions with long constraints) fail in sidecar

[pdabre12]

The failing presto-cpp-macos-build-engine job seems unrelated to the changes.