Skip to content

fix: add hstore support and decode arrays of custom-OID types - #71

Merged
aesslinger merged 4 commits into
mainfrom
fix/hstore-extract-null
Sep 15, 2026
Merged

aesslinger merged 4 commits into
mainfrom
fix/hstore-extract-null

Conversation

@aesslinger

@aesslinger aesslinger commented Sep 14, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • hstore has no well-known Postgres OID, so extract.rs's Type:: dispatch table never matched it — columns silently decoded as null ([Bug]: HSTORE support in new postgreSQL plugin #68, Bug: HSTORE fields being shown as null even when containing data #69). Fixed by dispatching on ty.name() == "hstore", matching the builtin driver's extract/simple.rs pattern, and decoding via tokio-postgres's native HashMap<String, Option<String>> support.
  • Also ports the write side from tabularis#427 (referenced from Phase 2.3: Extension-aware type system #27/Phase 2: PostgreSQL-specific features beyond the built-in driver (tabularis#16) #9): binding.rs previously rejected any JSON object bound to a non-JSON column, so editing an hstore cell failed with "Cannot bind a JSON object to a non-JSON column". Now resolves the real hstore OID per column via pg_type (extension types aren't well-known OIDs) and binds through the same HashMap shape tokio-postgres encodes natively.
  • Found during review: the same "no well-known OID" gap applied to arrays of any custom-OID element type, not just scalar hstore columns — enum[], hstore[], and in practice every array type without a hardcoded fast-path (numeric[], date[], json[], money[], inet[], etc.) also silently decoded to null (Arrays of custom-OID types (enum[], hstore[], etc.) decode to null #72). Fixed with a generic 1-D array decoder that recurses per-element, matching the builtin driver's generic Kind::Array dispatch. Existing hardcoded array fast-paths (int2/int4/int8/float4/float8/bool/text/varchar) are untouched.
  • Hardening found in self-review: the new array decoder initially pre-allocated its output Vec using the wire header's claimed element count, before validating the buffer actually contained that many elements — a truncated/corrupted array value could claim up to i32::MAX elements and trigger a multi-gigabyte allocation attempt. Fixed to grow incrementally instead, with a regression test.
  • Unrelated but blocking: bumps rustls 0.23.43 → 0.23.45, fixing RUSTSEC-2026-0285 (CVE-2025-61730), a new advisory published while this PR was open that started failing CI's Security audit gate for every branch in the repo, including this one. Cargo.toml already allowed the patched version; the lockfile was just stale.

Closes #68. Closes #69 (already closed as a duplicate of #68). Closes #72.

Also filed #73 for an unrelated pre-existing bug found while regression-testing this: the hardcoded array fast-paths decode the whole column to null when any element is NULL (Vec<T> vs Vec<Option<T>>) — confirmed unchanged by this PR (same behavior on main), left for separate follow-up.

Test plan

  • cargo test --lib — 152 passing, including new unit tests for hstore decode/bind, the generic array decoder (hand-built wire bytes, following this repo's TDD-without-live-db convention), and the untrusted-length allocation-safety fix
  • cargo clippy --all-targets -- -D warnings — clean
  • cargo fmt --all — clean
  • cargo audit --ignore RUSTSEC-2026-0235 — clean (only the pre-existing, already-ignored chacha20 yanked-crate warning remains)
  • New live-db tests pass against a real PostgreSQL 16 instance (self-contained, create their own extension/types/tables): execute_query_returns_a_real_hstore_value_not_null, execute_query_returns_real_array_values_for_custom_oid_element_types
  • Manual JSON-RPC smoke test against a live database: insert_record/update_record with hstore objects round-trip correctly
  • Full regression matrix (40+ checks) run against a live database covering every previously-supported scalar type, every existing hardcoded array type (confirmed byte-identical to pre-fix), and every newly-covered type — no regressions found
  • All 7 CI checks green: Test, Clippy/fmt, Security audit, Live PostgreSQL integration, Markdown lint, PR title, Manifest validation
  • mergeStateStatus: CLEAN, no conflicts with main (branch is up to date with main, which hasn't moved since this branch was cut)

hstore has no well-known Postgres OID, so extract.rs's Type:: dispatch
table never matched it and columns silently decoded as null (#68, #69).
Dispatch on ty.name() == "hstore" instead, matching the builtin driver's
extract/simple.rs pattern, and decode via tokio-postgres's native
HashMap<String, Option<String>> support.

Also ports the write side from tabularis#427: binding.rs rejected any
JSON object bound to a non-JSON column, so editing an hstore cell failed
outright. Resolves the real hstore OID per column via pg_type (extension
types aren't well-known OIDs) and binds through the same HashMap shape.
@aesslinger aesslinger added the prerelease:rc Version suggestion targets a release candidate label Sep 14, 2026
@github-actions

Copy link
Copy Markdown

Version suggestion

Based on this PR's title (fix) and the prerelease:rc label:

Current 1.0.0-rc.2
Suggested next tag v1.0.0-rc.3

This is informational only — no tag or release is created automatically yet.

extract.rs's dispatch only special-cased a handful of well-known array
OIDs (int2/int4/int8/float4/float8/bool/text/varchar), so any array whose
element type isn't one of those — enum[], hstore[], and in practice every
other type without a hardcoded fast-path (numeric[], date[], json[],
money[], inet[], etc.) — fell through to the generic string fallback and
silently decoded to null (#72).

Adds a generic 1-D array decoder (ArrayValue) that parses the array wire
format directly and recurses per-element via extract_element_from_bytes,
matching the builtin driver's generic Kind::Array dispatch
(extract/mod.rs + extract/array.rs::try_extract_elem). Placed after the
existing hardcoded array arms so their behavior is unchanged. Multi-
dimensional arrays still fall back to null, consistent with this file's
existing 1-D-only array handling.

Verified against a live database that every previously-working scalar
and array type is byte-identical to before, and that the newly-covered
array types match the builtin driver's exact JSON shape for their scalar
form. Filed #73 for an unrelated pre-existing bug found during this
audit: hardcoded array types decode the whole column to null when any
element is NULL (Vec<T> vs Vec<Option<T>>), unaffected by this fix.
@aesslinger aesslinger changed the title fix: add hstore support to value extraction and binding fix: add hstore support and decode arrays of custom-OID types Sep 14, 2026
rustls 0.23.43 incorrectly accepted TLS 1.3 handshake messages sent at
the wrong encryption level when packed into the same record as a
key-changing message (CVE-2025-61730) — fixed upstream in 0.23.45.
CI's cargo-audit gate started failing on this PR once the advisory
was published, unrelated to this branch's actual changes.
ArrayValue::from_sql called Vec::with_capacity(len) using the array
header's claimed element count before validating the buffer actually
contains that many elements. A truncated or corrupted array value can
claim up to i32::MAX elements while carrying far fewer bytes, which
would attempt a multi-gigabyte allocation on the first decode attempt,
before the per-element truncation check ever runs.

Vec::new() grows by amortized doubling as elements are actually read,
so the allocation stays proportional to what's genuinely present in
the buffer. Added a regression test with a claimed i32::MAX-element
array and no element bytes following it.
@aesslinger
aesslinger merged commit 60c4baf into main Sep 15, 2026
14 checks passed
@aesslinger
aesslinger deleted the fix/hstore-extract-null branch September 15, 2026 12:05
aesslinger added a commit that referenced this pull request Sep 15, 2026
Ships PR #71 (hstore read/write support, plus decoding for arrays of
any custom-OID element type — fixes #68, #69, #72), PR #75 (hardcoded
array types no longer null out the whole array when one element is
NULL — fixes #73), PR #76 (table/column comments exposed via
get_tables/get_columns, closing #74), and PR #77 (SHOW/CALL with a
limit param no longer produces a LIMIT syntax error, while
SELECT/WITH/VALUES/TABLE/EXPLAIN keep real SQL-level pagination —
fixes #70). Also carries the rustls 0.23.43 -> 0.23.45 security bump
(RUSTSEC-2026-0285) merged as part of PR #71's branch.

Verified: .tabularium re-validated as valid JSON with version matching
the upcoming v1.0.0-rc.3 tag; cargo build --release/test (171 unit +
18 live-DB against the local pg-tabularis-test container)/clippy/fmt/
audit all pass; the release binary returns a valid initialize
JSON-RPC response.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

prerelease:rc Version suggestion targets a release candidate

Projects

None yet

1 participant