Update to arrow 26, change timezones #4039

tustvold · 2022-10-31T03:50:44Z

Which issue does this PR close?

re apache/arrow-rs#2953

Rationale for this change

Get latest arrow
Fixes #3938

What changes are included in this PR?

Are there any user-facing changes?

tustvold · 2022-10-31T03:52:07Z

datafusion/expr/src/function.rs

@@ -219,7 +219,7 @@ pub fn return_type(
        }
        BuiltinScalarFunction::Now => Ok(DataType::Timestamp(
            TimeUnit::Nanosecond,
-            Some("UTC".to_owned()),
+            Some("+00:00".to_owned()),


This change is necessary because by default (without the chrono-tz feature) arrow-rs doesn't know how to interpret this timezone. With the recent improvements to timezone handling, it now complains - (it prints "Unknown Timezone" when rending the timestamps). TBC this is an improvement over silently ignoring the timezone which it did before

FYI @waitingkuo

I basically applied this same pattern to a few other tests and now things looks great 👍

tustvold · 2022-10-31T04:34:08Z

I'm getting failures in test_sort_fetch_memory_calculation which appears to have been added as part of #3593 by @isidentical. I'm having a hard time working out why it is failing, perhaps someone might be able help out?

Dandandan · 2022-10-31T06:44:16Z

I'm getting failures in test_sort_fetch_memory_calculation which appears to have been added as part of #3593 by @isidentical. I'm having a hard time working out why it is failing, perhaps someone might be able help out?

Somehow the implementation spilled memory, because it believes memory usage exceeded the limit. Maybe some change happened in arrow 26 not shrinking the buffer somewhere?

isidentical · 2022-10-31T08:23:56Z

@tustvold @Dandandan it seems like the limit is never applied for multi-column sorts when lexsort_to_indices is called. Probably due to the changes in apache/arrow-rs#2929, where we previously created an array of (&value_indices)[0..len] but now just create it from value_indices directly without any limit (the single-column case is handled separately; and it still respects the limit).

Dandandan · 2022-10-31T08:27:38Z

@tustvold @Dandandan it seems like the limit is never applied for multi-column sorts when lexsort_to_indices is called. Probably due to the changes in apache/arrow-rs#2929, where we previously created an array of (&value_indices)[0..len] but now just create it from value_indices directly without any limit (the single-column case is handled separately; and it still respects the limit).

Ah, great find. @tustvold might be worth to block the RC to fix this issue?

tustvold · 2022-10-31T08:41:04Z

Aah nice find will workaround.

Given it can easily be worked around by slicing the returned array I don't think we need to block the RC

alamb · 2022-10-31T10:45:37Z

First thing I recommend we do is to file an arrow-rs ticket explaining the issue and then we can discuss a new RC based on that issue. Doing that now

alamb · 2022-10-31T10:49:29Z

Filed apache/arrow-rs#2990 -- will work on that shortly

alamb · 2022-10-31T11:26:47Z

Proposed fix apache/arrow-rs#2991

alamb · 2022-10-31T18:20:42Z

I created a new release candidate for arrow 26.0.0

alamb · 2022-11-03T20:36:15Z

Arrow 26 has been released -- apache/arrow-rs#2953

andygrove · 2022-11-03T21:31:51Z

I plan on creating DataFusion 14.0.0 rc1 once this is merged.

alamb · 2022-11-04T17:04:32Z

I am working on updating this PR and polishing it for merge

alamb · 2022-11-04T17:00:31Z

datafusion/core/tests/sql/mod.rs

-    let arr_micros = TimestampMicrosecondArray::from_opt_vec(ts_micros, None);
-    let arr_millis = TimestampMillisecondArray::from_opt_vec(ts_millis, None);
-    let arr_secs = TimestampSecondArray::from_opt_vec(ts_secs, None);
+    let arr_nanos = TimestampNanosecondArray::from(ts_nanos);


alamb · 2022-11-04T18:33:28Z

datafusion/optimizer/tests/integration-test.rs

@@ -237,25 +235,26 @@ fn timestamp_nano_ts_none_predicates() -> Result<()> {
    // a scan should have the now()... predicate folded to a single
    // constant and compared to the column without a cast so it can be
    // pushed down / pruned
-    let expected = "Projection: test.col_int32\n  Filter: test.col_ts_nano_utc < TimestampNanosecond(1666612093000000000, Some(\"UTC\"))\
-                    \n    TableScan: test projection=[col_int32, col_ts_nano_none]";
+    let expected =


Turns out this also fixes #3938 🎉

andygrove · 2022-11-04T18:34:42Z

sorry @alamb, I merged a PR that created a small conflict here

alamb · 2022-11-04T18:35:00Z

sorry @alamb, I merged a PR that created a small conflict here

No worries -- I just fixed it

ursabot · 2022-11-04T19:33:13Z

Benchmark runs are scheduled for baseline = 4a67d0d and contender = dd081d6. dd081d6 is a master commit associated with this PR. Results will be available as each benchmark for each run completes.
Conbench compare runs links:
[Skipped ⚠️ Benchmarking of arrow-datafusion-commits is not supported on ec2-t3-xlarge-us-east-2] ec2-t3-xlarge-us-east-2
[Skipped ⚠️ Benchmarking of arrow-datafusion-commits is not supported on test-mac-arm] test-mac-arm
[Skipped ⚠️ Benchmarking of arrow-datafusion-commits is not supported on ursa-i9-9960x] ursa-i9-9960x
[Skipped ⚠️ Benchmarking of arrow-datafusion-commits is not supported on ursa-thinkcentre-m75q] ursa-thinkcentre-m75q
Buildkite builds:
Supported benchmarks:
ec2-t3-xlarge-us-east-2: Supported benchmark langs: Python, R. Runs only benchmarks with cloud = True
test-mac-arm: Supported benchmark langs: C++, Python, R
ursa-i9-9960x: Supported benchmark langs: Python, R, JavaScript
ursa-thinkcentre-m75q: Supported benchmark langs: C++, Java

alamb · 2022-11-04T20:16:45Z

🚀 FYI @Ted-Jiang

* Update to arrow 26 * Update datafusion-cli * Update datafusion-cli lockfile * More test fixes * Re-add dyn_cmp_dict dev-dependencies * Update datafusion-cli lock * fix: Update tests * Update datafusion optimizer tests * Fix test Co-authored-by: Andrew Lamb <[email protected]>

Update to arrow 26

c459fea

github-actions bot added core Core DataFusion crate logical-expr Logical plan and expressions optimizer Optimizer rules physical-expr Physical Expressions sql SQL Planner labels Oct 31, 2022

tustvold commented Oct 31, 2022

View reviewed changes

Update datafusion-cli

e3178bc

alamb mentioned this pull request Oct 31, 2022

Regression in 26.0.0 RC1: limit is not applied for multi-column sorts lexsort_to_indices apache/arrow-rs#2990

Closed

tustvold added 3 commits November 1, 2022 08:03

Update datafusion-cli lockfile

0252a2b

More test fixes

81dfd85

Re-add dyn_cmp_dict dev-dependencies

521a2e5

alamb mentioned this pull request Nov 1, 2022

DataFusion does not support wasm32-unknown-unknown target #177

Closed

Merge remote-tracking branch 'upstream/master' into update-arrow-26

898e76f

alamb added 2 commits November 4, 2022 12:54

Merge remote-tracking branch 'apache/master' into update-arrow-26

22f7dae

Update datafusion-cli lock

64e458c

alamb added 2 commits November 4, 2022 13:08

fix: Update tests

c8ea100

Update datafusion optimizer tests

8e222e0

alamb reviewed Nov 4, 2022

View reviewed changes

alamb marked this pull request as ready for review November 4, 2022 17:19

alamb changed the title ~~Update to arrow 26~~ Update to arrow 26, change timezones Nov 4, 2022

alamb mentioned this pull request Nov 4, 2022

Add metrics for parquet page level skipping #4105

Merged

Fix test

ea538d6

alamb reviewed Nov 4, 2022

View reviewed changes

Merge remote-tracking branch 'apache/master' into update-arrow-26

98174c9

andygrove approved these changes Nov 4, 2022

View reviewed changes

Dandandan approved these changes Nov 4, 2022

View reviewed changes

andygrove merged commit dd081d6 into apache:master Nov 4, 2022

alamb mentioned this pull request Nov 7, 2022

Improved UX of creating TimestampNanosecondArray with timezones apache/arrow-rs#3042

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update to arrow 26, change timezones #4039

Update to arrow 26, change timezones #4039

tustvold commented Oct 31, 2022 •

edited by alamb

Loading

tustvold Oct 31, 2022 •

edited

Loading

alamb Nov 4, 2022

tustvold commented Oct 31, 2022

Dandandan commented Oct 31, 2022 •

edited

Loading

isidentical commented Oct 31, 2022

Dandandan commented Oct 31, 2022

tustvold commented Oct 31, 2022

alamb commented Oct 31, 2022

alamb commented Oct 31, 2022

alamb commented Oct 31, 2022

alamb commented Oct 31, 2022 •

edited

Loading

alamb commented Nov 3, 2022

andygrove commented Nov 3, 2022

alamb commented Nov 4, 2022

alamb Nov 4, 2022

alamb Nov 4, 2022

andygrove commented Nov 4, 2022

alamb commented Nov 4, 2022

ursabot commented Nov 4, 2022

alamb commented Nov 4, 2022

Update to arrow 26, change timezones #4039

Update to arrow 26, change timezones #4039

Conversation

tustvold commented Oct 31, 2022 • edited by alamb Loading

Which issue does this PR close?

Rationale for this change

What changes are included in this PR?

Are there any user-facing changes?

tustvold Oct 31, 2022 • edited Loading

Choose a reason for hiding this comment

alamb Nov 4, 2022

Choose a reason for hiding this comment

tustvold commented Oct 31, 2022

Dandandan commented Oct 31, 2022 • edited Loading

isidentical commented Oct 31, 2022

Dandandan commented Oct 31, 2022

tustvold commented Oct 31, 2022

alamb commented Oct 31, 2022

alamb commented Oct 31, 2022

alamb commented Oct 31, 2022

alamb commented Oct 31, 2022 • edited Loading

alamb commented Nov 3, 2022

andygrove commented Nov 3, 2022

alamb commented Nov 4, 2022

alamb Nov 4, 2022

Choose a reason for hiding this comment

alamb Nov 4, 2022

Choose a reason for hiding this comment

andygrove commented Nov 4, 2022

alamb commented Nov 4, 2022

ursabot commented Nov 4, 2022

alamb commented Nov 4, 2022

tustvold commented Oct 31, 2022 •

edited by alamb

Loading

tustvold Oct 31, 2022 •

edited

Loading

Dandandan commented Oct 31, 2022 •

edited

Loading

alamb commented Oct 31, 2022 •

edited

Loading