From: Dmitry Ivanov <d.ivanov@postgrespro.ru>
To: Robert Haas <robertmhaas@gmail.com>
Cc: Dmitry Ivanov <d.ivanov@postgrespro.ru>,
 Michael Paquier <michael.paquier@gmail.com>,
 Andres Freund <andres@anarazel.de>,
 Erik Rijkers <er@xs4all.nl>,
 Amit Langote <amitlangote09@gmail.com>,
 Amit Langote <Langote_Amit_f8@lab.ntt.co.jp>,
 Rajkumar Raghuwanshi <rajkumar.raghuwanshi@enterprisedb.com>,
 Ashutosh Bapat <ashutosh.bapat@enterprisedb.com>,
 Pg Hackers <pgsql-hackers@postgresql.org>,
 <pgsql-hackers-owner@postgresql.org>
Subject: Re: Declarative partitioning - another take
Date: Mon, 12 Dec 2016 20:14:14 +0300
MIME-Version: 1.0
Message-ID: <e6c56fe9-4b87-4f64-ac6f-bc99675f3f9e@postgrespro.ru>
In-Reply-To: 
 <CA+TgmoarP9qWMifAfc=98M3Gz=WFsKVpYEdkjXKx5dhf7tk+tw@mail.gmail.com>
References: <890ce68d-2cff-ccf3-67f7-6f5d3264dbfc@lab.ntt.co.jp>
 <CA+HiwqE1-1w9GiPE3+oje00=y5mzTu8c8XCn=9jzKW4ik+3y9w@mail.gmail.com>
 <CA+Tgmob9o3+xgYHtGbfzUWm9bknRryx53Q5pEVFkzVHSDO=QXQ@mail.gmail.com>
 <78ae9888-7372-245d-cf6b-f655e6fe6c12@lab.ntt.co.jp>
 <ce3c665ba06aed715386e79ef2abc616@xs4all.nl>
 <CA+HiwqFJ2qyA7eWfu0XLE77zhN88jMeQTxBBD_J-Uw=7YWkG_A@mail.gmail.com>
 <CA+Tgmob==oNYLN2M-pVVaLNMZxqvXk3_EoUnJqqRdd-8tm0CNw@mail.gmail.com>
 <bf186a19e6b16d104b997e7c1698cb56@xs4all.nl>
 <CA+Tgmob6DJEQBd=qH2wZG1onYZ9R1Sji3q+GqCRUqQi+ug28Rw@mail.gmail.com>
 <20161208043942.ekjvyf7ddbxehh7c@alap3.anarazel.de>
 <CAB7nPqS1jYR5huoTAmCRWsjeYC=ap1qmonjM+rjKuHjqOzGaDg@mail.gmail.com>
 <CA+TgmobTxn2+0x96h5Le+GOK5kw3J37SRveNfzEdx9s5-Yd8vA@mail.gmail.com>
 <426b2b01-61e0-43aa-bd84-c6fcf516f1c3@postgrespro.ru>
 <CA+Tgmoa8ieXx5fK1iC41jWZxCKhvnjS688RG_JO+yo7zhnOxHA@mail.gmail.com>
 <c22db3f5-93f6-4015-a727-8eac50d38de1@postgrespro.ru>
 <CA+TgmoarP9qWMifAfc=98M3Gz=WFsKVpYEdkjXKx5dhf7tk+tw@mail.gmail.com>
Organization: Postgres Professional
User-Agent: Trojita/0.7; Qt/5.7.0; xcb; Linux; 
Content-Type: multipart/mixed;
	boundary="trojita=_13b243bb-64ca-499f-ba65-988d81f20596"
Precedence: bulk
Sender: pgsql-hackers-owner@postgresql.org

This is a multipart/mixed message in MIME format.

--trojita=_13b243bb-64ca-499f-ba65-988d81f20596
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Transfer-Encoding: quoted-printable

Hi guys,

Looks like I've just encountered a bug. Please excuse me for the messy=20
email, I don't have much time at the moment.


Here's the test case:

create table test(val int) partition by range (val);
create table test_1 partition of test for values from (1) to (1000)=20
partition by range(val);
create table test_2 partition of test for values from (1000) to (2000)=20
partition by range(val);
create table test_1_1 partition of test_1 for values from (1) to (500)=20
partition by range(val);
create table test_1_2 partition of test_1 for values from (500) to (1000)=20
partition by range(val);
create table test_1_1_1 partition of test_1_1 for values from (1) to (500);
create table test_1_2_1 partition of test_1_2 for values from (500) to=20
(1000);


/* insert a row into "test_1_2_1" */
insert into test values(600);


/* what we EXPECT to see */
select *, tableoid::regclass from test;
 val |  tableoid=20=20
-----+------------
 600 | test_1_2_1
(1 row)


/* what we ACTUALLY see */
insert into test values(600);
ERROR:  no partition of relation "test_1_1" found for row
DETAIL:  Failing row contains (600).


How does this happen? This is how "PartitionDispatch" array looks like:

test | test_1 | test_2 | test_1_1 | test_1_2

which means that this code (partition.c : 1025):


/*
 * We can assign indexes this way because of the way
 * parted_rels has been generated.
 */
pd[i]->indexes[j] =3D -(i + 1 + m);


doesn't work, since partitions are not always placed right after the parent=
=20
(implied by index "m").

We have to take into account the total amount of partitions we've=20
encountered so far (right before index "i").

I've attached a patch with a hotfix, but the code looks so-so and has a=20
smell. I think it must be rewritten. This bug hunt surely took a while: I=20
had to recheck all of the steps several times.


--=20
Dmitry Ivanov
Postgres Professional: http://www.postgrespro.com
Russian Postgres Company=

--trojita=_13b243bb-64ca-499f-ba65-988d81f20596
Content-Type: text/x-patch
Content-Disposition: attachment;
	filename=hacky_partitioning_hotfix.diff
Content-Transfer-Encoding: base64

ZGlmZiAtLWdpdCBhL3NyYy9iYWNrZW5kL2NhdGFsb2cvcGFydGl0aW9uLmMg
Yi9zcmMvYmFja2VuZC9jYXRhbG9nL3BhcnRpdGlvbi5jCmluZGV4IDIxOWQz
ODAuLjExOWE0MWQgMTAwNjQ0Ci0tLSBhL3NyYy9iYWNrZW5kL2NhdGFsb2cv
cGFydGl0aW9uLmMKKysrIGIvc3JjL2JhY2tlbmQvY2F0YWxvZy9wYXJ0aXRp
b24uYwpAQCAtOTUwLDcgKzk1MCw4IEBAIFJlbGF0aW9uR2V0UGFydGl0aW9u
RGlzcGF0Y2hJbmZvKFJlbGF0aW9uIHJlbCwgaW50IGxvY2ttb2RlLAogCQkJ
ICAgKnBhcnRlZF9yZWxzOwogCUxpc3RDZWxsICAgKmxjOwogCWludAkJCWks
Ci0JCQkJazsKKwkJCQlrLAorCQkJCWNoaWxkcmVuX3NvX2ZhciA9IDA7CiAK
IAkvKgogCSAqIExvY2sgcGFydGl0aW9ucyBhbmQgbWFrZSBhIGxpc3Qgb2Yg
dGhlIHBhcnRpdGlvbmVkIG9uZXMgdG8gcHJlcGFyZQpAQCAtMTAyNiwxMSAr
MTAyNywxOCBAQCBSZWxhdGlvbkdldFBhcnRpdGlvbkRpc3BhdGNoSW5mbyhS
ZWxhdGlvbiByZWwsIGludCBsb2NrbW9kZSwKIAkJCQkgKiBXZSBjYW4gYXNz
aWduIGluZGV4ZXMgdGhpcyB3YXkgYmVjYXVzZSBvZiB0aGUgd2F5CiAJCQkJ
ICogcGFydGVkX3JlbHMgaGFzIGJlZW4gZ2VuZXJhdGVkLgogCQkJCSAqLwot
CQkJCXBkW2ldLT5pbmRleGVzW2pdID0gLShpICsgMSArIG0pOworCQkJCXBk
W2ldLT5pbmRleGVzW2pdID0gLSgxICsgbSArIGNoaWxkcmVuX3NvX2Zhcik7
CiAJCQkJbSsrOwogCQkJfQogCQl9CiAJCWkrKzsKKworCQkvKgorCQkgKiBD
aGlsZHJlbiBvZiB0aGlzIHBhcmVudCBhcmUgcGxhY2VkCisJCSAqIGFmdGVy
IGFsbCBjaGlsZHJlbiBvZiBhbGwgcHJldmlvdXMgcGFyZW50cywKKwkJICog
c28gd2UgaGF2ZSB0byB0YWtlIHRoaXMgaW50byBhY2NvdW50LgorCQkgKi8K
KwkJY2hpbGRyZW5fc29fZmFyICs9IHBhcnRkZXNjLT5ucGFydHM7CiAJfQog
CiAJcmV0dXJuIHBkOwo=

--trojita=_13b243bb-64ca-499f-ba65-988d81f20596
Content-Type: text/plain
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
MIME-Version: 1.0


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

--trojita=_13b243bb-64ca-499f-ba65-988d81f20596--