Closed
Bug 491954
Opened 15 years ago
Closed 15 years ago
Views should select exact columns
Categories
(Toolkit :: Places, defect)
Toolkit
Places
Tracking
()
VERIFIED
FIXED
mozilla1.9.2a1
People
(Reporter: sdwilsh, Assigned: mak)
References
Details
(Keywords: verified1.9.1)
Attachments
(2 files, 1 obsolete file)
28.35 KB,
patch
|
Details | Diff | Splinter Review | |
2.28 KB,
patch
|
sdwilsh
:
review+
|
Details | Diff | Splinter Review |
Our views should select exact columns so we don't kill our ability to downgrade between versions. This *must* be fixed before we ship 1.9.1 or we'll stick ourselves into a nasty nasty hole.
Flags: blocking1.9.1?
Assignee | ||
Updated•15 years ago
|
Assignee: nobody → mak77
Status: NEW → ASSIGNED
Assignee | ||
Comment 1•15 years ago
|
||
well, really we should avoid "SELECT *" everywhere we UNION. Just to be safe for future changes i've removed all SELECT * FROM real_table, and replaced with COLUMNS macros. Plus moved indexes to their own file.
Attachment #376363 -
Flags: review?(sdwilsh)
Assignee | ||
Updated•15 years ago
|
Severity: normal → critical
OS: Mac OS X → All
Hardware: x86 → All
Reporter | ||
Comment 2•15 years ago
|
||
Comment on attachment 376363 [details] [diff] [review] patch >+++ b/toolkit/components/places/src/nsAnnotationService.cpp >@@ -127,8 +128,7 @@ nsAnnotationService::Init() > > // mDBGetAnnotation > rv = mDBConn->CreateStatement(NS_LITERAL_CSTRING( >- "SELECT * " >- "FROM moz_annos " >+ "SELECT " MOZ_ANNOS_COLUMNS " FROM moz_annos " This isn't needed since we don't use temporary tables here. Please don't add complexity like this unless we have to. >@@ -136,8 +136,7 @@ nsAnnotationService::Init() > > // mDBGetItemAnnotation > rv = mDBConn->CreateStatement(NS_LITERAL_CSTRING( >- "SELECT * " >- "FROM moz_items_annos " >+ "SELECT " MOZ_ITEMS_ANNOS_COLUMNS " FROM moz_items_annos " ditto >+++ b/toolkit/components/places/src/nsPlacesIndexes.h >@@ -0,0 +1,165 @@ >+/* vim: sw=2 ts=2 sts=2 expandtab Can I suggest coping what storage uses please? See http://mxr.mozilla.org/mozilla-central/source/storage/style.txt r=sdwilsh with changes
Attachment #376363 -
Flags: review?(sdwilsh) → review+
Assignee | ||
Comment 3•15 years ago
|
||
Attachment #376363 -
Attachment is obsolete: true
Assignee | ||
Comment 4•15 years ago
|
||
http://hg.mozilla.org/mozilla-central/rev/e6c08971cd7e
Status: ASSIGNED → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.2a1
Assignee | ||
Updated•15 years ago
|
Attachment #376685 -
Flags: approval1.9.1?
Assignee | ||
Comment 5•15 years ago
|
||
Comment on attachment 376685 [details] [diff] [review] patch v1.1 this should either block or have approval, we need this change to allow any future schema upgrade.
Assignee | ||
Updated•15 years ago
|
Attachment #376685 -
Flags: approval1.9.1?
Assignee | ||
Comment 7•15 years ago
|
||
thanks http://hg.mozilla.org/releases/mozilla-1.9.1/rev/f3ca8b75d84e
Keywords: fixed1.9.1
Assignee | ||
Comment 8•15 years ago
|
||
argh, in the change i exactly forgot to change the view creation, me bad! additional patch coming.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Assignee | ||
Comment 9•15 years ago
|
||
Attachment #376825 -
Flags: review?(sdwilsh)
Reporter | ||
Comment 10•15 years ago
|
||
Comment on attachment 376825 [details] [diff] [review] additional patch v1.0 ugh - I recall thinking that too but I was going to add it at the end of the review comments, and apparently I forgot. *sigh* r=sdwilsh
Attachment #376825 -
Flags: review?(sdwilsh) → review+
Assignee | ||
Comment 11•15 years ago
|
||
http://hg.mozilla.org/mozilla-central/rev/ca08e548a105
Status: REOPENED → RESOLVED
Closed: 15 years ago → 15 years ago
Resolution: --- → FIXED
Assignee | ||
Comment 12•15 years ago
|
||
http://hg.mozilla.org/releases/mozilla-1.9.1/rev/308ae27ef9d8
Comment 13•15 years ago
|
||
Marking verified fixed based on checkins into trunk and 1.9.1 branch. There is no way for me to check this manually.
Status: RESOLVED → VERIFIED
Keywords: fixed1.9.1 → verified1.9.1
You need to log in
before you can comment on or make changes to this bug.
Description
•