Re: Assert failure in base_yyparse

On Fri, May 9, 2025 at 6:50 PM Alvaro Herrera <alvherre(at)kurilemu(dot)de> wrote:
> I agree that blocking the index from using the option name that xmltable
> parsing uses internally is okay. Maybe we can rename it to something
> like "__pg__is_not_null" or something like that, which would reduce the
> chances of troubling people; the existing name sounds too much like a
> valid name that users could want to use.
>
> Also, maybe rather than just "syntax error" we could say something like
> "option name XYZ cannot be used in XMLTABLE".

Agreed on both points.

Evgeniy's patch checks the type of the option argument at the last
moment and raises an error if it's not Boolean. I think it might be
better to check whether the user-supplied name collides with the
internally reserved name earlier in the process. According to the
doc, users should specify column nullability using [NOT NULL | NULL]
in the column definition, rather than explicitly setting an
"is_not_null" option.

Attached is a patch that implements this. It also renames the
internally used option name and updates the error message.

> I wonder if we have any other names used by the parser that can cause
> this kind of problem. In a quick look through gram.y I didn't find any
> other place that would fabricate a name and also accept arbitrary
> user-specified names to use, so this seems to be the only place affected
> by this particular bug.

Yeah. Thanks for checking.

Thanks
Richard