oracle / node-oracledb

Oracle Database driver for Node.js maintained by Oracle Corp.
http://oracle.github.io/node-oracledb/
Other
2.24k stars 1.07k forks source link

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

Open sla100 opened 1 year ago

sla100 commented 1 year ago
  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;
}
sharadraju commented 1 year ago

@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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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

Would love to hear your thoughts! Thanks!

sla100 commented 1 year ago

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 commented 1 year ago

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.