From bcf4364cd8354d0754b76c82b945478e2f5e5822 Mon Sep 17 00:00:00 2001 From: Dor Alagem Date: Mon, 6 Apr 2026 22:34:03 +0300 Subject: [PATCH 1/3] Fix SQLITE_MISUSE to surface SQLite error message Previously, SQLITE_MISUSE was silently ignored (empty break), so errors like invalid JSON path syntax ('$[-1]') were never shown to the user. Now the error message from sqlite3_errmsg() is surfaced the same way as any other execution error, matching the behaviour of the sqlite3 CLI. Fixes #4113 --- src/RunSql.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/RunSql.cpp b/src/RunSql.cpp index 537c5e2023..b49507a37a 100644 --- a/src/RunSql.cpp +++ b/src/RunSql.cpp @@ -247,6 +247,7 @@ bool RunSql::executeNextStatement() break; } case SQLITE_MISUSE: + error = QString::fromUtf8(sqlite3_errmsg(pDb.get())); break; default: error = QString::fromUtf8(sqlite3_errmsg(pDb.get())); From 41a34fa11a98d6460567dedbe61a2ef1e31aec47 Mon Sep 17 00:00:00 2001 From: Dor Alagem Date: Fri, 10 Apr 2026 16:47:17 +0300 Subject: [PATCH 2/3] Address #4121 review: fix blank-line hang and surface JSON path warning MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two fixes responding to @mgrojo's review feedback: 1. MainWindow::executeQuery(): the blank-line-skip loop after a newline was unbounded. On inputs like `select json_extract('[3]', '$[-1]');` followed by trailing blank lines, execute_from_line would walk past the last line, QScintilla would return an empty string for every out-of-range line, and the loop would spin forever, hanging the UI. Add a bounds check against editor->lines(). Patch provided by @mgrojo in the review discussion. 2. Surface invalid SQLite JSON path syntax. SQLite silently returns NULL for json_extract() paths like `$[-1]` (bare negative index), which is valid MySQL/MariaDB syntax but not SQLite syntax — the SQLite equivalent is `$[#-1]`. Because SQLite returns success with NULL instead of an error, the earlier SQLITE_MISUSE surfacing alone did not help the single-line case @mgrojo flagged ("we're back to square one"). Reintroduce a lightweight header-only checker (JsonPathChecker.h) that regex-scans each statement for the `$[-N]` anti-pattern and emits a new RunSql::statementWarning signal, which MainWindow surfaces in the SQL results panel. The statement still runs — the warning is informational so users stop losing rows silently. --- CMakeLists.txt | 1 + src/JsonPathChecker.h | 50 +++++++++++++++++++++++++++++++++++++++++++ src/MainWindow.cpp | 14 +++++++++++- src/RunSql.cpp | 9 ++++++++ src/RunSql.h | 1 + 5 files changed, 74 insertions(+), 1 deletion(-) create mode 100644 src/JsonPathChecker.h diff --git a/CMakeLists.txt b/CMakeLists.txt index 1a17f534aa..ea20da01ba 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -154,6 +154,7 @@ target_sources(${PROJECT_NAME} src/Palette.h src/CondFormat.h src/RunSql.h + src/JsonPathChecker.h src/ProxyDialog.h src/SelectItemsPopup.h src/TableBrowser.h diff --git a/src/JsonPathChecker.h b/src/JsonPathChecker.h new file mode 100644 index 0000000000..cf879435cd --- /dev/null +++ b/src/JsonPathChecker.h @@ -0,0 +1,50 @@ +#ifndef JSONPATHCHECKER_H +#define JSONPATHCHECKER_H + +#include +#include +#include + +/** + * Detects invalid SQLite JSON path syntax in a SQL statement and returns a + * warning message if found, or an empty string if the statement looks fine. + * + * The specific pattern detected here is a bare negative array index inside a + * JSON path expression, e.g. '$[-1]'. This is valid syntax in MySQL/MariaDB + * but is NOT valid in SQLite. SQLite uses the '#' symbol to refer to the array + * length, so the correct SQLite equivalent for "last element" is '$[#-1]', not + * '$[-1]'. + * + * The danger of this mistake is that SQLite silently returns NULL for the + * invalid path instead of raising an error. When such an expression appears in + * a WHERE clause, it causes rows to be silently dropped from the result set — + * a data-loss bug that is very hard to diagnose. + * + * See: https://github.com/sqlitebrowser/sqlitebrowser/issues/4113 + * See: https://sqlite.org/json1.html (path syntax section) + */ +inline QString checkForInvalidJsonPaths(const QString& query) +{ + // Match any JSON path segment of the form $[], e.g. $[-1], $[-42]. + // The negative look-ahead (?!#) ensures we do NOT match the valid SQLite form $[#-N]. + static const QRegularExpression invalidJsonPathRx( + QStringLiteral(R"(\$\[(?!#)-\d+\])"), + QRegularExpression::CaseInsensitiveOption + ); + + const QRegularExpressionMatch match = invalidJsonPathRx.match(query); + if(match.hasMatch()) + { + return QCoreApplication::translate("RunSql", + "Warning: the JSON path '%1' uses a bare negative array index which is not " + "supported by SQLite. SQLite silently returns NULL for such paths, which may " + "cause rows to be silently missing from your results. " + "Use '$[#-N]' syntax instead (e.g. '$[#-1]' for the last element). " + "See: https://sqlite.org/json1.html") + .arg(match.captured(0)); + } + + return {}; +} + +#endif // JSONPATHCHECKER_H diff --git a/src/MainWindow.cpp b/src/MainWindow.cpp index 02335ca918..7e10aed97d 100644 --- a/src/MainWindow.cpp +++ b/src/MainWindow.cpp @@ -1245,7 +1245,12 @@ void MainWindow::executeQuery() if (char_at_index == '\r' || char_at_index == '\n') { execute_from_line++; // The next lines could be empty, so skip all of them too. - while(editor->text(execute_from_line).trimmed().isEmpty()) + // Guard against running off the end of the document: if the query ends + // with trailing blank lines (as happens with some SQL samples) the + // previous unbounded loop would spin forever because QScintilla returns + // an empty string for out-of-range line numbers. + while(execute_from_line <= editor->lines() && + editor->text(execute_from_line).trimmed().isEmpty()) execute_from_line++; execute_from_index = 0; } @@ -1293,6 +1298,13 @@ void MainWindow::executeQuery() query_logger(false, status_message, from_position, to_position); }, Qt::QueuedConnection); + // A statementWarning is emitted when a suspicious pattern is detected that + // SQLite would otherwise silently accept (e.g. a bare negative JSON path + // index like '$[-1]' — see issue #4113). The query still runs; this just + // surfaces the warning in the SQL results area so the user notices. + connect(execute_sql_worker.get(), &RunSql::statementWarning, sqlWidget, [query_logger](const QString& warning_message, int from_position, int to_position) { + query_logger(false, warning_message, from_position, to_position); + }, Qt::QueuedConnection); connect(execute_sql_worker.get(), &RunSql::statementExecuted, sqlWidget, [query_logger, this](const QString& status_message, int from_position, int to_position) { ui->actionSqlResultsSave->setEnabled(false); ui->actionSqlResultsSaveAsView->setEnabled(false); diff --git a/src/RunSql.cpp b/src/RunSql.cpp index b49507a37a..a17237b9b5 100644 --- a/src/RunSql.cpp +++ b/src/RunSql.cpp @@ -2,6 +2,7 @@ #include "sqlite.h" #include "sqlitedb.h" #include "Data.h" +#include "JsonPathChecker.h" #include #include @@ -108,6 +109,14 @@ bool RunSql::executeNextStatement() QString error; if (sql3status == SQLITE_OK) { + // Check for known silent-failure patterns before executing. These are + // cases where SQLite will return success without an error but may + // silently drop data (e.g. rows in a WHERE clause) due to an invalid + // argument such as a bare negative index in a JSON path. + const QString json_path_warning = checkForInvalidJsonPaths(executed_query); + if(!json_path_warning.isEmpty()) + emit statementWarning(json_path_warning, execute_current_position, end_of_current_statement_position); + // What type of query was this? StatementType query_type = getQueryType(executed_query); diff --git a/src/RunSql.h b/src/RunSql.h index 6a9f13c2bf..53d6c490cd 100644 --- a/src/RunSql.h +++ b/src/RunSql.h @@ -55,6 +55,7 @@ class RunSql : public QThread void statementErrored(QString message, int from_position, int to_position); void statementExecuted(QString message, int from_position, int to_position); void statementReturnsRows(QString query, int from_position, int to_position, qint64 time_in_ms); + void statementWarning(QString message, int from_position, int to_position); void structureUpdated(); /** From 46e471bf748f98ef8b6250fb7b251da30f12afc4 Mon Sep 17 00:00:00 2001 From: Dor Alagem Date: Sat, 11 Apr 2026 21:17:25 +0300 Subject: [PATCH 3/3] fix(#4121): correct warning display and broaden JSON path regex MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Use ok=true in query_logger for statementWarning so the editor does not highlight the statement as an error; the query ran successfully and only a potential misuse is flagged. - Extend the invalid-JSON-path regex from the root-only form ($[-N]) to also catch nested-path negative indices: $.array[-1] — negative index after a key step $[0][-1] — negative index after another array step Achieved by matching the leading anchor as one of: $, .word, or ]. A capture group now isolates just the [-N] step so the warning message shows the problematic part without the leading anchor. --- src/JsonPathChecker.h | 24 ++++++++++++++++++------ src/MainWindow.cpp | 4 +++- 2 files changed, 21 insertions(+), 7 deletions(-) diff --git a/src/JsonPathChecker.h b/src/JsonPathChecker.h index cf879435cd..efbe6271bb 100644 --- a/src/JsonPathChecker.h +++ b/src/JsonPathChecker.h @@ -25,10 +25,22 @@ */ inline QString checkForInvalidJsonPaths(const QString& query) { - // Match any JSON path segment of the form $[], e.g. $[-1], $[-42]. - // The negative look-ahead (?!#) ensures we do NOT match the valid SQLite form $[#-N]. + // Match any JSON path step that uses a bare negative array index, e.g.: + // $[-1] — negative index at the root + // $.array[-1] — negative index after a key step + // $[0][-1] — negative index after another array step + // + // The three alternates in the leading group cover these three cases: + // \$ — root anchor (e.g. $[-1]) + // \.[\w*@]+ — key step (e.g. .array[-1]) + // \] — array step (e.g. [0][-1]) + // + // The negative look-ahead (?!#) ensures we do NOT match the valid SQLite + // form $[#-N], which uses '#' as the array-length placeholder. + // Capture group 1 isolates the bare negative index step (e.g. '[-1]') so + // the warning message shows the problematic part without the leading anchor. static const QRegularExpression invalidJsonPathRx( - QStringLiteral(R"(\$\[(?!#)-\d+\])"), + QStringLiteral(R"((?:\$|\.[\w*@]+|\])(\[(?!#)-\d+\]))"), QRegularExpression::CaseInsensitiveOption ); @@ -36,12 +48,12 @@ inline QString checkForInvalidJsonPaths(const QString& query) if(match.hasMatch()) { return QCoreApplication::translate("RunSql", - "Warning: the JSON path '%1' uses a bare negative array index which is not " + "Warning: the JSON path step '%1' uses a bare negative array index which is not " "supported by SQLite. SQLite silently returns NULL for such paths, which may " "cause rows to be silently missing from your results. " - "Use '$[#-N]' syntax instead (e.g. '$[#-1]' for the last element). " + "Use '[#-N]' syntax instead (e.g. '$[#-1]' for the last element). " "See: https://sqlite.org/json1.html") - .arg(match.captured(0)); + .arg(match.captured(1)); } return {}; diff --git a/src/MainWindow.cpp b/src/MainWindow.cpp index 7e10aed97d..06c51cacd0 100644 --- a/src/MainWindow.cpp +++ b/src/MainWindow.cpp @@ -1302,8 +1302,10 @@ void MainWindow::executeQuery() // SQLite would otherwise silently accept (e.g. a bare negative JSON path // index like '$[-1]' — see issue #4113). The query still runs; this just // surfaces the warning in the SQL results area so the user notices. + // We pass ok=true so the editor does NOT highlight the statement as an + // error — the query executed successfully, it is just potentially wrong. connect(execute_sql_worker.get(), &RunSql::statementWarning, sqlWidget, [query_logger](const QString& warning_message, int from_position, int to_position) { - query_logger(false, warning_message, from_position, to_position); + query_logger(true, warning_message, from_position, to_position); }, Qt::QueuedConnection); connect(execute_sql_worker.get(), &RunSql::statementExecuted, sqlWidget, [query_logger, this](const QString& status_message, int from_position, int to_position) { ui->actionSqlResultsSave->setEnabled(false);