Opened 4 years ago

Closed 4 years ago

Last modified 3 years ago

#15580 closed Bug (fixed)

DB cache backend uses LIMIT / OFFSET query for culling

Reported by: ikelly Owned by: nobody
Component: Core (Cache system) Version: master
Severity: Normal Keywords: oracle
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

As a result, the DB cache backend cannot safely be used with Oracle.

Attachments (1)

15580.patch (2.5 KB) - added by ikelly 4 years ago.

Download all attachments as: .zip

Change History (8)

comment:1 Changed 4 years ago by ikelly

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

For Oracle, the existing query should probably be replaced with:

SELECT cache_key
FROM (SELECT cache_key, rank() OVER (ORDER BY cache_key) AS rank FROM  %s)
WHER rank = %%s

comment:2 Changed 4 years ago by russellm

  • Triage Stage changed from Unreviewed to Accepted

This is obviously a limitation -- however, it's not clear to me what the resolution should be. I really don't want to replace the internals of the DB cache backend with the ORM just so that we can get LIMIT and OFFSET support for Oracle. We may just need to introduce an 'Oracle' DB backend, and specialize that one call.

comment:3 Changed 4 years ago by lukeplant

  • Type set to Bug

comment:4 Changed 4 years ago by lukeplant

  • Severity set to Normal

Changed 4 years ago by ikelly

comment:5 Changed 4 years ago by ikelly

  • Has patch set

I've added a patch that pushes out this one query to connection.ops rather than to the ORM. I don't think tests are needed since this is already caught by regressiontests.cache.tests.DBCacheTests.test_cull.

Russell, please let me know if this looks okay to you.

comment:6 Changed 4 years ago by aaugustin

  • Easy pickings unset
  • Resolution set to fixed
  • Status changed from new to closed
  • UI/UX unset

I reported a duplicate, #16481 -- my bad -- and it was fixed at r16635, arguably in a less elegant manner.

I'm closing this as "fixed", because technically it is fixed.

I'll try to get a core developer to take a look at this patch. It could be worth reverting r16635 and applying this patch instead.

comment:7 Changed 3 years ago by akaariai

I committed ikelly's patch in d3c2eb103f6682c029a850e60dc4cf85896b6aa2 to make the cache database backend 3rd party database friendly.

Note: See TracTickets for help on using tickets.
Back to Top