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 |
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.
Use dynamic storage for row columns
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.
Replace ArrayList foreach with index loops
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 👍 / 👎.
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)