Dandandan commented on code in PR #11721:
URL: https://github.com/apache/datafusion/pull/11721#discussion_r1697459557


##########
datafusion/functions-aggregate/src/approx_percentile_cont.rs:
##########
@@ -104,6 +105,12 @@ impl ApproxPercentileCont {
             None
         };
 
+        if args.ignore_nulls {
+            return not_impl_err!(
+                "IGNORE NULLS clause not yet supported for 
APPROX_PERCENTILE_CONT"

Review Comment:
   Hm... for now I think it makes sense to ignore the `ignore_nulls` argument 
(like we did before) and remove any nulls which makes the most sense. Null 
handling is broken anyway (resulting in 0 instead of NULL), so no one will rely 
on percentile_cont on columns with nulls.
   I'll file a ticket to discuss standardizing `PERCENTILE_CONT` and having it 
behave like PostgreSQL / Spark  / Snowflake, etc.... 



-- 
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]

Reply via email to