Обсуждение: pgsql: Fix calculation of relid sets for partitionwise child joins.
Fix calculation of relid sets for partitionwise child joins. Applying add_outer_joins_to_relids() to a child join doesn't actually work, even if we've built a SpecialJoinInfo specialized to the child, because that function will also compare the join's relids to elements of the main join_info_list, which only deal in regular relids not child relids. This mistake escaped detection by the existing partitionwise join tests because they didn't test any cases where add_outer_joins_to_relids() needs to add additional OJ relids (that is, any cases where join reordering per identity 3 is possible). Instead, let's apply adjust_child_relids() to the relids of the parent join. This requires minor code reordering to collect the relevant AppendRelInfo structures first, but that's work we'd do shortly anyway. Report and fix by Richard Guo; cosmetic changes by me Discussion: https://postgr.es/m/CAMbWs49NCNbyubZWgci3o=_OTY=snCfAPtMnM-32f3mm-K-Ckw@mail.gmail.com Branch ------ master Details ------- https://git.postgresql.org/pg/commitdiff/3c90dcd039149716454378bd270673f781b5c19f Modified Files -------------- src/backend/optimizer/path/joinrels.c | 14 +++++---- src/backend/optimizer/util/relnode.c | 18 +++++++---- src/test/regress/expected/partition_join.out | 46 ++++++++++++++++++++++++++++ src/test/regress/sql/partition_join.sql | 9 ++++++ 4 files changed, 75 insertions(+), 12 deletions(-)
On Fri, 2023-07-21 at 16:00 +0000, Tom Lane wrote: > Fix calculation of relid sets for partitionwise child joins. In CI, I'm seeing a compiler warning here: https://cirrus-ci.com/task/6112262960709632 [22:28:11.772] joinrels.c: In function ‘try_partitionwise_join’: [22:28:11.772] joinrels.c:1546:11: error: variable ‘child_joinrelids’ set but not used [-Werror=unused-but-set-variable] [22:28:11.772] 1546 | Relids child_joinrelids; [22:28:11.772] | ^~~~~~~~~~~~~~~~ [22:28:11.772] cc1: all warnings being treated as errors Regards, Jeff Davis
On Sat, Jul 22, 2023 at 4:22 AM Jeff Davis <pgsql@j-davis.com> wrote: > > On Fri, 2023-07-21 at 16:00 +0000, Tom Lane wrote: > > Fix calculation of relid sets for partitionwise child joins. > > In CI, I'm seeing a compiler warning here: > > https://cirrus-ci.com/task/6112262960709632 > > [22:28:11.772] joinrels.c: In function ‘try_partitionwise_join’: > [22:28:11.772] joinrels.c:1546:11: error: variable ‘child_joinrelids’ > set but not used [-Werror=unused-but-set-variable] > [22:28:11.772] 1546 | Relids child_joinrelids; > [22:28:11.772] | ^~~~~~~~~~~~~~~~ > [22:28:11.772] cc1: all warnings being treated as errors Same here - https://github.com/BRupireddy/postgres/runs/15251297440. Might have to mark the child_joinrelids PG_USED_FOR_ASSERTS_ONLY. -- Bharath Rupireddy PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services: https://aws.amazon.com
Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com> writes:
> On Sat, Jul 22, 2023 at 4:22 AM Jeff Davis <pgsql@j-davis.com> wrote:
>> On Fri, 2023-07-21 at 16:00 +0000, Tom Lane wrote:
>>> Fix calculation of relid sets for partitionwise child joins.
>> In CI, I'm seeing a compiler warning here:
>> [22:28:11.772] joinrels.c: In function ‘try_partitionwise_join’:
>> [22:28:11.772] joinrels.c:1546:11: error: variable ‘child_joinrelids’
>> set but not used [-Werror=unused-but-set-variable]
Right, I failed to test it without --enable-cassert, so I did not
see this warning.
> Might have to mark the child_joinrelids PG_USED_FOR_ASSERTS_ONLY.
It seemed better to me to put the adjust_child_relids call into
the Assert macro, so the compiler knows it doesn't have to run
adjust_child_relids in the non-Assert case.
regards, tom lane