Skip to content

timestamp in cursor comparison > expect() has problem with precision / nls #771

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

Closed
MalchiasTS opened this issue Oct 31, 2018 · 7 comments
Closed

Comments

@MalchiasTS
Copy link

In our project we use a lot of timestamp(6) columns. Our calls to ut.expect() run into problems with comparisons between sys_refcursors that contain timestamp(6) columns.

I prepared three little attachments that should help demonstrate the problem and show our current solution. But this solution is not perfect, as it only fixes the problem for our timestamp(6) columns and we would like to see a more general solution.

If you can help in any way, that would be great.

Attachments

  1. setup.sql - creates one table and two packages designed to demonstrate the problem.
  2. problem_demo.sql - shows our NLS_SESSION_PARAMETERS, starts the ut-test and quotes the fail message we get from this unit test run ... if we do not patch the current UT3 installation
  3. our_solution_so_far.txt - describes how we got around the problem with a very small patch, but we lack the utplsql knowlege to find a general fix, so we ask you for help.

setup.sql - creates one table and two packages designed to demonstrate the problem

/*
    name
        setup.sql
    version
        1.0, 2018-10-31
    author
        MalachiasTS

    purpose
        Build a table and two packages in the current schema 
        which are needed to demonstrate a problem with the 
        comparison timestamp columns in sys_refcursors.

    creates
        tables
            TMP_NLS_TEST
        packages
            P_NLS_TEST
            P_NLS_TEST_UT
*/

--drop table tmp_nls_test;

create table tmp_nls_test (
    id number,
	ats3 timestamp (3),
	ats6 timestamp (6),
	ats9 timestamp (9)
);

insert into tmp_nls_test (id, ats3, ats6, ats9) 
values (1, systimestamp, systimestamp, systimestamp);
commit;

--drop package p_nls_test;

create or replace package p_nls_test
is
    procedure insert_into_nls_test (
        i_id         tmp_nls_test.id%type,
        i_timestamp3 tmp_nls_test.ats3%type,
        i_timestamp6 tmp_nls_test.ats6%type,
        i_timestamp9 tmp_nls_test.ats9%type
    );
end p_nls_test;
/

create or replace package body p_nls_test
as
    procedure insert_into_nls_test (
        i_id         tmp_nls_test.id%type,
        i_timestamp3 tmp_nls_test.ats3%type,
        i_timestamp6 tmp_nls_test.ats6%type,
        i_timestamp9 tmp_nls_test.ats9%type
    )
    is
    begin
        insert into tmp_nls_test (id, ats3, ats6, ats9) 
            values (i_id, i_timestamp3, i_timestamp6, i_timestamp9);

        --commit;
    end insert_into_nls_test;
end p_nls_test;
/

--drop package p_nls_test_ut;

create or replace package p_nls_test_ut
is
    -- %suite(nls_test)
    -- %suitepath(nls_test)

    -- %test(check that inserts into tmp_nls_test work correctly)
    procedure ut_insert_into_nls_test;
end p_nls_test_ut;
/

create or replace package body p_nls_test_ut
is
    procedure ut_insert_into_nls_test
    is
        lv_new_id number;
        lv_ts timestamp(9);

        lc_exp sys_refcursor;
        lc_act sys_refcursor;
    begin
        -- get new id
        select max(id)+1
        into lv_new_id
        from tmp_nls_test;

        -- get a valid timestamp with timezone
        select systimestamp
        into lv_ts
        from dual;

        -- insert a record with these values
        p_nls_test.insert_into_nls_test (lv_new_id, null, lv_ts, null);


        -- build two cursors
        ut.set_nls();

        -- fill cursor with expected values by selecting from dual
        open lc_exp for
            select
                lv_new_id           as id, 
                to_timestamp (null) as ats3, 
                lv_ts               as ats6,  
                to_timestamp (null) as ats9
            from dual;

        -- fill cursor with actual table values.
        open lc_act for
            select id, ats3, ats6, ats9
            from tmp_nls_test
            where id = 2;
            
        ut.reset_nls();

        -- compare variable with table values
        ut.expect (lc_act).to_equal (lc_exp);
        
    end ut_insert_into_nls_test;
end p_nls_test_ut;
/

problem_demo.sql - shows our NLS_SESSION_PARAMETERS, starts the ut-test and quotes the fail message we get from this unit test run ... if we do not patch the current UT3 installation.

/*
select * from NLS_SESSION_PARAMETERS;

PARAMETER               VALUE
----------------------------------------------------------
NLS_LANGUAGE            AMERICAN
NLS_TERRITORY           AMERICA
NLS_CURRENCY            $
NLS_ISO_CURRENCY        AMERICA
NLS_NUMERIC_CHARACTERS  .,
NLS_CALENDAR            GREGORIAN
NLS_DATE_FORMAT         yyyy-mm-dd hh24:mi:ss
NLS_DATE_LANGUAGE       AMERICAN
NLS_SORT                BINARY
NLS_TIME_FORMAT         HH.MI.SSXFF AM
NLS_TIMESTAMP_FORMAT    yyyy-mm-dd hh24:mi:ssxff
NLS_TIME_TZ_FORMAT      HH.MI.SSXFF AM TZR
NLS_TIMESTAMP_TZ_FORMAT yyyy-mm-dd hh24:mi:ssxff tzh:tzm
NLS_DUAL_CURRENCY       $
NLS_COMP                BINARY
NLS_LENGTH_SEMANTICS	BYTE
NLS_NCHAR_CONV_EXCP     FALSE
*/

begin
    ut.run ('p_nls_test_ut');
end;
/

/*
DBMS OUTPUT:

nls_test
  nls_test
    check that inserts into tmp_nls_test work correctly [.149 sec] (FAILED - 1)
 
Failures:
 
  1) ut_insert_into_nls_test
      Actual: refcursor [ count = 1 ] was expected to equal: refcursor [ count = 1 ]
      Diff:
      Rows: [ 1 differences ]
        Row No. 1 - Actual:   <ATS6>2018-10-31T17:11:51.150893</ATS6>
        Row No. 1 - Expected: <ATS6>2018-10-31T17:11:51.150893000</ATS6>
      at "<schema>.P_NLS_TEST_UT.UT_INSERT_INTO_NLS_TEST", line 46 ut.expect (lc_act).to_equal (lc_exp);
      
       
Finished in .15082 seconds
1 tests, 1 failed, 0 errored, 0 disabled, 0 warning(s)
 */

our_solution_so_far.txt - describes how we got around the problem with a very small patch, but we lack the utplsql knowlege to find a general fix, so we ask you for help.

What fixed it for us was ...

We patched two rows in the header of package UT_UTILS:

line 120:

    gc_timestamp_format         constant varchar2(100) := 'yyyy-mm-dd"T"hh24:mi:ssxff';
->: gc_timestamp_format         constant varchar2(100) := 'yyyy-mm-dd"T"hh24:mi:ssxff6';    -- patched

line 121:

    gc_timestamp_tz_format      constant varchar2(100) := 'yyyy-mm-dd"T"hh24:mi:ssxff tzh:tzm';
->: gc_timestamp_tz_format      constant varchar2(100) := 'yyyy-mm-dd"T"hh24:mi:ssxff6 tzh:tzm';  -- patched

This patch only fixes it for timestamp(6).
Lucky for us, if we use timestamp types at all, then we always use TIMESTAMP(6) ... presently.

What would be a general solution?

@jgebal
Copy link
Member

jgebal commented Oct 31, 2018

Hi @MalchiasTS
Thank you for submitting the issue, detailed steps to reproduce and proposed workaround.
The problem you're facing is related to how Oracle binds PLSQL variables in SQL statements.

SQL and PLSQL engine are separate and it seems like Oracle is passing timestamp variables as variables with maximum precission 9 rather than passing them with declared precision.

The proposed fix will only work for timestamp with precision = 6 and will not work for any lower precision.
It will also cut precision of any timestamp above 6 to just 6 digits. This might lead to false-positive test results.

I have updated your example to get it to work without any changes to framework.

create or replace package body p_nls_test_ut
is
    procedure ut_insert_into_nls_test
    is
        lv_new_id number;
        lv_ts timestamp(9);

        lc_exp sys_refcursor;
        lc_act sys_refcursor;
    begin
        -- get new id
        select max(id)+1
        into lv_new_id
        from tmp_nls_test;

        -- get a valid timestamp with timezone
        select systimestamp
        into lv_ts
        from dual;

        -- insert a record with these values
        p_nls_test.insert_into_nls_test (lv_new_id, null, lv_ts, null);


        -- build two cursors
        ut.set_nls();

        -- fill cursor with expected values by selecting from dual
        open lc_exp for
            select
                lv_new_id           as id, 
                to_timestamp (null) as ats3, 
                cast(lv_ts as timestamp(6))              as ats6,  
                to_timestamp (null) as ats9
            from dual;

        -- fill cursor with actual table values.
        open lc_act for
            select id, ats3, ats6, ats9
            from tmp_nls_test
            where id = 2;
            
        ut.reset_nls();

        -- compare variable with table values
        ut.expect (lc_act).to_equal (lc_exp);
        
    end ut_insert_into_nls_test;
end p_nls_test_ut;
/

begin
    ut.run ('p_nls_test_ut');
end;
/
nls_test
  nls_test
    check that inserts into tmp_nls_test work correctly [.032 sec]
 
Finished in .034613 seconds
1 tests, 0 failed, 0 errored, 0 disabled, 0 warning(s)

The changed part is use of CAST on the PLSQL bind variable that is passed to SQL statement:

            select
                lv_new_id           as id, 
                to_timestamp (null) as ats3, 
                cast(lv_ts as timestamp(6))              as ats6,  
                to_timestamp (null) as ats9
            from dual;

Please let me know if the solution above, is what you were seeking for.

To make some additional investigation I've done a check using DBMS_SQL.DESCRIBE_COLUMNS.

DECLARE
  lv_ts  timestamp(6);
  lc_exp sys_refcursor;

  c           NUMBER;
  d           NUMBER;
  col_cnt     INTEGER;
  f           BOOLEAN;
  rec_tab     DBMS_SQL.DESC_TAB;
  col_num    NUMBER;
  PROCEDURE print_rec(rec in DBMS_SQL.DESC_REC) IS
  BEGIN
    DBMS_OUTPUT.NEW_LINE;
    DBMS_OUTPUT.PUT_LINE('col_type            =    '
                         || rec.col_type);
    DBMS_OUTPUT.PUT_LINE('col_maxlen          =    '
                         || rec.col_max_len);
    DBMS_OUTPUT.PUT_LINE('col_name            =    '
                         || rec.col_name);
    DBMS_OUTPUT.PUT_LINE('col_name_len        =    '
                         || rec.col_name_len);
    DBMS_OUTPUT.PUT_LINE('col_schema_name     =    '
                         || rec.col_schema_name);
    DBMS_OUTPUT.PUT_LINE('col_schema_name_len =    '
                         || rec.col_schema_name_len);
    DBMS_OUTPUT.PUT_LINE('col_precision       =    '
                         || rec.col_precision);
    DBMS_OUTPUT.PUT_LINE('col_scale           =    '
                         || rec.col_scale);
    DBMS_OUTPUT.PUT('col_null_ok         =    ');
    IF (rec.col_null_ok) THEN
      DBMS_OUTPUT.PUT_LINE('true');
    ELSE
      DBMS_OUTPUT.PUT_LINE('false');
    END IF;
  END;
BEGIN
  lv_ts := systimestamp;
  
  open lc_exp for 
    select
        lv_ts as ats9, 
        cast(lv_ts as timestamp(6)) as ats6
    from dual;

  c := DBMS_SQL.TO_CURSOR_NUMBER(lc_exp);
  
  DBMS_SQL.DESCRIBE_COLUMNS(c, col_cnt, rec_tab);

  col_num := rec_tab.first;
  IF (col_num IS NOT NULL) THEN
    LOOP
      print_rec(rec_tab(col_num));
      col_num := rec_tab.next(col_num);
      EXIT WHEN (col_num IS NULL);
    END LOOP;
  END IF;
 
  DBMS_SQL.CLOSE_CURSOR(c);
END;
/

It gives:

col_type            =    180
col_maxlen          =    11
col_name            =    ATS9
col_name_len        =    4
col_schema_name     =    
col_schema_name_len =    0
col_precision       =    0
col_scale           =    9
col_null_ok         =    true

col_type            =    180
col_maxlen          =    11
col_name            =    ATS6
col_name_len        =    4
col_schema_name     =    
col_schema_name_len =    0
col_precision       =    0
col_scale           =    6
col_null_ok         =    true

So you can see that SCALE of variable without CAST = 6.

@jgebal
Copy link
Member

jgebal commented Oct 31, 2018

Perhaps we should update documentation with this note as it seems something that might be significant.

@MalchiasTS
Copy link
Author

That solves it for us. Thank you very much. Outstanding work - as usual :-)

@emanueol
Copy link

emanueol commented Nov 1, 2018

Hi, why not simply change declaration:
lv_ts timestamp(9);

into:
lv_ts timestamp(6);

or better define pl/sql variable the same as table:
lv_ts tmp_nls_test.ats6%type;

@MalchiasTS
Copy link
Author

MalchiasTS commented Nov 1, 2018

Hi @emanueol

to answer your questions: Because - strangely enough - neither of your suggestions solve the problem. Both will lead to the failure I described. Only the cast (lv_ts as timestamp(6)) actually does the trick.

@jgebal
Copy link
Member

jgebal commented Nov 17, 2018

I have a second thought on this.
Maybe we should treat all as TIMESTAMP(9) and ignore defined precision when comparing data.

  • What we win is that users don't need to use CAST when passing timestamp variable
  • What we loose is ability to detect datatype precision differences

If we force users to use CAST however, we don't compare the actual data-type precision but the result of cast operation. Also, for data-types like NUMBER/VARCHAR we only look at the actual data, not at precision or scale of variables.

So it seems that the fix could be as simple as change of one line in ut_utils, similar to what was originally suggested.

FROM:

  gc_timestamp_format         constant varchar2(100) := 'yyyy-mm-dd"T"hh24:mi:ssxff';

TO:

  gc_timestamp_format         constant varchar2(100) := 'yyyy-mm-dd"T"hh24:mi:ssxff9'; 

Note that that would be ff9, not like originally proposed ff6, so we can always compare with max precision.

@MalchiasTS , @Shoelace @emanueol - any thoughts?

@jgebal jgebal reopened this Nov 17, 2018
@jgebal
Copy link
Member

jgebal commented Nov 18, 2018

After a bit of investigation and testing I have realized, it is not doable in PLSQL.
Setting explicit format to ...ff9 does not help, Oracle still converts bind variables into timestamp with specific precision and there is nothing that can be done about it on the framework side.

The only thing to be done is to raise a defect/bug to Oracle Support to get that resolved.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants