Skip to content

docs: doc the requirement for ParquetHandler to do type widening #3313

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
dengsh12 wants to merge 2 commits into
base: main
Choose a base branch
from
+16 −7
Open

docs: doc the requirement for ParquetHandler to do type widening #3313

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a13803c
add comments
dengsh12 Sep 11, 2026
6aa9c47
ai review
dengsh12 Sep 12, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 16 additions & 7 deletions kernel/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -792,13 +792,22 @@ pub trait ParquetHandler: AsAny {
///
/// # Type coercion
///
/// A matched Parquet column whose physical type differs from the `physical_schema`
/// [`StructField`] must be coerced to the requested type. In particular, timestamp columns MUST
/// be normalized to the protocol specified microsecond precision: a `TIMESTAMP(MILLIS)` (or
/// any other non-microsecond unit) column read into a `TIMESTAMP` / `TIMESTAMP_NTZ` field
/// must be rescaled to microseconds (a finer unit such as nanosecond is truncated). The
/// default engine does this via `arrow::compute::cast` while reordering columns to the
/// requested schema.
/// When `physical_schema` requests a type different from the matched Parquet column, the reader
/// must coerce it to the requested type when the conversion is allowed by the [Delta protocol
/// type widening rules]. Examples include `INTEGER` -> `LONG`, `FLOAT` -> `DOUBLE`, and `DATE`

@github-actions github-actions Bot Sep 12, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Nit1 The examples INTEGER -> LONG, FLOAT -> DOUBLE, and DATE -> TIMESTAMP_NTZ sit directly under the type widening rules link, while the next paragraph lists INT32 -> DATE and INT64 -> TIMESTAMP/TIMESTAMP_NTZ as extra Kernel requirements. A reader could infer these coercions are gated on the type-widening table feature rather than being reader-side coercions needed to materialize the requested physical_schema. Raised by: maintainer-codex-reviewer. Suggested fix: add a short clause noting these coercions are required whenever physical_schema asks for a compatible type and are not conditional on the type-widening feature being enabled.

/// -> `TIMESTAMP_NTZ`. See the protocol for the complete list.
Comment on lines +795 to +798

@dengsh12 dengsh12 Sep 12, 2026 •
edited
Loading

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

DefaultEngine does this, but some engine has not implemented this -- will inform them

///
/// Kernel additionally requires readers to support `INT32` -> `DATE` and `INT64` ->
/// `TIMESTAMP` / `TIMESTAMP_NTZ` conversions for checkpoint `stats_parsed` and
/// `partitionValues_parsed` fields.
///
/// Timestamp columns MUST be normalized to the protocol specified microsecond precision: a
/// `TIMESTAMP(MILLIS)` (or any other non-microsecond unit) column read into a `TIMESTAMP` /
/// `TIMESTAMP_NTZ` field must be rescaled to microseconds (a finer unit such as nanosecond is
/// truncated). The default engine does this via `arrow::compute::cast` while reordering columns
/// to the requested schema.
Comment on lines +807 to +808

@nicklan nicklan Sep 12, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

nit:

Suggested change
/// truncated). The default engine does this via `arrow::compute::cast` while reordering columns
/// to the requested schema.
/// truncated). The default engine does this via `arrow::compute::cast`.

///
/// [Delta protocol type widening rules]: https://github.com/delta-io/delta/blob/master/PROTOCOL.md#type-widening
///
/// # Metadata Columns
///
Expand Down
Loading