fix: catalog rename metadata, data_url field, and ToSqlParam/IntoValue type coverage#134
Merged
StefanSteiner merged 5 commits intoJun 10, 2026
Conversation
`reconcile_in` previously treated a rename as a delete + fresh stub: the disappeared table's catalog row was deleted, and the new table name got a bare stub with all prose fields (source_url, purpose, notes, load_tool, load_params, created_by, loaded_at) wiped. Fix: before the delete/stub loop, detect renames by matching disappeared catalog entries to new live tables via row count. A match is only accepted when BOTH sides are unambiguous (exactly one disappeared entry has that row count AND exactly one new table has it), preventing false-positive metadata assignment when multiple tables share a count (e.g. two empty tables). Entries with NULL row_count are excluded from matching. Matched entries get their table_name updated in place via a simple UPDATE — all other fields (including loaded_at and last_refreshed_at) are preserved untouched. Adds: - `rename_catalog_entry` helper (UPDATE SET table_name only) - Test: creates a table with full metadata, renames it, reconciles, asserts every field (source_url, purpose, notes, load_tool, row_count, loaded_at, last_refreshed_at) survived. Closes tableau#59.
…tableau#60) Adds a new `data_url` TEXT column to `_table_catalog`, exposed through `set_table_metadata`. This is the machine-actionable download URL for the raw data file — distinct from `source_url` (human-readable page) and `source_description` (prose). With `data_url` and the existing `load_params`, an automated refresh becomes fully mechanical: fetch `data_url`, infer format from the extension, re-ingest with the recorded parameters. Changes: - `CATALOG_COLUMNS`: add `data_url TEXT` to the schema. - `ensure_exists_in` migration: `ALTER TABLE ADD COLUMN data_url TEXT` so pre-existing catalogs gain the column without recreation. - `CatalogEntry`: add `data_url: Option<String>` + `to_json()` output. - `MetadataFields`: add `data_url` + include in `is_empty()` check. - `set_metadata_in`: generate the `data_url = ...` assignment. - `upsert_stub_in` INSERT: include `data_url` in the column list (explicit NULL for consistency with the 14-column schema). - `SetTableMetadataParams` (server): add `data_url` param + thread it. - Tool description updated to mention `data_url`. - SELECTs in `list_in`/`get_in`: fetch the new column. - `row_to_entry`: parse `data_url` from the row JSON. - Module doc table: add `data_url` entry. - Test: `set_metadata_data_url_roundtrip` — set, read back, clear with empty string. Closes tableau#60.
…au#65) Adds binary-bind ToSqlParam impls so these types can be passed to query_params without manual stringification: - Interval: PG binary [us:i64 BE][days:i32 BE][months:i32 BE]. - JSON (serde_json::Value): PG `json` binary form is the UTF-8 text (no jsonb version byte); serde_json is already an unconditional dep, so no feature flag is needed. - Numeric: PG binary NUMERIC, scale=0 only. Hyper rejects scaled binary NUMERIC params server-side with SQLSTATE 0A000 regardless of encoding or explicit CAST (verified empirically), so scaled support is not possible over the current binary bind path — tracked in tableau#132. For scale>0, encode_param sets dscale>0 so the param fails fast at the server rather than silently binding a mis-scaled whole number; the byte payload is deliberately server-rejected, NOT a faithful scaled encoding (correct decimal-aligned base-10000 grouping is tableau#132). Unit tests cover the scale=0 byte layout (42, 0, -1, 123456789), the dscale-for-rejection behavior, Interval bytes, and JSON bytes. Integration tests round-trip JSON, Interval, scale=0 Numeric, and pin the scale>0 fail-fast rejection (flips to success when tableau#132 lands). Partially addresses tableau#65 (Geography in the next commit).
Adds Geography support to the Inserter (COPY / HyperBinary path), which the issue's gap analysis flagged as missing. Geography can now be passed to Inserter::add_row via the IntoValue trait. - Inserter::add_geography(&Geography) delegates to the existing varbinary path (add_bytes), passing the raw as_bytes() payload. The chunk-level write_varbinary already prepends the [u32_le len] framing, so we must NOT pass to_hyper_binary() output (which would double-prefix the length and corrupt the value). - impl IntoValue for Geography and &Geography (Geography owns a Vec and isn't Copy, so these take/pass by reference — mirrors the &Interval / &Numeric reference impls). - Round-trip test (WKT POINT in → byte-identical raw bytes out) and a nullable test (Some + None via the Option<T> blanket impl → SQL NULL). NOTE: ToSqlParam for Geography (query-parameter binding) is NOT included — Hyper has no PostgreSQL-binary input function for the geography type (error 42883 on bind), so it is impossible over the current binary-only bind path. Tracked in tableau#133 (needs per-parameter text-format bind, same root cause as tableau#132). Geography also does not yet implement RowValue, so results are read as raw bytes; that reader is future work. Closes tableau#65.
… doc (tableau#65) Final-review polish for the tableau#65 ToSqlParam work: - params.rs module doc: list the newly-supported types (Numeric scale=0, Interval, serde_json::Value, bytes) and note Geography is Inserter-only (tableau#133), so the 'Supported Types' list matches the actual impls. - test_interval_param: assert the param renders to "P2M5D" (ISO-8601), proving the [us BE][days BE][months BE] field encoding decoded correctly — not just that a non-null value came back. - Add test_option_numeric_param: exercises the Option<T> blanket impl (Some(Numeric scale=0) → value, None → SQL NULL).
StefanSteiner
force-pushed
the
fix/catalog-rename-preserves-metadata
branch
from
7f25466 to
ae618fb
Compare
StefanSteiner
changed the title
Contributor
Author
|
Five fixes across the MCP catalog and the Rust API param/insert layer. All |
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Five fixes across the MCP catalog and the Rust API param/insert layer. All
additive — patch release (v0.5.3 → v0.5.4).
Changes
ALTER TABLE RENAMEno longer drops_table_catalogprosemetadata.
reconciledetects renames (1:1 row-count match) and renames thecatalog row in place; ambiguous matches fall back to delete+stub.
data_urlcolumn in_table_catalog(+set_table_metadataparam): the machine-actionable download URL, distinct from the human-readable
source_url, enabling mechanical refresh.ToSqlParamforInterval,serde_json::Value(JSON), andNumeric(scale=0);IntoValue+Inserter::add_geographyforGeography.Known limitations (filed as follow-ups)
Hyper's binary bind path can't accept everything; surfaced empirically and
documented in code:
Numericparams (scale > 0) are rejected by Hyper(
0A000). The impl encodes scale=0 correctly and makes scale>0 fail fastrather than silently bind a mis-scaled integer.
ToSqlParam for Geographyis impossible (Hyper has no pg-binaryinput function for
geography,42883). Geography is supported via theInserter (
IntoValue) path instead.Testing
P2M5D) / scale=0 Numeric/
Option<Numeric>/ Geography (byte-identical) and pin the scale>0fail-fast rejection; catalog rename +
data_urlround-trip tests.cargo test -p hyperdb-api+-p hyperdb-mcpgreen; workspaceclippy -D warnings,fmt --check, andcargo denyclean. No new deps.