The following review has been posted through the commitfest application: make installcheck-world: tested, failed Implements feature: not tested Spec compliant: not tested Documentation: not tested
On linux/gcc the patch generates a warning in nodeAgg.c that is fairly easy to fix. Using -Werror to make catching the error easier, I get: make[3]: Entering directory `/home/mark/postgresql.clean/src/backend/executor' gcc -Wall -Wmissing-prototypes -Wpointer-arith -Wdeclaration-after-statement -Wendif-labels -Wmissing-format-attribute -Wformat-security -fno-strict-aliasing -fwrapv -O2 -Werror -I../../../src/include -D_GNU_SOURCE -c -o nodeAgg.o nodeAgg.c -MMD -MP -MF .deps/nodeAgg.Po cc1: warnings being treated as errors nodeAgg.c: In function ‘ExecAgg’: nodeAgg.c:2053: error: ‘result’ may be used uninitialized in this function make[3]: *** [nodeAgg.o] Error 1 make[3]: Leaving directory `/home/mark/postgresql.clean/src/backend/executor' make[2]: *** [executor-recursive] Error 2 make[2]: Leaving directory `/home/mark/postgresql.clean/src/backend' make[1]: *** [all-backend-recurse] Error 2 make[1]: Leaving directory `/home/mark/postgresql.clean/src' make: *** [all-src-recurse] Error 2 There are two obvious choices to silence the warning: diff --git a/src/backend/executor/nodeAgg.c b/src/backend/executor/nodeAgg.c index f059560..8959f5b 100644 --- a/src/backend/executor/nodeAgg.c +++ b/src/backend/executor/nodeAgg.c @@ -2066,6 +2066,7 @@ ExecAgg(AggState *node) break; case AGG_PLAIN: case AGG_SORTED: + default: result = agg_retrieve_direct(node); break; } which I like better, because I don't expect it would create an extra instruction in the compiled code, but also: diff --git a/src/backend/executor/nodeAgg.c b/src/backend/executor/nodeAgg.c index f059560..eab2605 100644 --- a/src/backend/executor/nodeAgg.c +++ b/src/backend/executor/nodeAgg.c @@ -2050,7 +2050,7 @@ lookup_hash_entries(AggState *aggstate) TupleTableSlot * ExecAgg(AggState *node) { - TupleTableSlot *result; + TupleTableSlot *result = NULL; if (!node->agg_done) { -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers