Skip to content

Conversation

Erigara
Copy link

@Erigara Erigara commented Sep 1, 2025

Which issue does this PR close?

Rationale for this change

It more convenient to debug tings when timestamps are properly shown.

What changes are included in this PR?

Fix in methods and tests to reflect changes.

Are these changes tested?

Changes are covered by adjusted tests.

Are there any user-facing changes?

Yes, when user would display Row or individual Field timestamps would be shown with proper precision.

@github-actions github-actions bot added the parquet Changes to the parquet crate label Sep 1, 2025
/// Helper method to convert Parquet timestamp into a string.
/// Input `value` is a number of milliseconds since the epoch in UTC.
/// Datetime is displayed in local timezone.
#[inline]
fn convert_timestamp_millis_to_string(value: i64) -> String {
convert_timestamp_secs_to_string(value / 1000)
let dt = Utc.timestamp_millis_opt(value).unwrap();
format!("{}", dt.format("%Y-%m-%d %H:%M:%S%.3f %:z"))
Copy link
Member

Choose a reason for hiding this comment

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

The input is i64, so it will never attach a time zone, and the output will always be' +00:00'. Do we need to add "%:z" in the output?

From Utc.timestamp_millis_opt, it seems that it will always return a UTC timezone value, so maybe we need to change the comment for this function(the comment said the result will be in local timezone).

Copy link
Member

Choose a reason for hiding this comment

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

Another question not related to the current PR: this function convert_timestamp_millis_to_string will be used in Field, and Field contains an i64 for TimestampMillis/TimestampMicros(the milliseconds/microseconds from unix epoch), does output the raw timestamp(the i64 value) here is enough? and translate the timestamp to dt.format value when displaying something like LogicalType::Timestamp

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
parquet Changes to the parquet crate
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Dont truncate timestamps on display for Row
2 participants