pg.ddx.io  pgsql-committers@postgresql.org mailing list archive  
help / color / mirror / Atom feed
pgsql: Fix calculation of relid sets for partitionwise child joins.
5+ messages / 3 participants
[nested] [flat]

Commits from this thread:
  2023-07-22 bda97e47af 11237e5a46  Avoid compiler warning in non-assert builds.
* pgsql: Fix calculation of relid sets for partitionwise child joins.
@ 2023-07-21 16:00 Tom Lane <tgl@sss.pgh.pa.us>
  2023-07-21 22:51 ` Re: pgsql: Fix calculation of relid sets for partitionwise child joins. Jeff Davis <pgsql@j-davis.com>
  0 siblings, 1 reply; 5+ messages in thread

From: Tom Lane @ 2023-07-21 16:00 UTC (permalink / raw)
  To: pgsql-committers@lists.postgresql.org

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(-)



^ permalink  raw  reply  [nested|flat] 5+ messages in thread

* Re: pgsql: Fix calculation of relid sets for partitionwise child joins.
  2023-07-21 16:00 pgsql: Fix calculation of relid sets for partitionwise child joins. Tom Lane <tgl@sss.pgh.pa.us>
@ 2023-07-21 22:51 ` Jeff Davis <pgsql@j-davis.com>
  2023-07-22 03:41   ` Re: pgsql: Fix calculation of relid sets for partitionwise child joins. Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
  0 siblings, 1 reply; 5+ messages in thread

From: Jeff Davis @ 2023-07-21 22:51 UTC (permalink / raw)
  To: Tom Lane <tgl@sss.pgh.pa.us>; pgsql-committers@lists.postgresql.org

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






^ permalink  raw  reply  [nested|flat] 5+ messages in thread

* Re: pgsql: Fix calculation of relid sets for partitionwise child joins.
  2023-07-21 16:00 pgsql: Fix calculation of relid sets for partitionwise child joins. Tom Lane <tgl@sss.pgh.pa.us>
  2023-07-21 22:51 ` Re: pgsql: Fix calculation of relid sets for partitionwise child joins. Jeff Davis <pgsql@j-davis.com>
@ 2023-07-22 03:41   ` Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
  2023-07-22 14:35     ` Re: pgsql: Fix calculation of relid sets for partitionwise child joins. Tom Lane <tgl@sss.pgh.pa.us>
  0 siblings, 1 reply; 5+ messages in thread

From: Bharath Rupireddy @ 2023-07-22 03:41 UTC (permalink / raw)
  To: Jeff Davis <pgsql@j-davis.com>; +Cc: Tom Lane <tgl@sss.pgh.pa.us>; pgsql-committers@lists.postgresql.org

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





^ permalink  raw  reply  [nested|flat] 5+ messages in thread

* Re: pgsql: Fix calculation of relid sets for partitionwise child joins.
  2023-07-21 16:00 pgsql: Fix calculation of relid sets for partitionwise child joins. Tom Lane <tgl@sss.pgh.pa.us>
  2023-07-21 22:51 ` Re: pgsql: Fix calculation of relid sets for partitionwise child joins. Jeff Davis <pgsql@j-davis.com>
  2023-07-22 03:41   ` Re: pgsql: Fix calculation of relid sets for partitionwise child joins. Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
@ 2023-07-22 14:35     ` Tom Lane <tgl@sss.pgh.pa.us>
  0 siblings, 0 replies; 5+ messages in thread

From: Tom Lane @ 2023-07-22 14:35 UTC (permalink / raw)
  To: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>; +Cc: Jeff Davis <pgsql@j-davis.com>; pgsql-committers@lists.postgresql.org

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





^ permalink  raw  reply  [nested|flat] 5+ messages in thread

* pgsql: Fix calculation of relid sets for partitionwise child joins.
@ 2023-07-21 16:00 Tom Lane <tgl@sss.pgh.pa.us>
  0 siblings, 0 replies; 5+ messages in thread

From: Tom Lane @ 2023-07-21 16:00 UTC (permalink / raw)
  To: pgsql-committers@lists.postgresql.org

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
------
REL_16_STABLE

Details
-------
https://git.postgresql.org/pg/commitdiff/f75595cd80d134d5a63929a90a0347ff9729b79a

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(-)



^ permalink  raw  reply  [nested|flat] 5+ messages in thread


end of thread, other threads:[~2023-07-22 14:35 UTC | newest]

Thread overview: 5+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2023-07-21 16:00 pgsql: Fix calculation of relid sets for partitionwise child joins. Tom Lane <tgl@sss.pgh.pa.us>
2023-07-21 22:51 ` Jeff Davis <pgsql@j-davis.com>
2023-07-22 03:41   ` Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
2023-07-22 14:35     ` Tom Lane <tgl@sss.pgh.pa.us>
2023-07-21 16:00 pgsql: Fix calculation of relid sets for partitionwise child joins. Tom Lane <tgl@sss.pgh.pa.us>

This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox