You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "felipecrv (via GitHub)" <gi...@apache.org> on 2023/04/13 22:45:51 UTC
[GitHub] [arrow] felipecrv opened a new pull request, #35123: GH-35122: [C++] Add support for scalars in run_end_encode and run_end_decode
felipecrv opened a new pull request, #35123:
URL: https://github.com/apache/arrow/pull/35123
### Rationale for this change
To make `run_end_encode` and `run_end_decode` usable in all contexts, they should gracefully handle `Scalar` inputs.
### What changes are included in this PR?
- Make `run_end_encode` turn a regular `Scalar` into a `RUN_END_ENCODED` array.
- Make `run_end_decode` turn a `RUN_END_ENCODED`
### Are these changes tested?
Yes, by unit tests.
### Are there any user-facing changes?
No.
--
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] felipecrv commented on pull request #35123: GH-35122: [C++] Add support for scalars in run_end_encode and run_end_decode
Posted by "felipecrv (via GitHub)" <gi...@apache.org>.
felipecrv commented on PR #35123:
URL: https://github.com/apache/arrow/pull/35123#issuecomment-1507698653
@jorisvandenbossche @benibus @zeroshade
@westonpace is it OK/expected for/from vector functions to handle scalar inputs and always return an array? Because that's what I'm doing here.
--
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] westonpace commented on pull request #35123: GH-35122: [C++] Add support for scalars in run_end_encode and run_end_decode
Posted by "westonpace (via GitHub)" <gi...@apache.org>.
westonpace commented on PR #35123:
URL: https://github.com/apache/arrow/pull/35123#issuecomment-1512503286
Perhaps a related question. Why are run end encode/decode vector functions in the first place? Wouldn't they be scalar functions?
--
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 #35123: GH-35122: [C++] Add support for scalars in run_end_encode and run_end_decode
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #35123:
URL: https://github.com/apache/arrow/pull/35123#issuecomment-1507697399
* Closes: #35122
--
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] westonpace commented on pull request #35123: GH-35122: [C++] Add support for scalars in run_end_encode and run_end_decode
Posted by "westonpace (via GitHub)" <gi...@apache.org>.
westonpace commented on PR #35123:
URL: https://github.com/apache/arrow/pull/35123#issuecomment-1516810393
Fair points. I don't think I know the function registry well enough to answer.
--
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] felipecrv commented on pull request #35123: GH-35122: [C++] Add support for scalars in run_end_encode and run_end_decode
Posted by "felipecrv (via GitHub)" <gi...@apache.org>.
felipecrv commented on PR #35123:
URL: https://github.com/apache/arrow/pull/35123#issuecomment-1520338844
@jorisvandenbossche I think the requirements you listed make a good case for this not being a scalar function. I will close the PR because of that and my failed attempt in using this as a function that takes a scalar.
--
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] felipecrv commented on pull request #35123: GH-35122: [C++] Add support for scalars in run_end_encode and run_end_decode
Posted by "felipecrv (via GitHub)" <gi...@apache.org>.
felipecrv commented on PR #35123:
URL: https://github.com/apache/arrow/pull/35123#issuecomment-1508487120
@jorisvandenbossche I'm now a bit unsure about this PR because in the context I tried to use this implementation, I would need to return an array expanded from the Scalar that had length equal to the other columns in the `ExecBatch`.
Think `SELECT run_end_encode(3) + col FROM ...`
--
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] felipecrv commented on pull request #35123: GH-35122: [C++] Add support for scalars in run_end_encode and run_end_decode
Posted by "felipecrv (via GitHub)" <gi...@apache.org>.
felipecrv commented on PR #35123:
URL: https://github.com/apache/arrow/pull/35123#issuecomment-1513227799
> Perhaps a related question. Why are run end encode/decode vector functions in the first place? Wouldn't they be scalar functions?
I just continued the work of Tobias and didn't put much thinking into what they should have been. I don't see any reason for them to not be scalar functions.
--
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] jorisvandenbossche commented on pull request #35123: GH-35122: [C++] Add support for scalars in run_end_encode and run_end_decode
Posted by "jorisvandenbossche (via GitHub)" <gi...@apache.org>.
jorisvandenbossche commented on PR #35123:
URL: https://github.com/apache/arrow/pull/35123#issuecomment-1516156580
While they follow the typical requirements of a scalar function of same shape for input / output, the kernel is not strictly speaking scalar because it's not just independently element-wise (i.e. the order of the values of the input array impact the exact result array, although you could say that the exact run_ends/values are an implementation detail, as long as they still represent "logically" the same values).
I don't know if this internal/theoretical detail is enough to not consider it as a scalar function, as in _practice_, I assume you can use this function in most places where the kernel is restricted to be scalar? (eg in projections)
--
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] jorisvandenbossche commented on pull request #35123: GH-35122: [C++] Add support for scalars in run_end_encode and run_end_decode
Posted by "jorisvandenbossche (via GitHub)" <gi...@apache.org>.
jorisvandenbossche commented on PR #35123:
URL: https://github.com/apache/arrow/pull/35123#issuecomment-1507985296
Checking some other vector functions passing them a scalar, there are that raise a NotImplementedError (eg sort_indices, rank) or that return an array (eg replace_with_mask, cumulative_sum, indices_nonzero).
So returning an array seems fine.
--
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] westonpace commented on pull request #35123: GH-35122: [C++] Add support for scalars in run_end_encode and run_end_decode
Posted by "westonpace (via GitHub)" <gi...@apache.org>.
westonpace commented on PR #35123:
URL: https://github.com/apache/arrow/pull/35123#issuecomment-1507723741
As far as I know, vector functions aren't used anywhere internally beyond CallFunction, so if it runs, then I think it's ok.
--
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] felipecrv closed pull request #35123: GH-35122: [C++] Add support for scalars in run_end_encode and run_end_decode
Posted by "felipecrv (via GitHub)" <gi...@apache.org>.
felipecrv closed pull request #35123: GH-35122: [C++] Add support for scalars in run_end_encode and run_end_decode
URL: https://github.com/apache/arrow/pull/35123
--
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