Received: from malur.postgresql.org ([217.196.149.56]) by arkaria.postgresql.org with esmtps (TLS1.3:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1nwA2v-0003kK-8W for pgsql-hackers@arkaria.postgresql.org; Tue, 31 May 2022 22:09:01 +0000 Received: from localhost ([127.0.0.1] helo=malur.postgresql.org) by malur.postgresql.org with esmtp (Exim 4.92) (envelope-from ) id 1nwA2u-00007n-3Z for pgsql-hackers@arkaria.postgresql.org; Tue, 31 May 2022 22:09:00 +0000 Received: from magus.postgresql.org ([2a02:c0:301:0:ffff::29]) by malur.postgresql.org with esmtps (TLS1.3:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1nwA2t-0008WV-N2 for pgsql-hackers@lists.postgresql.org; Tue, 31 May 2022 22:08:59 +0000 Received: from mail-lf1-x134.google.com ([2a00:1450:4864:20::134]) by magus.postgresql.org with esmtps (TLS1.3:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.92) (envelope-from ) id 1nwA2q-0004zL-LJ for pgsql-hackers@lists.postgresql.org; Tue, 31 May 2022 22:08:59 +0000 Received: by mail-lf1-x134.google.com with SMTP id y32so23670875lfa.6 for ; Tue, 31 May 2022 15:08:56 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=yugabyte.com; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=9J5VANLF+KAD6i/wiF3XaufdJ+uZ6qM99zsvrGJ6pNE=; b=S6FgMnslfEmWst4BlZ+FMYhql7OSN8ZHV50fOg3EbCiBK5s2v7HFUlu4MKQvpQqMCl v2p5ac5BeEmL09wSuspsyl6NM5Fzw8LVb2r3xnRAR6ew0NU5bvGy71yxIXEO1wDx5fLD ufOOHbacoyLY7Vhz7Jp/rXZiKqPEG+h5AFB4Y= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=9J5VANLF+KAD6i/wiF3XaufdJ+uZ6qM99zsvrGJ6pNE=; b=IOPVnRKfC/3kYBMz0hb6u+Z3orW8iYcudAjHJnqmECyojbjVldwPa8iAEEw1FF32mY lkMXYmOiyOTC/p+7ZPi+rK4egQQ8aNud7WGE5kUQxrolGu+ACohIHnTfoe9A2mWeLvID KBngdBJH7f29qKDxHM2FfawGRZkXWJvZ2efZVgsyYkAbBjHNiq9SP6dBk2jI/1hgWxmn iJH6bpFfFvbc69vqLSub6Dr4ataeAn7eVGr6fdTYwdyYRnOIlB04g/kzjw59uuVf73rM CwAbmiuiq6MEYmdQG1CjCVBdwQYy/5lsVZNTSL9s3q1vaMExfQzE0BKtoGTj32jdaIsU G8sg== X-Gm-Message-State: AOAM530ADHYJg4s4+MHMENBNvzZH0B1sDNW05f5CpPG3hrmKRoDE9SNV mbxERThvqfbEHJYFhqfopqP1h+7bk6zNpE1qu1/tf+Q99CAa4g== X-Google-Smtp-Source: ABdhPJyqzWO/iteHWuQbj4GVoZ5+/rwSRxs0AZ47Iu7cHc2LJZCenx1rKOK8sfOvIvGe+FXQQ1d7+3xC8e9co3SGsXc= X-Received: by 2002:a19:6917:0:b0:478:fe4c:5b6f with SMTP id e23-20020a196917000000b00478fe4c5b6fmr1615901lfc.29.1654034935867; Tue, 31 May 2022 15:08:55 -0700 (PDT) MIME-Version: 1.0 References: <163714495450.16056.3566953095730878367@malur.postgresql.org> <28bcc918-ec5d-5287-963f-86c678db9ae0@postgrespro.ru> In-Reply-To: From: Zhihong Yu Date: Tue, 31 May 2022 15:14:25 -0700 Message-ID: Subject: Re: Add SPLIT PARTITION/MERGE PARTITIONS commands To: Dmitry Koval Cc: Matthias van de Meent , PostgreSQL Hackers Content-Type: multipart/alternative; boundary="000000000000cc867405e0560458" List-Id: List-Help: List-Subscribe: List-Post: List-Owner: List-Archive: Archived-At: Precedence: bulk --000000000000cc867405e0560458 Content-Type: text/plain; charset="UTF-8" On Tue, May 31, 2022 at 1:43 PM Zhihong Yu wrote: > > > On Tue, May 31, 2022 at 12:43 PM Dmitry Koval > wrote: > >> >Just out of curiosity, why is SPLIT / MERGE support not included for >> >HASH partitions? Because sibling partitions can have a different >> >modulus, you should be able to e.g. split a partition with (modulus, >> >remainder) of (3, 1) into two partitions with (mod, rem) of (6, 1) and >> >(6, 4) respectively, with the reverse being true for merge operations, >> >right? >> >> You are right, SPLIT/MERGE operations can be added for HASH-partitioning >> in the future. But HASH-partitioning is rarer than RANGE- and >> LIST-partitioning and I decided to skip it in the first step. >> Maybe community will say that SPLIT/MERGE commands are not needed... (At >> first step I would like to make sure that it is no true) >> >> P.S. I attached patch with 1-line warning fix (for cfbot). >> -- >> With best regards, >> Dmitry Koval >> >> Postgres Professional: http://postgrespro.com > > > Hi, > For attachPartTable, the parameter wqueue is missing from comment. > The parameters of CloneRowTriggersToPartition are called parent > and partition. I think it is better to name the parameters to > attachPartTable in a similar manner. > > For struct SplitPartContext, SplitPartitionContext would be better name. > > + /* Store partition contect into list. */ > contect -> context > > Cheers > Hi, For transformPartitionCmdForMerge(), nested loop is used to detect duplicate names. If the number of partitions in partcmd->partlist, we should utilize map to speed up the check. For check_parent_values_in_new_partitions(): + if (!find_value_in_new_partitions(&key->partsupfunc[0], + key->partcollation, parts, nparts, datum, false)) + found = false; It seems we can break out of the loop when found is false. Cheers --000000000000cc867405e0560458 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable


=
On Tue, May 31, 2022 at 1:43 PM Zhiho= ng Yu <zyu@yugabyte.com> wrot= e:


On Tue, May 31, 2022 at 12:43 PM Dmitry Koval <= d.koval@postgre= spro.ru> wrote:
=C2=A0>Just out of curiosity, why is SPLIT / MERGE support not in= cluded for
=C2=A0>HASH partitions? Because sibling partitions can have a different<= br> =C2=A0>modulus, you should be able to e.g. split a partition with (modul= us,
=C2=A0>remainder) of (3, 1) into two partitions with (mod, rem) of (6, 1= ) and
=C2=A0>(6, 4) respectively, with the reverse being true for merge operat= ions,
=C2=A0>right?

You are right, SPLIT/MERGE operations can be added for HASH-partitioning in the future. But HASH-partitioning is rarer than RANGE- and
LIST-partitioning and I decided to skip it in the first step.
Maybe community will say that SPLIT/MERGE commands are not needed... (At first step I would like to make sure that it is no true)

P.S. I attached patch with 1-line warning fix (for cfbot).
--
With best regards,
Dmitry Koval

Postgres Professional: http://postgrespro.com

<= div>Hi,
For=C2=A0attachPartTable, the parameter=C2=A0wqueue is mi= ssing from comment.
The parameters of=C2=A0CloneRowTriggersToPart= ition are called=C2=A0parent and=C2=A0partition. I think it is better to na= me the parameters to attachPartTable in a similar manner.
For struct SplitPartContext, SplitPartitionContext would be bet= ter name.

+ =C2=A0 =C2=A0 =C2=A0 /* Store partitio= n contect into list. */
contect ->=C2=A0context
<= div>
Cheers
Hi,
= For=C2=A0transformPartitionCmdForMerge(), nested loop is used to detect dup= licate names.
If the number of partitions in=C2=A0partcmd->par= tlist, we should utilize map to speed up the check.

For=C2=A0check_parent_values_in_new_partitions():

+ =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 if (!find_value_in_new_partitions(&a= mp;key->partsupfunc[0],
+ =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 key->partcollation, parts, nparts= , datum, false))
+ =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 foun= d =3D false;

It seems we can break out of the = loop when found is false.

Cheers
--000000000000cc867405e0560458--