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

Dates with a year from 1 to 100 are returned as 19xx #1570

Open
sla100 opened this issue Jun 7, 2023 · 6 comments
Open

Dates with a year from 1 to 100 are returned as 19xx #1570

sla100 opened this issue Jun 7, 2023 · 6 comments

Comments

@sla100
Copy link

sla100 commented Jun 7, 2023

  1. What versions are you using?
    6.0.0
  2. Is it an error or a hang or a crash?
    error
  3. What error(s) or behavior you are seeing?
    Dates with a year from 1 to 100 are returned as 19xx
  4. Include a runnable Node.js script that shows the problem.
  const years = [
    9999,
    2023,
    1000,
    100,
    10,
    1,
    -1,
    -10,
    -100
    -1000,
  ];
  const sql = `SELECT ${years.map(year => `DATE '${year}-01-01'`).join(', ')} FROM DUAL`;
  const {rows: [row]} = await conn.execute(sql);
  deepEqual(years, row.map(d=>d.getFullYear()));

The reason is related to the behavior of the Date constructor.

Values from 0 to 99 map to the years 1900 to 1999.

Patch for _makeDate method in lib/settings.js:

if (useLocal) {
  const d = new Date(year, month - 1, day, hour, minute, second, fseconds);
  if (0 < year < 100) {
    d.setFullYear(year);
  }
  return d;
}
@sla100 sla100 added the bug label Jun 7, 2023
@sharadraju
Copy link
Member

@sla100 Thank you for reporting this. We will need to look into this behavior and ascertain if this default (buggy?) behavior of Date constructor has been used by other users.
Since the suggested code change is not in the code for Thin mode, I believe this bug could have existed with earlier versions also. Need to check it out.

@sla100
Copy link
Author

sla100 commented Jun 7, 2023

Since the suggested code change is not in the code for Thin mode, I believe this bug could have existed with earlier versions also. Need to check it out.

Hi. IMHO settings/_makeDate work @6 both with thin and thick mode. In earlier versions, the Date object was created trough the unix-style time value (number) which generated much greater inaccuracies.

@sharadraju
Copy link
Member

sharadraju commented Jun 8, 2023

Thanks for explaining, @sla100.
I am able to replicate the issue. We will decide on adding the fix in 6.0.2 release, based on some testing.

@sharadraju
Copy link
Member

sharadraju commented Jun 13, 2023

@sla100 We are doing some extensive testing on this issue. When we tested, a lot of additional test scenarios/questions are popping up.

  • The Year 0 to 99 case needs to be handled for TIMESTAMP WITH TIMEZONE and TIMESTAMP WITH LOCAL TIMEZONE datatypes also (i.e. when useLocal variable is false in lib\settings.js) and that creates the additional overhead of handling timezone differences
  • How do we handle Year '0' which is an invalid year, in the Oracle Database world?
    ECMAScript uses a proleptic Gregorian calendar, which does not recognize Year 0. In the Proleptic calendar Year 1 AD is preceded by Year 1 BC. So Year 0 is left open to interpretation.
  • In node-oracledb Thick Mode, using Year 0 throws the following error:
    Error: ORA-01841: (full) year must be between -4713 and +9999, and not be 0
  • We also need to handle timezone differences during the transition from 1 AD to 1 BC and vice versa (may not be that urgent or practical for all real-time users)
  • Also, there may be users who are using this 2-digit year interpretation to represent 19xx and 20xx years.

Would love to hear your thoughts! Thanks!

@sla100
Copy link
Author

sla100 commented Jun 14, 2023

Would love to hear your thoughts! Thanks!

My opinion may be unpopular :). For me, the DATE type in Oracle is a collection of six numbers. This is how it is saved in the records and transmitted through the network to the client. The interpretation of these values may be highly context dependent and may not even be "point in time" directly.

For example let's considet that designer specifies that the field is called "RETURN DATE" and the rules say that the goods can be returned by the end of working hours of a given store.
When value in record are DATE ''2023-06-14', the store in Paris is open until 8pm and my phone's time zone is Europe/Athens i want to see "return until 2023-06-14 19:00:00". But only the conjunction of this full knowledge will allow you to display that information.

So I would like the converter to be able to get this tuple of numbers instead of the Date object which the application server's time zone. Something like:

oracledb.fetchTypeHandler = ({dbType}) => {
  if (dbType === oracledb.DB_TYPE_DATE) {
    return {
      parseType: oracledb.RAW,
      converter: ({year, mont, day}) => `${year}-${month}-${day}`, // ISO string, Temporal constructor etc.
    };
  }
};

@sharadraju
Copy link
Member

Thanks @sla100 for the explanation of the date requirements.
We are exploring the option of doing this (retrieving date as a set of numbers) as an enhancement for a future major release.

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

2 participants