Re: Unsafe use of relation->rd_options without checking its type

Поиск
Список
Период
Сортировка
От neha khatri
Тема Re: Unsafe use of relation->rd_options without checking its type
Дата
Msg-id CAFO0U+-oeRHyofrWOdwfmDuZxh5E3uqTG85OrcJMe5-fj1JiKQ@mail.gmail.com
обсуждение исходный текст
Ответ на Unsafe use of relation->rd_options without checking its type  (Tom Lane <tgl@sss.pgh.pa.us>)
Список pgsql-hackers

                                                             ^

The reason for the error is that transformOnConflictArbiter applies
RelationIsUsedAsCatalogTable() to something that it doesn't know to
be a plain relation --- it's a view in this case.  And that macro
blindly assumes that relation->rd_options is a StdRdOptions struct,
when in this case it's actually a ViewOptions struct.

Now that I've seen this I wonder which other uses of rd_options are
potentially broken.  RelationIsUsedAsCatalogTable() is hardly the
only macro that is assuming with little justification that it's
applied to the right kind of reloptions.
 
Right, there are many macros, which blindly assume the rd_options
to be of specific type. Here is the list of such macros :

 RelationGetFillFactor  
 RelationIsUsedAsCatalogTable
 RelationGetParallelWorkers
 RelationIsSecurityView        
 RelationHasCheckOption        
 RelationHasLocalCheckOption    
 RelationHasCascadedCheckOption 
 BrinGetPagesPerRange        
 GinGetUseFastUpdate          
 GinGetPendingListCleanupSize

For the macros associated with specific index type, it might be alright to
assume the type of the rd_options because those have very limited usage.
However for the rest of the macros, the code does not limit its usage. This can
lead to problems as you described above . 



We could band-aid this by having the RelationIsUsedAsCatalogTable()
macro check relation->relkind, but I'm inclined to think that the
right fix going forward is to insist that StdRdOptions, ViewOptions,
and the other in-memory representations of reloptions ought to be
self-identifying somehow.  We could add a field to them similar to
nodeTag, but one of the things that was envisioned to begin with
is relation types that have StdRdOptions as the first part of a
larger struct.  I'm not sure how to make that work with a tag.

Thoughts?
 
Yes, it seems appropriate to tag all types of the rd_options with an 
identification and maybe check for that identification within each macro.
Currently, there are following types of rd_options as far as I could find:

 GiSTOptions
 BrinOptions
 GinOptions
 StdRdOptions
 ViewOptions
 BloomOptions (from contrib)

I am not clear on how the identification field in the above structures can
be a problem,  for the relations have StdRdOptions as the first part of a
larger structure.

Regards,
Neha

В списке pgsql-hackers по дате отправления:

Предыдущее
От: Thomas Munro
Дата:
Сообщение: WIP: [[Parallel] Shared] Hash
Следующее
От: Vladimir Gordiychuk
Дата:
Сообщение: Re: Logical decoding and walsender timeouts