make queries static strings in storage

RESOLVED FIXED

Status

Cloud Services
Server: Sync
RESOLVED FIXED
7 years ago
7 years ago

People

(Reporter: tarek, Assigned: RaFromBRC)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

all in title
(Assignee)

Updated

7 years ago
Assignee: nobody → rmiller
Status: NEW → ASSIGNED
(Assignee)

Comment 1

7 years ago
Created attachment 547576 [details] [diff] [review]
change the static queries to use raw sql
Attachment #547576 - Flags: review?(telliott)
Attachment #547576 - Flags: review?(tarek)
Comment on attachment 547576 [details] [diff] [review]
change the static queries to use raw sql

My personal opinion is that if we're going to have static queries for clarity, I don't even see the benefit of keeping a dict.

Plain queries like:

QUERY = """\
select poof in doof
where toos = 1
"""

should be enough.

But that's just my opinion, I don't want to sound harsh here, so r+-ing
Attachment #547576 - Flags: review?(tarek) → review+
(Assignee)

Comment 3

7 years ago
(In reply to comment #2)
> My personal opinion is that if we're going to have static queries for
> clarity, I don't even see the benefit of keeping a dict.
> 
> Plain queries like:
> 
> QUERY = """\
> select poof in doof
> where toos = 1
> """
>
> should be enough.

I didn't want to have to change any of the calling code (i.e. I wanted get_query() to continue to work exactly as before).  I could have put the queries at module scope and used globals() to look them up, but one query isn't created until get_query() is called, so I would have had to either stash that in the global dict (yuck) or done two lookups (not a big deal, really, but not the choice I took).
Comment on attachment 547576 [details] [diff] [review]
change the static queries to use raw sql

Patch appears to translate correctly. I've made a note in Bug 624542 that we should really bench that COLLECTION_MODIFIED query.
Attachment #547576 - Flags: review?(telliott) → review+
Applied by Rob at http://hg.mozilla.org/services/server-storage/rev/e04b8f36ebae
Status: ASSIGNED → RESOLVED
Last Resolved: 7 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.