Re: Partitioned tables and covering indexes

2018-04-12 Thread Robert Haas
On Thu, Apr 12, 2018 at 6:14 AM, Alexander Korotkov wrote: > I'm OK with collation of included columns to be the same as collation > of underlying table columns. But I still think we should throw an error > when user is trying to specify his own collation of included columns. I agree. The colla

Re: Partitioned tables and covering indexes

2018-04-12 Thread Teodor Sigaev
pushed. Hope, second try will be successful. Teodor Sigaev wrote: Thank you, pushed 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.

Re: Partitioned tables and covering indexes

2018-04-12 Thread Teodor Sigaev
Thanks to everyone, this part is pushed. I will waiting a bit before pushing topic-stater patch to have a time to look on buildfarm. Alvaro, could you add a comment to CompareIndexInfo() to clarify why it doesn't compare indoptions (like DESC/ASC etc)? It doesn't matter for uniqueness of index

Re: Partitioned tables and covering indexes

2018-04-12 Thread Alexander Korotkov
On Thu, Apr 12, 2018 at 1:14 AM, Teodor Sigaev wrote: > Peter Geoghegan wrote: > >> On Wed, Apr 11, 2018 at 2:29 PM, Peter Eisentraut >> wrote: >> >>> But in this case it doesn't even do equality comparison, it just returns >>> the value. >>> >> >> That's the idea that I tried to express. The po

Re: Partitioned tables and covering indexes

2018-04-12 Thread Alexander Korotkov
On Thu, Apr 12, 2018 at 1:14 AM, Teodor Sigaev wrote: > That's the idea that I tried to express. The point is that we need to >>> tell the user that there is no need to worry about it, rather than >>> that they're wrong to ask about it. Though we should probably actually >>> just throw an error.

Re: Partitioned tables and covering indexes

2018-04-11 Thread Teodor Sigaev
That's the idea that I tried to express. The point is that we need to tell the user that there is no need to worry about it, rather than that they're wrong to ask about it. Though we should probably actually just throw an error. Or maybe it should be the collation of the underlying table columns

Re: Partitioned tables and covering indexes

2018-04-11 Thread Teodor Sigaev
Peter Geoghegan wrote: On Wed, Apr 11, 2018 at 2:29 PM, Peter Eisentraut wrote: But in this case it doesn't even do equality comparison, it just returns the value. That's the idea that I tried to express. The point is that we need to tell the user that there is no need to worry about it, ra

Re: Partitioned tables and covering indexes

2018-04-11 Thread Peter Eisentraut
On 4/11/18 17:38, Peter Geoghegan wrote: > On Wed, Apr 11, 2018 at 2:29 PM, Peter Eisentraut > wrote: >> But in this case it doesn't even do equality comparison, it just returns >> the value. > > That's the idea that I tried to express. The point is that we need to > tell the user that there is n

Re: Partitioned tables and covering indexes

2018-04-11 Thread Peter Geoghegan
On Wed, Apr 11, 2018 at 2:29 PM, Peter Eisentraut wrote: > But in this case it doesn't even do equality comparison, it just returns > the value. That's the idea that I tried to express. The point is that we need to tell the user that there is no need to worry about it, rather than that they're wr

Re: Partitioned tables and covering indexes

2018-04-11 Thread Peter Eisentraut
On 4/11/18 17:08, Peter Geoghegan wrote: >> However, I don't see any point in defining collations here, because >> INCLUDE attributes exist solely for index-only scans. So, index just >> can return value of INCLUDE attribute "as is", no point to do something >> with collation. >> >> So, I propose

Re: Partitioned tables and covering indexes

2018-04-11 Thread Peter Geoghegan
On Wed, Apr 11, 2018 at 1:58 PM, Alexander Korotkov wrote: > It appears that INCLUDE columns might have collation defined. > For instance, following query is working: > > create index t_s1_s2_idx on t (s1) include (s2 collate "en_US.UTF-8"); > > However, I don't see any point in defining collation

Re: Partitioned tables and covering indexes

2018-04-11 Thread Alexander Korotkov
On Wed, Apr 11, 2018 at 10:47 PM, Alvaro Herrera wrote: > Teodor Sigaev wrote: > > > Patch attached. > > I wonder why this is a problem in opfamilies but not collations. > If we don't compare collations, wouldn't it make more sense to break out > of the loop once the number of keys is reached? >

Re: Partitioned tables and covering indexes

2018-04-11 Thread Alvaro Herrera
Teodor Sigaev wrote: > Patch attached. I wonder why this is a problem in opfamilies but not collations. If we don't compare collations, wouldn't it make more sense to break out of the loop once the number of keys is reached? When this code was written, there was no question as to what length the

Re: Partitioned tables and covering indexes

2018-04-11 Thread Teodor Sigaev
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. Patch attached. But it seems to me, field's names of IndexInfo structure are a bit confused now: int

Re: Partitioned tables and covering indexes

2018-04-11 Thread Teodor Sigaev
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

Re: Partitioned tables and covering indexes

2018-04-11 Thread Teodor Sigaev
Thank you, pushed 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.  T

Re: Partitioned tables and covering indexes

2018-04-10 Thread Amit Langote
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 logi

Re: Partitioned tables and covering indexes

2018-04-10 Thread Jaime Casanova
On 10 April 2018 at 10: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 l

Re: Partitioned tables and covering indexes

2018-04-10 Thread Teodor Sigaev
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 index

Re: Partitioned tables and covering indexes

2018-04-10 Thread Alexander Korotkov
On Tue, Apr 10, 2018 at 12:47 PM, Amit Langote < langote_amit...@lab.ntt.co.jp> wrote: > On 2018/04/10 16:07, Jaime Casanova wrote: > > Hi, > > > > Trying covering indexes on partitioned tables i get this error > > """ > > postgres=# create index on t1_part (i) include (t); > > ERROR: cache looku

Re: Partitioned tables and covering indexes

2018-04-10 Thread Amit Langote
On 2018/04/10 16:07, Jaime Casanova wrote: > Hi, > > Trying covering indexes on partitioned tables i get this error > """ > postgres=# create index on t1_part (i) include (t); > ERROR: cache lookup failed for opclass 0 > """ > > To reproduce: > > create table t1_part (i int, t text) partition b