You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by GitBox <gi...@apache.org> on 2021/09/20 21:20:09 UTC
[GitHub] [arrow] lidavidm opened a new pull request #11191: ARROW-14027: [C++] Handle scalars in Grouper
lidavidm opened a new pull request #11191:
URL: https://github.com/apache/arrow/pull/11191
This adds basic support for scalar values in the groupers. The non-fast grouper directly supports scalar. GrouperFastImpl will expand scalars to arrays first; this is because the rest of the implementation deals with buffers, which we don't really have with a scalar, so expanding scalars was the most expedient solution at a cost in performance. There's also a bit of refactoring to consistently expose a view method for primitive scalars, which can later be used to simplify some kernel implementations.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] lidavidm commented on pull request #11191: ARROW-14027: [C++] Handle scalars in Grouper
Posted by GitBox <gi...@apache.org>.
lidavidm commented on pull request #11191:
URL: https://github.com/apache/arrow/pull/11191#issuecomment-929621073
Whoops, fixed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] github-actions[bot] commented on pull request #11191: ARROW-14027: [C++] Handle scalars in Grouper
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #11191:
URL: https://github.com/apache/arrow/pull/11191#issuecomment-923321575
https://issues.apache.org/jira/browse/ARROW-14027
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] bkietz closed pull request #11191: ARROW-14027: [C++] Handle scalars in Grouper
Posted by GitBox <gi...@apache.org>.
bkietz closed pull request #11191:
URL: https://github.com/apache/arrow/pull/11191
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] bkietz commented on pull request #11191: ARROW-14027: [C++] Handle scalars in Grouper
Posted by GitBox <gi...@apache.org>.
bkietz commented on pull request #11191:
URL: https://github.com/apache/arrow/pull/11191#issuecomment-929613447
Ah, looks like a clang-format error https://github.com/apache/arrow/pull/11191/checks?check_run_id=3736913835#step:7:4369
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] bkietz closed pull request #11191: ARROW-14027: [C++] Handle scalars in Grouper
Posted by GitBox <gi...@apache.org>.
bkietz closed pull request #11191:
URL: https://github.com/apache/arrow/pull/11191
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] bkietz commented on a change in pull request #11191: ARROW-14027: [C++] Handle scalars in Grouper
Posted by GitBox <gi...@apache.org>.
bkietz commented on a change in pull request #11191:
URL: https://github.com/apache/arrow/pull/11191#discussion_r717904967
##########
File path: cpp/src/arrow/compute/kernels/hash_aggregate.cc
##########
@@ -541,9 +616,27 @@ struct GrouperFastImpl : Grouper {
~GrouperFastImpl() { map_.cleanup(); }
Result<Datum> Consume(const ExecBatch& batch) override {
+ // ARROW-14027: expand scalar arguments for now
Review comment:
Nit: elsewhere we call "materialization of a batch.length constant array from a scalar" broadcasting, I'd prefer to keep that consistent
```suggestion
// ARROW-14027: broadcast scalar arguments for now
```
##########
File path: cpp/src/arrow/compute/kernels/hash_aggregate.cc
##########
@@ -214,8 +248,12 @@ struct DictionaryKeyEncoder : FixedWidthKeyEncoder {
DictionaryKeyEncoder(std::shared_ptr<DataType> type, MemoryPool* pool)
: FixedWidthKeyEncoder(std::move(type)), pool_(pool) {}
- Status Encode(const ArrayData& data, uint8_t** encoded_bytes) override {
- auto dict = MakeArray(data.dictionary);
+ Status Encode(const Datum& data, int64_t batch_length,
+ uint8_t** encoded_bytes) override {
+ auto dict =
+ data.is_array()
+ ? MakeArray(data.array()->dictionary)
+ : MakeArray(data.scalar_as<DictionaryScalar>().value.dictionary->data());
Review comment:
```suggestion
: data.scalar_as<DictionaryScalar>().value.dictionary;
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org