Skip to content

least (copy), greatest (copy) and dateadd mock #70

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

Merged
merged 11 commits into from
Dec 30, 2024
Merged

Conversation

osipovartem
Copy link
Contributor

  • Copied least and greatest UDF functions from this PR (already merged, but not released). After datafusion release we should remove this local copy
  • Added mock for dateadd function to support sql syntax
  • Changed logic for datafusion -> snowflake timestamps convertor for a while (still receive the error 252005: Failed to convert: field min_last_success: TIMESTAMP_NTZ::min_last_success, Error: invalid literal for int() with base 10: 'min_last_success')

@osipovartem
Copy link
Contributor Author

/ptal @slyons @DanCodedThis

@osipovartem osipovartem changed the title Functions least (copy), greatest (copy) and dateadd mock Dec 27, 2024
@DanCodedThis
Copy link
Contributor

For dateadd we can either use utf8 or it's variants or extend the sql parser to actualy work with underlying types, but the addition operator should work out of the box, more info in issue #68 (keep in mind there are two branhces now)

…th curl "select dateadd('\''days'\'', 3,'\''2024-12-26'\'')"
…o the ZII pattern from Casey Muratori (MollyRocket) meaning the defaults or do we want some specific panic?
@DanCodedThis
Copy link
Contributor

Also when using nanoseconds, maybe I'm using to small of a value but I can't seem to see the change, or is it a part of that "thing" where timestamps don't work with them yet?

@DanCodedThis
Copy link
Contributor

And us (microsenconds) also

eadgbear
eadgbear previously approved these changes Dec 27, 2024
@rampage644
Copy link
Contributor

Closes #75

@DanCodedThis
Copy link
Contributor

i64 max is 9_223_372_036_854_775_807, and since we are adding nanoseconds even with years, if somebody decides to add 30 years to any timestamp/date/time we will have an overflow (in the value that is being added, not the timestamp), we are flying dangerously close to the sun for my liking, or are we?

I initially decided not to use MonthDayNano since days and month are i32, and even though we distinguish years, month, weeks, etc., anybody by mistake can write any value that may be bigger than i32 max, accidentally (since we always receive i64, ex.: was writing nanoseconds switched the form ns to yy) or is it just an edge case?

Plus the other problem is leap years, and "leap months", which should be handled internally by timestamps (didn't check yet, but it should be common sense), but because we are adding nanoseconds, our months are always 30 days and our years are always 365 days.

I'm going to look into this, and after that the PR should be ready

Copy link
Contributor

@DanCodedThis DanCodedThis left a comment

Choose a reason for hiding this comment

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

Dateadd completed

DanCodedThis
DanCodedThis previously approved these changes Dec 30, 2024
Copy link
Contributor

@DanCodedThis DanCodedThis left a comment

Choose a reason for hiding this comment

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

Dateadd completed

@DanCodedThis
Copy link
Contributor

/ptal @osipovartem

@DanCodedThis DanCodedThis merged commit 80ecd8e into main Dec 30, 2024
@osipovartem osipovartem deleted the functions branch January 13, 2025 09:02
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.

4 participants