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.

Sign up for GitHub

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

Jump to bottom

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

Closed
wants to merge 3 commits into from
Closed

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

wants to merge 3 commits into from

Conversation

charettes
Copy link
Member

@charettes charettes commented Jan 27, 2025 •
edited
Loading

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
django/db/backends/oracle/features.py Outdated
Comment on lines 220 to 223
@cached_property
def supports_tuple_comparison_lookups(self):
# Support is known to be missing on 23.2 but available on 23.4.
return self.connection.oracle_version >= (23, 4)
Copy link
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
Copy link
Member Author

charettes commented Jan 28, 2025 •
edited
Loading

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
Copy link
Member

felixxm commented Feb 2, 2025

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
Copy link
Member Author

charettes commented Feb 2, 2025 •
edited
Loading

@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
Copy link
Member Author

charettes commented Feb 2, 2025 •
edited
Loading

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
Copy link
Member

felixxm commented Feb 6, 2025

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).

@charettes
Copy link
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 force-pushed the tuple-lookups_as_fallback branch from b6898ff to 3016df2 Compare February 8, 2025 16:13
@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
@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
Copy link
Member

@felixxm felixxm left a comment

Choose a reason for hiding this comment

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

Thanks 👍

@felixxm
Copy link
Member

felixxm commented Feb 9, 2025

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