Skip to content

Conversation

hazzik
Copy link
Member

@hazzik hazzik commented Sep 25, 2025

In Oracle current_timestamp and systimestamp return TIMESTAMP WITH TIME ZONE type, which in Oracle ManagedDataAccess v23 turns into DateTimeOffset of expected DateTime, so some tests fail.

Switching it to localtimestamp to fix the problem.

@hazzik hazzik changed the title Fix CurrentTimestampSelectString in Oracle9iDialect Fix current_timestamp registration in Oracle9iDialect Sep 25, 2025
@hazzik hazzik changed the title Fix current_timestamp registration in Oracle9iDialect Register localtimestap as current_timestamp in Oracle9iDialect Sep 25, 2025
@fredericDelaporte fredericDelaporte changed the title Register localtimestap as current_timestamp in Oracle9iDialect Register localtimestamp as current_timestamp in Oracle9iDialect Sep 25, 2025
@fredericDelaporte
Copy link
Member

So, using current_timestamp for an update or insert with Oracle was resulting into an UTC date stored in database, eventually converted back to the client current local on read if the typing of the table columns was timestamp with time zone too ; if I understand correctly. While on most other db NHibernate handles, it is a local time timestamp which is stored.

So, that is a bug we are fixing here, but one that would require some data migration for the app affected, wouldn't it?

@hazzik
Copy link
Member Author

hazzik commented Sep 26, 2025

Not sure on this front. I’ll try to add some verifications/test.

If that change is not acceptable, we would need to add workaround to AbstractDateTimeType to be able to read from DateTimeOffset.

@fredericDelaporte
Copy link
Member

As a bug, fixing should always be acceptable. But here, as applications could possibly rely on the effects of this bug to be functional, if that is the case, we need to document it as a possible breaking change.

@hazzik
Copy link
Member Author

hazzik commented Oct 1, 2025

Ok, I've checked and here is the behavior:

So, using current_timestamp for an update or insert with Oracle was resulting into an UTC date stored in database

No.

Both localtimestamp and current_timestamp would be converted to same value:

  • if storing to TIMESTAMP column it would be stored as date time at session time zone.
  • If storing to TIMESTAMP WITH TIME ZONE column it would be stored as date time with session time zone.

So I have following DDL:

create table Entity (
        Id RAW(16) not null,
       Name VARCHAR2(255),
       DT1 TIMESTAMP,
       DT2 TIMESTAMP WITH TIME ZONE,
       primary key (Id)
    )

Then following insert:

INSERT INTO ENTITY (ID, NAME, DT1, DT2)
SELECT SYS_GUID(), 'Bob', localtimestamp, localtimestamp FROM DUAL
UNION ALL
SELECT SYS_GUID(), 'Alice', current_timestamp, current_timestamp FROM DUAL;

And query:

SELECT * FROM ENTITY;

I'm getting following:

image

I tried to do the same in code with NH, and the behavior is the same.

@hazzik
Copy link
Member Author

hazzik commented Oct 1, 2025

To the above: my server is in UTC, and session UTC+10

@hazzik
Copy link
Member Author

hazzik commented Oct 1, 2025

So, I would say that this change is pretty safe and should not be breaking. However, we can say a few words in a probably breaking changes section if we want.

In any way, I would like this to be included in 5.6, especially if we want to treat this as a breaking change

hazzik added a commit to hazzik/nhibernate-core that referenced this pull request Oct 1, 2025
Copy link
Member

@fredericDelaporte fredericDelaporte left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought the union might cause some additional type conversion before inserting and checked with separated insert queries, same results.

@fredericDelaporte fredericDelaporte enabled auto-merge (squash) October 4, 2025 18:27
@fredericDelaporte fredericDelaporte merged commit 9a7761b into nhibernate:master Oct 4, 2025
18 checks passed
@fredericDelaporte fredericDelaporte deleted the oracle-current-timestamp branch October 4, 2025 20:36
@fredericDelaporte fredericDelaporte added this to the 5.6 milestone Oct 4, 2025
@hazzik
Copy link
Member Author

hazzik commented Oct 5, 2025

@fredericDelaporte yeah, I was inserting separately. Just combined query when posting here.

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

Successfully merging this pull request may close these issues.

2 participants