From: Arne Roland <A.Roland@index.de>
To: Alvaro Herrera <alvherre@alvh.no-ip.org>, Julien Rouhaud
	<rjuju123@gmail.com>
CC: pgsql-hackers <pgsql-hackers@postgresql.org>
Subject: Re: missing indexes in indexlist with partitioned tables
Thread-Topic: missing indexes in indexlist with partitioned tables
Thread-Index: 
 AQHXsJoeXw6ojQG4NkOLdiX4KUBdD6vkTOpcgIAWJQCAA2SO7oAAbRh////16wCAADDYxYAAo/oAgABsDQCAAiIn7g==
Date: Wed, 19 Jan 2022 21:13:55 +0000
Message-ID: <225acbe1b6e242df8025ea6e75de2158@index.de>
References: 
 <20220118065750.v5zigpuisljc6jmk@jrouhaud>,<202201181324.idvthmrhbtf5@alvherre.pgsql>
In-Reply-To: <202201181324.idvthmrhbtf5@alvherre.pgsql>
Accept-Language: en-US, de-DE
Content-Language: en-US
Content-Type: multipart/alternative;
	boundary="_000_225acbe1b6e242df8025ea6e75de2158indexde_"
MIME-Version: 1.0
Archived-At: 
 <https://www.postgresql.org/message-id/225acbe1b6e242df8025ea6e75de2158%40index.de>
Precedence: bulk

--_000_225acbe1b6e242df8025ea6e75de2158indexde_
Content-Type: text/plain; charset="iso-8859-1"
Content-Transfer-Encoding: quoted-printable

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 i=
t 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 w=
ell. We solve the general join pruning case. If we make the index non-uniqu=
e 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 wa=
y, I will adjust the patch so. Just to make it more clear for people lookin=
g 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 not=
e that this is a very new test case from a patch committed less than one we=
ek ago.

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

@=C1lvaro Herrera:
If you want to help me, please don't put forward an abstract list of respon=
sibilities. 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 reg=
ression 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 s=
tructures 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, b=
ut I should be able to update the tests later.

Regards
Arne


--_000_225acbe1b6e242df8025ea6e75de2158indexde_
Content-Type: text/html; charset="iso-8859-1"
Content-Transfer-Encoding: quoted-printable

<html>
<head>
<meta http-equiv=3D"Content-Type" content=3D"text/html; charset=3Diso-8859-=
1">
<style type=3D"text/css" style=3D"display:none;"><!-- P {margin-top:0;margi=
n-bottom:0;} --></style>
</head>
<body dir=3D"ltr">
<div id=3D"divtagdefaultwrapper" style=3D"font-size:12pt;color:#000000;font=
-family:Calibri,Helvetica,sans-serif;" dir=3D"ltr">
<p>Hi!</p>
<p></p>
<div>&gt; From: Alvaro Herrera &lt;alvherre@alvh.no-ip.org&gt;<br>
&gt; [...]<br>
&gt; Hmm, these plan changes look valid to me.&nbsp; A left self-join using=
 the<br>
&gt; primary key column?&nbsp; That looks optimizable all right.<br>
&gt; [...]<br>
&gt; What I still don't know is whether this patch is actually desirable or=
<br>
&gt; not.&nbsp; If the only cases it affects is self-joins, is there much a=
ctual<br>
&gt; value?<br>
<br>
This is not really about self joins. That was just the most simple example,=
 because otherwise we need a second table.<br>
It's unique, it's not relevant whether it's the same table. In most cases i=
t won't. I was talking about join pruning.<br>
<br>
<div>&gt; I suspect that the author of partition-wise joins would want to c=
hange<br>
&gt; these queries so that whatever was being tested by these self-joins is=
<br>
&gt; tested by some other means (maybe just create an identical partitioned=
<br>
&gt; table via CREATE TABLE fract_t2 ... ; INSERT INTO fract_t2 SELECT FROM=
<br>
&gt; fract_t).&nbsp; But at the same time, the author of this patch should<=
br>
<br>
Your suggestion doesn't work, because with my patch we solve that case as w=
ell. We solve the general join pruning case. If we make the index non-uniqu=
e however, we should be able to test the
<span>fractional</span> case the same way.</div>
<br>
&gt; b) add some test cases to verify that his desired<br>
&gt; behavior is tested somewhere, not just in a test case that's<br>
&gt; incidentally broken by his patch.<br>
<br>
My patch already includes such a test, look at @@ -90,6 &#43;90,13 @@ src/t=
est/regress/sql/partition_join.sql<br>
Since the selfjoin part&nbsp;was confusing to you, it might be worthwhile t=
o do test that with two different tables. While I see no need to test in th=
at way, I will adjust the patch so. Just to make it more clear for people l=
ooking at those tests in the
<span>future</span>.<br>
<br>
a) make<br>
<div>&gt; sure that the submitted patch updates these test results so that =
the<br>
&gt; test pass [...]</div>
<br>
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 not=
e that this is a very new test case from a patch committed less than one we=
ek ago.<br>
<br>
I'm glad Julien Rouhaud pointed out, that Tomas patch is committed it by no=
w. That was very helpful to me, as I can now integrate the two tests.</div>
<div><br>
</div>
<div>@<span>=C1lvaro Herrera</span>:<br>
If you want to help me, please don't put forward an abstract list of respon=
sibilities. 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 da=
ily basis.<br>
Is there a simple way for me to check for that?<br>
<br>
I will probably integrate those two tests, since they can work of similar s=
tructures 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, b=
ut I should be able to update the
 tests later.<br>
<br>
Regards<br>
Arne<br>
<br>
</div>
</div>
</body>
</html>

--_000_225acbe1b6e242df8025ea6e75de2158indexde_--