Skip to content
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.

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

refactor: use jiff crate replace chrono in date/timestamp func #16787

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

TCeason
Copy link
Collaborator

@TCeason TCeason commented Nov 7, 2024

I hereby agree to the terms of the CLA available at: https://docs.databend.com/dev/policies/cla/

Summary

In jiff https://docs.rs/jiff/latest/jiff/index.html#time-zone-features, we enable tzdb-bundle-always(When enabled, Jiff will forcefully depend on the jiff-tzdb crate). It's not a good idea when use jiff crate.

But if not enable it, jiff will read tzinfo from file /usr/share/zoneinfo. This file relate tzdata in system env. And we init timezone in function ctx, if system not install tzdata will cause panic. So we enable feature tzdb-bundle-always as default.

Done:

register_diff_functions(registry)
register_to_number_functions(registry)
register_add_functions(registry)
register_sub_functions(registry)
register_timestamp_to_date
register_date_to_timestamp
register_to_string
register_string_to_date
register_string_to_timestamp
register_real_time_functions
register_rounder_functions

Tests

  • Unit Test
  • Logic Test
  • Benchmark Test
  • No Test - Explain why

Type of change

  • Bug Fix (non-breaking change which fixes an issue)
  • New Feature (non-breaking change which adds functionality)
  • Breaking Change (fix or feature that could cause existing functionality not to work as expected)
  • Documentation Update
  • Refactoring
  • Performance Improvement
  • Other (please describe):

This change is Reviewable

@TCeason TCeason marked this pull request as draft November 7, 2024 10:59
@github-actions github-actions bot added the pr-refactor this PR changes the code base without new features or bugfix label Nov 7, 2024
@TCeason TCeason force-pushed the jiff branch 10 times, most recently from 884c9d1 to f284827 Compare November 8, 2024 14:16
Done:

register_diff_functions(registry);
register_to_number_functions(registry);
register_add_functions(registry);
register_sub_functions(registry);
register_timestamp_to_date
register_date_to_timestamp
register_to_string
register_string_to_date
register_string_to_timestamp

TODO:

register_real_time_functions(registry);
register_rounder_functions(registry);
@TCeason TCeason marked this pull request as ready for review November 13, 2024 05:28
@TCeason TCeason changed the title refactor: use jiff crate replace chrono refactor: use jiff crate replace chrono in date/timestamp func Nov 13, 2024
@TCeason TCeason marked this pull request as draft November 13, 2024 05:42
@@ -98,7 +98,8 @@ pub enum FunctionEval {
#[derive(Clone)]
pub struct FunctionContext {
pub tz: TzLUT,
Copy link
Collaborator

@andylokandy andylokandy Nov 13, 2024

Choose a reason for hiding this comment

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

Can we remove it? Or convert from jiff_tz?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Cannot. Date_format need this. Jeff not support format string with no ascii format

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr-refactor this PR changes the code base without new features or bugfix
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants