MIME-Version: 1.0
References: <163714495450.16056.3566953095730878367@malur.postgresql.org>
 <c73a1746-0cd0-6bdd-6b23-3ae0b7c0c582@postgrespro.ru>
 <82ed3466-9855-ccd6-e2a3-a3a92166ff01@postgrespro.ru>
In-Reply-To: <82ed3466-9855-ccd6-e2a3-a3a92166ff01@postgrespro.ru>
From: Zhihong Yu <zyu@yugabyte.com>
Date: Wed, 13 Jul 2022 12:03:46 -0700
Message-ID: 
 <CALNJ-vSuzLxR+tyCiLA4s8sznn_M8aXdSVSYx-Sxb5E9jUG+8A@mail.gmail.com>
Subject: Re: Add SPLIT PARTITION/MERGE PARTITIONS commands
To: Dmitry Koval <d.koval@postgrespro.ru>
Cc: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Content-Type: multipart/alternative; boundary="0000000000005c779905e3b45b1f"
Archived-At: 
 <https://www.postgresql.org/message-id/CALNJ-vSuzLxR%2BtyCiLA4s8sznn_M8aXdSVSYx-Sxb5E9jUG%2B8A%40mail.gmail.com>
Precedence: bulk

--0000000000005c779905e3b45b1f
Content-Type: text/plain; charset="UTF-8"

On Wed, Jul 13, 2022 at 11:28 AM Dmitry Koval <d.koval@postgrespro.ru>
wrote:

> Hi!
>
> Patch stop applying due to changes in upstream.
> Here is a rebased version.
>
> --
> With best regards,
> Dmitry Koval
>
> Postgres Professional: http://postgrespro.com

Hi,

+attachPartTable(List **wqueue, Relation rel, Relation partition,
PartitionBoundSpec *bound)

I checked naming of existing methods, such as AttachPartitionEnsureIndexes.
I think it would be better if the above method is
named attachPartitionTable.

+   if (!defaultPartCtx && OidIsValid(defaultPartOid))
+   {
+       pc = createSplitPartitionContext(table_open(defaultPartOid,
AccessExclusiveLock));

Since the value of pc would be passed to defaultPartCtx, there is no need
to assign to pc above. You can assign directly to defaultPartCtx.

+   /* Drop splitted partition. */

splitted -> split

+   /* Rename new partition if it is need. */

need -> needed.

Cheers

--0000000000005c779905e3b45b1f
Content-Type: text/html; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

<div dir=3D"ltr"><div dir=3D"ltr"><br></div><br><div class=3D"gmail_quote">=
<div dir=3D"ltr" class=3D"gmail_attr">On Wed, Jul 13, 2022 at 11:28 AM Dmit=
ry Koval &lt;<a href=3D"mailto:d.koval@postgrespro.ru">d.koval@postgrespro.=
ru</a>&gt; wrote:<br></div><blockquote class=3D"gmail_quote" style=3D"margi=
n:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex=
">Hi!<br>
<br>
Patch stop applying due to changes in upstream.<br>
Here is a rebased version.<br>
<br>
-- <br>
With best regards,<br>
Dmitry Koval<br>
<br>
Postgres Professional: <a href=3D"http://postgrespro.com" rel=3D"noreferrer=
" target=3D"_blank">http://postgrespro.com</a></blockquote><div>Hi,</div><d=
iv><br></div><div>+attachPartTable(List **wqueue, Relation rel, Relation pa=
rtition, PartitionBoundSpec *bound)=C2=A0</div><div><br></div><div>I checke=
d naming of existing methods, such as=C2=A0AttachPartitionEnsureIndexes.</d=
iv><div>I think it would be better if the above method is named=C2=A0attach=
PartitionTable.</div><div><br></div><div>+ =C2=A0 if (!defaultPartCtx &amp;=
&amp; OidIsValid(defaultPartOid))<br>+ =C2=A0 {<br>+ =C2=A0 =C2=A0 =C2=A0 p=
c =3D createSplitPartitionContext(table_open(defaultPartOid, AccessExclusiv=
eLock));<br></div><div><br></div><div>Since the value of pc would be passed=
 to=C2=A0defaultPartCtx, there is no need to assign to pc above. You can as=
sign directly to=C2=A0defaultPartCtx.</div><div><br></div><div>+ =C2=A0 /* =
Drop splitted partition. */<br></div><div><br></div><div>splitted -&gt; spl=
it<br></div><div><br></div><div>+ =C2=A0 /* Rename new partition if it is n=
eed. */<br></div><div><br></div><div>need -&gt; needed.</div><div><br></div=
><div>Cheers</div></div></div>

--0000000000005c779905e3b45b1f--