Skip to content

Accept ARRAY vector literals and typed value errors - #329

Open
EnRaiha wants to merge 5 commits into
mainfrom
fix/issue306-vector-array-coercion
Open

EnRaiha wants to merge 5 commits into
mainfrom
fix/issue306-vector-array-coercion

Conversation

@EnRaiha

@EnRaiha EnRaiha commented Sep 16, 2026

Copy link
Copy Markdown
Contributor

Summary

ARRAY[0.1, 0.2, 0.3] folds its literals to Decimal in the planner. Strict element coercion kept Float and Integer, dropped every other element, and reported the literal as having 0 elements — and that mismatch surfaced as XX000, an internal error, for what is a client value problem. A non-numeric element was equally opaque: no element index, no type, and the same internal class.

This PR fixes the coercion and the class. Elements of every numeric shape are read; a bad element is refused by index and type; and a client value error keeps its BadRequest class across the Data Plane boundary instead of degrading to XX000.

Behaviour changes

Path Before After
INSERT … ARRAY[0.1, 0.2, 0.3] into VECTOR(3), strict schema XX000, "got 0 elements" the row inserts
ARRAY[0.1, 'nope', 0.3] XX000 42601, naming the element index and its type
ARRAY[NaN] / ARRAY[Inf] folded through refused
A client value error crossing the Data Plane or the cluster wire XX000 42601

Closes #306.

Root cause

  • extract_vector_floats in strict_format/coerce.rs matched Value::Float and Value::Integer only. The planner's literal folding produces Decimal, so a literal ARRAY[0.1, 0.2, 0.3] contributed zero elements and the column reported got 0 elements.
  • The follow-up problem was the error class, not the coercion: crate::Error::BadRequest had no mirror in the bridge's ErrorCode enum, so the conversion fell through the catch-all to ErrorCode::Internal { detail } — every client value error crossed the Data Plane boundary as XX000, even though the direct (single-node) path already assigned 42601.
  • Two strict-encoder wrap sites (handlers/convert.rs, point/update/post_image.rs), the update-from-join collect pass, and two doors found by the audit rounds (below) had the same fold-into-Internal shape.

What changed

Commit 0fd44a626 — coercion:

  • extract_vector_floats reads Float, Integer, Decimal, and numeric String elements; a non-numeric element is rejected with its index and type; non-finite values are refused with the value.
  • The four binary_tuple wrap sites (stored_body.rs, stage_write/body.rs ×2, stage_upsert.rs) preserve BadRequest instead of wrapping it.

Commit ddefcaa0f — wire coverage:

  • document_strict_inserts_a_decimal_array_literal_into_a_vector_column (the filed case)
  • document_strict_reports_a_non_numeric_element_by_index_and_type (message names the element; SQLSTATE pinned)

Commit 6c29ac9fd — the bridge:

  • ErrorCode::BadRequest { detail } added to the bridge envelope enum with the same doc-comment pattern as its siblings, and preserved in From<crate::Error>.
  • Mirrored on the cluster wire (DataPlaneErrorCode, appended variant; the enum's own doc says variants are appended, never reordered) with both round-trip conversions.
  • pgwire renders it 42601 (the code the direct path already assigns). The node-abort classifier treats it as not established: the same code can be raised after an earlier step of a multi-step plan applied, so refusal-without-apply is not provable from the code alone.

Fixes from the audit rounds:

  • f912fb2b1: two more doors preserved BadRequesthandlers/convert.rs (CONVERT row encode) and point/update/post_image.rs (strict re-encode). Tests added: the abort classifier covers BadRequest; the cluster wire round-trips it verbatim.
  • e3e129563: the update-from-join collect pass (handlers/update_from_join.rs), which encodes strict post-images, preserved it too.

Regression proof

On base main, without this change: the filed case reports expected VECTOR(3), got 0 elements (SQLSTATE XX000), and the bad-element case finds no element message. With this change both pass, and the bad-element SQLSTATE is asserted as 42601.

Tested

  • cargo nextest run -p nodedb --test wire -E 'test(cases::engine_surface_document_strict)' — 2 new cases pass
  • error-path regressions: 42 passed across error_from_data_plane, data_plane_error_wire, write_abort, and the sqlstate/error-code unit tests
  • convert/typeguard modules: 22 passed (sql_convert_column_defs, sql_typeguard_default_gate, sql_typeguard_defaults)
  • cargo fmt --all -- --check — clean
  • cargo clippy -p nodedb -p nodedb-cluster --lib -- -D warnings — clean
  • preflight — pass

Review

A separate, read-only parity audit ran over this branch before submission. It found the same defect class on further sites and asked for two tests; all are fixed and verified here.

Finding Resolution
Two strict-encode doors folded BadRequest into Internal (handlers/convert.rs, point/update/post_image.rs) both preserve the class; the abort classifier and the cluster wire round-trip are covered by tests (f912fb2b1)
The update-from-join collect pass folded it too preserved on that path; targeted tests pass (e3e129563)
Cross-version compatibility of the appended wire variant the enum's append-only discipline; CI covers the workspace build

Notes

  • Host note: on this machine the native wire tests overflow the default thread stack and need RUST_MIN_STACK=16M. The same abort reproduces on an unrelated branch with no native-path changes, so it is a host property, not this change. CI runs them at the repository default.
  • The wire variant is appended and never reordered; existing slots are untouched.

Commits

  • 0fd44a626fix(strict-format): coerce ARRAY vector literals into a strict VECTOR
  • ddefcaa0f — ARRAY vector literal wire coverage
  • 6c29ac9fdfix(bridge): carry a bad request across the Data Plane boundary
  • f912fb2b1fix(executor): keep a bad request out of XX000 on the convert and post-image doors
  • e3e129563fix(executor): keep a bad request typed on the update-from-join collect pass

Copilot AI lite review requested due to automatic review settings September 16, 2026 20:05

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@EnRaiha EnRaiha added the run-ci Opt this PR into the full test suite; re-add to force a re-run label Sep 16, 2026
@EnRaiha
EnRaiha force-pushed the fix/issue306-vector-array-coercion branch from aa4ec43 to e3e1295 Compare September 17, 2026 02:16
@EnRaiha EnRaiha removed the run-ci Opt this PR into the full test suite; re-add to force a re-run label Sep 17, 2026
ARRAY[0.1, 0.2, 0.3] folds its literals to Decimal. extract_vector_floats
kept Float and Integer, dropped the rest, and reported "got 0 elements" as
XX000.

- accept Float, Integer, Decimal, and numeric String elements
- reject a non-numeric element by index and type
- refuse non-finite values
- preserve BadRequest at the binary_tuple wrap sites, so a client value
  error never surfaces as XX000
Error::BadRequest fell into the bridge's catch-all and reached the client as
XX000. A client value error is not an internal fault.

- ErrorCode::BadRequest mirrors the crate error across the Data Plane and the
  cluster wire (appended variant, existing slots untouched)
- pgwire renders it 42601, the code the direct path already assigns
- write-abort treats it as not established: the same code can be raised after
  an earlier step of a multi-step plan applied
…t-image doors

Two strict-encoder wrap sites still moved a client value error into
ErrorCode::Internal while the sibling sites preserved it. Mirror the sibling
arm so a bad value keeps its 42601 on every path, and pin both directions:
the abort classifier never treats a bad request as definitely-unapplied, and
the cluster wire round-trips it verbatim.
…ct pass

The collect pass encodes strict post-images, so a client value error can
surface there. It was folded into ErrorCode::Internal; preserve it like the
sibling doors so the client keeps 42601.
@EnRaiha EnRaiha added the run-ci Opt this PR into the full test suite; re-add to force a re-run label Sep 17, 2026
@EnRaiha
EnRaiha force-pushed the fix/issue306-vector-array-coercion branch from e3e1295 to 654b7cc Compare September 17, 2026 04:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

run-ci Opt this PR into the full test suite; re-add to force a re-run

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ARRAY vector literal loses every element on document_strict INSERT, reported as 0 elements under XX000

2 participants