Re: Problem with parallel_workers option (Was Re: [PATCH] get rid of StdRdOptions, use individual binary reloptions representation for each relation kind instead)

On 2019-Jan-07, Nikolay Shaplov wrote:

> Asserts are cool thing. I found some unexpected stuff.
>
> parallel_workers option is claimed to be heap-only option.
>
> But in src/backend/optimizer/util/plancat.c in get_relation_info
> RelationGetParallelWorkers is being called for both heap and toast tables (and
> not only for them).

Ugh.

I wonder if it makes sense for a toast table to have parallel_workers.
I suppose it's not useful, since a toast table is not supposed to be
scanned in bulk, only accessed through the tuptoaster interface. But on
the other hand, you *can* do "select * from pg_toast_NNN", and it almost
all respects a toast table is just like a regular heap table.

> Because usually there are no reloptions for toast, it returns default -1
> value. If some reloptions were set for toast table, RelationGetParallelWorkers
> will return a value from uninitialized memory.

Well, if it returns a negative number or zero, the rest of the server
should behave identically to it returning the -1 that was intended. And
if it returns a positive number, the worst that will happen is that a
Path structure somewhere will have a positive number of workers, but
since queries on toast tables are not planned in the regular way, most
likely those Paths will never exist anyway.

So while I agree that this is a bug, it seems pretty benign.

Unless I overlook something.

--
Álvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services