Skip to content

Conversation

@rampage644
Copy link
Contributor

@rampage644 rampage644 commented Feb 11, 2025

This is a quick fix for dateadd UDF - it just makes it work such that its physical execution works for dbt snowplow
workload.

  • Use interval instead of duration type for hour/minute/second
  • Handle empty array case

Ideally, it should be reimplemented differently:

  • Non-deprecated methods should be used
  • It should be designed to handle ColumnarValue::Array as a first class citizen (whereas scalar is an exception, not otherwise)
  • (Debatable) Perhaps even 2nd argument should be treated as a columnar value, not scalar

Good example is in datafusion-examples directory.

Fixes - somewhat incorrect - #223

 * Use interval instead of duration type for hour/minute/second
 * Handle empty array case
@DanCodedThis
Copy link
Contributor

DanCodedThis commented Feb 11, 2025

Copy link
Contributor

@Vedin Vedin left a comment

Choose a reason for hiding this comment

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

It's fine for now and I believe work pretty fast. I still don't like to create additional array for scalar values. We should consider using something similar to evaluate_array_scalar in the future.

@rampage644
Copy link
Contributor Author

@DanCodedThis Feel free to take over this (i.e. decision to merge is on you), feel free to remove all code I've written altogether.

@DanCodedThis DanCodedThis merged commit 3bfee66 into main Feb 12, 2025
5 checks passed
@DanCodedThis DanCodedThis deleted the bug/fix-dateadd-fn branch February 12, 2025 10:39
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.

6 participants