|From:||Nikita Glukhov <n(dot)gluhov(at)postgrespro(dot)ru>|
|To:||Alexander Korotkov <a(dot)korotkov(at)postgrespro(dot)ru>|
|Cc:||Darafei Praliaskouski <me(at)komzpa(dot)net>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>, Heikki Linnakangas <hlinnaka(at)iki(dot)fi>, Fedor Sigaev <teodor(at)sigaev(dot)ru>|
|Subject:||Re: compress method for spgist - 2|
|Views:||Raw Message | Whole Thread | Download mbox|
On 06.12.2017 21:49, Alexander Korotkov wrote:
> On Wed, Dec 6, 2017 at 6:08 PM, Nikita Glukhov
> <n(dot)gluhov(at)postgrespro(dot)ru <mailto:n(dot)gluhov(at)postgrespro(dot)ru>> wrote:
> On 05.12.2017 23:59, Alexander Korotkov wrote:
>> On Tue, Dec 5, 2017 at 1:14 PM, Darafei Praliaskouski
>> <me(at)komzpa(dot)net <mailto:me(at)komzpa(dot)net>> wrote:
>> The following review has been posted through the commitfest
>> 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.
> I think we are reconstructing a leaf datum, so documentation was
> correct but the code in spgWalk() and freeScanStackEntry() wrongly
> used attType instead of attLeafType. Fixed patch is attached.
> Reconstructed datum is used for index-only scan. Thus, it should be
> original indexed datum of attType, unless we have decompress method
> and pass reconstructed datum through it.
But if we have compress method and do not have decompress method then
index-only scan seems to be impossible.
>> 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.
> I've added compress method existence check to spgvalidate().
> It would be nice to evade double calling of config method. Possible
> option could be to memorize difference between attribute type and leaf
> type in high bit of functionset, which is guaranteed to be free.
I decided to simply set compress method's bit in functionset when this
method it is not required.
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company
|Next Message||Masahiko Sawada||2017-12-07 02:07:55||Re: User defined data types in Logical Replication|
|Previous Message||Vitaliy Garnashevich||2017-12-07 00:17:13||Re: Bitmap scan is undercosted?|