Feature/sqlite compiletime#1180
Conversation
|
Hi, as mentioned, will you do the standard library pendant? (provide api functions) else this isn't publicly usable unless users define @extern natives by themselves? |
c31188a to
4eabb8b
Compare
|
Updated this PR with the clean implementation (including handle validation, statement connection ownership tracking, auto-finalizing open statements when closing connections, and explicit interpreter error messages). Also opened the WurstStdlib2 PR providing the native |
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1742a4fbd7
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
d03161c to
5d16740
Compare
5d16740 to
6e51eb3
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6e51eb3786
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| ResultSet rs = sqliteResultSets.remove(statement.getVal()); | ||
| if (rs != null) rs.close(); | ||
| sqliteExecutedStatements.remove(statement.getVal()); | ||
| stmt.clearParameters(); |
There was a problem hiding this comment.
When a prepared statement with bound parameters is reset and stepped again without rebinding every parameter, this line clears all JDBC bindings. SQLite's reset operation leaves bindings in place, so common reuse patterns like binding a WHERE parameter once, reading a row, calling sqlite_reset(stmt), and stepping again silently query with NULL/no values or fail constraints instead of reusing the previous values. Please reset execution state without clearing bindings; expose a separate clear-bindings operation if needed.
Useful? React with 👍 / 👎.
sqlite_reset previously called PreparedStatement.clearParameters(), wiping all bound parameters. This diverges from SQLite's sqlite3_reset(), which resets execution state but leaves bindings in place, and broke the common reuse pattern of binding a parameter once, stepping, resetting, and stepping again without rebinding. sqlite_reset now only clears the execution state (open result set and the executed marker). A separate sqlite_clear_bindings native maps to sqlite3_clear_bindings() for callers that explicitly want to drop bindings. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Address adversarial review findings on the SQLite compiletime natives: - sqlite_exec now runs every statement of a ';'-separated script instead of silently dropping all but the first (JDBC/xerial only executes the first). Splitting respects string literals, quoted identifiers and comments. - sqlite_open rejects smuggled URI query parameters and explicitly disables extension loading, closing a load_extension() -> dlopen native-code vector reachable from a malicious compiletime dependency. - sqlite_close now always closes the connection even if finalizing a statement throws, aggregating errors, so the connection can no longer leak. - (re)binding a parameter invalidates prior execution state, so binding new values after a step (without an explicit sqlite_reset) re-executes instead of being silently ignored. - Added sqlite_column_is_null so callers can distinguish SQL NULL from an empty string / zero value. - closeAllSqliteResources no longer resets the handle counter, keeping handles monotonic so a stale handle can never alias a new resource. - Documented the 1-based bind / 0-based column index convention and the 32-bit truncation of sqlite_column_int. - Removed dead, cleanup-escaping fallback branches in RunTests. Tests: multi-statement exec, rebind-without-reset re-execution and NULL detection compiletime tests, plus unit tests for the SQL statement splitter. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Follow-up: binding preservation + hardening passPushed two commits addressing the
|
Follow-up to the second adversarial review. Replace the hand-rolled SQL statement splitter in sqlite_exec with a direct call to SQLite's native sqlite3_exec (via the xerial driver's low-level DB handle). The splitter could not parse trigger BEGIN...END bodies, CASE...END, or the [id] / `id` identifier-quoting forms, so those scripts were shredded into invalid fragments; delegating to SQLite's own parser handles all of them. The splitter and its unit tests are removed; a native test now exercises a multi-statement script containing a trigger and a ';'-bearing quoted identifier. Also from the review: - sqlite_open now only rejects query parameters on the "file:" URI form, so a plain path legitimately containing '?' (e.g. on Linux) is accepted; enableLoadExtension(false) remains the defense-in-depth backstop for all paths (the extension-loading vector was verified closed either way). - Documented that (re)binding a parameter closes any open result set. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2 participants
This PR adds compiletime support for sqlite operations. Not thoroughly tested, but simple stuff works:
Self-contained Examples:
SQLite.txt
SQLiteHelder.txt