Skip to content

Core: Deprecate 'downcast-ns-timestamp-to-us-on-write' config key#3596

Open
qzyu999 wants to merge 1 commit into
apache:mainfrom
qzyu999:deprecate-downcast-on-write-config-key
Open

Core: Deprecate 'downcast-ns-timestamp-to-us-on-write' config key#3596
qzyu999 wants to merge 1 commit into
apache:mainfrom
qzyu999:deprecate-downcast-on-write-config-key

Conversation

@qzyu999

@qzyu999 qzyu999 commented Jul 2, 2026

Copy link
Copy Markdown

Which issue does this PR close?

Minor config naming issue raised in #3594 (review)

Rationale for this change

The config key downcast-ns-timestamp-to-us-on-write (env: PYICEBERG_DOWNCAST_NS_TIMESTAMP_TO_US_ON_WRITE) says 'on-write' but is also used in the read path (ArrowScan) to downcast nanosecond timestamps to microseconds. The name is misleading.

What changes are included in this PR?

  • Introduce a new canonical config key downcast-ns-timestamp-to-us (env: PYICEBERG_DOWNCAST_NS_TIMESTAMP_TO_US)
  • Add _get_downcast_ns_timestamp_to_us() helper that checks the new key first, falls back to the old key with a DeprecationWarning\
  • Migrate all call sites to use the helper
  • Update docs and error messages
  • Add 3 unit tests for backwards compatibility (legacy key alone, new key alone, both set)

Are there any user-facing changes?

  • New config key: downcast-ns-timestamp-to-us / PYICEBERG_DOWNCAST_NS_TIMESTAMP_TO_US\
  • Backwards compatible: the old key still works but emits a DeprecationWarning\
  • Error message updated: the TypeError for unsupported 'ns' precision now references the new key name

…of 'downcast-ns-timestamp-to-us'

The config key 'downcast-ns-timestamp-to-us-on-write' says 'on-write' but is also used in the read path (ArrowScan). Rename the canonical key to 'downcast-ns-timestamp-to-us' which accurately describes the behavior regardless of direction.

The old key still works but emits a DeprecationWarning. The new key takes precedence when both are set.
@qzyu999 qzyu999 force-pushed the deprecate-downcast-on-write-config-key branch from 59ade22 to c887117 Compare July 2, 2026 21:01
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.

1 participant