You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@ignite.apache.org by "tkalkirill (via GitHub)" <gi...@apache.org> on 2026/09/02 12:30:57 UTC
[PR] IGNITE-29033 SQL Calcite: Support UDF and UDTF Overloading [ignite]
tkalkirill opened a new pull request, #13544:
URL: https://github.com/apache/ignite/pull/13544
https://issues.apache.org/jira/browse/IGNITE-29033
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-29033 SQL Calcite: Support UDF and UDTF Overloading [ignite]
Posted by "tkalkirill (via GitHub)" <gi...@apache.org>.
tkalkirill commented on code in PR #13544:
URL: https://github.com/apache/ignite/pull/13544#discussion_r3948794448
##########
modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/schema/IgniteSchema.java:
##########
@@ -94,23 +96,13 @@ public void removeTable(String tblName) {
* @param func SQL function.
*/
public void addFunction(String name, Function func) {
- for (Function existingFun : getFunctions(name)) {
- List<FunctionParameter> params = func.getParameters();
- List<FunctionParameter> existingParams = existingFun.getParameters();
-
- if (params.size() != existingParams.size())
- continue;
-
- for (int i = 0; i < params.size(); ++i) {
- FunctionParameter p = params.get(i);
- FunctionParameter existingP = existingParams.get(i);
+ IgniteTypeFactory typeFactory = Commons.typeFactory();
- if (!p.getType(Commons.typeFactory()).equalsSansFieldNames(existingP.getType(Commons.typeFactory())))
- break;
+ for (Function existingFun : getFunctions(name)) {
+ if (sameParameters(func.getParameters(), existingFun.getParameters(), typeFactory)) {
Review Comment:
I tried this change and checked `SqlUtil.lookupRoutine` directly. Both overloads remain candidates for both `INTEGER` and `INTEGER NOT NULL`, and reversing the candidate order changes which overload is selected.
The earlier integration test was misleading: both methods had the same Java name, so compilation of the generated Java code could resolve the overload again. It did not prove that SQL resolution distinguishes nullability.
I reverted the change and kept the comparison ignoring nullability. Treating int and Integer as the same SQL signature avoids ambiguous selection and additional rules users would need to remember. This limitation is documented in both annotations.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-29033 SQL Calcite: Support UDF and UDTF Overloading [ignite]
Posted by "zstan (via GitHub)" <gi...@apache.org>.
zstan commented on code in PR #13544:
URL: https://github.com/apache/ignite/pull/13544#discussion_r3922137184
##########
modules/calcite/src/test/java/org/apache/ignite/internal/processors/query/calcite/integration/UserDefinedFunctionsIntegrationTest.java:
##########
@@ -94,6 +94,27 @@ public void testSameSignatureNotRegistered() throws Exception {
assertEquals(1, schema.getFunctions("SAMESIGN").size());
}
+ /** */
+ @Test
+ public void testOverloadedFunctions() {
+ client.getOrCreateCache(new CacheConfiguration<Integer, Object>("overloaded-functions")
+ .setSqlSchema("UDF")
+ .setSqlFunctionClasses(OverloadedFunctionsLibrary.class));
+
+ SchemaPlus schema = queryProcessor(client).schemaHolder().schema("UDF");
+
+ assertEquals(2, schema.getFunctions("OVERLOADED").size());
+ assertEquals(2, schema.getFunctions("OVERLOADED_TABLE").size());
+ assertEquals(1, schema.getFunctions("SQL_EQUIVALENT").size());
Review Comment:
I wonder - why ? Behavior is different from java - is it ok ? It can confuse and brings, as minimal, unexpected behavior ? If it calsite related design - plz show me the related links ? And as minimal - i expect it need to be documented somehow ?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-29033 SQL Calcite: Support UDF and UDTF Overloading [ignite]
Posted by "tkalkirill (via GitHub)" <gi...@apache.org>.
tkalkirill merged PR #13544:
URL: https://github.com/apache/ignite/pull/13544
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-29033 SQL Calcite: Support UDF and UDTF Overloading [ignite]
Posted by "zstan (via GitHub)" <gi...@apache.org>.
zstan commented on code in PR #13544:
URL: https://github.com/apache/ignite/pull/13544#discussion_r3922137184
##########
modules/calcite/src/test/java/org/apache/ignite/internal/processors/query/calcite/integration/UserDefinedFunctionsIntegrationTest.java:
##########
@@ -94,6 +94,27 @@ public void testSameSignatureNotRegistered() throws Exception {
assertEquals(1, schema.getFunctions("SAMESIGN").size());
}
+ /** */
+ @Test
+ public void testOverloadedFunctions() {
+ client.getOrCreateCache(new CacheConfiguration<Integer, Object>("overloaded-functions")
+ .setSqlSchema("UDF")
+ .setSqlFunctionClasses(OverloadedFunctionsLibrary.class));
+
+ SchemaPlus schema = queryProcessor(client).schemaHolder().schema("UDF");
+
+ assertEquals(2, schema.getFunctions("OVERLOADED").size());
+ assertEquals(2, schema.getFunctions("OVERLOADED_TABLE").size());
+ assertEquals(1, schema.getFunctions("SQL_EQUIVALENT").size());
Review Comment:
I wonder - why ? Behavior is different from java - is it ok ? It can confuse and brings, as minimal, unexpected behavior ? If it calcite related design - plz show me the related links ? And as minimal - i expect it need to be documented somehow ?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-29033 SQL Calcite: Support UDF and UDTF Overloading [ignite]
Posted by "zstan (via GitHub)" <gi...@apache.org>.
zstan commented on code in PR #13544:
URL: https://github.com/apache/ignite/pull/13544#discussion_r3935285371
##########
modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/schema/IgniteSchema.java:
##########
@@ -94,23 +96,13 @@ public void removeTable(String tblName) {
* @param func SQL function.
*/
public void addFunction(String name, Function func) {
- for (Function existingFun : getFunctions(name)) {
- List<FunctionParameter> params = func.getParameters();
- List<FunctionParameter> existingParams = existingFun.getParameters();
-
- if (params.size() != existingParams.size())
- continue;
-
- for (int i = 0; i < params.size(); ++i) {
- FunctionParameter p = params.get(i);
- FunctionParameter existingP = existingParams.get(i);
+ IgniteTypeFactory typeFactory = Commons.typeFactory();
- if (!p.getType(Commons.typeFactory()).equalsSansFieldNames(existingP.getType(Commons.typeFactory())))
- break;
+ for (Function existingFun : getFunctions(name)) {
+ if (sameParameters(func.getParameters(), existingFun.getParameters(), typeFactory)) {
Review Comment:
If you will use non ignoring nullability comparison:
```
if (!paramType.equalsSansFieldNames(existingParamType))
return false;
```
Seems you can change behavior as it was in java and reduce further overloading missing errors
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-29033 SQL Calcite: Support UDF and UDTF Overloading [ignite]
Posted by "tkalkirill (via GitHub)" <gi...@apache.org>.
tkalkirill commented on code in PR #13544:
URL: https://github.com/apache/ignite/pull/13544#discussion_r3923738230
##########
modules/calcite/src/test/java/org/apache/ignite/internal/processors/query/calcite/integration/UserDefinedFunctionsIntegrationTest.java:
##########
@@ -94,6 +94,27 @@ public void testSameSignatureNotRegistered() throws Exception {
assertEquals(1, schema.getFunctions("SAMESIGN").size());
}
+ /** */
+ @Test
+ public void testOverloadedFunctions() {
+ client.getOrCreateCache(new CacheConfiguration<Integer, Object>("overloaded-functions")
+ .setSqlSchema("UDF")
+ .setSqlFunctionClasses(OverloadedFunctionsLibrary.class));
+
+ SchemaPlus schema = queryProcessor(client).schemaHolder().schema("UDF");
+
+ assertEquals(2, schema.getFunctions("OVERLOADED").size());
+ assertEquals(2, schema.getFunctions("OVERLOADED_TABLE").size());
+ assertEquals(1, schema.getFunctions("SQL_EQUIVALENT").size());
Review Comment:
This behavior intentionally differs from Java because overload resolution uses SQL types. Both int and Integer correspond to SQL INTEGER, so Calcite cannot reliably distinguish these overloads and their resolution would depend on registration order.
Therefore, overloads must have different SQL parameter types or parameter order. I’ve also documented this behavior in QuerySqlFunction and QuerySqlTableFunction.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-29033 SQL Calcite: Support UDF and UDTF Overloading [ignite]
Posted by "zstan (via GitHub)" <gi...@apache.org>.
zstan commented on code in PR #13544:
URL: https://github.com/apache/ignite/pull/13544#discussion_r3965037104
##########
modules/calcite/src/test/java/org/apache/ignite/internal/processors/query/calcite/integration/UserDefinedFunctionsIntegrationTest.java:
##########
@@ -94,6 +94,27 @@ public void testSameSignatureNotRegistered() throws Exception {
assertEquals(1, schema.getFunctions("SAMESIGN").size());
}
+ /** */
+ @Test
+ public void testOverloadedFunctions() {
Review Comment:
ok, lets align this test with already existing, i mean : append LogListener - check plz ?
```
public void testSameSignatureNotRegistered() throws Exception {
LogListener logChecker = LogListener.matches("Unable to register function 'SAMESIGN'. Other function " +
"with the same name and parameters is already registered").build();
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org