Skip to content

Conversation

@kosiew
Copy link

@kosiew kosiew commented Dec 22, 2025

Which issue does this PR close?

Rationale for this change

The to_timestamp* scalar UDF implementations (to_timestamp, to_timestamp_seconds, to_timestamp_millis, to_timestamp_micros, to_timestamp_nanos) each contained near-identical boilerplate for:

  • Default implementations
  • deprecated new() constructors (kept for backward compatibility)
  • new_with_config() constructors that extract timezone from ConfigOptions
  • with_updated_config() implementations that rebuild the UDF with the new config

This duplication increases maintenance cost and the risk of the functions drifting apart over time.

What changes are included in this PR?

  • Introduces a impl_to_timestamp_constructors! macro to generate:

    • Default impl
    • deprecated new() constructor
    • new_with_config() constructor that pulls execution.time_zone from ConfigOptions
  • Applies the macro to all five ToTimestamp* UDF structs.

  • Introduces an impl_with_updated_config! macro to generate the identical with_updated_config() method body.

  • Replaces per-type with_updated_config() method implementations with the macro invocation.

Are these changes tested?

  • No new tests added.
  • This PR is a pure refactor (behavior-preserving): the generated constructors and with_updated_config() bodies are identical to the previous hand-written implementations.
  • Existing datetime/to_timestamp UDF tests should continue to cover runtime behavior.

Are there any user-facing changes?

  • No user-facing behavior changes expected.
  • Public API remains the same (deprecated new() constructors are still present); this change only reduces internal boilerplate.

Copy link
Owner

@Omega359 Omega359 left a comment

Choose a reason for hiding this comment

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

Thanks, merging.

@Omega359 Omega359 merged commit 1579ec3 into Omega359:timestamp-17998 Dec 22, 2025
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