Hi,
Sorry for sending this again, accidentally used an old email of David's.
On 2023-02-06 14:48:44 +0000, PG Bug reporting form wrote:
> When executing the following query，assert failed may be triggered, which
> may be related to RANDOM（）：
>
>
> CREATE TABLE table0 ( column0 INT ) ;
> INSERT INTO table0 VALUES ( 1 ) , ( 2 ) , ( 3 ) , ( 4 ) , ( 5 ) , ( 6 ) , (
> 7 ) , ( 8 ) , ( 9 ) , ( 10 ) ;
> SELECT WIDTH_BUCKET ( SUM ( 1 ) FILTER ( WHERE ( SELECT LAG ( TRUE , CAST (
> RANDOM ( ) AS INT ) , column0 <= 1 ) OVER ( ) ) ) OVER ( PARTITION BY 1
> ORDER BY column0 RANGE BETWEEN CURRENT ROW AND 1 FOLLOWING ) , 1 , 2 , 3 )
> FROM table0 ;
Yes, you're right that this is caused by the random(), partially.
A simplified trigger of the problem is:
SELECT
SUM (1)
-- triggers the problem, volatile function in subplan, not removed due to correlated subquery
FILTER (WHERE ( SELECT COALESCE(random() < 0.5, i = 0)))
-- doesn't trigger the problem
--FILTER (WHERE random() < 0.5 )
OVER ( ORDER BY i RANGE BETWEEN CURRENT ROW AND 1 FOLLOWING)
FROM generate_series(0, 1000) g1(i);
If you replace 1 FOLLOWING with UNBOUNDED FOLLOWING, this crashes on 9.4-10
too, just a bit less reliably.
The problem is that normally we supress the moving aggregate optimization if a
volatile function is contained in the filter. But unfortunately,
contain_volatile_functions() doesn't descend into subplans. So we don't see
the volatile expression.
This is documented:
* We will recursively look into Query nodes (i.e., SubLink sub-selects)
* but not into SubPlans. This is a bit odd, but intentional. If we are
* looking at a SubLink, we are probably deciding whether a query tree
* transformation is safe, and a contained sub-select should affect that;
* for example, duplicating a sub-select containing a volatile function
* would be bad. However, once we've got to the stage of having SubPlans,
* subsequent planning need not consider volatility within those, since
* the executor won't change its evaluation rules for a SubPlan based on
* volatility.
Which seems, uh, a big assumption for something as generally named as
contain_volatile_functions(). It makes sense for pushdown / pullup
transformation, but in many, if not most, other cases, it seems pretty
broken. Why is that safe for equivclass.c, indxpath.c, partprune.c, etc?
Completely separately, but IMO the decision whether to use the moving
aggregate optimization ought to be a plan time decision, rather than an
execution time one...
Greetings,
Andres Freund
