On Fri, 2025-03-07 at 20:02 +0300, Nikolay Shaplov wrote:
> While working with my New Options Engine patch
> https://commitfest.postgresql.org/patch/4688/
> I found out that I can detach a small portion of it as a separate
> patch.
> It has own value, even if big patch is never committed, and it would
> make
> smoother further committing of big patch if we ever get to it.
>
> Patch description is following:
Thanks for the patch!
> 1. `isnull` variable is actually needed in a very narrow scope,
> so
> it is better to keep it in that scope, not keeping it in mind in
> while dealing with the rest of the code.
Since we already have datum=0 logic there, there is indeed no need to
have isnull variable on same scope.
>
> 2. Toast table RelOptions are never altered directly with ALTER
> command.
> One should do ATLER to a heap relation and use toast. reloption
> namespace
> to address toast's reloption. If you get `ATExecSetRelOptions` called
> with
> `RELKIND_TOASTVALUE` relation in the args, something is really wrong.
> We
> should throw asserts, errors and whistle as loud as we can.
Seems reasonable to me. Toast tables are handled later by
"heap_reloptions(RELKIND_TOASTVALUE, ..." in same ATExecSetRelOptions()
function code.
LGTM
-------
Regards,
Timur Magomedov
