Thread: pgsql: Do execGrouping.c via expression eval machinery, take two.
Do execGrouping.c via expression eval machinery, take two. This has a performance benefit on own, although not hugely so. The primary benefit is that it will allow for to JIT tuple deforming and comparator invocations. Large parts of this were previously committed (773aec7aa), but the commit contained an omission around cross-type comparisons and was thus reverted. Author: Andres Freund Discussion: https://postgr.es/m/20171129080934.amqqkke2zjtekd4t@alap3.anarazel.de Branch ------ master Details ------- https://git.postgresql.org/pg/commitdiff/bf6c614a2f2c58312b3be34a47e7fb7362e07bcb Modified Files -------------- src/backend/executor/execExpr.c | 143 +++++++++++++++++ src/backend/executor/execExprInterp.c | 29 ++++ src/backend/executor/execGrouping.c | 249 +++++++----------------------- src/backend/executor/nodeAgg.c | 145 ++++++++++------- src/backend/executor/nodeGroup.c | 24 +-- src/backend/executor/nodeRecursiveunion.c | 11 +- src/backend/executor/nodeSetOp.c | 54 +++---- src/backend/executor/nodeSubplan.c | 110 +++++++++++-- src/backend/executor/nodeUnique.c | 31 ++-- src/backend/executor/nodeWindowAgg.c | 38 +++-- src/backend/utils/adt/orderedsetaggs.c | 56 +++---- src/include/executor/execExpr.h | 1 + src/include/executor/executor.h | 34 ++-- src/include/executor/nodeAgg.h | 14 +- src/include/nodes/execnodes.h | 27 ++-- 15 files changed, 566 insertions(+), 400 deletions(-)
Hi, On 2018-02-16 22:48:39 +0000, Andres Freund wrote: > Do execGrouping.c via expression eval machinery, take two. > > This has a performance benefit on own, although not hugely so. The > primary benefit is that it will allow for to JIT tuple deforming and > comparator invocations. > > Large parts of this were previously committed (773aec7aa), but the > commit contained an omission around cross-type comparisons and was > thus reverted. > > Author: Andres Freund > Discussion: https://postgr.es/m/20171129080934.amqqkke2zjtekd4t@alap3.anarazel.de This triggered a failure on rhinoceros, in the sepgsql test: https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=rhinoceros&dt=2018-02-16%2023%3A45%3A02 The relevant diff is: + LOG: SELinux: allowed { execute } scontext=unconfined_u:unconfined_r:sepgsql_regtest_superuser_t:s0-s0:c0.c255 tcontext=system_u:object_r:sepgsql_proc_exec_t:s0tclass=db_procedure name="pg_catalog.int4eq(integer,integer)" and that's because we now invoke the function access hook for grouping equal, which we previously didn't. I personally think the new behaviour makes more sense, but if somebody wants to argue differently? The only argument against I can see is that there's some other cases where also don't yet invoke it, but that seems weak. I never fully grasped the exact use-case for the function execute hook is, so maybe Kaigai and/or Robert could comment? Greetings, Andres Freund
Hi, On 2018-02-16 22:48:39 +0000, Andres Freund wrote: > Do execGrouping.c via expression eval machinery, take two. > > This has a performance benefit on own, although not hugely so. The > primary benefit is that it will allow for to JIT tuple deforming and > comparator invocations. > > Large parts of this were previously committed (773aec7aa), but the > commit contained an omission around cross-type comparisons and was > thus reverted. > > Author: Andres Freund > Discussion: https://postgr.es/m/20171129080934.amqqkke2zjtekd4t@alap3.anarazel.de This triggered a failure on rhinoceros, in the sepgsql test: https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=rhinoceros&dt=2018-02-16%2023%3A45%3A02 The relevant diff is: + LOG: SELinux: allowed { execute } scontext=unconfined_u:unconfined_r:sepgsql_regtest_superuser_t:s0-s0:c0.c255 tcontext=system_u:object_r:sepgsql_proc_exec_t:s0tclass=db_procedure name="pg_catalog.int4eq(integer,integer)" and that's because we now invoke the function access hook for grouping equal, which we previously didn't. I personally think the new behaviour makes more sense, but if somebody wants to argue differently? The only argument against I can see is that there's some other cases where also don't yet invoke it, but that seems weak. I never fully grasped the exact use-case for the function execute hook is, so maybe Kaigai and/or Robert could comment? Greetings, Andres Freund
Hi, On 2018-02-16 16:03:37 -0800, Andres Freund wrote: > This triggered a failure on rhinoceros, in the sepgsql test: > https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=rhinoceros&dt=2018-02-16%2023%3A45%3A02 > > The relevant diff is: > + LOG: SELinux: allowed { execute } scontext=unconfined_u:unconfined_r:sepgsql_regtest_superuser_t:s0-s0:c0.c255 tcontext=system_u:object_r:sepgsql_proc_exec_t:s0tclass=db_procedure name="pg_catalog.int4eq(integer,integer)" > and that's because we now invoke the function access hook for grouping > equal, which we previously didn't. > > I personally think the new behaviour makes more sense, but if somebody > wants to argue differently? The only argument against I can see is that > there's some other cases where also don't yet invoke it, but that seems > weak. > > I never fully grasped the exact use-case for the function execute hook > is, so maybe Kaigai and/or Robert could comment? Fixed by adjusting the test output: https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=rhinoceros&dt=2018-02-21%2002%3A45%3A01 Greetings, Andres Freund
Hi, On 2018-02-16 16:03:37 -0800, Andres Freund wrote: > This triggered a failure on rhinoceros, in the sepgsql test: > https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=rhinoceros&dt=2018-02-16%2023%3A45%3A02 > > The relevant diff is: > + LOG: SELinux: allowed { execute } scontext=unconfined_u:unconfined_r:sepgsql_regtest_superuser_t:s0-s0:c0.c255 tcontext=system_u:object_r:sepgsql_proc_exec_t:s0tclass=db_procedure name="pg_catalog.int4eq(integer,integer)" > and that's because we now invoke the function access hook for grouping > equal, which we previously didn't. > > I personally think the new behaviour makes more sense, but if somebody > wants to argue differently? The only argument against I can see is that > there's some other cases where also don't yet invoke it, but that seems > weak. > > I never fully grasped the exact use-case for the function execute hook > is, so maybe Kaigai and/or Robert could comment? Fixed by adjusting the test output: https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=rhinoceros&dt=2018-02-21%2002%3A45%3A01 Greetings, Andres Freund