922350 - Warning is displayed, for SQL statement labeled "do not warn"

Status: RESOLVED INVALID

(Toolkit :: Places, defect)

Description

[Daniel Holbert [:dholbert]]

I just noticed this go by during a reftest run, in my local debug build:
{
WARNING: SQL statement '/* do not warn (bug 659740 - SQLite may ignore index if few visits exist) */SELECT ROUND((strftime('%s','now','localtime','utc') - v.visit_date/1000000)/86400), IFNULL(r.visit_type, v.visit_type), v.visit_date FROM moz_historyvisits v LEFT JOIN moz_historyvisits r ON r.id = v.from_visit AND v.visit_type BETWEEN 5 AND 6 WHERE v.place_id = :page_id ORDER BY v.visit_date DESC ' (c34c1c70) should have been finalized before closing the connection: file /mozilla/storage/src/mozStorageConnection.cpp, line 850
}

The "WARNING... do not warn" seemed a bit odd to me. :)

HG Archeology says that this "do not warn" comment was added here:
 http://hg.mozilla.org/mozilla-central/rev/8b9ce2b1e438#l1.53
and then reindented here:
 http://hg.mozilla.org/mozilla-central/rev/28288ae6d58c#l10.92
both in 2011.

bug 659740 comment 8 suggested that the "do not warn" is supposed to actually prevent warnings for this SQL statement:
> I think we are going to see an increase in warnings, [...]
> Based on this, I've added a /* do not warn */ since the indices
> exist but are ignored by the optimizer.
...but it apparently isn't preventing warnings, since I got a warning for this statement.

mak, is something broken here?

Comment 1

[Daniel Holbert [:dholbert]]

FWIW, I hit this at the end of my reftest run, amidst a bunch of other shutdown-spew, shortly after "REFTEST TEST-START | Shutdown".  This was when running reftests like so:
  $ ./mach reftest layout/reftests/flexbox/

Comment 2

[Marco Bonardo [:mak] (Away Apr 25 - May 5)]

the do not warn is referred to a performance warning, not to this finalization warning. So it doesn't matter it states "do not warn", it's just another unrelated warning.

On the other side this warning should not appear after the fix for bug 914005, was your version older than that?

Comment 3

[Daniel Holbert [:dholbert]]

(In reply to Marco Bonardo [:mak] from comment #2)
> On the other side this warning should not appear after the fix for bug
> 914005, was your version older than that?

I think it was before that was fixed; this was among a bunch of other storage-related shutdown-output, so I think the build was probably old-ish and yet-to-receive that other bug's fix.

> the do not warn is referred to a performance warning, not to this
> finalization warning. So it doesn't matter it states "do not warn", it's
> just another unrelated warning.

OK -- I suppose this is INVALID, then.