Skip to content

fix: projection dropping a cftime coord crashes batch reading - #230

Merged
alxmrs merged 1 commit into
xqlsystems:mainfrom
ghostiee-11:fix/cftime-projection-drops-dim
Aug 2, 2026
Merged

fix: projection dropping a cftime coord crashes batch reading#230
alxmrs merged 1 commit into
xqlsystems:mainfrom
ghostiee-11:fix/cftime-projection-drops-dim

Conversation

@ghostiee-11

Copy link
Copy Markdown
Contributor

A projection that drops a cftime dimension coordinate (for example GROUP BY on a non-time dim, which removes time from the scan) crashed with KeyError: 'Column time does not exist in schema' during batch reading.

iter_record_batches preloaded every dimension's coord values and called schema.field(name) for cftime coords, but the projected batch schema no longer contains the dropped dim. datetime64 coords skip that branch, so only cftime datasets broke.

The dropped dim is never read in the batch loop (it iterates only the schema's fields), so this skips preloading any dim absent from the projected schema.

Verified across all calendars (Gregorian/NoLeap mapped to timestamp, 360-day/Julian mapped to int64) and every projected dimension. The regression test in test_df.py fails without the fix (KeyError) and passes with it.

iter_record_batches preloaded every dim's coord values and called
schema.field(name) for cftime coords, but a projection (e.g. GROUP BY on a
non-time dim) drops columns from the batch schema, so the lookup raised
KeyError. Skip dims absent from the projected schema; they are never read in
the batch loop, which iterates only the schema's fields.
@ghostiee-11
ghostiee-11 force-pushed the fix/cftime-projection-drops-dim branch from 61b1d74 to 03d0c75 Compare July 23, 2026 19:31
@ghostiee-11

Copy link
Copy Markdown
Contributor Author

@alxmrs, Can you please review this

@alxmrs

alxmrs commented Aug 2, 2026

Copy link
Copy Markdown
Collaborator

Sorry for the late reply. TAL now.

@alxmrs alxmrs left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Thanks for the fix.

@alxmrs
alxmrs merged commit 6b3077c into xqlsystems:main Aug 2, 2026
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants