chore: Improve Virtual Column Block Meta Generation - #20348
Conversation
github-actions
Bot
added
the
pr-chore
3f741ad to
821760a
Compare
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1bccb680e3
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
1bccb68 to
6a1ee17
Compare
zhang2014
left a comment
zhang2014
left a comment
There was a problem hiding this comment.
Went through the whole diff. The segment-local schema / id remapping / footer fallback logic looks right to me, and I did not find a path that reads wrong data. Three inline comments below: the first one (enter_column_ref type rewrite) is the one I think needs a change before merge, the other two are perf suggestions on the insert path and the block-meta pruner.
| fn enter_column_ref(&mut self, column: &ColumnRef<String>) -> RewriteResult { | ||
| let Some(data_type) = self | ||
| .column_types | ||
| .and_then(|column_types| column_types.get(&column.id)) | ||
| else { | ||
| return Ok(None); | ||
| }; | ||
| if data_type == &column.data_type { | ||
| return Ok(None); | ||
| } | ||
|
|
||
| let mut column = column.clone(); | ||
| column.data_type = data_type.clone(); | ||
| Ok(Some(column.into())) | ||
| } |
There was a problem hiding this comment.
This rewrite can leave input_domains inconsistent with the expression that is actually folded, and I believe that panics.
The ref is rewritten to the physical type here, but the parent is only re-type-checked when it is a FunctionCall (check_function in enter_function_call). When that re-check fails, the Err(_) => Ok(None) branch keeps the original call with the Nullable(Variant) column ref — while RangeIndex::apply has already inserted a Nullable(<physical>) domain for that name into input_domains (range_index.rs L136-137).
ConstantFolder::fold_with_domain then feeds a Domain::Number/Domain::String into a Variant-typed calc_domain, and those unwrap the downcast (TypedUnaryCalcDomain::domain_eval / EraseCalcDomainGeneric*Arg::domain_eval do VariantType::try_downcast_domain(..).unwrap()).
Shape that triggers it: any Variant-only function applied directly to a virtual column ref, no Cast in between, on a block whose stats for that path are typed:
WHERE json_typeof(v['a']) = 'string' -- likewise as_string(v['a']), is_string(v['a']), json_array_length(v['a']) ...check_function("json_typeof", [Nullable(UInt64)]) fails (there is no Number -> Variant auto-cast rule, only Variant -> X), the original expr is kept, the folder sees a UInt64 domain for a Variant ref and panics.
Two possible fixes:
- Only rewrite a ref whose direct parent is a
Cast/TryCast— that is the only shape that benefits (is_true(TRY_CAST(v['a'] AS UInt8 NULL) = 10)), and it is the shape the planner emits forv['a'] = 10. - Or keep the rewrite but only inject the typed domain for refs that were actually rewritten; anything left as
Nullable(Variant)must keepDomain::full(&ty).
A test next to test_range_index_prunes_try_cast_virtual_column_by_physical_type with json_typeof("v.a") = 'string' and UInt64 stats should reproduce it (I could not run it locally because the jsonb git rev fetch kept timing out, so please double check).
| let path = jsonb::keypath::KeyPaths { | ||
| paths: key_paths.to_vec(), | ||
| } | ||
| .to_owned() | ||
| .to_canonical_path(); | ||
| *self.source_paths[source_index].entry(path).or_default() += 1; |
There was a problem hiding this comment.
perf: this is now on the regular write path — StreamBlockProperties/BlockBuilder create a JsonPathStatisticsBuilder for every MutationKind other than Recluster/Compact/Refresh when enable_virtual_column = true, and observe_path runs once per scalar leaf per row.
Per leaf it does key_paths.to_vec() + .to_owned() (one String per path segment) + .to_canonical_path() (another String), and entry(path) needs the owned String even when the path already exists — so it is 3+ allocations per leaf for a map that is almost always a hit.
VirtualColumnBuilder::extract_virtual_values already handles the same problem with a hash-first lookup (SipHasher24 over key_paths -> StackHashMap<u128, usize>) and only builds the owned path the first time it is seen. Reusing that here (hash -> index into Vec<(String, u64)>) would make the common path allocation-free.
Minor: max_path_statistics is only applied in finalize, so source_paths can grow unbounded within a block. Refusing new keys once the cap is reached (and setting path_statistics_complete = false) would bound memory as well.
| column.paths.iter().any(|item| { | ||
| item.path | ||
| .strip_prefix(prefix) | ||
| .is_some_and(|suffix| suffix.starts_with('.') || suffix.starts_with('[')) | ||
| }) |
There was a problem hiding this comment.
nit/perf: paths is sorted by canonical path, so this can be a partition_point for prefix + "." / prefix + "[" instead of strip_prefix over every path. try_prune_from_block_meta calls it once per (virtual field x block); since the schema is shared by all blocks of a segment, the answer could also be computed once per segment and reused across blocks.
2 participants
I hereby agree to the terms of the CLA available at: https://docs.databend.com/dev/policies/cla/
Summary
This PR improves the metadata management, generation lifecycle, and read efficiency of Fuse virtual columns.
Changes
Persist segment-level virtual column schemas
Generate virtual column data during rewrite operations
OPTIMIZE TABLE ... COMPACTor reclustering.Replace the table-level virtual schema with segment-local schemas
TableMeta.virtual_schemaand stops maintaining a global virtual column schema at the table level.SegmentInfo.summary.virtual_segment_schema, allows each segment to use a layout derived from its own JSON path distributionRead virtual column metadata from BlockMeta
Support range pruning for virtual columns
RangeIndexto consume statistics from virtual columns.min,maxandnull-countstatistics to prune blocks before reading source Variant data.Add diagnostic table functions for virtual columns schemas, block metadata, sidecar files, and simulated generation results. These functions make it easier to inspect how JSON paths are classified, verify that blocks in a segment use a consistent layout, diagnose pruning behavior, and estimate virtual sidecar sizes before rebuilding them.
fuse_virtual_column_segment_schemadisplays the segment-local mapping between canonical JSON paths and virtual column IDs.fuse_virtual_column_block_metadisplays virtual column metadata for each block, resolved against its segment-local schema, including physical types, offsets, sizes, statistics, and shared-path information.fuse_virtual_column_parquet_metareads virtual sidecar Parquet footer metadata and displays the direct and shared physical columns stored in each sidecar file.fuse_virtual_column_buildsimulates virtual column generation for a block or an entire segment without modifying table data. It supports an optionalmax_direct_columnsargument for evaluating different direct/shared layouts.fixes: #[Link the issue here]
Tests
Type of change
AI assistance
This change is