Repository navigation
Conversation
Member
|
@save-buffer There might be a bit of background reading here but do you want to take a look at the filter kernel itself? |
save-buffer
reviewed
Aug 12, 2022
save-buffer
left a comment
save-buffer
left a comment
Contributor
There was a problem hiding this comment.
Overall seems pretty reasonable, just a couple of style points and an overall question about the encoding.
| }; | ||
|
|
||
| int64_t accumulated_run_length = 0; | ||
| if (values_is_valid_ == NULLPTR) { |
Contributor
There was a problem hiding this comment.
Could you break up this if into two functions? I'm having trouble following the control flow with the nested ifs. Maybe something like Exec_ValuesNotNull, Exec_ValuesMaybeNull
| return std::distance(accumulated_run_lengths, it); | ||
| } | ||
|
|
||
| void AddArtificialOffsetInChildArray(ArrayData* array, int64_t offset) { |
Contributor
There was a problem hiding this comment.
You could make this return Status and do RETURN_NOT_OK. I know this is a testing function but I'd say it's probably better that the test wraps this function in ARROW_CHECK_OK
| uint8_t* out_is_valid_; | ||
| int64_t* out_run_length_; | ||
| T* out_data_; | ||
| int64_t& out_logical_length_; |
Contributor
There was a problem hiding this comment.
I personally don't have a strong opinion about this but I think according to the style guide this should be a pointer (since it's an output).
| out_data_ = reinterpret_cast<T*>(out_arr->child_data[0]->buffers[1]->mutable_data()); | ||
| } | ||
|
|
||
| void Exec() { |
Contributor
There was a problem hiding this comment.
So I'd have thought that you could just run the existing Filter function on the run lengths array and the values array and be done. I think the reason you had to rewrite it is because an RLE array's length field is the logical length and not the number of runs, is that correct?
Contributor
Author
There was a problem hiding this comment.
The primary reason is that runs of filter array does not necessarily have the the same run lengths as the RLE array we are filtering. For example if the data is filtered based on the value of a different column.
This PR aims to upgrade ORC to version 1.7.6. Apache ORC 1.7.6 is the most recent maintenance release with the following bug fixes. - https://github.com/apache/orc/releases/tag/v1.7.6 - https://orc.apache.org/news/2022/08/17/ORC-1.7.6/ Authored-by: William Hyun <[email protected]> Signed-off-by: Sutou Kouhei <[email protected]>
… PKGBUILD (apache#13917) Authored-by: Sutou Kouhei <[email protected]> Signed-off-by: Sutou Kouhei <[email protected]>
…datafusion-c (apache#13923) Binary uploader is dev/release/05-binary-upload.sh and dev/release/post-02-binary.sh. We need to customize .deb package name. This also adds missing environment variable entries. Authored-by: Sutou Kouhei <[email protected]> Signed-off-by: Sutou Kouhei <[email protected]>
…me or index (apache#13652) Authored-by: anjakefala <[email protected]> Signed-off-by: David Li <[email protected]>
Relating to the building of the functionality for Compute in Go with Arrow, this is the implementation of ArraySpan / ExecValue / ExecResult etc. It was able to be separated out from the function interface definitions, so I was able to make this PR while apache#13924 is still being reviewed Authored-by: Matt Topol <[email protected]> Signed-off-by: Matt Topol <[email protected]>
Authored-by: Matt Topol <[email protected]> Signed-off-by: Matt Topol <[email protected]>
Authored-by: Matt Topol <[email protected]> Signed-off-by: Matt Topol <[email protected]>
…put method (apache#13267) **Summary** An additional parameter in Flight do_put to specify chunk size in R. **Problem** Currently, all data is sent through in a single message. It's a likely scenario that users will want the ability to control the batch sizes without building a custom do_put method. **Solution** Additional (optional) parameter to specify chunk size. Lead-authored-by: Christopher.Dunderdale <[email protected]> Co-authored-by: Christopher Dunderdale <[email protected]> Signed-off-by: Dewey Dunnington <[email protected]>
…ease stringr functions (apache#13937) Authored-by: Nic Crane <[email protected]> Signed-off-by: Dewey Dunnington <[email protected]>
Authored-by: Matt Topol <[email protected]> Signed-off-by: Matt Topol <[email protected]>
Turns out that the `pragma_table_info` function in modernc.org/sqlite's package doesn't work correctly in go1.17 either, only in go1.18. As this is only used for testing and the example sqlite flightsql server, rather than anything needed in the flightsql package itself, the bulid failure is easily solved by marking the example and its tests to be only built in go1.18. As we already have a git workflow that runs with go1.18, the CI will still continue to test the example code, but mamba builds using go1.17 won't break anymore. Authored-by: Matt Topol <[email protected]> Signed-off-by: Matt Topol <[email protected]>
Because our official .jar packages are built in manylinux2014 image. Authored-by: Sutou Kouhei <[email protected]> Signed-off-by: Sutou Kouhei <[email protected]>
…TS=0 (apache#13904) Authored-by: Miles Granger <[email protected]> Signed-off-by: Joris Van den Bossche <[email protected]>
) `std::random_shuffle` was deprecated in C++14 and removed altogether in C++17 in favor of `std::shuffle`. The codebase only uses these shuffling functions in tests and benchmarks and `std::shuffle` is used in all instances except this one. Authored-by: Bryce Mecum <[email protected]> Signed-off-by: Antoine Pitrou <[email protected]>
Authored-by: SHIMA Tatsuya <[email protected]> Signed-off-by: Jonathan Keane <[email protected]>
…he#13939) Authored-by: Raúl Cumplido <[email protected]> Signed-off-by: Sutou Kouhei <[email protected]>
After apache#13267 there is some trailing whitespace left over in one of the files. Authored-by: Dewey Dunnington <[email protected]> Signed-off-by: Sutou Kouhei <[email protected]>
apache#12824) In ARROW-15857 (apache#12734) we fixed the nightly failures on rhub/fedora-clang-devel by a kludge modifying the default makefile, but also upstreamed the fixes (rstudio/sass#104 and r-hub/rhub-linux-builders#60). These upstreams are now both released, so we can remove the kludge from modification of the docker image. Lead-authored-by: Dewey Dunnington <[email protected]> Co-authored-by: Dewey Dunnington <[email protected]> Signed-off-by: Dewey Dunnington <[email protected]>
…requests (apache#13781) These changes aim to document the issue lifecycle - how to create new issues (largely already existed) as well as how to engage with and understand issue state later on. This also serves to document proposed processes: 1. Assigned issues idle more than 90 days may be unassigned (discussed and approved on ML) 2. Unassigned issues in "In Progress" status should be set to "Open" 3. Establishing consistent usage of Status and Resolution fields. Authored-by: Todd Farmer <[email protected]> Signed-off-by: Antoine Pitrou <[email protected]>
…che#13932) since size_t is passed as the first argument to the StringFormatter, it needs to be templated with Int64Type instead of Int32Type Lead-authored-by: Aldrin M <[email protected]> Co-authored-by: Aldrin Montana <[email protected]> Signed-off-by: Antoine Pitrou <[email protected]>
ccache supports MSVC since 4.6.0: https://ccache.dev/releasenotes.html#_ccache_4_6 > Added support for caching calls to Microsoft Visual C++ (MSVC) and > clang-cl (MSVC compatibility for Clang). * no ccache: 22m23s: https://github.com/apache/arrow/runs/7983003808 * not cached: 39m35s: https://github.com/kou/arrow/runs/7984875241 * cached: 9m31s: https://github.com/kou/arrow/runs/7985401473 Authored-by: Sutou Kouhei <[email protected]> Signed-off-by: Antoine Pitrou <[email protected]>
…ache#13941) Typical real life Arrow datasets contain map of primitive type. This PR introduce MapBinder mapping of primitive types map entries Authored-by: igor.suhorukov <[email protected]> Signed-off-by: David Li <[email protected]>
This implements the interface for Kernels along with a `KernelSignature` struct and the type matching for `InputType` and `OutputType` via a `TypeMatcher` interface. Along with tests for all of the above. Authored-by: Matt Topol <[email protected]> Signed-off-by: Matt Topol <[email protected]>
…e#13951) Authored-by: Alenka Frim <[email protected]> Signed-off-by: Alenka Frim <[email protected]>
…y ones integrated in selection tests
3 tasks done
|
Thank you for your contribution. Unfortunately, this pull request has been marked as stale because it has had no activity in the past 365 days. Please remove the stale label or comment below, or this PR will be closed in 14 days. Feel free to re-open this if it has been closed in error. If you do not have repository permissions to reopen the PR, please tag a maintainer. |
github-actions
Bot
added
the
Status: stale-warning
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.