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
