Skip to content

default_null_columns is never populated, so a newly added column fails on older chunks #110

Description

@abernatskiy

(With Claude): The null-filling path for schema evolution exists but is unreachable, and the storage reader does not implement it at all.

1. The set is always empty. TableSet::set_nullable and set_column_default (crates/query/src/plan/table.rs:39,44) are the only ways to populate column_defaults, and neither is called anywhere in the repo:

$ git grep -n "set_nullable\|set_column_default" -- crates/
crates/query/src/plan/table.rs:39:    pub fn set_column_default(...)
crates/query/src/plan/table.rs:44:    pub fn set_nullable(...)

So default_null_columns() always returns an empty HashSet, and the branch that would inject NullArrays in crates/query/src/scan/parquet/file.rs:170 is dead code. Every projected column missing from a chunk schema takes the else arm instead:

tracing::error!("column ...is not found in...");
anyhow::bail!(ColumnDoesNotExist::new(self.table_name.to_string(), name));

2. The storage reader ignores the parameter. impl TableReader for SnapshotTableReader takes it as _default_null_columns (crates/query/src/scan/storage/reader.rs:23) and never uses it, so populating the set would fix the parquet path only.

Effect. Adding a column to a chunk builder is not backward compatible: selecting the new field over any chunk written before the change returns ColumnDoesNotExist rather than nulls. A concrete instance is the solana transaction_config column added in 8de9324 with query support in aa62e79 — selecting transactionConfig over chunks predating 8de9324 errors. The same applies to any future column addition on any dataset kind.

Suggested fix: declare newly added columns via set_nullable in the per-dataset query definitions, and honour default_null_columns in SnapshotTableReader::read so both scan paths behave the same. Worth deciding whether the default should instead be to null-fill any missing projected column, since the current behaviour makes every schema addition a breaking change for historical ranges.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions