Cache API sqlite code is not hitting storage key index due to IS binary operator

RESOLVED FIXED in Firefox 41

Status

()

Core
DOM
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: bkelly, Assigned: bkelly)

Tracking

unspecified
mozilla41
Points:
---

Firefox Tracking Flags

(firefox41 fixed)

Details

Attachments

(1 attachment)

(Assignee)

Description

3 years ago
When I implemented storing keys as blobs I had to switch all the "key=value" constraints in the sql to "key IS value".  This is because zero-length blobs are represented as NULL in sqlite.

The IS operator works, but unfortunately triggers a full table scan instead of using the key index.

This can be fixed by using separate query strings.  If the key we are searching for is NULL, then explicitly use "IS NULL".  Otherwise use the "=value" comparison.

For some reason "IS NULL" works with the index, but "IS value" does not.
Hrm, have you asked the sqlite team about this? I wonder if that's a bug or just part of the language...
(Assignee)

Comment 2

3 years ago
Its documented behavior on their query optimization page.  So I don't think it's a bug.
(Assignee)

Comment 3

3 years ago
Created attachment 8608776 [details] [diff] [review]
Modify Cache API sqlite code to use IS NULL literal when comparing an empty key. r=ehsan

Dynamically build the query string so we can use IS NULL explicitly if the key is the empty string.  Otherwise we use key=:key.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=cfa4e72ba663
Assignee: nobody → bkelly
Status: NEW → ASSIGNED
Attachment #8608776 - Flags: review?(ehsan)

Updated

3 years ago
Attachment #8608776 - Flags: review?(ehsan) → review+

Comment 5

3 years ago
https://hg.mozilla.org/mozilla-central/rev/78db1cd5ffef
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
status-firefox41: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla41
You need to log in before you can comment on or make changes to this bug.