Re: missing indexes in indexlist with partitioned tables - Mailing list pgsql-hackers

From Zhihong Yu
Subject Re: missing indexes in indexlist with partitioned tables
Date
Msg-id CALNJ-vTJbf94i33eCc7bE0RN++qKpnsgRSQJAUFsksibpbO8mQ@mail.gmail.com
Whole thread Raw
In response to Re: missing indexes in indexlist with partitioned tables  (Arne Roland <A.Roland@index.de>)
Responses Re: missing indexes in indexlist with partitioned tables
List pgsql-hackers


On Wed, Jan 19, 2022 at 1:50 PM Arne Roland <A.Roland@index.de> wrote:

Hi,


I came up with a slightly more intuitive way to force the different outcome for the fractional test, that does hardly change anything.


I'm not sure, whether the differentiation between fract_x and fract_t is worth it, since there shouldn't be any difference, but as mentioned before I added it for potential future clarity.


Thanks for your feedback again!


Regards

Arne



From: Arne Roland
Sent: Wednesday, January 19, 2022 10:13:55 PM
To: Alvaro Herrera; Julien Rouhaud
Cc: pgsql-hackers
Subject: Re: missing indexes in indexlist with partitioned tables
 

Hi!

> From: Alvaro Herrera <alvherre@alvh.no-ip.org>
> [...]
> Hmm, these plan changes look valid to me.  A left self-join using the
> primary key column?  That looks optimizable all right.
> [...]
> What I still don't know is whether this patch is actually desirable or
> not.  If the only cases it affects is self-joins, is there much actual
> value?

This is not really about self joins. That was just the most simple example, because otherwise we need a second table.
It's unique, it's not relevant whether it's the same table. In most cases it won't. I was talking about join pruning.

> I suspect that the author of partition-wise joins would want to change
> these queries so that whatever was being tested by these self-joins is
> tested by some other means (maybe just create an identical partitioned
> table via CREATE TABLE fract_t2 ... ; INSERT INTO fract_t2 SELECT FROM
> fract_t).  But at the same time, the author of this patch should

Your suggestion doesn't work, because with my patch we solve that case as well. We solve the general join pruning case. If we make the index non-unique however, we should be able to test the fractional case the same way.

> b) add some test cases to verify that his desired
> behavior is tested somewhere, not just in a test case that's
> incidentally broken by his patch.

My patch already includes such a test, look at @@ -90,6 +90,13 @@ src/test/regress/sql/partition_join.sql
Since the selfjoin part was confusing to you, it might be worthwhile to do test that with two different tables. While I see no need to test in that way, I will adjust the patch so. Just to make it more clear for people looking at those tests in the future.

a) make
> sure that the submitted patch updates these test results so that the
> test pass [...]

Just for the record: I did run the tests, but I did miss that the commit of Tomas fix for fractional optimization is already on the master. Please note that this is a very new test case from a patch committed less than one week ago.

I'm glad Julien Rouhaud pointed out, that Tomas patch is committed it by now. That was very helpful to me, as I can now integrate the two tests.

@Álvaro Herrera:
If you want to help me, please don't put forward an abstract list of responsibilities. If anything I obviously need practical help, on how I can catch on recent changes quicker and without manual intervention. I don't have a modified buildfarm animal running, that tries to apply my patch and run regression tests for my patch on a daily basis.
Is there a simple way for me to check for that?

I will probably integrate those two tests, since they can work of similar structures without need to recreate the tables again and again. I have clear understanding how that new test works. I have to attend a few calls now, but I should be able to update the tests later.

Regards
Arne

Hi,

-           if (indexRelation->rd_rel->relkind == RELKIND_PARTITIONED_INDEX)
+           if (inhparent && (!index->indisunique || indexRelation->rd_rel->relkind != RELKIND_PARTITIONED_INDEX)) 

The check on RELKIND_PARTITIONED_INDEX seems to negate what the comment above says:

+            * Don't add partitioned indexes to the indexlist

Cheers

pgsql-hackers by date:

Previous
From: Julien Rouhaud
Date:
Subject: Re: Schema variables - new implementation for Postgres 15
Next
From: Michael Paquier
Date:
Subject: Re: do only critical work during single-user vacuum?