On Wed, Jun 17, 2026 at 12:58 PM Tatsuo Ishii <[email protected]> wrote:
>
> Hi Chao,
>
> > Hi Tatsuo-san,
> >
> > Sorry for jumping in. I didn’t review the patch, but while browsing recent 
> > commits, I noticed that this commit has a small issue: the errmsg should 
> > start with a lowercase character, according to the error style guide [1]:
> > ```
> > Primary error messages: Do not capitalize the first letter. Do not end a 
> > message with a period. Do not even think about ending a message with an 
> > exclamation point.
> > ```
> >
> > The attached diff is a quick fix that changes “Only” to “only” and updates 
> > the expected output accordingly.
> >
> > [1] https://www.postgresql.org/docs/current/error-style-guide.html
>
> You are right. I don't know why I forgot the rule. Patch pushed.
> Thank you!

Thanks for working on this!

I have a couple of comments.

When executing a non-existent function with IGNORE NULLS, for example,

    SELECT no_such_func() IGNORE NULLS;

previously we got:

    ERROR: function no_such_func() does not exist

but now we get:

    ERROR: only window functions accept RESPECT/IGNORE NULLS

Isn't the previous error more helpful in this case? It makes me wonder
whether the IGNORE NULLS check is being performed too early. Perhaps
it would be better to move the check to other place.


I also noticed that when using an aggregate function as a window
function, for example,

    SELECT sum(i) IGNORE NULLS OVER() FROM ...;

we now get:

    ERROR: only window functions accept RESPECT/IGNORE NULLS

whereas previously the error was:

    ERROR: aggregate functions do not accept RESPECT/IGNORE NULLS

The new message seems confusing because `sum()` is being used as
a window function in this case, so saying "only window functions accept ..."
doesn't seem accurate.


    /*
     * NULL TREATEMENT is only allowed for window functions per spec.
     */

Also, I noticed a typo: TREATEMENT should be TREATMENT.


The attached patch implements these changes. Thoughts?

Regards,

-- 
Fujii Masao

Attachment: v1-0001-Refine-NULL-treatment-checks-for-non-window-funct.patch
Description: Binary data

Reply via email to