fix: a SQL comment before or after a statement is a parse error (#698)

.openspec/specs/002-parser/spec.md

@@ -410,6 +410,14 @@ The tokenizer MUST convert SQL text into a stream of tokens. Each token MUST car
 
 **Tests:** `src/parser/tokenizer.rs::test_tokenize_parameters`
 
+#### Scenario: A leading or trailing comment is trivia, not syntax
+
+- GIVEN `-- c\nCREATE TABLE t(a);`, `/* c */ CREATE TABLE t(a);`, `CREATE TABLE t(a); -- c`, or a comment-only input
+- WHEN parsed
+- THEN the comment is skipped and the statement parses (a comment-only input succeeds as a no-op), matching what the pinned oracle accepts
+
+**Tests:** `tests/corpus/comment_trivia_test.rs::leading_line_comment_before_statement_is_accepted`, `tests/corpus/comment_trivia_test.rs::leading_block_comment_before_statement_is_accepted`, `tests/corpus/comment_trivia_test.rs::trailing_line_comment_after_statement_is_accepted`, `tests/corpus/comment_trivia_test.rs::comment_only_input_is_a_successful_no_op`, `src/parser/tokenizer.rs::skip_leading_trivia_skips_whitespace_and_both_comment_styles`
+
 ### Requirement 2: Grammar Compatibility [MUST]
 
 The parser MUST accept all SQL that SQLite accepts, and reject all SQL that SQLite rejects.

CHANGELOG.md

@@ -8,6 +8,14 @@ All notable changes to sqlite-rs. Format follows [Keep a Changelog](https://keep
 
 ### Fixed
 
+- A SQL comment before or after a statement (`-- c\nCREATE TABLE t(a);`,
+  `CREATE TABLE t(a); -- c`, or comment-only input) was a parse error,
+  even though `split_statements` already grouped a leading comment with
+  its statement — the statement dispatchers keyword-sniffed the raw
+  text and expected the first token to be a keyword. `codegen::dispatch`
+  and the `exec`/`query` CLI entry points now skip leading trivia via
+  `parser::skip_leading_trivia` before sniffing; comment-only input is a
+  successful no-op, matching the oracle (#698, spec 002 Req 1).
 - `hash_agg_find` allocated a fresh `Vec<u8>` key buffer and `Vec<Value>`
   key-values buffer per row. It now reuses `HashAggState`-held scratch
   buffers via take/give-back, cloning into `GroupSlot`/the index

src/bin/sqlite-rs/exec.rs

@@ -103,7 +103,7 @@ pub fn run_exec(path: &Path, sql: &str) -> ExitCode {
 /// any statement starting with `CREATE`/`DROP`/`ALTER` invalidates,
 /// even one that ends up failing or being a no-op.
 fn is_schema_changing(stmt: &str) -> bool {
-    let head = stmt.trim_start();
+    let head = sqlite_rs::parser::skip_leading_trivia(stmt);
     ["CREATE", "DROP", "ALTER"].iter().any(|kw| {
         head.get(..kw.len())
             .is_some_and(|h| h.eq_ignore_ascii_case(kw))

src/bin/sqlite-rs/query.rs

@@ -192,8 +192,7 @@ pub fn run_query(raw_args: Vec<String>) -> ExitCode {
     // point (`parse_explain`, grammar V4) rather than `parse_select` —
     // only checked when the statement actually starts with `EXPLAIN`,
     // so an ordinary `SELECT` never pays for the extra parse attempt.
-    let starts_with_explain = sql
-        .trim_start()
+    let starts_with_explain = sqlite_rs::parser::skip_leading_trivia(&sql)
         .get(..7)
         .is_some_and(|head| head.eq_ignore_ascii_case("explain"));
     let (select, eqp_mode) = if starts_with_explain {

src/codegen/dispatch.rs

@@ -13,6 +13,7 @@ use crate::parser::error::{
     parse_create_view, parse_delete, parse_drop_index, parse_drop_table, parse_insert,
     parse_pragma, parse_rollback, parse_update,
 };
+use crate::parser::skip_leading_trivia;
 use crate::schema::{TableSchema, ViewSchema};
 use crate::vdbe::Program;
 
@@ -83,7 +84,8 @@ impl From<CodegenError> for DispatchError {
 /// there. `compile_statement` below instead does its own borrowed,
 /// non-allocating scan for the hot dispatch path.
 pub fn leading_keywords(sql: &str) -> Vec<String> {
-    sql.split_whitespace()
+    skip_leading_trivia(sql)
+        .split_whitespace()
         .take(3)
         .map(|w| w.to_ascii_uppercase())
         .collect()
@@ -142,7 +144,7 @@ pub fn compile_statement(
             .ok_or_else(|| DispatchError::NoSuchIndex(name.to_string()))
     };
 
-    let mut words = sql.split_whitespace();
+    let mut words = skip_leading_trivia(sql).split_whitespace();
     let first_word = words.next().unwrap_or("");
     let head = canonical(first_word);
     let second = canonical(words.next().unwrap_or(""));

src/parser.rs

@@ -16,4 +16,4 @@ pub use error::{
     parse_delete, parse_drop_index, parse_drop_table, parse_drop_view, parse_explain, parse_insert,
     parse_pragma, parse_rollback, parse_select, parse_update, ParseOutcome,
 };
-pub use tokenizer::{ends_with_semicolon, split_statements};
+pub use tokenizer::{ends_with_semicolon, skip_leading_trivia, split_statements};

src/parser/tokenizer.rs

@@ -1179,8 +1179,8 @@ pub fn ends_with_semicolon(sql: &str) -> bool {
     matches!(last_real, Some(tok) if tok.kind == TokenKind::Semicolon)
 }
 
-/// Empty statements (a bare `;`, leading/trailing whitespace-only) are
-/// dropped, matching `sqlite3`'s own script handling.
+/// Empty statements (a bare `;`, leading/trailing whitespace-only, or
+/// comment-only) are dropped, matching `sqlite3`'s own script handling.
 pub fn split_statements(sql: &str) -> Vec<String> {
     let tokens = Tokenizer::tokenize(sql);
     let mut statements = Vec::new();
@@ -1201,11 +1201,26 @@ pub fn split_statements(sql: &str) -> Vec<String> {
     statements
 }
 
+/// Skips leading whitespace and comments in `sql`, returning the rest of
+/// the source starting at the first real token (or the empty string if
+/// `sql` is nothing but trivia). Comments are trivia, not syntax — code
+/// that keyword-sniffs a raw statement string (e.g. `codegen::dispatch`)
+/// needs to look past a leading comment the same way the tokenizer does,
+/// rather than treating the comment's own text as the "first word".
+pub fn skip_leading_trivia(sql: &str) -> &str {
+    let offset = Tokenizer::tokenize(sql)
+        .first()
+        .map(|tok| tok.span.offset as usize)
+        .unwrap_or(0);
+    sql.get(offset..).unwrap_or("")
+}
+
 fn push_trimmed(statements: &mut Vec<String>, slice: &str) {
     let trimmed = slice.trim();
-    if !trimmed.is_empty() {
-        statements.push(trimmed.to_string());
+    if trimmed.is_empty() || skip_leading_trivia(trimmed).is_empty() {
+        return;
     }
+    statements.push(trimmed.to_string());
 }
 
 /// The remaining unconsumed source, from the given byte cursor.
@@ -1256,6 +1271,45 @@ mod tests {
         assert_eq!(stmts, vec!["BEGIN", "ROLLBACK"]);
     }
 
+    #[test]
+    fn split_statements_keeps_a_leading_comment_attached_to_its_statement() {
+        // #698: `split_statements` already groups a leading comment with
+        // the statement that follows it (matching `sqlite3`'s own script
+        // handling) — the bug was downstream, in code that keyword-sniffs
+        // that returned text. This test pins the grouping behavior this
+        // ticket must not disturb.
+        let stmts = split_statements("-- c\nCREATE TABLE t(a);");
+        assert_eq!(stmts, vec!["-- c\nCREATE TABLE t(a)"]);
+    }
+
+    #[test]
+    fn split_statements_drops_a_comment_only_statement() {
+        // #698: a comment-only statement must not survive as a
+        // "statement" for the dispatcher to choke on — it's a no-op.
+        let stmts = split_statements("-- just a comment");
+        assert!(stmts.is_empty());
+
+        let stmts = split_statements("BEGIN; /* comment only */; COMMIT");
+        assert_eq!(stmts, vec!["BEGIN", "COMMIT"]);
+    }
+
+    #[test]
+    fn skip_leading_trivia_skips_whitespace_and_both_comment_styles() {
+        assert_eq!(
+            skip_leading_trivia("-- c\nCREATE TABLE t(a)"),
+            "CREATE TABLE t(a)"
+        );
+        assert_eq!(
+            skip_leading_trivia("/* c */ CREATE TABLE t(a)"),
+            "CREATE TABLE t(a)"
+        );
+        assert_eq!(
+            skip_leading_trivia("  CREATE TABLE t(a)"),
+            "CREATE TABLE t(a)"
+        );
+        assert_eq!(skip_leading_trivia("-- only a comment"), "");
+    }
+
     #[test]
     fn ends_with_semicolon_true_for_a_trailing_top_level_semicolon() {
         assert!(ends_with_semicolon("SELECT * FROM t;"));

tests/corpus/comment_trivia_test.rs

@@ -0,0 +1,201 @@
+// Copyright 2026 Schuberg Philis
+// SPDX-License-Identifier: Apache-2.0
+//! Oracle-diffed regression tests for #698: a SQL comment before or
+//! after a statement must not be a parse error — comments are trivia,
+//! not syntax, and the tokenizer already skips them everywhere except
+//! at the statement-dispatch boundary (`codegen::dispatch`'s raw
+//! keyword-sniffing, and `split_statements`'s comment-only-statement
+//! handling).
+//!
+//! The oracle is invoked via stdin, not argv: a leading `--` in an argv
+//! element is read as a CLI option by `sqlite3` itself (confirmed:
+//! `sqlite3 db.db "-- c\nCREATE TABLE t(a)"` fails with `Error: unknown
+//! option: - c` before it ever reaches the SQL parser), independent of
+//! how the argument reached the process. `sqlite-rs exec`'s own
+//! argument parsing is purely positional (`src/bin/sqlite-rs/main.rs`),
+//! so it has no such restriction and is fed the same text via argv.
+
+use std::io::Write;
+use std::path::{Path, PathBuf};
+use std::process::{Command, Output, Stdio};
+use std::sync::atomic::{AtomicU64, Ordering};
+
+use crate::oracle::{pinned_oracle, skip_no_oracle};
+
+const CLI: &str = env!("CARGO_BIN_EXE_sqlite-rs");
+
+fn scratch_db(label: &str) -> PathBuf {
+    static COUNTER: AtomicU64 = AtomicU64::new(0);
+    let n = COUNTER.fetch_add(1, Ordering::Relaxed);
+    let dir = std::env::temp_dir().join(format!(
+        "sqlite-rs-comment-trivia-{label}-{}-{n}",
+        std::process::id()
+    ));
+    std::fs::remove_dir_all(&dir).ok();
+    std::fs::create_dir_all(&dir).unwrap();
+    dir.join("scratch.db")
+}
+
+fn run_exec(db: &Path, sql: &str) -> Output {
+    Command::new(CLI)
+        .arg("exec")
+        .arg(db)
+        .arg(sql)
+        .output()
+        .unwrap_or_else(|e| panic!("running {CLI} exec {} {sql:?}: {e}", db.display()))
+}
+
+fn run_query(db: &Path, sql: &str) -> Output {
+    Command::new(CLI)
+        .arg("query")
+        .arg(db)
+        .arg(sql)
+        .output()
+        .unwrap_or_else(|e| panic!("running {CLI} query {} {sql:?}: {e}", db.display()))
+}
+
+/// Runs `sql` against a fresh oracle-created `db` via stdin (never
+/// argv — see the module doc for why), returning stdout as text.
+fn oracle_via_stdin(oracle: &Path, db: &Path, sql: &str) -> String {
+    let mut child = Command::new(oracle)
+        .arg(db)
+        .stdin(Stdio::piped())
+        .stdout(Stdio::piped())
+        .stderr(Stdio::piped())
+        .spawn()
+        .unwrap_or_else(|e| panic!("spawning oracle on {}: {e}", db.display()));
+    child
+        .stdin
+        .take()
+        .unwrap()
+        .write_all(sql.as_bytes())
+        .unwrap();
+    let output = child.wait_with_output().unwrap();
+    assert!(
+        output.status.success(),
+        "oracle rejected {sql:?}: {}",
+        String::from_utf8_lossy(&output.stderr)
+    );
+    String::from_utf8_lossy(&output.stdout).into_owned()
+}
+
+/// Asserts our `exec` accepts `sql` and, when the pinned oracle is
+/// available, that the two engines agree on the resulting stored DDL
+/// text for `t` (`expected_ddl`, e.g. `"CREATE TABLE t(a)"` — a
+/// statement's own leading/trailing comment must not leak into the
+/// stored text, but a *mid*-statement comment, being part of the
+/// statement's own span, must).
+fn assert_accepted_and_matches_oracle(label: &str, sql: &str, expected_ddl: &str) {
+    let db = scratch_db(label);
+    let output = run_exec(&db, sql);
+    assert!(
+        output.status.success(),
+        "{label}: our exec rejected {sql:?}: {}",
+        String::from_utf8_lossy(&output.stderr)
+    );
+    let our_schema = Command::new(CLI)
+        .arg("dump")
+        .arg(&db)
+        .output()
+        .unwrap_or_else(|e| panic!("running {CLI} dump {}: {e}", db.display()));
+    assert!(our_schema.status.success());
+    let our_schema = String::from_utf8_lossy(&our_schema.stdout).into_owned();
+    assert!(
+        our_schema.contains(expected_ddl),
+        "our schema for {label}: {our_schema:?}"
+    );
+
+    let Some(oracle) = pinned_oracle() else {
+        skip_no_oracle(label);
+        return;
+    };
+    let oracle_db = scratch_db(&format!("{label}-oracle"));
+    let oracle_schema = oracle_via_stdin(&oracle, &oracle_db, &format!("{sql}\n.schema"));
+    // `.schema`'s own trailing `;` differs from `dump`'s; compare only
+    // the table's actual DDL text, which is what #698 is about (whether
+    // a comment leaks into or blocks the statement), not dump-format
+    // fidelity (covered by `dump_oracle_test.rs`).
+    assert!(
+        oracle_schema.contains(expected_ddl),
+        "oracle schema for {label}: {oracle_schema:?}"
+    );
+}
+
+#[test]
+fn leading_line_comment_before_statement_is_accepted() {
+    assert_accepted_and_matches_oracle(
+        "leading-line",
+        "-- c\nCREATE TABLE t(a);",
+        "CREATE TABLE t(a)",
+    );
+}
+
+#[test]
+fn leading_block_comment_before_statement_is_accepted() {
+    assert_accepted_and_matches_oracle(
+        "leading-block",
+        "/* c */ CREATE TABLE t(a);",
+        "CREATE TABLE t(a)",
+    );
+}
+
+#[test]
+fn trailing_line_comment_after_statement_is_accepted() {
+    assert_accepted_and_matches_oracle("trailing", "CREATE TABLE t(a); -- c", "CREATE TABLE t(a)");
+}
+
+#[test]
+fn mid_statement_comment_still_works() {
+    assert_accepted_and_matches_oracle(
+        "mid-statement",
+        "CREATE TABLE t(a /* col */);",
+        "CREATE TABLE t(a /* col */)",
+    );
+}
+
+#[test]
+fn comment_only_input_is_a_successful_no_op() {
+    let db = scratch_db("comment-only");
+    let output = run_exec(&db, "-- just a comment");
+    assert!(
+        output.status.success(),
+        "comment-only input should be a no-op, not an error: {}",
+        String::from_utf8_lossy(&output.stderr)
+    );
+
+    if let Some(oracle) = pinned_oracle() {
+        let oracle_db = scratch_db("comment-only-oracle");
+        // Succeeding at all (no error/panic from `wait_with_output`'s
+        // status assertion) is the acceptance bar here.
+        oracle_via_stdin(&oracle, &oracle_db, "-- just a comment");
+    } else {
+        skip_no_oracle("comment_only_input_is_a_successful_no_op (oracle cross-check)");
+    }
+}
+
+#[test]
+fn comment_like_text_inside_a_string_literal_stays_literal() {
+    let db = scratch_db("string-literal");
+    assert!(run_exec(&db, "CREATE TABLE t(a)").status.success());
+
+    let output = run_query(&db, "SELECT '-- not a comment'");
+    assert!(
+        output.status.success(),
+        "stderr: {}",
+        String::from_utf8_lossy(&output.stderr)
+    );
+    assert_eq!(
+        String::from_utf8_lossy(&output.stdout),
+        "-- not a comment\n"
+    );
+
+    if let Some(oracle) = pinned_oracle() {
+        let oracle_db = scratch_db("string-literal-oracle");
+        let oracle_out = oracle_via_stdin(&oracle, &oracle_db, "SELECT '-- not a comment';");
+        assert_eq!(oracle_out, "-- not a comment\n");
+    } else {
+        skip_no_oracle(
+            "comment_like_text_inside_a_string_literal_stays_literal (oracle cross-check)",
+        );
+    }
+}

tests/corpus/main.rs

@@ -25,6 +25,7 @@ mod btree_insert_test;
 mod btree_test;
 mod cli_e2e_test;
 mod cli_write_test;
+mod comment_trivia_test;
 mod crash_torture_test;
 mod cte_test;
 mod declared_collate_test;