Crash of pg_hint_plan with Postgres 16~ with join paths #139
Labels
Comments
|
Hello! I've got server crash that provided the same backtrace as you presented, but the reproduction is different: And the backtrace: I've used this branches: Server configuration: |
|
Hi, Although I'm not yet able to understand the reason well, I identify postgres/postgres@2489d76 making the error. You can reproduce following combinations.
|
mikecaat
pushed a commit
to mikecaat/pg_hint_plan
that referenced
this issue
Aug 22, 2023
find_join_hint() calculates the join level using joinrelids which is a bitmapset of relations within the join with the assumption that the relids has only base relations. But, relids became to have outer-join relids with PostgreSQL commit (2489d76c). So, the calculation logic became wrong and crash on some condition. The commit fixes the logic to calculate joinrelids to consider only base relations. This patch is for PoC. I think we need to think the following. * Is the test enough and stable? * Don't we need to change OuterInnerJoinCreate() and transform_join_hints which call find_join_hint()?
|
IIUC, the reason of crash is the relids can have outer-join relids currently. I make an above PoC patch to solve the issue. Since I'm not familiar with the planner code and I'm studying now, please let me know if you have any comment. BTW, since the pg_hint_plan tests fails with current PostgreSQL head, please try with postgres/postgres@a8b7424 |
With PG16 coming soon at the door, we are going to do something here very soon. I'll try to look at your patch. (I'm a bit overdue with the SQL script updates, as well..) |
The tests now pass on current Postgres HEAD. I've fixed the issues with EXPLAIN and the costs. |
michaelpq
added a commit
that referenced
this issue
Aug 30, 2023
find_join_hint() calculates the join level using joinrelids which is a bitmapset of relations. However, since the upstream commit 2489d76c introduced in 16, it is possible that these include outer-join relids, something that pg_hint_plan cannot digest because it expects only case relations. This commit filters out any outer-join relids from the lists of relids considered during the join calculations. This could cause crashes because of corrupted relids lists. There may be a smarter way to fix that, but this is proving to be good enough for now. Per issue #139. Reported-by: Michael Paquier Author: Masahiro Ikeda Backpatch-through: 16
michaelpq
added a commit
that referenced
this issue
Aug 30, 2023
find_join_hint() calculates the join level using joinrelids which is a bitmapset of relations. However, since the upstream commit 2489d76c introduced in 16, it is possible that these include outer-join relids, something that pg_hint_plan cannot digest because it expects only case relations. This commit filters out any outer-join relids from the lists of relids considered during the join calculations. This could cause crashes because of corrupted relids lists. There may be a smarter way to fix that, but this is proving to be good enough for now. Per issue #139. Reported-by: Michael Paquier Author: Masahiro Ikeda Backpatch-through: 16
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
I have been able to trigger a crash of pg_hint_plan after using the following query on HEAD, when using Postgres 16:
Here is a backtrace of the parts relevant to pg_hint_plan:
I was first suspecting something in 21dfca13, but the crash reproduces as well on a build that reverts it, so it seems to me that something is not in line with the module's code and the recent planner changes.
The text was updated successfully, but these errors were encountered: