nagai/fune
3
0
Fork 0
forked from mirrors/gecko-dev
Code Pull requests Activity

Bug 1884308 - Chunk bookmark GUIDs when retrieving details to avoid going over Sqlite variables limit. r=mconley,places-reviewers,Standard8

Browse source 
Differential Revision: https://phabricator.services.mozilla.com/D204660
This commit is contained in:
Marco Bonardo 2024-03-15 16:18:00 +00:00
parent 27ecbe8d71
commit 5a15e88f6a
8 changed files with 100 additions and 41 deletions
repo.diff.show_diff_stats Download patch file Download diff file Expand all files Collapse all files

5
dom/cache/Connection.cpp vendored
View file

@ -226,6 +226,11 @@ Connection::GetVariableLimit(int32_t* aResultOut) {
return mBase->GetVariableLimit(aResultOut);
}
NS_IMETHODIMP
Connection::SetVariableLimit(int32_t aLimit) {
return mBase->SetVariableLimit(aLimit);
}
NS_IMETHODIMP
Connection::BeginTransaction() { return mBase->BeginTransaction(); }

6
storage/mozIStorageAsyncConnection.idl
View file

@ -70,8 +70,12 @@ interface mozIStorageAsyncConnection : nsISupports {
*
* If you bind more params than this limit, `create{Async}Statement` will
* fail with a "too many SQL variables" error.
* It's possible to lower the limit, but it's not possible to set it higher
* than the default value, passing an higher value will silently truncate to
* the default. Lowering the limit is particularly useful for testing code
* that may bind many variables.
*/
readonly attribute int32_t variableLimit;
attribute int32_t variableLimit;
/**
* Returns true if a transaction is active on this connection.

12
storage/mozStorageConnection.cpp
View file

@ -2468,6 +2468,18 @@ Connection::GetVariableLimit(int32_t* _limit) {
return NS_OK;
}
NS_IMETHODIMP
Connection::SetVariableLimit(int32_t limit) {
if (!connectionReady()) {
return NS_ERROR_NOT_INITIALIZED;
}
int oldLimit = ::sqlite3_limit(mDBConn, SQLITE_LIMIT_VARIABLE_NUMBER, limit);
if (oldLimit < 0) {
return NS_ERROR_UNEXPECTED;
}
return NS_OK;
}
NS_IMETHODIMP
Connection::BeginTransaction() {
if (!connectionReady()) {

4
storage/test/unit/test_storage_connection.js
View file

@ -980,6 +980,10 @@ add_task(async function test_variableLimit() {
info("Open connection");
let db = Services.storage.openDatabase(getTestDB());
Assert.equal(db.variableLimit, 32766, "Should return default limit");
db.variableLimit = 999;
Assert.equal(db.variableLimit, 999, "Should return the set limit");
db.variableLimit = 33000;
Assert.equal(db.variableLimit, 32766, "Should silently truncate");
await asyncClose(db);
});

75
toolkit/components/places/Bookmarks.sys.mjs
View file

@ -3335,42 +3335,38 @@ async function retrieveFullBookmarkPath(guid, options = {}) {
*/
async function getBookmarkDetailMap(aGuids) {
return lazy.PlacesUtils.withConnectionWrapper(
"Bookmarks.geBookmarkDetails",
"Bookmarks.geBookmarkDetailMap",
async db => {
const rows = await db.executeCached(
`
SELECT
b.guid,
b.id,
b.parent,
IFNULL(h.frecency, 0),
IFNULL(h.hidden, 0),
IFNULL(h.visit_count, 0),
h.last_visit_date,
(
SELECT group_concat(pp.title ORDER BY pp.title)
FROM moz_bookmarks bb
JOIN moz_bookmarks pp ON pp.id = bb.parent
JOIN moz_bookmarks gg ON gg.id = pp.parent
WHERE bb.fk = h.id
AND gg.guid = '${Bookmarks.tagsGuid}'
),
t.guid, t.id, t.title
FROM moz_bookmarks b
LEFT JOIN moz_places h ON h.id = b.fk
LEFT JOIN moz_bookmarks t ON t.guid = target_folder_guid(h.url)
WHERE b.guid IN (${lazy.PlacesUtils.sqlBindPlaceholders(aGuids)})
`,
aGuids
);
return new Map(
rows.map(row => {
const lastVisitDate = row.getResultByIndex(6);
return [
row.getResultByIndex(0),
{
let entries = new Map();
for (let chunk of lazy.PlacesUtils.chunkArray(aGuids, db.variableLimit)) {
await db.executeCached(
`
SELECT
b.guid,
b.id,
b.parent,
IFNULL(h.frecency, 0),
IFNULL(h.hidden, 0),
IFNULL(h.visit_count, 0),
h.last_visit_date,
(
SELECT group_concat(pp.title ORDER BY pp.title)
FROM moz_bookmarks bb
JOIN moz_bookmarks pp ON pp.id = bb.parent
JOIN moz_bookmarks gg ON gg.id = pp.parent
WHERE bb.fk = h.id
AND gg.guid = '${Bookmarks.tagsGuid}'
),
t.guid, t.id, t.title
FROM moz_bookmarks b
LEFT JOIN moz_places h ON h.id = b.fk
LEFT JOIN moz_bookmarks t ON t.guid = target_folder_guid(h.url)
WHERE b.guid IN (${lazy.PlacesUtils.sqlBindPlaceholders(chunk)})
`,
chunk,
row => {
const lastVisitDate = row.getResultByIndex(6);
entries.set(row.getResultByIndex(0), {
id: row.getResultByIndex(1),
parentId: row.getResultByIndex(2),
frecency: row.getResultByIndex(3),
@ -3383,10 +3379,11 @@ async function getBookmarkDetailMap(aGuids) {
targetFolderGuid: row.getResultByIndex(8),
targetFolderItemId: row.getResultByIndex(9),
targetFolderTitle: row.getResultByIndex(10),
},
];
})
);
});
}
);
}
return entries;
}
);
}

24
toolkit/components/places/tests/bookmarks/test_insert_thousands_bookmarks.js Normal file
View file

@ -0,0 +1,24 @@
/* Any copyright is dedicated to the Public Domain.
http://creativecommons.org/publicdomain/zero/1.0/ */
// Test `insertTree()` with more bookmarks than the Sqlite variables limit.
add_task(async function () {
const NUM_BOOKMARKS = 1000;
await PlacesUtils.withConnectionWrapper("test", async db => {
db.variableLimit = NUM_BOOKMARKS - 100;
Assert.greater(
NUM_BOOKMARKS,
db.variableLimit,
"Insert more bookmarks than the Sqlite variables limit."
);
});
let children = [];
for (let i = 0; i < NUM_BOOKMARKS; ++i) {
children.push({ url: "http://www.mozilla.org/" + i });
}
await PlacesUtils.bookmarks.insertTree({
guid: PlacesUtils.bookmarks.toolbarGuid,
children,
});
});

2
toolkit/components/places/tests/bookmarks/xpcshell.toml
View file

@ -70,6 +70,8 @@ support-files = ["bookmarks_long_tag.json"]
["test_bookmarks_update.js"]
["test_insert_thousands_bookmarks.js"]
["test_insertTree_fixupOrSkipInvalidEntries.js"]
["test_keywords.js"]

13
toolkit/modules/Sqlite.sys.mjs
View file

@ -1688,12 +1688,23 @@ OpenedConnection.prototype = Object.freeze({
* Returns the maximum number of bound parameters for statements executed
* on this connection.
*
* @type {number}
* @returns {number} The bound parameters limit.
*/
get variableLimit() {
return this.unsafeRawConnection.variableLimit;
},
/**
* Set the the maximum number of bound parameters for statements executed
* on this connection. If the passed-in value is higher than the maximum
* default value, it will be silently truncated.
*
* @param {number} newLimit The bound parameters limit.
*/
set variableLimit(newLimit) {
this.unsafeRawConnection.variableLimit = newLimit;
},
/**
* The integer schema version of the database.
*