Closed Bug 491954 Opened 12 years ago Closed 12 years ago
Views should select exact columns
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.
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)
Severity: normal → critical
OS: Mac OS X → All
Hardware: x86 → All
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+
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.2a1
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.
Blocking as per comment 0
Flags: blocking1.9.1? → blocking1.9.1+
argh, in the change i exactly forgot to change the view creation, me bad! additional patch coming.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
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+
Status: REOPENED → RESOLVED
Closed: 12 years ago → 12 years ago
Resolution: --- → FIXED
Marking verified fixed based on checkins into trunk and 1.9.1 branch. There is no way for me to check this manually.
You need to log in before you can comment on or make changes to this bug.