Re: minor gripe about lax reloptions parsing for views

From: Mark Dilger <mark(dot)dilger(at)enterprisedb(dot)com>
To: Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org>
Cc: PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: minor gripe about lax reloptions parsing for views
Date: 2021-10-01 19:34:53
Message-ID: 8F154BB9-971C-4585-8079-504838E41938@enterprisedb.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

> On Oct 1, 2021, at 6:15 AM, Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org> wrote:
>
> On 2021-Sep-30, Mark Dilger wrote:
>
>> The solution is simple enough: stop using HEAP_RELOPT_NAMESPACES when
>> parsing reloptions for views and instead create a
>> VIEW_RELOPT_NAMESPACES array which does not include "toast".
>
> It seems a reasonable (non-backpatchable) change to me.

I agree. It's neither important enough to be back-patched nor completely non-breaking. Somebody could be passing bogus reloptions and relying on the parser to ignore them.

>> I've already fixed this, mixed into some other work. I'll pull it out
>> as its own patch if there is any interest.
>
> Yeah.
>
> I suppose you'll need a new routine that returns the namespace array to
> use based on relkind.

The patch does it this way. The new routine can just return NULL for relkinds that don't accept "toast" as an option namespace. We don't need to create the VIEW_RELOPT_NAMESPACES array mentioned upthread.

The patch changes the docs for index storage option "fillfactor". The existing documentation imply that all index methods support this parameter, but in truth built-in methods brin and gin do not, and we should not imply anything about what non-built-in methods do.

The changes to create_view.sql demonstrate what the patch has fixed.

The other changes to regression tests provide previously missing test coverage of storage options for which the behavior is unchanged. I prefer to have coverage, but if the committer who picks this up disagrees, those changes could just be ignored. I'd also be happy to remove them and repost if the committer prefers.

Attachment Content-Type Size
v1-0001-Reject-storage-options-in-toast-namespace-in-view.patch application/octet-stream 41.8 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Jeremy Schneider 2021-10-01 19:37:31 Re: BUG #16583: merge join on tables with different DB collation behind postgres_fdw fails
Previous Message David Zhang 2021-10-01 19:23:42 Re: Fix uninitialized variable access (src/backend/utils/mmgr/freepage.c)