wgtmac commented on code in PR #55449:
URL: https://github.com/apache/arrow/pull/55449#discussion_r4547356552
##########
cpp/src/arrow/util/alp/alp_sampler_internal.h:
##########
@@ -34,7 +34,7 @@ namespace arrow::util::alp {
template <AlpFloatingType T>
class ARROW_EXPORT AlpSampler {
public:
- AlpSampler() = default;
+ explicit AlpSampler(int64_t sampling_window_size);
Review Comment:
> If we want to sample 2048 values (8*256) from the 122880 window size, why
not just do that directly instead of all the chunking and counting? We could
trivially sample using a fast random generator (with a deterministic seed), and
it would probably be better as well.
I think this is worth exploring. As the Parquet spec only describes the
decoding scheme and all parquet-cpp ALP-related logics are written in the
internal files, it is safe for us to switch encoding algorithm at any time.
Should we do this in a separate PR once we are more confident when a better
approach has been found?
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
