Improve atan and anan2 sql function for mysql #31924
Conversation
| @@ -50,6 +51,8 @@ public static void registryUserDefinedFunction(final String schemaName, final Sc | |||
| schemaPlus.add("pg_catalog.intervaltonum", ScalarFunctionImpl.create(SQLFederationFunctionUtils.class, "intervalToNum")); | |||
| schemaPlus.add("pg_catalog.gs_password_notifyTime", ScalarFunctionImpl.create(SQLFederationFunctionUtils.class, "gsPasswordNotifyTime")); | |||
| schemaPlus.add("bit_count", ScalarFunctionImpl.create(MySQLBitCountFunction.class, "bitCount")); | |||
| schemaPlus.add("atan", ScalarFunctionImpl.create(SqlFunctions.class, "atan2")); | |||
| schemaPlus.add("atan2", ScalarFunctionImpl.create(SqlFunctions.class, "atan")); | |||
There was a problem hiding this comment.
Is SqlFunctions a built-in class in Caclite? Why doesn't Calcite use the functions in this class by default?
There was a problem hiding this comment.
Yes, SqlFunctions is a built-in class in Calcite that implements the execution logic for atan and atan2 functions. However, in Calcite, the atan function method signature only allows one parameter, while the atan2 function method signature allows two parameters.
MySQL handles the atan function differently depending on the number of parameters: it uses a different execution logic for one parameter (atan) versus two parameters (atan2).
When Calcite encounters a call to the atan function with two parameters, it can find the implementation based on the function name but fails during parameter matching because in Calcite, the atan function can only accept a single parameter.
To resolve this, I added an implementation for the atan2 function alongside atan. Now, at runtime, Calcite can find both implementations of these functions. It will match based on the number of parameters and invoke the appropriate method, thereby calling the atan2 function implementation when two parameters are provided.
In MySQL, the atan2 function can indeed be executed with just one parameter.
Ref: https://github.com/mysql/mysql-server/blob/trunk/sql/item_func.cc#L2906
There was a problem hiding this comment.
That is, they are equivalent in the MySQL dialect. Thanks for clarifying that, this is a great PR.
Ref #31912.
Changes proposed in this pull request:
mysql is compatible with calls to different numbers of parameters
Before committing this PR, I'm sure that I have checked the following options:
./mvnw clean install -B -T1C -Dmaven.javadoc.skip -Dmaven.jacoco.skip -e.