В письме от понедельник, 7 января 2019 г. 13:56:48 MSK пользователь Alvaro 
Herrera написал:

> > 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.
 
If parallel_workers is not intended to be used anywhere except heap and 
matview, then may be better to make fix more relaxed. Like 

if  (relation->rd_rel->relkind == RELKIND_RELATION ||                          
     relation->rd_rel->relkind == RELKIND_MATVIEW  )   
 rel->rel_parallel_workers =                                         
                                    RelationGetParallelWorkers(relation,-1);
else
 rel->rel_parallel_workers = -1;

> > 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.
It is mild until somebody introduce PartitionedlRelOptions. Then 
PartitionedlRelOptions * will be converted to StdRdOptions* and we will get 
segmentation fault...

So may be there is no need for back fix, but better to fix it now :-)
May be with the patch for StdRdOptions removal.


Reply via email to