feat: Add SQLite compiletime native bindings#450
Conversation
0e3c57d to
b585cbd
Compare
|
|
||
| /** A row returned from a SELECT query. Access columns by index via col(). */ | ||
| public class SqlResult | ||
| string array[2000] cols |
There was a problem hiding this comment.
Big array members are not a good idea in wurst, I think just use ArrayList here and for select and remove LinkedList import.
|
@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: 7db7067e44
ℹ️ 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".
|
|
||
| /** A row returned from a SELECT query. Access columns by index via col(). */ | ||
| public class SqlResult | ||
| string array[2000] cols |
There was a problem hiding this comment.
Because Wurst implements sized class-member arrays by generating one backing array per element, this declaration emits 2000 backing arrays/accessor branches for every project that uses SqlResult, even when a query returns only one or two columns. That is a large code-size/compile-time cost in a stdlib wrapper and makes every col() access go through the generated dispatcher; store the actual column values in a dynamic list/ArrayList instead of a 2000-slot member array.
Useful? React with 👍 / 👎.
7db7067 to
81cd773
Compare
|
Updated this PR with the requested fixes:
|
Adds @extern declarations for compiletime SQLite JDBC bindings exposed by the WurstScript compiler (requires wurstscript/WurstScript PR #1180). Native functions: sqlite_open, sqlite_prepare, sqlite_bind_*, sqlite_step, sqlite_column_*, sqlite_reset, sqlite_finalize, sqlite_close, sqlite_exec. High-level API: - SqlResult: row accessor with col, colInt, colReal, colBool helpers - SqliteDb: OOP connection wrapper with select, selectFirst, exists, count - SQL: configurable singleton for single-database workflows Parameterised query support: - execPrepared, selectPrepared, selectFirstPrepared, existsPrepared - Prevents SQL injection by using sqlite_prepare + sqlite_bind_string - Includes injection prevention test Fixes vs original draft: - Moved sqlite_column_count after first sqlite_step (JDBC ResultSet metadata requires an executed statement) - Added parameterised query variants to prevent SQL injection - Added comprehensive doc comments about error behaviour and upstream dep - Unified test annotations to @test (lowercase) - Removed unused ErrorHandling import - Fixed array size to use literal (WurstScript requirement)
Because Wurst implements sized class-member arrays by generating one backing array per element, the previous 2000-slot member array in SqlResult emitted massive code and compiled slowly. This refactors SqlResult to use a dynamic ArrayList<string> to store only the actual returned columns. Additionally updates all LinkedList usages to ArrayList to match.
81cd773 to
59480f1
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 59480f1d1e
ℹ️ 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".
| for p in params | ||
| sqlite_bind_string(stmt, idx, p) |
There was a problem hiding this comment.
When prepared queries are used, this iterates an ArrayList<string> with for p in params, but ArrayList in this repo does not expose an iterator and is documented/implemented for index-based traversal only. That makes the new SQLite package fail to typecheck as soon as this method is compiled; the same pattern is repeated in the new tests over rows, so these should be changed to for i = 0 to params.size() - 1 / params.get(i) style loops.
Useful? React with 👍 / 👎.
The API used `for x in ArrayList` in 6 places, but this stdlib's ArrayList has no iterator by design (index loops only), so the PR did not compile. Replaced those with index loops. Also completed the binding surface against WurstScript PR #1180: - expose sqlite_column_is_null and sqlite_clear_bindings natives (were missing) - SqlResult now records per-column NULL flags and exposes colIsNull(), the only reliable way to tell SQL NULL apart from an empty string (col() returns "" for both) Expanded SQLiteTests with a native-surface section proving every native: typed bind/read-back, column_count, column_is_null, step-past-last-row, reset (rewind + preserve bindings), clear_bindings, reset+rebind reuse, multi-statement exec, trigger blocks, and finalize+reprepare. All 354 stdlib tests pass (50 SQLite) with the PR #1180 compiler. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Evaluation + update (compiled & tested against WurstScript #1180)I built the compiler from #1180 and ran the stdlib test suite against it. Findings and the changes I pushed: 🔴 Critical: the PR did not compile
Fixed by switching to index loops ( 🟡 Incomplete native surface vs #1180#1180 ships 16 natives; this PR declared 14, omitting the two that make the cursor API complete:
Both are now declared, and ✅ Tests: full native-surface coverageAdded a native-surface section proving every native during compiletime: typed bind/read-back ( Proof50 of those are SQLite tests, all green. (Before the fix: 11 compile errors, 0 tests runnable.)
|
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0c9bca1b30
ℹ️ 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".
2 participants
Adds native
@externdeclarations for compiletime SQLite database operations inwurst/file/SQLite.wurst.High-level API
SqlResult: Dynamic column storage usingArrayList<string>.SqliteDb: OOP connection wrapper supporting raw and parameterised queries (execPrepared,selectPrepared,selectFirstPrepared,existsPrepared).SQL: Configurable singleton wrapper.Acceptance Criteria
Exposed Native Functions:
sqlite_open(string path) returns intsqlite_prepare(int conn, string q) returns intsqlite_bind_int(int stmt, int idx, int val)sqlite_bind_real(int stmt, int idx, real val)sqlite_bind_string(int stmt, int idx, string val)sqlite_step(int stmt) returns booleansqlite_column_count(int stmt) returns intsqlite_column_int(int stmt, int idx) returns intsqlite_column_real(int stmt, int idx) returns realsqlite_column_string(int stmt, int idx) returns stringsqlite_reset(int stmt)sqlite_finalize(int stmt)sqlite_close(int conn)sqlite_exec(int conn, string q)