Repository navigation
Defer numeric property formatting - #229
Merged
Merged
Conversation
Keep numeric plan properties as numbers until they are rendered. This preserves their exact values for Plan Insights and other consumers while still presenting readable counts, byte sizes, and durations in node and metadata panels. Represent each scalar property as a PropertyEntry containing a string or number plus an optional NumberFormat hint. DuckDB and Hyper infer compatible formats from property names; Postgres limits inference to rounded counts because its timing and memory values use different units. Other loaders retain raw scalar values without inferred formatting. Move number presentation into a shared formatter and PropertyList used by both graph nodes and plan metadata. Keep complex JSON values flat and stringified so nested properties remain outside this change. Update loader consumers, documentation, focused formatter coverage, and loader output snapshots for the scalar entry representation.
vogelsgesang
force-pushed
the
defer-number-formatting
branch
from
October 29, 2026 02:19
46eaa24 to
2f540a6
Compare
Restore the formatter and property-model documentation carried by the original nested-properties work, and document that plan metadata uses the same scalar entry type as node properties. Keep loader parsing utilities distinct from the renderer-facing number-format module. Retain the original direct formatter cases, verify that Hyper and Postgres attach formats without changing raw numeric values, and cover formatting through the shared PropertyList renderer.
vogelsgesang
commented
Nov 4, 2026
Use count consistently for approximate cardinality formatting and replace the legacy metric formatter with the shared count formatter for graph edge labels. This keeps cardinalities consistent with property values and the formatting behavior from the original nested-properties work. Add a strict string property accessor and use it for Tableau XML fields that are semantically textual, while retaining scalar access for identifiers that may legitimately be numbers. Keep exported property accessors together after the model interfaces and clarify inference helper names. Update focused expectations and loader compatibility snapshots for the new count name and edge-label presentation.
Replace name-based format inference with loader-owned dictionaries. Unknown numeric properties use exact formatting with thousand separators, including very small fractions. Require an explicit format at the formatting boundary. Cover DuckDB 2.0 operator timing, CPU time, row-group counts, row counts, and intermediate sizes. Format PostgreSQL timing and memory values in their original millisecond and 1024-byte kB units without changing stored values. Add shared Umbra/CedarDB cardinalities and microsecond pipeline timings, documenting which timing fields belong to each system. Verify their units against local Umbra and CedarDB execution probes. Add millisecond, microsecond, and kilobyte formats that scale only during rendering. Update loader, formatter, and UI tests, documentation, and loader output snapshots while keeping nested properties serialized.
vogelsgesang
commented
Nov 4, 2026
Keep pipeline start and stop offsets exact while formatting durations with their source units. Restrict DuckDB's operator format dictionary to tree properties, since current profile metadata retains nested metric groups. Link the DuckDB and PostgreSQL metric documentation from their dictionaries. Put exact formatting first, place numeric dictionaries after structural configuration, and clarify the graph node's property-list variable. Trim unnecessary README detail and document nested JSON scalar conversion. Split formatter tests by format, move accessor and scalar-conversion tests beside the modules they cover, and align loader assertions around preserved values and selected formats. Remove the redundant DuckDB case and duplicate PostgreSQL assertion, and update the start/stop output snapshots.
vogelsgesang
commented
Nov 4, 2026
Skip numeric, Boolean, null, object, and array category candidates and try the next configured property source. Use the configured fallback when no source yields a string, while preserving empty strings as valid categories. Remove categorical insights' forceToString dependency so serialized objects and numeric values cannot become categories through implicit conversion. Cover alternative sources and fallback behavior without changing the existing insight golden results.
Preserve NaN and signed Infinity as numeric property values with their selected number format. Render them through formatNumber without unit or count suffixes, keeping presentation decisions outside the loaders. Update loader preservation and PropertyList rendering coverage, remove the unnecessary scalar-conversion test, and drop the redundant DuckDB metadata comment requested in review.
vogelsgesang
commented
Nov 4, 2026
Inline string and JSON conversion into toPropertyEntry and remove the forceToString helper. Route Hyper pipeline statistics and error metadata through the same property-entry constructor as other loader properties. Update the helper documentation and remove the loader-utils test file requested in review. Existing loader and insight golden results remain unchanged.
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
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.
Keep numeric plan properties as numbers until they are rendered. This preserves their exact values and source units for Plan Insights and other consumers while still presenting readable counts, byte sizes, and durations in node and metadata panels.
Represent each scalar property as a PropertyEntry containing a string or number plus an optional NumberFormat hint. Hyper, DuckDB, Postgres, and the shared Umbra/CedarDB loader select formats from explicit property-name dictionaries. Unknown numeric properties use exact formatting with thousand separators. Add source-unit-aware formats for PostgreSQL milliseconds and 1024-byte kB and Umbra/CedarDB microseconds; stored values are never scaled.
Move number presentation into a shared formatter and PropertyList used by both graph nodes and plan metadata. Cover DuckDB 2.0 operator metrics, including timing, CPU time, row-group counts, and intermediate sizes. Keep complex JSON values flat and stringified so nested properties remain outside this change.
Update loader consumers, documentation, focused formatter and loader coverage, and loader output snapshots for the scalar entry representation. Confirm Umbra/CedarDB timing units with local execution probes.