Re: [PROPOSAL] Covering + unique indexes.

15.09.2015 12:18, David Rowley:
> On 12 September 2015 at 00:45, Anastasia Lubennikova
> <a(dot)lubennikova(at)postgrespro(dot)ru <mailto:a(dot)lubennikova(at)postgrespro(dot)ru>>
> wrote:
>
> I've started work on a patch that allows to combine covering and
> unique functionality.
>
>
> Great to hear someone is working on this!

Thank you! It looks like very interesting project to me)

> Next issue is pg_index changes.
> Now there's only a boolean flag
>
> * bool indisunique; /* is this a unique index? */
>
> But new algorithm requires to store a single number
>
> * unit16n_unique_columns; /* number of first columns of index
> which has unique constrains. */
>
> I think, that numbers of all attributes themselves are not needed.
> Is it right?
>
>
> I think the total number of attributes is already in indnatts.
> I imagine you're planning to keep the indexed columns at the start of
> the indkey[] array, and just use n_unique_columns to mark how many of
> the indkey[] attributes to check as indexed columns. I'd imagine the
> change would be fairly simple from a planner point of view as you'd
> just need to check columns 1 to n_unique_columns instead of 1 to
> indnatts. Although I have a tendency to under estimate these things :(

I've asked that for the same reason. I'm not too deep in access method
and btree code, so I don't want to miss any hidden dependencies.
As I see it, changes are required in _bt_isequal()
<http://doxygen.postgresql.org/nbtinsert_8c.html#abcfb7a3dcd40a5d1759652239f3a0115>.
Instead of

for (i = 1; i <= keysz; i++) {} // where /keysz/ is actually /natts//=
rel->rd_rel->relnatts;
/Look at _bt_check_uinque
<http://doxygen.postgresql.org/nbtinsert_8c.html#a96eb8c53ffdf53f139b037194a9721a3>
and pg_class
<http://doxygen.postgresql.org/pg__class_8h.html#ac8bf924d36feee5f3ca4c36aa01c75ec>
for more info.

cycle should be
for (i = 1; i <= nuinique; i++) {} // where /nunique /is value of
/rel->rd_index->//indnuinque

//indnuinque /is a new field, which I suggest to add to pg_index
<http://doxygen.postgresql.org/pg__index_8h.html#a5065be0408f03576083a30c97b43a09a>
instead of /indisunique/ (or may be in addition to it).

But I'm doubt about the problem which Jim has mentioned.

>Are we certain that no index type could ever support an index on (f1,
f2, f3) UNIQUE(f1, f3)? Even if it >doesn't make sense for btree,
perhaps some other index could handle it.

If it's really so, we certainly have to keep in pg_index
<http://doxygen.postgresql.org/pg__index_8h.html#a5065be0408f03576083a30c97b43a09a>
not just number of unique columns (/indnunique/), but their numbers
themselves (an array /indnuniqueatts/ on the model of /indnatts/).

> I imagine you don't want to name the new column n_unique_columns,
> since it does not fit too well with non-unique indexes.

Hm, I think that it would be quite clear to set it to zero for
non-unique indexes.
/(nunique == 0)/ is equal to /(indisunique==false)/.

But maybe I've missed some reason why we should to save /indisunique/ flag.

> Perhaps just indindexedatts, or something slightly along those lines.
> But perhaps it would be a good idea to also rename "ncolumns" in code,
> to ensure that any non-updated code does not even compile. Perhaps
> including "tot" or "total" in there might help indicate it's new meaning.
I hope, that all these changes are not needed. Just continue to use
/ncolumns/ for all indexed columns. And new variable /nuinique/ (or
something like that) is for number of unique columns in the index.

--
Anastasia Lubennikova
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company