Skip to content

Fixed #36148 -- Enabled native tuple comparison lookups on Oracle >= 23.4 and SQLite >= 3.37. - #19107

Closed
charettes wants to merge 3 commits into
django:mainfrom
charettes:tuple-lookups_as_fallback
Closed

Fixed #36148 -- Enabled native tuple comparison lookups on Oracle >= 23.4 and SQLite >= 3.37.#19107
charettes wants to merge 3 commits into
django:mainfrom
charettes:tuple-lookups_as_fallback

Conversation

@charettes

@charettes charettes commented Jan 27, 2025 •
edited
Loading

Copy link
Copy Markdown
Member

Per discussion

Trac ticket number

ticket-36148

Branch description

I can't get my hands on Oracle <= 23.3 but I know that

  • CI which is running 19 doesn't support it
  • It seems to be missing on Oracle 23.2
  • It's working on Oracle 23.4
  • ORACLE_VERSION=23.4.0.0 PYTHON_VERSION=3.12 docker compose run --rm oracle --parallel 1 --keepdb composite_pk passes
  • ORACLE_VERSION=23.5.0.0 PYTHON_VERSION=3.12 docker compose run --rm oracle --parallel 1 --keepdb composite_pk passes

@github-actions github-actions Bot added the no ticket Based on PR title, no linked Trac ticket label Jan 27, 2025
charettes
charettes commented Jan 27, 2025
View reviewed changes
Comment thread django/db/backends/oracle/features.py Outdated
Comment on lines 220 to 223

@charettes charettes Jan 27, 2025

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This can be handy in bulk_batch_size as if this is supported we can likely avoid considering one placeholder per composite field see https://github.com/django/django/pull/19088/files#r1931008541.

@charettes charettes mentioned this pull request Jan 27, 2025
Merged
@charettes
charettes force-pushed the tuple-lookups_as_fallback branch 3 times, most recently from bb6a5aa to 6d0e2ce Compare January 28, 2025 01:05
@charettes charettes changed the title Enabled native tuple comparison lookups on Oracle >= 23.4. Fixed #36148 -- Enabled native tuple comparison lookups on Oracle >= 23.4. Jan 28, 2025
@github-actions github-actions Bot removed the no ticket Based on PR title, no linked Trac ticket label Jan 28, 2025
@charettes
charettes marked this pull request as ready for review January 28, 2025 01:07
@charettes
charettes force-pushed the tuple-lookups_as_fallback branch from 6d0e2ce to 9952fc4 Compare January 28, 2025 20:19
@charettes

charettes commented Jan 28, 2025 •
edited
Loading

Copy link
Copy Markdown
Member Author

Discovered that SQLite does support native tuple containment checks by digging a bit

@felixxm
felixxm self-requested a review January 29, 2025 15:56
@felixxm

felixxm commented Feb 2, 2025

Copy link
Copy Markdown
Member

Per discussion

Trac ticket number

ticket-36148

Branch description

I can't get my hands on Oracle <= 23.3 but I know that

* CI which is running 19 doesn't support it

* [It seems to be missing on Oracle 23.2](https://dbfiddle.uk/Z1p0AxvV)

* [It's working on Oracle 23.4](https://onecompiler.com/oracle/437c9m79g)

Oracle 23c has been rebranded to Oracle 23ai (starting with version 23.4), which would explain many new features in the "minor" release.

@charettes

charettes commented Feb 2, 2025 •
edited
Loading

Copy link
Copy Markdown
Member Author

@felixxm I just spent some more time going through the release notes again and I couldn't any mention of it.

It might have been a feature that flew the radar though given that support the VALUES constructor was added and it's effectively a way to define a sequence of rows.

That would explain why (c0, ..., cn) IN (SELECT s0, ..., sn FROM ...) was supported on Oracle 19 but not (c0, ..., cn) = (L0, ..., LN) for literal values.

Looking how we circumvented the lack of VALUES support for Oracle on inserts I suspect the same could work for literal values instead!

@charettes

charettes commented Feb 2, 2025 •
edited
Loading

Copy link
Copy Markdown
Member Author

There we go it seems that we can work around the TupleExact.as_oracle on 19 by using (SELECT L0, ..., L5 FROM DUAL) instead of (L0, ..., L5).

It appears that < and > are not implemented though.

@felixxm

felixxm commented Feb 6, 2025

Copy link
Copy Markdown
Member

Looking how we circumvented the lack of VALUES support for Oracle on inserts I suspect the same could work for literal values instead!

Unfortunately we had to revert 175b049 (see 5424151 and ticket-35655).

felixxm
felixxm reviewed Feb 6, 2025
View reviewed changes
Comment thread django/db/models/fields/tuple_lookups.py Outdated
@charettes

charettes commented Feb 6, 2025

Copy link
Copy Markdown
Member Author

Unfortunately we had to revert 175b049 (see 5424151 and ticket-35655).

The issue might only be for INSERT though an not for (c0, ..., cn) IN (SELECT v0, ..., vn FROM DUAL). Not sure it's worth doing given it allows us to exercise the fallback code anyway.

@charettes
charettes force-pushed the tuple-lookups_as_fallback branch from 9952fc4 to b6898ff Compare February 8, 2025 14:53
charettes
charettes commented Feb 8, 2025
View reviewed changes
Comment thread django/db/models/fields/tuple_lookups.py Outdated
@charettes
charettes force-pushed the tuple-lookups_as_fallback branch from b6898ff to 3016df2 Compare February 8, 2025 16:13
charettes
charettes commented Feb 8, 2025
View reviewed changes
Comment thread django/db/models/fields/tuple_lookups.py Outdated
@charettes charettes changed the title Fixed #36148 -- Enabled native tuple comparison lookups on Oracle >= 23.4. Fixed #36148 -- Enabled native tuple comparison lookups on Oracle >= 23.4 and SQLite > 3.37. Feb 8, 2025
@charettes charettes changed the title Fixed #36148 -- Enabled native tuple comparison lookups on Oracle >= 23.4 and SQLite > 3.37. Fixed #36148 -- Enabled native tuple comparison lookups on Oracle >= 23.4 and SQLite >= 3.37. Feb 8, 2025
@charettes
charettes force-pushed the tuple-lookups_as_fallback branch from 3016df2 to 47ab068 Compare February 9, 2025 02:09
felixxm
felixxm reviewed Feb 9, 2025
View reviewed changes
Comment thread django/db/backends/sqlite3/features.py Outdated
@charettes @felixxm
Refs django#373 -- Delegated Tuple compilation to compiler.
c326cfe 
This should allow third-party backends to define Tuple.as_vendor()
overrides that are taken into consideration which calling as_sql()
directly prevents.
@charettes @felixxm
Refs #36148 -- Relied on a feature switch to define tuple lookups sup…
a0a765d 
…port.

This should allow backends more easily opt-in or out of native support and rely
on the fallback if unavailable.
@charettes @felixxm
Refs #36148 -- Enabled native tuple comparison lookups on SQLite 3.37…
bfc7364 
…+ and Oracle 23.4+.

VALUES must be explicitly specified when declaring a sequence of tuples
on SQLite < 3.37 but it's not required on >= 3.37.

See sqlite/sqlite@9289f51 which addressed the last remaining issue with
IN.
@felixxm
felixxm force-pushed the tuple-lookups_as_fallback branch from 1ae16d3 to bfc7364 Compare February 9, 2025 14:15
felixxm
felixxm approved these changes Feb 9, 2025
View reviewed changes

@felixxm felixxm left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks 👍

@felixxm

felixxm commented Feb 9, 2025

Copy link
Copy Markdown
Member

Merged in c326cfe, a0a765d, and 4a3ad9e.

@felixxm felixxm closed this Feb 9, 2025
@charettes
charettes deleted the tuple-lookups_as_fallback branch February 9, 2025 16:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@felixxm felixxm felixxm approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@charettes @felixxm