Re: [PATCH] get rid of StdRdOptions, use individual binary reloptions representation for each relation kind instead

From: Nikolay Shaplov <dhyan(at)nataraj(dot)su>
To: pgsql-hackers(at)lists(dot)postgresql(dot)org
Cc: Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, Dmitry Dolgov <9erthalion6(at)gmail(dot)com>
Subject: Re: [PATCH] get rid of StdRdOptions, use individual binary reloptions representation for each relation kind instead
Date: 2019-01-07 16:26:12
Message-ID: 20909571.QVcMXUKARm@x200m
Views: Raw Message | Whole Thread | Download mbox
Thread:
Lists: pgsql-hackers

В письме от четверг, 3 января 2019 г. 17:15:08 MSK пользователь Alvaro Herrera
написал:

> > Can we think about backward compatibility aliases?
.....
> > And keep them for as log as needed to avoid #if VERSION in thirdparty
> > code?
> Well, if you do this, at some point you need to tell the extension
> author to change the code. Or they can just keep the code unchanged
> forever. I don't think it's worth the bother.
Ok, you are the Boss ;-)

I've reverted all the macros names back to Relation* except those related to
fillfactor.

> I would have liked to get a StaticAssert in the definition, but I don't
> think it's possible. A standard Assert() should be possible, though.

This is a really good idea. I felt uncomfortable with this (ViewOptions *)
type convertation without checking that it is really View. With Assert I fell
much more safe.

I've added AssertMacro to all options related macroses.

And so, adding asserts discovered a bug with parallel_workers options. That
are defined as Heap only option, but in get_relation_info in src/backend/
optimizer/util/plancat.c a RelationGetParallelWorkers macros is also called
for toasts and other kinds of relations.
I've started a new thread dedicated to this issue, since it needs to be fixed
regardless to this patch.
And for now I added here a hotfix for this issue that is good enough for now.

I also reworked some comments. BTW do you know what is this comments spoke
about:

* RelationGetFillFactor() and RelationGetTargetPageFreeSpace() can only
* be applied to relations that use this format or a superset for
* private options data.

It is speaking about some old times when there can be some other type of
options? 'cause I do not understand what it is speaking about.
I've removed it, I hope I did not remove anything important.

Attachment Content-Type Size
get-rid-of-StrRdOptions_3.diff text/x-patch 35.5 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Lætitia Avrot 2019-01-07 16:45:04 Re: Grant documentation about "all tables"
Previous Message Magnus Hagander 2019-01-07 16:12:45 Re: Using logical replication with older version subscribers