Repository navigation
Improve Codebase :: Defects #1385
Description
I have started on this path with #1382 but I thought it would be worth creating an issue to track all of the changes I'm thinking about that will come after this. This audit was done with the help of Claude.
A1. Rows are silently dropped after a SQL error on the same page
What happens. If a .sql file has a statement that fails, and a later statement returns rows without naming a component, those rows disappear without an error.
This page should show two rows. It shows one:
select 'table' as component;
select 1 as x;
select nonexistent_function(); -- fails
select 2 as x; -- expected: rendered. actual: silently droppedSuggested fix. Make Some mean "a component is open":
close_componentusestake()instead ofas_mut().- Add an owned
next_component_indexcounter toHtmlRenderContextrather than reading it off the child. - Drop the
saved_componentrestore. A following row then opens a fresh default component.
A2. Retry-After is never sent on connection-pool timeouts
What happens. When the database connection pool times out, SQLPage returns 429 Too Many Requests but never attaches the Retry-After header.
Suggested fix. Move the Retry-After header into the if let Some(status) arm, keyed on the status being 429, next to the existing 401 WWW-Authenticate handling. Both headers are then functions of the final status.
A3. Asset cache-busting URLs change on every build
What happens. The hashed filenames used for sqlpage.js, sqlpage.css and the other embedded assets are supposed to change only when the asset's contents change. They currently change on every build, including rebuilds of the same commit. Rebuilding SQLPage invalidates every browser and CDN cache entry for its assets.
Suggested fix. Hash contents, which compress_asset already has in memory at :47, instead of re-reading the compressed file. This also deletes the 20-line streaming read in hashed_filename.
A4. Browser initialisation runs twice on every page load
What happens. Every function registered with add_init_fn runs twice on a normal page load. For most this is idempotent. For forms it is not: every <input type="file"> with a size limit gets two change listeners, and every form with data-auto-submit gets two change → form.submit() listeners.
Suggested fix. Two independent halves:
- Make readiness an exclusive choice, not three additive registrations. This alone fixes the doubled listeners.
- Give init functions the root they should scan:
add_init_fn(f: (root: ParentNode) => void), called withdocumentinitially and withevent.targetonfragment-loaded— the conventioninit_bootstrap_componentsalready uses.
A5. COPY ... NULL 'x' CSV silently imports the literal text
What happens. When a CSV import uses PostgreSQL's legacy COPY syntax with a NULL marker, SQLPage ignores the marker. Rows that should import as NULL import as the literal string instead.
COPY t (a, b) FROM 'f' NULL 'NA' CSV -- 'NA' is dropped; the text "NA" is importedSuggested fix. ...todo...
A6. Template properties have no effect
| Property | Where | What happens |
|---|---|---|
class on login |
login.handlebars:1 |
<div class="container" {{class}}> — the value is emitted as a bare attribute, not inside class. The documented top-level class property (99_shared_id_class_attributes.sql:56) does nothing. |
id on text |
text.handlebars:3 |
Emits id="" whenever title is set but no id is given. |
link on empty_state |
empty_state.handlebars:23-28 |
The .empty-action <a> renders unconditionally, so a card with no link/link_text shows an empty button. |
Related, found while checking the above. modal.handlebars:49 uses class="btn me-primary". There is no .me-primary rule in any bundled stylesheet (sqlpage.css, tabler.css, tabler-vendors.css and
tom-select.bootstrap5.css). This looks like a typo for btn-primary.
Two more Bootstrap 4 leftovers that have no effect under Tabler:
login.handlebars:16and:54useform-group— 0 matches intabler.css.columns.handlebars:12usesfont-weight-medium; the defined class isfw-medium.
A7. Table action columns break the documented CSS class convention
What happens. CHANGELOG.md:400-406 documents that every table column gets a ._col_{column_name} class, so users can style or hide a column with CSS. This works for ordinary columns and for edit_url/delete_url. It does not work for custom_actions or _sqlpage_actions: you can style the cells but not the header.
Suggested fix. Use _col_{{this.name}} on the header, matching the cell.
A8. A test fixture is dead, and its live twin points at a file that no longer exists
What happens. tests/core/select_temp_t.sql is never used. Nothing in the repository refers to it.
Why it matters. It is a near-duplicate of tests/sql_test_files/select_temp_t.sql. The only reference anywhere points at the second file:
tests/sql_test_files/component_rendering/temp_table_accessible_in_run_sql_nomssql_nooracle.sql:4
sqlpage.run_sql('tests/sql_test_files/select_temp_t.sql')
Suggested fix. Delete the dead file. Correct the comment in the live one.
Activity
lovasoa commentedon Aug 18, 2026
Same as #1386 (comment) . The effort is appreciated, but an issue named "Improve Codebase :: Defects" is hard to organize around.
81reap commentedon Aug 18, 2026
will hold off on taking any action until I get feedback here #1389 (comment)