-
Notifications
You must be signed in to change notification settings - Fork 99
Implement missing scalar functions + integration tests for all scalar functions #549
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
Conversation
f82bc3d to
9929001
Compare
Scalar functions are declared in the same namespace (enum) as other tokens, such as COMMA, SPACE, LPARENT, etc. This makes scalar function names conflict with token names. For example, there is a SPACE token and a SPACE scalar function. To avoid naming conflicts, all scalar functions now have an FN_ prefix.
3e41302 to
5d215f2
Compare
|
There seem to be a problem in our staging Cloud environment, it mixes positions of haystack and needle. This should return 7, but it returns 0: However for this it returns 7, but should return 0 (according to the documentation and all other non-staging instances) UPDATE: It turns out that |
The order of parameters in `locate` can be configured, which might cause problems in environments with non-default settings. `position` always work the same in that sense.
5c25fa4 to
a3c9363
Compare
Previously, the driver processed ODBC escape sequences before replacing
ODBC parameter markers (?) with ClickHouse positional parameters. This
caused incorrect parameter ordering for functions whose argument order
differs between the ODBC standard and ClickHouse (for example, LOCATE →
position).
Example (old behavior):
{FN LOCATE(?, ?)}
→ position(?, ?)
→ position({odbc_parameter_1:String}, {odbc_parameter_2:String})
At this point, the original parameter order is lost, making it
impossible to reorder arguments correctly. For LOCATE, the correct
mapping requires the second parameter to come first.
New behavior:
{FN LOCATE(?, ?)}
→ {FN LOCATE({odbc_parameter_1:String}, {odbc_parameter_2:String})}
→ position({odbc_parameter_2:String}, {odbc_parameter_1:String})
This change fixes the issue by replacing ODBC parameter markers before
processing ODBC escape sequences. The parser and lexer were also
extended to support ClickHouse positional parameter syntax.
5e4cf31 to
5a7ab81
Compare
For example:
SELECT {FN INSERT(?, ?, ?, ?)}
Additional add mapping from ODBC's INSERT to ClickHouse's overlayUTF8
fedd495 to
fcd394b
Compare
b47d18a to
7e0e0b1
Compare
Additionally removed unnecessary comment
for functions TIMESTAMPADD and TIMESTAMPDIFF
3dd0ea0 to
7032aa1
Compare
ab18db3 to
cab3dba
Compare
Previously, the parser would produce a partially processed query if it managed to parse part of the function. This led to confusing queries and even more confusing error messages. It was also almost impossible to decipher what the original query was. Now, the parser either parses the entire query or returns the original query unchanged. This will still produce an error message, but at least it will be clear what the original query was, making it easier to debug.
c4c7264 to
b4e5d34
Compare
TODO
{ts ...}escape sequence generatesDateTimevalues without fractional seconds, while other drivers generate timestamps with fractional seconds. For example,SELECT {ts '2024-01-15 10:00:00.500'}generates2024-01-15 10:00:00.500in the SQL Server ODBC Driver, but2024-01-15 10:00:00in the ClickHouse ODBC driver.Notes on the implementation
RANDfunction produces the same number if it is called multiple times within the same query. This limitation can be resolved, but it requires reworking the query parser in the ODBC driver. Additionally, theRANDfunction does not currently support a seed parameter.CURRENT_TIMEandCURTIMEreturn a datetime value (usingnow64()), which binds correctly to ODBC’sSQL_TIME_STRUCT, but can cause unexpected issues in SQL statements that rely on these functions returning only a time value.CONVERTdoes not work with theSQL_TIMEparameter.WEEKfunction generally works, but it does not produce results consistent with other ODBC drivers. Specifically, it uses ClickHouse’s toISOWeek()function, which counts the first and last week of the year only if it has four or more days in that year. Because of this, the results are slightly different from what is expected (see the table below). This can be fixed; however, it would break many existing reports, so the current trade-off is to leave it as is.
TODO
String
Numeric
Date/Time
System
Conversion