Re: missing indexes in indexlist with partitioned tables

From: Arne Roland <A(dot)Roland(at)index(dot)de>
To: Amit Langote <amitlangote09(at)gmail(dot)com>
Cc: Zhihong Yu <zyu(at)yugabyte(dot)com>, Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org>, Julien Rouhaud <rjuju123(at)gmail(dot)com>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: missing indexes in indexlist with partitioned tables
Date: 2022-01-31 18:14:10
Message-ID: 71191ef79d7a4892b99a62970f10771d@index.de
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi!

From: Amit Langote <amitlangote09(at)gmail(dot)com>
Sent: Tuesday, January 25, 2022 09:04
Subject: Re: missing indexes in indexlist with partitioned tables
> [...]
> "partindexlist" really made me think about a list of "partial indexes"
> for some reason. I think maybe "partedindexlist" is what you are
> looking for; "parted" is commonly used as short for "partitioned" when
> naming variables.
>
> The comment only mentions "further pruning" as to what partitioned
> indexes are to be remembered in RelOptInfo, but it's not clear what
> that means. It may help to be more specific.

Thanks for the feedback! I've changed that. The current version is attached.

> Finally, I don't understand why we need a separate field to store
> indexes found in partitioned base relations. AFAICS, nothing but the
> sites you are interested in (relation_has_unique_index_for() and
> rel_supports_distinctness()) would ever bother to look at a
> partitioned base relation's indexlist. Do you think putting them into
> in indexlist might break something?

I have thought about that before. AFAICT there is nothing in core, which breaks. However I am not sure, I want to mix those two kinds of index nodes. First of all the structure is different, partedIndexes don't have physical attributes after all. This is technical implementation detail relating to the current promise, that entries of the indexlist are indexes we can use. And by use, I mean use for statistics or the executor.
I'm more concerned about future changes regarding the order and optimization of processing harder here. The order in which we do things in the planner is a bit messy, and I wouldn't mind seeing details about that change. Looking at the current wacky order in the optimizer, I'm not convinced, that nothing will want to have a look at the indexlist, before partitioned tables are unpacked.

Since it would be easy to introduce this new variable later, wouldn't mind adding it to the indexlist directly for now. But changing the underlying promise of what it contains, seems noteworthy and more intrusive to me.

> > Side note: I personally think the name inhparent is mildly confusing, since it's not really about inheritance. I don't have a significantly better idea though.
>
> Partitioned tables are "inheritance parent", so share the same code as
> what traditional inheritance parents have always used for planning.

I recall that manual partitioning via inheritance, that was cumbersome. Though that minor historical detail was not, what I was referring to. There are a lot of other cases, that cause us to set inhparent. IIRC we use this flag in some ddl commands, which have nothing to do with inheritance. It essentially is used as a variant to skip the indexlist creation. If such hacks weren't there, we could simply check for the relkind and indisunique.

Regards
Arne

Attachment Content-Type Size
0005-partIndexlistClean.patch text/x-patch 19.4 KB

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Bharath Rupireddy 2022-01-31 18:15:19 Re: Add checkpoint and redo LSN to LogCheckpointEnd log message
Previous Message Nathan Bossart 2022-01-31 18:10:52 Re: Catalog version access