MDEV-20022 sql_mode="oracle" does not support TO_NUMBER() function #4128

Olernov · 2025-06-19T05:47:11Z

The Jira issue number for this PR is: MDEV-20022

Description

This patch implements an Oracle style function to_number() with the following signatures:

to_number(number_or_string_subject)
to_number(string_subject, string_format)

The function is implemented in sql/item_numconvfunc.cc.

The function returns the DOUBLE data type for
all signatures and input data types.

The format parser understands the following components:

Digits: 0, 9
Hex digits: X
Group separators: comma (,) and G
Decimal delimiers: period (.) and D
Approximate number signature: EEEE
Currency/numeric flags: $ and B
Currency signatures: C, L, U
Sign signatures: S, MI, PR
Special format signatures: V, TM, TM9, TME
Format flag: FM

Note, the parser was implemented assuming that we'll also have the oppostite function to_char() soon for numeric input. So it parser all known components.
However, the function to_number() does not support:

Formats V, TM, TM9, TME. to_number() returns NULL if the format string has these components. These componens are supported only by to_char() in Oracle.

Features not inclided into this patch:

The ON CONVERSION ERROR clause
The third parameter (nlsparam)
Internationalized components: G, D, C, L, U. These features will be implemented later, under terms of MDEV-36978.

Notable changes in the other files:

item_func.h: adding Item_handled_func::Handler_double
simple_parser.h: adding a number of CONTAINER templates They help to save on duplicate code when creating classes suitable for passing into parsing templates such as OPT, OR2C, OR3C, etc
simple_parser.h: Adding parsing templates OR4C and OR5C
simple_parser.h: Moving the template "OPT" towars the beginning of the file Rule parsing templates TOKEN, TokenChoice, AND2, OR2C, OR3C, OR4C, OR5C, LIST now provide a sub-class Opt, to parse its optional rule.

Release Notes

TODO: What should the release notes say about this change?
Include any changed system variables, status variables or behaviour. Optionally list any https://mariadb.com/kb/ pages that need changing.

How can this PR be tested?

Tests are added to the suite.

If the changes are not amenable to automated testing, please explain why not and carefully describe how to test manually.

Basing the PR against the correct MariaDB version

This is a new feature or a refactoring, and the PR is based against the main branch.
This is a bug fix, and the PR is based against the earliest maintained branch in which the bug can be reproduced.

PR quality check

I checked the CODING_STANDARDS.md file and my PR conforms to this where appropriate.
For any trivial modifications to the PR, I am ok with the reviewer making the changes themselves.

CLAassistant · 2025-06-19T05:47:17Z

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

Olernov

Initial part of review comments. They are mostly cosmetic and intended to simplify the code and improve the readability

sql/simple_parser.h

Olernov · 2025-06-19T06:24:15Z

sql/simple_parser.h

@@ -56,12 +56,283 @@
  storage, like Extended_string_tokenizer.
 */

+/*
+  A future change proposal:


This parser looks like a library, and I believe it should be documented more fully. What steps should a developer take to implement their own new parser based on the simple parser? Currently it's not clear.

It's worth mentioning that the policy-based design is employed, otherwise the code looks very complicated for those not familiar with such an approach

Olernov · 2025-06-19T08:51:35Z

sql/simple_parser.h

+  */
+
+  template<class PARSER, class AParent, class A>
+  class CONTAINER1: public A


The class is named "Container" but what does it contain? If it doesn't contain anything, isn't there a better and more specific name for it? If its primary goal is "to derive the storage from another container" (as a comment says) maybe it should be named StorageDeriver or something?

Olernov · 2025-06-19T08:54:38Z

sql/simple_parser.h

+    Containers to collect parsed data into the goal structures
+
+    CONTAINER1 is needed to derive the storage from another container,
+    but to make a new distinct type.


What are the use cases when one would need such a derivation? How should they employ this class, can you please post an example?

sql/simple_parser.h

sql/item_numconvfunc.cc

Olernov · 2025-06-22T11:46:29Z

sql/item_numconvfunc.cc

+    GRAMMAR: fraction_pDV: fraction_pDV_signature [ fraction_body ]
+
+    GRAMMAR: fraction_pDVCLU: positional_currency [ fraction_body ]
+    GRAMMAR:                | fraction_pDV [ postfix_currency_signature ]


Consider adding some examples here and at other places, it's very hard to imagine how such a grammar is used in format strings

DaveGosselin-MariaDB

Very nice. Please mention somewhere (maybe in simple_parser.h?) that the design pattern employed here is "Policy Based Design" and is described by https://en.wikipedia.org/wiki/Modern_C%2B%2B_Design .

sql/item_numconvfunc.cc

This patch implements an Oracle style function to_number() with the following signatures: - to_number(number_or_string_subject) - to_number(string_subject, string_format) The function is implemented in sql/item_numconvfunc.cc. The function returns the DOUBLE data type for all signatures and input data types. The format parser understands the following components: - Digits: 0, 9 - Hex digits: X - Group separators: comma (,) and G - Decimal delimiers: period (.) and D - Approximate number signature: EEEE - Currency/numeric flags: $ and B - Currency signatures: C, L, U - Sign signatures: S, MI, PR - Special format signatures: V, TM, TM9, TME - Format flag: FM Note, the parser was implemented assuming that we'll also have the oppostite function to_char() soon for numeric input. So it parser all known components. However, the function to_number() does not support: - Formats V, TM, TM9, TME. to_number() returns NULL if the format string has these components. These componens are supported only by to_char() in Oracle. Features not inclided into this patch: - The ON CONVERSION ERROR clause - The third parameter (nlsparam) - Internationalized components: G, D, C, L, U. These features will be implemented later, under terms of MDEV-36978. Notable changes in the other files: - item_func.h: adding Item_handled_func::Handler_double - simple_parser.h: adding a number of *CONTAINER* templates They help to save on duplicate code when creating classes suitable for passing into parsing templates such as OPT, OR2C, OR3C, etc - simple_parser.h: Adding parsing templates OR4C and OR5C - simple_parser.h: Moving the template "OPT" towars the beginning of the file Rule parsing templates TOKEN, TokenChoice, AND2, OR2C, OR3C, OR4C, OR5C, LIST now provide a sub-class Opt, to parse its optional rule.

abarkov · 2025-07-25T07:05:43Z

Closed the PR to avoid buildbot building the same push two times:

one time because it's a bb-* branch
another time because it's a PR

New comments will still go into this PR.

Olernov commented Jun 23, 2025

View reviewed changes

svoj added the MariaDB Corporation label Jun 27, 2025

DaveGosselin-MariaDB self-requested a review July 17, 2025 13:34

DaveGosselin-MariaDB assigned abarkov Jul 17, 2025

abarkov force-pushed the bb-12.0-bar-MDEV-20022 branch 2 times, most recently from 045e38a to 6e753a9 Compare July 18, 2025 12:05

DaveGosselin-MariaDB requested changes Jul 18, 2025

View reviewed changes

abarkov force-pushed the bb-12.0-bar-MDEV-20022 branch 8 times, most recently from 4976593 to 5fea1c6 Compare July 23, 2025 07:38

abarkov force-pushed the bb-12.0-bar-MDEV-20022 branch from 5fea1c6 to 10931c6 Compare July 23, 2025 08:07

abarkov closed this Jul 25, 2025

Uh oh!

MDEV-20022 sql_mode="oracle" does not support TO_NUMBER() function #4128

MDEV-20022 sql_mode="oracle" does not support TO_NUMBER() function #4128

Conversation

Olernov commented Jun 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Release Notes

How can this PR be tested?

Basing the PR against the correct MariaDB version

PR quality check

Uh oh!

CLAassistant commented Jun 19, 2025

Uh oh!

Olernov left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Olernov Jun 19, 2025

Choose a reason for hiding this comment

Uh oh!

Olernov Jun 19, 2025

Choose a reason for hiding this comment

Uh oh!

Olernov Jun 19, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Olernov Jun 22, 2025

Choose a reason for hiding this comment

Uh oh!

DaveGosselin-MariaDB left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

abarkov commented Jul 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Olernov commented Jun 19, 2025 •

edited

Loading

abarkov commented Jul 25, 2025 •

edited

Loading