Skip to content
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

New Rule: date conversions with format model #40

Closed
silviomarghitola opened this issue Jun 3, 2019 · 8 comments
Closed

New Rule: date conversions with format model #40

silviomarghitola opened this issue Jun 3, 2019 · 8 comments
Assignees
Labels
enhancement New feature or request
Milestone

Comments

@silviomarghitola
Copy link

silviomarghitola commented Jun 3, 2019

** Language Usage / General** or Language Usage / Variables & Types / General
Always use a format model in the functions TO_DATE and TO_TIMESTAMP

@PhilippSalvisberg
Copy link
Collaborator

from SonarQube

"TO_NUMBER" should be used with a format model

The TO_NUMBER function is converting the value of BINARY_FLOAT, BINARY_DOUBLE, CHAR, VARCHAR2, NCHAR, or NVARCHAR2 datatype to a value of NUMBER datatype.

Without providing the format of the input, TO_NUMBER will consider the provided value is compliant with the default format. Relying on a default configuration is a source of error because it creates a dependency between the code and the configuration of the ORACLE server where this code is deployed.

The behaviour of the TO_NUMBER function will certainly not be the expected one if the configuration of the ORACLE server is changing.

Noncompliant Code Example

The following code with return 0 on an ORACLE server running with its default US configuration with p_string = "2,540"

IF ( TO_NUMBER(p_string) >= 0 and TO_NUMBER(p_string) <= TO_NUMBER('50.00') ) THEN
  RETURN 1;
ELSE
  RETURN 0;
END IF;

Compliant Solution

The following code with return 1 on an ORACLE server running with its default FR configuration with p_string = "2,540" because the comma will be interpreted as decimal separator instead of thousand separator.

IF ( TO_NUMBER(p_string, '99.99') >= 0 and TO_NUMBER(p_string, '99.99') <= TO_NUMBER('50.00','99.99') ) THEN
  RETURN 1;
ELSE
  RETURN 0;
END IF;

@PhilippSalvisberg
Copy link
Collaborator

from SonarQube:

"TO_DATE" and "TO_TIMESTAMP" should be used with a datetime format model

The TO_DATE and TO_TIMESTAMP functions are converting char of CHAR, VARCHAR2, NCHAR, or NVARCHAR2 datatype to respectively a value of DATE or TIMESTAMP datatype.

Without providing the format of the input char, TO_DATE will consider the provided char is compliant with the default date format. Relying on a default configuration is a source of error because it creates a dependency between the code and the configuration of the ORACLE server where this code is deployed.

According to the Oracle's documentation "the default date format is determined implicitly by the NLS_TERRITORY initialization parameter or can be set explicitly by the NLS_DATE_FORMAT parameter.". It means the behaviour of the TO_DATE function will certainly not be the expected one if the configuration of the ORACLE server is changing.

Noncompliant Code Example

SELECT TO_DATE( 'January 15, 2018, 11:00 A.M.')
FROM DUAL;

Compliant Solution

SELECT TO_DATE( 'January 15, 2018, 11:00 A.M.', 'Month dd, YYYY, HH:MI A.M.')
FROM DUAL;

@PhilippSalvisberg
Copy link
Collaborator

I agree for to_date and to_timestamp. But I do not agree for to_number, since to_number can deal with various input formats and produce a sensitive result.

@PhilippSalvisberg PhilippSalvisberg changed the title New Rule: string to number or date conversions with format model New Rule: date conversions with format model Jun 4, 2019
@PhilippSalvisberg PhilippSalvisberg added the enhancement New feature or request label Jun 4, 2019
@PhilippSalvisberg PhilippSalvisberg added this to the v4.0 milestone Jun 4, 2019
@rogertroller
Copy link
Collaborator

I would extend the rule towards:
to_[whatever] should be used with a default on conversion error clause.

@PhilippSalvisberg
Copy link
Collaborator

PhilippSalvisberg commented Jun 6, 2019

Good idea regarding the DEFAULT ... ON CONVERSION ERROR clause. Based on the SQL Rerference Manual for 12.2 the following conversion functions have such a clause:

  • TO_BINARY_DOUBLE
  • TO_BINARY_FLOAT
  • TO_DATE
  • TO_DS_INTERVAL
  • TO_NUMBER
  • TO_TIMESTAMP
  • TO_TIMESTAMP_TZ
  • TO_YMITERVAL
  • CAST (added)

I suggest to handle this in and additional rule.

I'd keep the scope of this rule to all conversions which rely on a database (NLS) settings. Based on that, it is necessary to define a format also for TO_NUMBER (yes, I changed my mind). Here's a list of functions that would require a format parameter:

  • Numbers (e.g. Germany uses a comma , as decimal point. Switzerland uses a dot . as decimal point):
    • TO_BINARY_DOUBLE
    • TO_BINARY_FLOAT
    • TO_NUMBER
  • Characters (different results depending on NLS settings, relevant for dates and numbers as input)
    • TO_CHAR
    • TO_NCHAR
  • Dates/Timestamps (various common format per country)
    • TO_DATE
    • TO_TIMESTAMP
    • TO_TIMESTAMP_TZ
  • Intervals (e.g. different format for hours)
    • TO_DSINTERVAL
    • TO_YMINTERVAL

Summary: I suggest to create based on this issue the following two rules:

  • Always use a format model in conversion functions that rely on NLS settings
  • Try to define a default value on conversion errors

@rogertroller
Copy link
Collaborator

CAST has it also...

@PhilippSalvisberg
Copy link
Collaborator

Thanks @rogertroller , I've added it in the comment above.

@kibeha
Copy link
Collaborator

kibeha commented Oct 7, 2020

One problem concerning TO_NUMBER is that there is no way to specify with a format model the same functionality as if not specifying format model.

See details in section "Usecase" here: https://community.oracle.com/tech/apps-infra/discussion/4404178/parameters-should-accept-keyword-default-to-indicate-we-want-same-behaviour-as-if-parameter-was-not

For the datetime TO_* functions, not specifying format model means using the format model from NLS settings (as well as utilizing the other various NLS parameters to decide how to interpret the elements of the format model.)

For TO_NUMBER, not specifying format model means a default of "any number of digits and a possible decimal separator", where the decimal separator is dependent on session NLS setting (and you can't use the third parameter of TO_NUMBER to specify NLS_NUMERIC_CHARACTERS without specifying a format model.)

If we say to always use format model for TO_NUMBER, that requires using a format model with many many 9's in it to emulate the same functionality as we get by using TO_NUMBER without format model.

I think I am going for multiple rules here:

  • Always use a format model in date/time conversion functions that rely on NLS settings
  • Try to use a format model in TO_NUMBER (with exceptions if you can control decimal character and do not know maximum number of digits)
  • Try to define a default value on conversion errors

(I'm adding a section under Language Usage called Function Usage - there's potential for more rules there ;-))

@kibeha kibeha closed this as completed in fb8d903 Oct 21, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

4 participants