summaryrefslogtreecommitdiff
path: root/src/backend/optimizer/path
diff options
context:
space:
mode:
authorDavid Rowley <drowley@postgresql.org>2022-12-07 00:09:36 +1300
committerDavid Rowley <drowley@postgresql.org>2022-12-07 00:09:36 +1300
commita8583272218ab6e97c7bdeb8a960d8d4de329b7e (patch)
treed89171a52e35c7c52c1025ffc7a888729b27eb0d /src/backend/optimizer/path
parent83a1a1b56645b7a55ec00e44f8018116ee87c720 (diff)
downloadpostgresql-a8583272218ab6e97c7bdeb8a960d8d4de329b7e.tar.gz
Fix 32-bit build dangling pointer issue in WindowAgg
9d9c02ccd added window "run conditions", which allows the evaluation of monotonic window functions to be skipped when the run condition is no longer true. Prior to this commit, once the run condition was no longer true and we stopped evaluating the window functions, we simply just left the ecxt_aggvalues[] and ecxt_aggnulls[] arrays alone to store whatever value was stored there the last time the window function was evaluated. Leaving a stale value in there isn't really a problem on 64-bit builds as all of the window functions which we recognize as monotonic all return int8, which is passed by value on 64-bit builds. However, on 32-bit builds, this was a problem as the value stored in the ecxt_values[] element would be a by-ref value and it would be pointing to some memory which would get reset once the tuple context is destroyed. Since the WindowAgg node will output these values in the resulting tupleslot, this could be problematic for the top-level WindowAgg node which must look at these values to filter out the rows that don't meet its filter condition. Here we fix this by just zeroing the ecxt_aggvalues[] and setting the ecxt_aggnulls[] array to true when the run condition first becomes false. This results in the WindowAgg's output having NULLs for the WindowFunc's columns rather than the stale or pointer pointing to possibly freed memory. These tuples with the NULLs can only make it as far as the top-level WindowAgg node before they're filtered out. To ensure that these tuples *are* always filtered out, we now insist that OpExprs making up the run condition are strict OpExprs. Currently, all the window functions which the planner recognizes as monotonic return INT8 and the operator which is used for the run condition must be a member of a btree opclass. In reality, these restrictions exclude nothing that's built-in to Postgres and are unlikely to exclude anyone's custom operators due to the requirement that the operator is part of a btree opclass. It would be unusual if those were not strict. Reported-by: Sergey Shinderuk, using valgrind Reviewed-by: Richard Guo, Sergey Shinderuk Discussion: https://postgr.es/m/29184c50-429a-ebd7-f1fb-0589c6723a35@postgrespro.ru Backpatch-through: 15, where 9d9c02ccd was added
Diffstat (limited to 'src/backend/optimizer/path')
-rw-r--r--src/backend/optimizer/path/allpaths.c12
1 files changed, 12 insertions, 0 deletions
diff --git a/src/backend/optimizer/path/allpaths.c b/src/backend/optimizer/path/allpaths.c
index 03ee6dc832..0c564be7f6 100644
--- a/src/backend/optimizer/path/allpaths.c
+++ b/src/backend/optimizer/path/allpaths.c
@@ -2400,6 +2400,18 @@ check_and_push_window_quals(Query *subquery, RangeTblEntry *rte, Index rti,
return true;
/*
+ * Currently, we restrict this optimization to strict OpExprs. The reason
+ * for this is that during execution, once the runcondition becomes false,
+ * we stop evaluating WindowFuncs. To avoid leaving around stale window
+ * function result values, we set them to NULL. Having only strict
+ * OpExprs here ensures that we properly filter out the tuples with NULLs
+ * in the top-level WindowAgg.
+ */
+ set_opfuncid(opexpr);
+ if (!func_strict(opexpr->opfuncid))
+ return true;
+
+ /*
* Check for plain Vars that reference window functions in the subquery.
* If we find any, we'll ask find_window_run_conditions() if 'opexpr' can
* be used as part of the run condition.