Improve Codebase :: Defects

[81reap]

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 dropped

Suggested fix. Make Some mean "a component is open":

• close_component uses take() instead of as_mut().
• Add an owned next_component_index counter to HtmlRenderContext rather than reading it off the child.
• Drop the saved_component restore. 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:

1. Make readiness an exclusive choice, not three additive registrations. This alone fixes the doubled listeners.
2. Give init functions the root they should scan: add_init_fn(f: (root: ParentNode) => void), called with document initially and with event.target on fragment-loaded — the convention init_bootstrap_components already 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 imported

Suggested 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:16 and :54 use form-group — 0 matches in tabler.css.
• columns.handlebars:12 uses font-weight-medium; the defined class is fw-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.