Re: missing indexes in indexlist with partitioned tables
От | David Rowley |
---|---|
Тема | Re: missing indexes in indexlist with partitioned tables |
Дата | |
Msg-id | CAApHDvrUEcq5TnhFUNzCPaq1992UgQncPJ2sAw9+GOOQEgXEUw@mail.gmail.com обсуждение исходный текст |
Ответ на | Re: missing indexes in indexlist with partitioned tables (Arne Roland <A.Roland@index.de>) |
Ответы |
Re: missing indexes in indexlist with partitioned tables
(Alvaro Herrera <alvherre@alvh.no-ip.org>)
Re: missing indexes in indexlist with partitioned tables (Amit Langote <amitlangote09@gmail.com>) |
Список | pgsql-hackers |
On Wed, 3 Aug 2022 at 11:07, Arne Roland <A.Roland@index.de> wrote: > Attached a rebased version of the patch. Firstly, I agree that we should fix the issue of join removals not working with partitioned tables. I had a quick look over this and the first thing that I thought was the same as what Amit mentioned in: On Tue, 25 Jan 2022 at 21:04, Amit Langote <amitlangote09@gmail.com> wrote: > Finally, I don't understand why we need a separate field to store > indexes found in partitioned base relations. AFAICS, nothing but the > sites you are interested in (relation_has_unique_index_for() and > rel_supports_distinctness()) would ever bother to look at a > partitioned base relation's indexlist. Do you think putting them into > in indexlist might break something? I kinda disagree with Alvaro's fix in 05fb5d661. I think indexlist is the place to store these details. That commit added the following comment: /* * Ignore partitioned indexes, since they are not usable for * queries. */ But neither are hypothetical indexes either, yet they're added to RelOptInfo.indexlist. I think the patch should be changed so that the existing list is used and we find another fix for the problems Alvaro fixed in 05fb5d661. Unfortunately, there was no discussion marked on that commit message, so it's not quite clear what the problem was. I'm unsure if there was anything other than CLUSTER that was broken. I see that cfdd03f45 added CLUSTER for partitioned tables in v15. I think the patch would need to go over the usages of RelOptInfo.indexlist to make sure that we don't need to add any further conditions to skip their usage for partitioned tables. I wrote the attached patch as I wanted to see what would break if we did this. The only problem I got from running make check was in get_actual_variable_range(), so I just changed that so it returns false when the given rel is a partitioned table. I only quickly did the changes to get_relation_info() and didn't give much thought to what the can* bool flags should be set to. I just mostly skipped all that code because it was crashing on relation->rd_tableam->scan_bitmap_next_block due to the rd_tableam being NULL. Also, just a friendly tip, Arne; I saw you named your patch 0006 for version 6. You'll see many 000n patches around the list, but those are generally done with git format-patch. That number normally means the patch in the patch series, not the version of the patch. I'm not sure if it'll help any, but my workflow for writing new patches against master tends to be: git checkout master git checkout -b some_feature_branch # write some code git commit -a # maybe more code git commit -a git format-patch -v1 master That'll create v1-0001 and v1-0002 patches. When I'm onto v2, I just change the version number from -v1 to -v2. David
Вложения
В списке pgsql-hackers по дате отправления: