| From: | Andrey Lepikhov <a(dot)lepikhov(at)postgrespro(dot)ru> |
|---|---|
| To: | Peter Geoghegan <pg(at)bowt(dot)ie> |
| Cc: | Peter Eisentraut <peter(dot)eisentraut(at)2ndquadrant(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>, Alexander Korotkov <a(dot)korotkov(at)postgrespro(dot)ru>, Thomas Munro <thomas(dot)munro(at)enterprisedb(dot)com>, Claudio Freire <klaussfreire(at)gmail(dot)com>, Anastasia Lubennikova <a(dot)lubennikova(at)postgrespro(dot)ru> |
| Subject: | Re: Making all nbtree entries unique by having heap TIDs participate in comparisons |
| Date: | 2018-11-02 10:06:32 |
| Message-ID: | 4869413d-71ad-7f3d-dc20-97961c29a4fb@postgrespro.ru |
| Views: | Raw Message | Whole Thread | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
I do the code review.
Now, it is first patch - v6-0001... dedicated to a logical duplicates
ordering.
Documentation is full and clear. All non-trivial logic is commented
accurately.
Patch applies cleanly on top of current master. Regression tests passed
and my "Retail Indextuple deletion" use cases works without mistakes.
But I have two comments on the code.
New BTScanInsert structure reduces parameters list of many functions and
look fine. But it contains some optimization part ('restorebinsrch'
field et al.). It is used very locally in the code -
_bt_findinsertloc()->_bt_binsrch() routines calling. May be you localize
this logic into separate struct, which will passed to _bt_binsrch() as
pointer. Another routines may pass NULL value to this routine. It is may
simplify usability of the struct.
Due to the optimization the _bt_binsrch() size has grown twice. May be
you move this to some service routine?
--
Andrey Lepikhov
Postgres Professional
https://postgrespro.com
The Russian Postgres Company
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Stephen Frost | 2018-11-02 10:47:19 | Re: INSTALL file |
| Previous Message | Amit Kapila | 2018-11-02 09:50:12 | Re: Resetting PGPROC atomics in ProcessInit() |