ARROW-12421: [Rust] [DataFusion] Fix topkexec failure

[Dandandan]

This reproduces the test failure for the TopKExec test that @andygrove was seeing in combination with a 24-core machine.

Also FYI @alamb

(Marked as ready for review to trigger CI)

[github-actions]

Thanks for opening a pull request!

If this is not a minor PR. Could you open an issue for this pull request on JIRA? https://issues.apache.org/jira/browse/ARROW

Opening JIRAs ahead of time contributes to the Openness of the Apache Arrow project.

Then could you also rename pull request title in the following format?

ARROW-${JIRA_ID}: [${COMPONENT}] ${SUMMARY}

or

MINOR: [${COMPONENT}] ${SUMMARY}

See also:

• Other pull requests
• Contribution Guidelines - How to contribute patches

[alamb]

Nice

[Dandandan]

@alamb @andygrove I pushed a version with a fix (I believe) for the TopKExec example. Not sure if the original version with try_fold could also be fixed to work, but this works at least. It now has a .clone on the map which is not a big deal for small k but probably would be better to avoid that.

[andygrove]

Thanks @Dandandan 🚀

So if the issue was just in the test, I should change my vote on the release to a +1?

[codecov-commenter]

Codecov Report

Attention: Patch coverage is 83.33333% with 2 lines in your changes missing coverage. Please review.

Project coverage is 78.92%. Comparing base (9c1e5bd) to head (7d058ea).

Files	Patch %	Lines
rust/datafusion/tests/user_defined_plan.rs	83.33%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master   #10077      +/-   ##
==========================================
+ Coverage   78.90%   78.92%   +0.01%     
==========================================
  Files         286      286              
  Lines       64717    64713       -4     
==========================================
+ Hits        51068    51075       +7     
+ Misses      13649    13638      -11     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[Dandandan]

@andygrove as far as I can see the issue was in the test and faulty TopKExec implementation only, not really in repartion or the optimization of the repartion node.

[andygrove]

ok, we probably need to get the flatbuffer dependency change in anyway, so we should re-introduce the repartition optimization. Do you want to create a PR for that? If not, I can get to it later today.

…

On Sat, Apr 17, 2021 at 10:45 AM Daniël Heres ***@***.***> wrote: @andygrove <https://github.com/andygrove> as far as I can see the issue was in the test and faulty TopKExec implementation only, not really in repartion or the optimization of the repartion node. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#10077 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAHEBRA26KAMAGCFYYAUZ4DTJG3JPANCNFSM43C7OCUA> .

[Dandandan]

I probably won't get to that anymore today. Thanks, would be great

[github-actions]

https://issues.apache.org/jira/browse/ARROW-12421

[alamb]

Thanks @Dandandan

@andygrove as far as I can see the issue was in the test and faulty TopKExec implementation only, not really in repartion or the optimization of the repartion node.

For the record, the TopKExec is an example (not part of the DataFusion codebase)

[Dandandan]

Thanks @alamb @andygrove .

It would still be valuable to see if we can improve the example, or even add it to DataFusion once we have a good implementation (without the extension stuff of course)!