asolimando commented on code in PR #25337:
URL: https://github.com/apache/datafusion/pull/25337#discussion_r4084725337
##########
datafusion/functions-aggregate/src/approx_percentile_cont.rs:
##########
@@ -198,11 +192,18 @@ impl ApproxPercentileCont {
if let Some(max_size) = tdigest_max_size {
ApproxPercentileAccumulator::new_with_max_size(
percentile,
+ is_descending,
data_type.clone(),
max_size,
)
+ .should_track_percentile()
Review Comment:
Nit: not sure how impactful this would be in practice, but I think we
shouldn't be calling this when you have a literal (and not an
expression/column), so to avoid the extra `Float64` "virtual" column.
##########
datafusion/functions-aggregate/src/percentile_cont.rs:
##########
@@ -517,7 +555,8 @@ where
}
fn evaluate(&mut self) -> Result<ScalarValue> {
- let value = calculate_percentile::<T, I>(&mut self.all_values,
self.percentile)?;
+ let percentile = effective_percentile(&self.percentile,
self.is_descending)?;
Review Comment:
Here we are making this call before checking whether any rows were
accumulated, you should have a guard like the following, at the beginning of
the method:
```
if self.all_values.is_empty() {
return ScalarValue::new_primitive::<T>(None, &self.data_type);
}
```
You can repro as follows:
```
cargo run --bin datafusion-cli -- -c "SELECT percentile_cont(x, m) FROM
(VALUES (10,0.5),(20,0.5),(30,0.5),(40,0.5)) AS t(x,m) WHERE x > 1000"
```
The error is `Error: Error during planning: Percentile value for
'PERCENTILE_CONT' could not be determined: no non-null percentile value was
seen`
I suggest to add this as an SLT test.
We have the same problem in `DistinctPercentileContAccumulator::evaluate`
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
