From: | Alexander Korotkov <a(dot)korotkov(at)postgrespro(dot)ru> |
---|---|
To: | Darafei Praliaskouski <me(at)komzpa(dot)net> |
Cc: | pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>, Heikki Linnakangas <hlinnaka(at)iki(dot)fi>, Fedor Sigaev <teodor(at)sigaev(dot)ru>, Nikita Glukhov <n(dot)gluhov(at)postgrespro(dot)ru> |
Subject: | Re: compress method for spgist - 2 |
Date: | 2017-12-05 20:59:33 |
Message-ID: | CAPpHfdtQtqHLEjgdDcML_QuOBtDjcTrQckaBxCGriaaO2HhUKw@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Tue, Dec 5, 2017 at 1:14 PM, Darafei Praliaskouski <me(at)komzpa(dot)net> wrote:
> The following review has been posted through the commitfest application:
> make installcheck-world: not tested
> Implements feature: not tested
> Spec compliant: not tested
> Documentation: tested, passed
>
> I've read the updated patch and see my concerns addressed.
>
> I'm looking forward to SP-GiST compress method support, as it will allow
> usage of SP-GiST index infrastructure for PostGIS.
>
> The new status of this patch is: Ready for Committer
>
I went trough this patch. I found documentation changes to be not
sufficient. And I've made some improvements.
In particular, I didn't understand why is reconstructedValue claimed to be
of spgConfigOut.leafType while it should be of spgConfigIn.attType both
from general logic and code. I've fixed that. Nikita, correct me if I'm
wrong.
Also, I wonder should we check for existence of compress method when
attType and leafType are not the same in spgvalidate() function? We don't
do this for now.
0002-spgist-polygon-8.patch is OK for me so soon.
------
Alexander Korotkov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company
Attachment | Content-Type | Size |
---|---|---|
0001-spgist-compress-method-9.patch | application/octet-stream | 24.7 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Thomas Munro | 2017-12-05 21:23:53 | Re: explain analyze output with parallel workers - question about meaning of information for explain.depesz.com |
Previous Message | Alvaro Herrera | 2017-12-05 20:58:24 | Re: [HACKERS] Proposal: Local indexes for partitioned table |