pg.ddx.io  pgsql-hackers@postgresql.org mailing list archive  
help / color / mirror / Atom feed
From: Teodor Sigaev <teodor@sigaev.ru>
To: Amit Langote <Langote_Amit_f8@lab.ntt.co.jp>
To: Alexander Korotkov <a.korotkov@postgrespro.ru>
Cc: Jaime Casanova <jaime.casanova@2ndquadrant.com>
Cc: Pg Hackers <pgsql-hackers@postgresql.org>
Subject: Re: Partitioned tables and covering indexes
Date: Wed, 11 Apr 2018 19:39:20 +0300
Message-ID: <5ee72852-3c4e-ee35-e2ed-c1d053d45c08@sigaev.ru> (raw)
In-Reply-To: <994fa677-2dac-8fe5-3f39-024cd6406522@lab.ntt.co.jp>
References: <CAJGNTeO=BguEyG8wxMpU_Vgvg3nGGzy71zUQ0RpzEn_mb0bSWA@mail.gmail.com>
	<48893c03-d1c8-62ef-b37d-b2f7dbdb8245@lab.ntt.co.jp>
	<CAPpHfdujMkQmhhzKG0sPB=27V9GXJeSVBGHt1rhHCPK89OfKfw@mail.gmail.com>
	<a5ca67cf-0448-7875-ae89-64495748c016@sigaev.ru>
	<994fa677-2dac-8fe5-3f39-024cd6406522@lab.ntt.co.jp>

Patch makes buildfarm almost red, patch is temporary reverted.

Actually, discovered bug is not related to patch except new test faces with it,
problem is: CompareIndexInfo() checks rd_opfamily for equality for all 
attributes, not only for key attribute.

Obviously, CompareIndexInfo() needs more work:
     for (i = 0; i < info1->ii_NumIndexAttrs; i++)
     {
         if (maplen < info2->ii_KeyAttrNumbers[i])

Seems, we can go out from ii_KeyAttrNumbers array.




Amit Langote wrote:
> Hi.
> 
> On 2018/04/11 0:36, Teodor Sigaev wrote:
>>>      Does the attached fix look correct?  Haven't checked the fix with
>>> ATTACH
>>>      PARTITION though.
>>>
>>>
>>> Attached patch seems to fix the problem.  However, I would rather get
>>> rid of modifying stmt->indexParams.  That seems to be more logical
>>> for me.  Also, it would be good to check some covering indexes on
>>> partitioned tables.  See the attached patch.
>>
>> Seems right way, do not modify incoming object and do not copy rather
>> large and deep nested structure as suggested by Amit.
> 
> Yeah, Alexander's suggested way of using a separate variable for
> indexParams is better.
> 
>> But it will  be better to have a ATTACH PARTITION test too.
> 
> I have added tests.  Actually, instead of modifying existing tests, I
> think it might be better to have a separate section at the end of
> indexing.sql to test covering indexes feature for partitioned tables.
> 
> Attached find updated patch.
> 
> Thanks,
> Amit
> 

-- 
Teodor Sigaev                                   E-mail: teodor@sigaev.ru
                                                    WWW: http://www.sigaev.ru/




view thread (22+ messages)  latest in thread

Message-ID: <5ee72852-3c4e-ee35-e2ed-c1d053d45c08@sigaev.ru>
Permalink:  ../5ee72852-3c4e-ee35-e2ed-c1d053d45c08@sigaev.ru/
Also on:    postgresql.org/message-id/5ee72852-3c4e-ee35-e2ed-c1d053d45c08@sigaev.ru

 · 

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: pgsql-hackers@postgresql.org
  Cc: teodor@sigaev.ru, Langote_Amit_f8@lab.ntt.co.jp, a.korotkov@postgrespro.ru, jaime.casanova@2ndquadrant.com
  Subject: Re: Partitioned tables and covering indexes
  In-Reply-To: <5ee72852-3c4e-ee35-e2ed-c1d053d45c08@sigaev.ru>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox