History of datetime_cast_sql() in Oracle

[Shai Berger]

Hi,

In my quest to solve Oracle bugs, I found the function datetime_cast_sql() in
the Oracle backend. It causes a problem these days, because it gets in the way
of querying a date as string (mydate__startswith='2005'). It is a function
that is defined as a no-op in the base backend, and overridden, among core
backends, only in Oracle. Removing the override, currently, just makes one
more test pass, and if we decide to remove it from the base, code in several
other places may become simpler (will not need to special-case datetime
values).

I was able to track the history of this function, into the mists of time. In
the beginning, code to cast strings into dates for Oracle was mixed in with
the general querying code. Then came the boulder-oracle-sprint of 2006-2007,
where, at some point, that piece of code was factored out to a module-level
function get_datetime_cast_sql(), which was defined in all backends as a no-op
except Oracle; this was later refactored into the DatabaseOperations class we
know today.

However, at about the same time that the DatabaseOperations was created, the
Oracle backend made another change: It started setting the session's datetime
format on login. This, as far as I understand, makes the special casting
operation redundant -- and current test results support my understanding.

So -- I want to fix, now, the thing that was, well, not broken, but bent, in
2007. And my question to you -- especially, those of you who participated in
the boulder sprint -- can you think of any reason why I shouldn't?

Thanks,
Shai.

PS: In the mainline commit history, the entire boulder-oracle-sprint shows up
as one commit, but its detailed history is in the attic/boulder-oracle-sprint
branch.

[Aymeric Augustin]

On 19 mai 2013, at 15:33, Shai Berger <sh...@platonix.com> wrote:

> I was able to track the history of this function, into the mists of time. In
> the beginning, code to cast strings into dates for Oracle was mixed in with
> the general querying code. Then came the boulder-oracle-sprint of 2006-2007,
> where, at some point, that piece of code was factored out to a module-level
> function get_datetime_cast_sql(), which was defined in all backends as a no-op
> except Oracle; this was later refactored into the DatabaseOperations class we
> know today.
>
> However, at about the same time that the DatabaseOperations was created, the
> Oracle backend made another change: It started setting the session's datetime
> format on login. This, as far as I understand, makes the special casting
> operation redundant -- and current test results support my understanding.
>
> So -- I want to fix, now, the thing that was, well, not broken, but bent, in
> 2007. And my question to you -- especially, those of you who participated in
> the boulder sprint -- can you think of any reason why I shouldn't?

Based on the impressive amount of research you've performed, and on the
facts you've found, I think you can go ahead and simplify the code.

I'll review the patch if you want. Thanks for working on this.

--
Aymeric.

[Ian Kelly]

On Sun, May 19, 2013 at 7:33 AM, Shai Berger <sh...@platonix.com> wrote:
> Hi,
>
> In my quest to solve Oracle bugs, I found the function datetime_cast_sql() in
> the Oracle backend. It causes a problem these days, because it gets in the way
> of querying a date as string (mydate__startswith='2005'). It is a function
> that is defined as a no-op in the base backend, and overridden, among core
> backends, only in Oracle. Removing the override, currently, just makes one
> more test pass, and if we decide to remove it from the base, code in several
> other places may become simpler (will not need to special-case datetime
> values).
>
> I was able to track the history of this function, into the mists of time. In
> the beginning, code to cast strings into dates for Oracle was mixed in with
> the general querying code. Then came the boulder-oracle-sprint of 2006-2007,
> where, at some point, that piece of code was factored out to a module-level
> function get_datetime_cast_sql(), which was defined in all backends as a no-op
> except Oracle; this was later refactored into the DatabaseOperations class we
> know today.
>
> However, at about the same time that the DatabaseOperations was created, the
> Oracle backend made another change: It started setting the session's datetime
> format on login. This, as far as I understand, makes the special casting
> operation redundant -- and current test results support my understanding.
>
> So -- I want to fix, now, the thing that was, well, not broken, but bent, in
> 2007. And my question to you -- especially, those of you who participated in
> the boulder sprint -- can you think of any reason why I shouldn't?

You may be right. One thing I would be concerned about is the
reliability of only doing implicit date conversions. If you google
for "ORA-01843" you can find a good number of forum posts that
recommend against relying on the NLS_TIMESTAMP_FORMAT setting. See
also Django ticket #20292.

[Shai Berger]

Hi Ian,

Thanks for your insights.

On Tuesday 21 May 2013 23:22:14 Ian Kelly wrote:
>
> You may be right. One thing I would be concerned about is the
> reliability of only doing implicit date conversions. If you google
> for "ORA-01843" you can find a good number of forum posts that
> recommend against relying on the NLS_TIMESTAMP_FORMAT setting.

I looked at some of those. Most of them warn against it on two grounds:

- The actual format may be different from what you expect
- Formats that use the MON specifier are actually language-dependent

Both of these are mostlly irrelevant for Django -- we set the timestamp format
explicitly, and we don't use MON. A user may change the date/timestamp format
via raw sql, and cause breakage (especially with the new persistent
connections), but -- given the history where we set the format on every
connection -- I think such users mostly deserve what they get. At most, we
should warn against it in the documentation.

> See also Django ticket #20292.

Comment #3 on that ticket suggests that explicit casts do little to improve
things there -- apparently Oracle 10 has a problem applying formats to unicode
strings. I don't have Oracle 10 accessible to test against, and Django's tests
include instances of saving models with DateFields -- so I know the problem
(as reported) is not reproducible against Oracle 11. So as far as I can see,
removing the explicit cast makes things no worse here too.

I just committed the removal in Oracle.

Thanks,
Shai.

[Shai Berger]

My understanding is that an overridable method in the base DatabaeOperations
constitutes a public interface (for 3rd party backends). It is not documented,
so I guess we can get away with skipping deprecation cycles; but don't you
think we should try to warn users who override it that it is no longer used?

> I'll review the patch if you want. Thanks for working on this.

Thanks, I think I'll call you up on that one. For now, I just removed the
Oracle override, to let it pass some more tests.

Have fun,
Shai.

[Aymeric Augustin]

On 22 mai 2013, at 01:32, Shai Berger <sh...@platonix.com> wrote:

> I guess we can get away with skipping deprecation cycles; but don't you
> think we should try to warn users who override it that it is no longer used?

We don't really have a process for that. Maintaining a database backend
for Django is a hard task :-/

--
Aymeric.