Skip to content

Commit c8d42cd

Browse files
calccryptocalccrypto
committed
rollup fixes
common cleanup SQL fixed accidental modification of index when doing dryrun changed non-obvious behavior in opendb opening in READONLY mode would cause modifydb function to be ignored now will error
1 parent ff426f1 commit c8d42cd

File tree

5 files changed

+63
-44
lines changed

5 files changed

+63
-44
lines changed

‎include/dbutils.h

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -169,8 +169,10 @@ int create_treesummary_tables(const char *name, sqlite3 *db, void *args);
169169

170170
int set_db_pragmas(sqlite3 *db);
171171

172+
typedef int (*modifydb_func)(const char *name, sqlite3 *db, void *args);
173+
172174
sqlite3 *opendb(const char *name, int flags, const int setpragmas, const int load_extensions,
173-
int (*modifydb_func)(const char *name, sqlite3 *db, void *args), void *modifydb_args);
175+
modifydb_func modifydb, void *modifydb_args);
174176

175177
int querytsdb(const char *name, struct sum *sum, sqlite3 *db, int ts);
176178

@@ -239,6 +241,9 @@ void sqlite_print_err_and_free(char *err, FILE *stream, char *format, ...);
239241

240242
int get_rollupscore(sqlite3 *db, int *rollupscore);
241243

244+
extern const char ROLLUP_CLEANUP[];
245+
extern const size_t ROLLUP_CLEANUP_SIZE;
246+
242247
int treesummary_exists_callback(void *args, int count, char **data, char **columns);
243248

244249
enum CheckRollupScore {

‎src/dbutils.c

Lines changed: 26 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -254,12 +254,13 @@ int set_db_pragmas(sqlite3 *db) {
254254
}
255255

256256
sqlite3 *opendb(const char *name, int flags, const int setpragmas, const int load_extensions,
257-
int (*modifydb_func)(const char *name, sqlite3 *db, void *args), void *modifydb_args) {
257+
modifydb_func modifydb, void *modifydb_args) {
258258
sqlite3 *db = NULL;
259259

260260
if (sqlite3_open_v2(name, &db, flags | SQLITE_OPEN_URI, GUFI_SQLITE_VFS) != SQLITE_OK) {
261261
if (!(flags & SQLITE_OPEN_CREATE)) {
262-
sqlite_print_err_and_free(NULL, stderr, "Cannot open database: %s %s rc %d\n", name, sqlite3_errmsg(db), sqlite3_errcode(db));
262+
sqlite_print_err_and_free(NULL, stderr, "Cannot open database: %s %s rc %d\n",
263+
name, sqlite3_errmsg(db), sqlite3_errcode(db));
263264
}
264265
sqlite3_close(db); /* close db even if it didn't open to avoid memory leaks */
265266
return NULL;
@@ -280,12 +281,20 @@ sqlite3 *opendb(const char *name, int flags, const int setpragmas, const int loa
280281
sqlite3_extension_init(db, NULL, NULL) ;
281282
}
282283

283-
if (!(flags & SQLITE_OPEN_READONLY) && modifydb_func) {
284-
if (modifydb_func(name, db, modifydb_args) != 0) {
285-
sqlite_print_err_and_free(NULL, stderr, "Cannot modify database: %s %s rc %d\n", name, sqlite3_errmsg(db), sqlite3_errcode(db));
284+
if (modifydb) {
285+
if (flags & SQLITE_OPEN_READONLY) {
286+
fprintf(stderr, "Database %s opened in READONLY mode, but a modifydb function was provided", name);
286287
sqlite3_close(db);
287288
return NULL;
288289
}
290+
else {
291+
if (modifydb(name, db, modifydb_args) != 0) {
292+
sqlite_print_err_and_free(NULL, stderr, "Cannot modify database: %s %s rc %d\n",
293+
name, sqlite3_errmsg(db), sqlite3_errcode(db));
294+
sqlite3_close(db);
295+
return NULL;
296+
}
297+
}
289298
}
290299

291300
return db;
@@ -1414,6 +1423,18 @@ static int count_pwd(void *args, int count, char **data, char **columns) {
14141423
return 0;
14151424
}
14161425

1426+
const char ROLLUP_CLEANUP[] =
1427+
"DROP INDEX IF EXISTS " SUMMARY "_idx;"
1428+
"DELETE FROM " PENTRIES_ROLLUP ";"
1429+
"DELETE FROM " SUMMARY " WHERE isroot != 1;"
1430+
"DELETE FROM " XATTRS_ROLLUP ";"
1431+
"SELECT filename FROM " EXTERNAL_DBS_ROLLUP " WHERE type == '" EXTERNAL_TYPE_XATTR_NAME "';"
1432+
"DELETE FROM " EXTERNAL_DBS_ROLLUP ";"
1433+
"VACUUM;"
1434+
"UPDATE " SUMMARY " SET rollupscore = 0;"; /* keep this last to allow it to be modified easily */
1435+
1436+
const size_t ROLLUP_CLEANUP_SIZE = sizeof(ROLLUP_CLEANUP);
1437+
14171438
/* Delete all entries in the XATTR_ROLLUP table */
14181439
int xattrs_rollup_cleanup(void *args, int count, char **data, char **columns) {
14191440
(void) count; (void) columns;

‎src/gufi_rollup.c

Lines changed: 22 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -698,22 +698,19 @@ static int do_rollup(struct RollUp *rollup,
698698
char *err = NULL;
699699
int exec_rc = SQLITE_OK;
700700

701-
char setup[] =
702-
/* clear out old rollup data */
703-
"DROP INDEX IF EXISTS " SUMMARY "_idx;"
704-
"DELETE FROM " PENTRIES_ROLLUP ";"
705-
"DELETE FROM " SUMMARY " WHERE isroot != 1;"
706-
"DELETE FROM " TREESUMMARY ";"
707-
"DELETE FROM " XATTRS_ROLLUP ";"
708-
709-
/* select all external xattr db names for cleanup in callback */
710-
"SELECT filename FROM " EXTERNAL_DBS " WHERE type == '" EXTERNAL_TYPE_XATTR_NAME "';"
711-
712-
"DELETE FROM " EXTERNAL_DBS_ROLLUP ";"
713-
714-
/* set the rollup score in the SQL statement */
715-
"UPDATE " SUMMARY " SET rollupscore = 0;";
701+
/*
702+
* clear out old rollup data
703+
*
704+
* treesummary table is not cleared here - it was cleared when the
705+
* db was opened
706+
*/
707+
char setup[ROLLUP_CLEANUP_SIZE];
708+
memcpy(setup, ROLLUP_CLEANUP, ROLLUP_CLEANUP_SIZE);
716709

710+
/*
711+
* set rollup score here instead of running 2 SQL statements
712+
* setting it to 0 here and then to 1 during copying
713+
*/
717714
setup[sizeof(setup) - sizeof("0;")] += ds->score;
718715

719716
refstr_t name = {
@@ -726,7 +723,8 @@ static int do_rollup(struct RollUp *rollup,
726723
timestamp_end_print(timestamp_buffers, id, "setup", setup);
727724

728725
if (exec_rc != SQLITE_OK) {
729-
sqlite_print_err_and_free(err, stderr, "Error: Failed to set up database for rollup: \"%s\": %s\n", rollup->data.name, err);
726+
sqlite_print_err_and_free(err, stderr, "Error: Failed to set up database for rollup: \"%s\": %s\n",
727+
rollup->data.name, err);
730728
rc = -1;
731729
goto end_rollup;
732730
}
@@ -827,6 +825,13 @@ static int rollup(void *args timestamp_sig) {
827825
ds->score = 0;
828826
ds->success = 1;
829827

828+
int openflag = SQLITE_OPEN_READWRITE;
829+
modifydb_func modifydb = create_treesummary_tables;
830+
if (in->dry_run) {
831+
openflag = SQLITE_OPEN_READONLY;
832+
modifydb = NULL;
833+
}
834+
830835
timestamp_end_print(timestamp_buffers, id, "setup", setup);
831836

832837
/*
@@ -835,7 +840,7 @@ static int rollup(void *args timestamp_sig) {
835840
* always create the treesummary table
836841
*/
837842
timestamp_create_start(open_curr_db);
838-
sqlite3 *dst = opendb(dbname, SQLITE_OPEN_READWRITE, 1, 0, create_treesummary_tables, NULL);
843+
sqlite3 *dst = opendb(dbname, openflag, 1, 0, modifydb, NULL);
839844
timestamp_end_print(timestamp_buffers, id, "opendb", open_curr_db);
840845

841846
/* can attempt to roll up */

‎src/gufi_unrollup.c

Lines changed: 3 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -158,27 +158,9 @@ static int processdir(QPTPool_t *ctx, const size_t id, void *data, void *args) {
158158
};
159159

160160
char *err = NULL;
161-
if (sqlite3_exec(db,
162-
"BEGIN TRANSACTION;"
163-
"DROP INDEX IF EXISTS " SUMMARY "_idx;"
164-
"DELETE FROM " PENTRIES_ROLLUP ";"
165-
"DELETE FROM " SUMMARY " WHERE isroot != 1;"
166-
"UPDATE " SUMMARY " SET rollupscore = 0 WHERE isroot == 1;"
167-
"DELETE FROM " XATTRS_ROLLUP ";"
168-
"SELECT filename FROM " EXTERNAL_DBS_ROLLUP " WHERE type == '" EXTERNAL_TYPE_XATTR_NAME "';"
169-
"DELETE FROM " EXTERNAL_DBS_ROLLUP ";"
170-
"END TRANSACTION;"
171-
/*
172-
* not removing tree summary table since it is useful
173-
* and there's no way to tell if the tree summary table
174-
* existed before roll up (can add a column if necessary)
175-
* (maybe make removing the tree summary table optional?)
176-
*/
177-
"VACUUM;",
178-
xattrs_rollup_cleanup,
179-
&name,
180-
&err) != SQLITE_OK) {
181-
sqlite_print_err_and_free(err, stderr, "Could not remove roll up data from \"%s\": %s\n", work->name, err);
161+
if (sqlite3_exec(db, ROLLUP_CLEANUP, xattrs_rollup_cleanup, &name, &err) != SQLITE_OK) {
162+
sqlite_print_err_and_free(err, stderr, "Could not remove roll up data from \"%s\": %s\n",
163+
work->name, err);
182164
rc = 1;
183165
}
184166
err = NULL;

‎test/unit/googletest/dbutils.cpp.in

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -115,6 +115,12 @@ TEST(opendb, bad_path) {
115115
}
116116
}
117117

118+
TEST(opendb, modify_readonly_db) {
119+
sqlite3 *db = opendb(SQLITE_MEMORY, SQLITE_OPEN_READONLY, 0, 0,
120+
[](const char *, sqlite3 *, void *){ return 0; }, nullptr);
121+
EXPECT_EQ(db, nullptr);
122+
}
123+
118124
TEST(opendb, bad_modify_db) {
119125
sqlite3 *db = opendb(SQLITE_MEMORY, SQLITE_OPEN_READWRITE, 0, 0,
120126
[](const char *, sqlite3 *, void *){ return 1; }, nullptr);

0 commit comments

Comments
 (0)