Some basic datatypes and functions in oracle compatibility mode do not work
(MDEV-19162)
|
|
| Status: | Closed |
| Project: | MariaDB Server |
| Component/s: | Server |
| Affects Version/s: | None |
| Fix Version/s: | 10.6.1 |
| Type: | Technical task | Priority: | Major |
| Reporter: | Faisal Saeed (Inactive) | Assignee: | Michael Widenius |
| Resolution: | Fixed | Votes: | 0 |
| Labels: | Compatibility | ||
| Attachments: |
|
||||||||||||||||||||
| Issue Links: |
|
||||||||||||||||||||
| Description |
|
This function is overloaded and supports NUMBER, DATE, DATETIME, TIMESTAMP etc as parameters and returns a formatted/converted TEXT value https://docs.oracle.com/cd/B28359_01/olap.111/b28126/dml_functions_2113.htm#OLADM691 This is currently not available under ORACLE compatibility mode. |
| Comments |
| Comment by woqutech [ 2021-01-05 ] | |
|
Here is my implementation, please reivew the patch in attachment, thanks!
Compared with the oracle implementation, there are the following limitations:
| |
| Comment by Michael Widenius [ 2021-01-05 ] | |
|
Some quick comments:
An example of the changes suggested above: else { return 1; }-> append_with_zero_prefill(str, l_time->second, 2); Replace I don't see why this code is needed. Either remove or add a comment: + if (size < MAX_DATE_STRING_REP_LENGTH) Note that val_str() may return a string that is not null terminated, which means that the format argument to | |
| Comment by Michael Widenius [ 2021-01-05 ] | |
|
Some comments from Alexander Barkov: | |
| Comment by woqutech.com [ 2021-01-06 ] | |
| |
| Comment by woqutech.com [ 2021-01-13 ] | |
|
Based on the review comments, I made changes. The main changes in this patch(10.6-to_char_01_13.patch) are: 1. An error code and error message have been added to indicate that error for oracle compatible function . If an error occurs during the execution of TO_CHAR, set warning and return NULL | |
| Comment by Michael Widenius [ 2021-01-24 ] | |
|
Answer to your previous question: "In MariaDB we should not just abort a statement (except in strict mode) as this may cause problems for non transactional tables? Yes, in not strict mode, functions that gets wrong input should return NULL. This is to make statements like the following repeatable for non In strict mode is set to STRICT_TRANS_TABLES then the above will generate an | |
| Comment by Michael Widenius [ 2021-01-25 ] | |
|
Reviewing new patch:
-In the test suite, use --error ER_WRONG_PARAMCOUNT_TO_NATIVE_FCT instead of +static bool check_quotation(const short cfmt, bool *quotation_flag) + Can be written a bit shorter: static bool check_quotation(const char cfmt, bool *quotation_flag) return *quotation_flag; +static int parse_sepcial(char cfmt, const char *ptr, const char *end, const short *arr) parse_sepcial -> parse_special + /* This is a more efficent version (which also fixes a bug where the code tried if (cfmt == '&' && ptr + 1 < end) { tmp1= my_toupper(system_charset_info, *(beg+1)); if (tmp1 >= 'A' && tmp1 <= 'Z') return 0; }+ /* + break; The above could be written as just: while (!INVALID_CHARACTER(tmp1) && tmp1 != '"'); .... +bool Item_func_tochar::parse_format_string(const String *format, uint *fmt_len) cftm and ulen should be moved to the following for() loop as they are bool Item_func_tochar::parse_format_string(const String *format, uint *fmt_len) 9 should be changed to 'max_month_name_length', to be calculated in + if (my_toupper(system_charset_info, *(ptr+2)) != 'Y') 9 should be changed to 'max_day_name_length', to be calculated in ... I noticed you have two almost identical calls to parse_format_string(), + if (str->length() > MAX_DATETIME_FORMAT_MODEL_LEN) + It's better to move the checking and setting the warning message to Another thing is that parse_format_string may access characters after +String* Item_func_tochar::val_str(String* str) + if (warning_message.length()) The above is ok for fixed length format strings but not for dynamic ones. push_warning_printf(thd, if (str->alloc(max_length)) The above is not good for the case of !fixed_length as this length is + * To prevent conflict with the ascii code of the separator, such as: ':', '%', '?' As we are storing the format in as a short int, I changed FMT_BASE to 128. All the above changes are done in my proposed patch for 10.6. Other thing done in the patch:
I had problem with one expression: TO_CHAR(c3, 'YY&&& In my new code, I got I checked with Oracle 18c at dbfiddle.uk and Oracle gave same answer as my I added also to the error message "Oracle compatibility function error: date format not recognized" a few characters from where things went wrong to make it easier to find errors: |