IGNORE/RESPECT NULLS can be specified for (prokind == 'f').
Hi.
While reviewing the row pattern recognition,
I noticed that the IGNORE NULLS and RESPECT NULLS clauses can be specified for
normal functions (prokind == 'f').
create or replace function dummy2(int) returns bigint AS $$ BEGIN
RETURN $1; END$$ STABLE LANGUAGE plpgsql;
select dummy2(1) IGNORE NULLS;
select dummy2(1) RESPECT NULLS;
This seems unexpected?
Also https://git.postgresql.org/cgit/postgresql.git/commit/?id=25a30bbd4235a49c854036c84fe90f2bc5a87652
Obviously, RESPECT NULLS and IGNORE NULLS can appear in SELECT query,
since they are dedicated keywords, shouldn't they be mentioned in
select.sgml?
While reviewing the row pattern recognition,
I noticed that the IGNORE NULLS and RESPECT NULLS clauses can be specified for
normal functions (prokind == 'f').create or replace function dummy2(int) returns bigint AS $$ BEGIN
RETURN $1; END$$ STABLE LANGUAGE plpgsql;
select dummy2(1) IGNORE NULLS;
select dummy2(1) RESPECT NULLS;This seems unexpected?
Yes. Other than true window functions are not allowed to use the null
treatment clause per spec.
Also https://git.postgresql.org/cgit/postgresql.git/commit/?id=25a30bbd4235a49c854036c84fe90f2bc5a87652
Obviously, RESPECT NULLS and IGNORE NULLS can appear in SELECT query,
since they are dedicated keywords, shouldn't they be mentioned in
select.sgml?
No, we should fix the code. See attached patch.
To fix this, ParseFuncOrColumn() now checks whether other than window
functions are used with the null treatment clause. If so, error out.
Also remove the unnecessary test for "aggregate functions do not
accept RESPECT/IGNORE NULLS" because it is now checked in the
early-stage new check. The window regression test expected file is
changed accordingly.
Regards,
--
Tatsuo Ishii
SRA OSS K.K.
English: http://www.sraoss.co.jp/index_en/
Japanese:http://www.sraoss.co.jp
Attachments:
v1-0001-Fix-to-not-allow-null-treatment-to-non-window-fun.patchtext/x-patch; charset=us-asciiDownload+11-10
While reviewing the row pattern recognition,
I noticed that the IGNORE NULLS and RESPECT NULLS clauses can be specified for
normal functions (prokind == 'f').create or replace function dummy2(int) returns bigint AS $$ BEGIN
RETURN $1; END$$ STABLE LANGUAGE plpgsql;
select dummy2(1) IGNORE NULLS;
select dummy2(1) RESPECT NULLS;This seems unexpected?
Yes. Other than true window functions are not allowed to use the null
treatment clause per spec.Also https://git.postgresql.org/cgit/postgresql.git/commit/?id=25a30bbd4235a49c854036c84fe90f2bc5a87652
Obviously, RESPECT NULLS and IGNORE NULLS can appear in SELECT query,
since they are dedicated keywords, shouldn't they be mentioned in
select.sgml?No, we should fix the code. See attached patch.
To fix this, ParseFuncOrColumn() now checks whether other than window
functions are used with the null treatment clause. If so, error out.Also remove the unnecessary test for "aggregate functions do not
accept RESPECT/IGNORE NULLS" because it is now checked in the
early-stage new check. The window regression test expected file is
changed accordingly.
I'm going to push the patch today if there's no objection.
--
Tatsuo Ishii
SRA OSS K.K.
English: http://www.sraoss.co.jp/index_en/
Japanese:http://www.sraoss.co.jp
On Wed, Jun 17, 2026 at 7:47 AM Tatsuo Ishii <ishii@postgresql.org> wrote:
To fix this, ParseFuncOrColumn() now checks whether other than window
functions are used with the null treatment clause. If so, error out.Also remove the unnecessary test for "aggregate functions do not
accept RESPECT/IGNORE NULLS" because it is now checked in the
early-stage new check. The window regression test expected file is
changed accordingly.I'm going to push the patch today if there's no objection.
The code looks good to me, and the commit message is clear and well-written.
To fix this, ParseFuncOrColumn() now checks whether other than window
functions are used with the null treatment clause. If so, error out.Also remove the unnecessary test for "aggregate functions do not
accept RESPECT/IGNORE NULLS" because it is now checked in the
early-stage new check. The window regression test expected file is
changed accordingly.I'm going to push the patch today if there's no objection.
The code looks good to me, and the commit message is clear and well-written.
Thank you for the review!
I have pushed the patch to master branch.
https://git.postgresql.org/gitweb/?p=postgresql.git;a=commit;h=4e5920e6de87dc1ed761d2a898b7786c50c11db2
--
Tatsuo Ishii
SRA OSS K.K.
English: http://www.sraoss.co.jp/index_en/
Japanese:http://www.sraoss.co.jp
On Jun 17, 2026, at 09:17, Tatsuo Ishii <ishii@postgresql.org> wrote:
To fix this, ParseFuncOrColumn() now checks whether other than window
functions are used with the null treatment clause. If so, error out.Also remove the unnecessary test for "aggregate functions do not
accept RESPECT/IGNORE NULLS" because it is now checked in the
early-stage new check. The window regression test expected file is
changed accordingly.I'm going to push the patch today if there's no objection.
The code looks good to me, and the commit message is clear and well-written.
Thank you for the review!
I have pushed the patch to master branch.
https://git.postgresql.org/gitweb/?p=postgresql.git;a=commit;h=4e5920e6de87dc1ed761d2a898b7786c50c11db2
--
Tatsuo Ishii
SRA OSS K.K.
English: http://www.sraoss.co.jp/index_en/
Japanese:http://www.sraoss.co.jp
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]https://www.postgresql.org/docs/current/error-style-guide.html:
```
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
Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/
Attachments:
nocfbot-fix-errmsg.diffapplication/octet-stream; name=nocfbot-fix-errmsg.diff; x-unix-mode=0644Download+3-3
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!
--
Tatsuo Ishii
SRA OSS K.K.
English: http://www.sraoss.co.jp/index_en/
Japanese:http://www.sraoss.co.jp
On Wed, Jun 17, 2026 at 12:58 PM Tatsuo Ishii <ishii@postgresql.org> 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
Attachments:
v1-0001-Refine-NULL-treatment-checks-for-non-window-funct.patchapplication/octet-stream; name=v1-0001-Refine-NULL-treatment-checks-for-non-window-funct.patchDownload+33-12
Hi,
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.
Agreed. BTW, if the check is moved there, it might be better to
change the error message according to the surroundings. The pattern is
"... specified, but %s is not a ..."
errmsg("OVER specified, but %s is not a window function nor an aggregate function",
So the message could be changed to something like:
errmsg("RESPECT/IGNORE NULLS is specified, but %s is not a window function"),
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.
I agree with your concern and am fine with the change.
/*
* NULL TREATEMENT is only allowed for window functions per spec.
*/Also, I noticed a typo: TREATEMENT should be TREATMENT.
Thanks for the correction.
The attached patch implements these changes. Thoughts?
Please see comment above.
Regards,
--
Tatsuo Ishii
SRA OSS K.K.
English: http://www.sraoss.co.jp/index_en/
Japanese:http://www.sraoss.co.jp
On Jun 18, 2026, at 10:44, Tatsuo Ishii <ishii@postgresql.org> wrote:
Hi,
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.Agreed. BTW, if the check is moved there, it might be better to
change the error message according to the surroundings. The pattern is
"... specified, but %s is not a ..."errmsg("OVER specified, but %s is not a window function nor an aggregate function",
So the message could be changed to something like:
errmsg("RESPECT/IGNORE NULLS is specified, but %s is not a window function"),
Yes, this is also my only comment for the patch.
Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/
On Thu, Jun 18, 2026 at 12:06 PM Chao Li <li.evan.chao@gmail.com> wrote:
Agreed. BTW, if the check is moved there, it might be better to
change the error message according to the surroundings. The pattern is
"... specified, but %s is not a ..."errmsg("OVER specified, but %s is not a window function nor an aggregate function",
So the message could be changed to something like:
errmsg("RESPECT/IGNORE NULLS is specified, but %s is not a window function"),
Thanks for the review!
I've updated the patch accordingly.
I removed "is" from "RESPECT/IGNORE NULLS is specified" to match other
similar error messages, which use the form "XXX specified, ...".
+ if (ignore_nulls != NO_NULLTREATMENT)
+ ereport(ERROR,
+ (errcode(ERRCODE_WRONG_OBJECT_TYPE),
+ errmsg("aggregate functions do not accept RESPECT/IGNORE NULLS"),
+ parser_errposition(pstate, location)));
I also moved this check to the end of the
"else if (fdresult == FUNCDETAIL_AGGREGATE)" block. That seems like
a better place, since the ordered-set aggregate checks should happen
before this check.
Regards,
--
Fujii Masao
Attachments:
v2-0001-Refine-error-reporting-for-null-treatment-on-non-.patchapplication/octet-stream; name=v2-0001-Refine-error-reporting-for-null-treatment-on-non-.patchDownload+31-12
On Jun 18, 2026, at 15:54, Fujii Masao <masao.fujii@gmail.com> wrote:
On Thu, Jun 18, 2026 at 12:06 PM Chao Li <li.evan.chao@gmail.com> wrote:
Agreed. BTW, if the check is moved there, it might be better to
change the error message according to the surroundings. The pattern is
"... specified, but %s is not a ..."errmsg("OVER specified, but %s is not a window function nor an aggregate function",
So the message could be changed to something like:
errmsg("RESPECT/IGNORE NULLS is specified, but %s is not a window function"),
Thanks for the review!
I've updated the patch accordingly.
I removed "is" from "RESPECT/IGNORE NULLS is specified" to match other
similar error messages, which use the form "XXX specified, ...".+ if (ignore_nulls != NO_NULLTREATMENT) + ereport(ERROR, + (errcode(ERRCODE_WRONG_OBJECT_TYPE), + errmsg("aggregate functions do not accept RESPECT/IGNORE NULLS"), + parser_errposition(pstate, location)));I also moved this check to the end of the
"else if (fdresult == FUNCDETAIL_AGGREGATE)" block. That seems like
a better place, since the ordered-set aggregate checks should happen
before this check.Regards,
--
Fujii Masao
<v2-0001-Refine-error-reporting-for-null-treatment-on-non-.patch>
V2 looks good to me.
Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/
I've updated the patch accordingly.
I removed "is" from "RESPECT/IGNORE NULLS is specified" to match other
similar error messages, which use the form "XXX specified, ...".+ if (ignore_nulls != NO_NULLTREATMENT) + ereport(ERROR, + (errcode(ERRCODE_WRONG_OBJECT_TYPE),
The errcode field can start without a parenthesis:
errcode(ERRCODE_WRONG_OBJECT_TYPE),
Other than that the patch looks good to me.
Regards,
--
Tatsuo Ishii
SRA OSS K.K.
English: http://www.sraoss.co.jp/index_en/
Japanese:http://www.sraoss.co.jp
Hello,
I would dearly appreciate it if you could make these errors in this
style:
ereport(ERROR,
(errcode(ERRCODE_WRONG_OBJECT_TYPE),
/* translator: first %s is a CREATE FUNCTION clause, eg. "IGNORE NULLS" */
errmsg("%s specified, but %s is not a window function",
"RESPECT/IGNORE NULLS", NameListToString(funcname)),
parser_errposition(pstate, location)));
to avoid keeping the clause name in the translatable string.
Thanks
--
Álvaro Herrera Breisgau, Deutschland — https://www.EnterpriseDB.com/
"Entristecido, Wutra (canción de Las Barreras)
echa a Freyr a rodar
y a nosotros al mar"
On Thu, Jun 18, 2026 at 11:46 PM Álvaro Herrera <alvherre@kurilemu.de> wrote:
Hello,
I would dearly appreciate it if you could make these errors in this
style:ereport(ERROR,
(errcode(ERRCODE_WRONG_OBJECT_TYPE),
/* translator: first %s is a CREATE FUNCTION clause, eg. "IGNORE NULLS" */
errmsg("%s specified, but %s is not a window function",
"RESPECT/IGNORE NULLS", NameListToString(funcname)),
parser_errposition(pstate, location)));to avoid keeping the clause name in the translatable string.
Thanks for the suggestion! I've updated the patch accordingly.
Patch attached.
Regards,
--
Fujii Masao
Attachments:
v3-0001-Refine-error-reporting-for-null-treatment-on-non-.patchapplication/octet-stream; name=v3-0001-Refine-error-reporting-for-null-treatment-on-non-.patchDownload+32-12
On Thu, Jun 18, 2026 at 5:28 PM Tatsuo Ishii <ishii@postgresql.org> wrote:
I've updated the patch accordingly.
I removed "is" from "RESPECT/IGNORE NULLS is specified" to match other
similar error messages, which use the form "XXX specified, ...".+ if (ignore_nulls != NO_NULLTREATMENT) + ereport(ERROR, + (errcode(ERRCODE_WRONG_OBJECT_TYPE),The errcode field can start without a parenthesis:
errcode(ERRCODE_WRONG_OBJECT_TYPE),
Yes, but other similar ereport() calls in parse_func.c also start
the errcode field with a parenthesis, so I followed the same style
here.
Regards,
--
Fujii Masao
On Fri, Jun 19, 2026 at 1:12 AM Fujii Masao <masao.fujii@gmail.com> wrote:
On Thu, Jun 18, 2026 at 11:46 PM Álvaro Herrera <alvherre@kurilemu.de> wrote:
Hello,
I would dearly appreciate it if you could make these errors in this
style:ereport(ERROR,
(errcode(ERRCODE_WRONG_OBJECT_TYPE),
/* translator: first %s is a CREATE FUNCTION clause, eg. "IGNORE NULLS" */
errmsg("%s specified, but %s is not a window function",
"RESPECT/IGNORE NULLS", NameListToString(funcname)),
parser_errposition(pstate, location)));to avoid keeping the clause name in the translatable string.
Thanks for the suggestion! I've updated the patch accordingly.
Patch attached.
Barring any objections, I will commit the patch.
Regards,
--
Fujii Masao
On Fri, Jun 19, 2026 at 1:12 AM Fujii Masao <masao.fujii@gmail.com> wrote:
On Thu, Jun 18, 2026 at 11:46 PM Álvaro Herrera <alvherre@kurilemu.de> wrote:
Hello,
I would dearly appreciate it if you could make these errors in this
style:ereport(ERROR,
(errcode(ERRCODE_WRONG_OBJECT_TYPE),
/* translator: first %s is a CREATE FUNCTION clause, eg. "IGNORE NULLS" */
errmsg("%s specified, but %s is not a window function",
"RESPECT/IGNORE NULLS", NameListToString(funcname)),
parser_errposition(pstate, location)));to avoid keeping the clause name in the translatable string.
Thanks for the suggestion! I've updated the patch accordingly.
Patch attached.Barring any objections, I will commit the patch.
No objection from me.
--
Tatsuo Ishii
SRA OSS K.K.
English: http://www.sraoss.co.jp/index_en/
Japanese:http://www.sraoss.co.jp
On Tue, Jun 23, 2026 at 8:38 PM Tatsuo Ishii <ishii@postgresql.org> wrote:
Barring any objections, I will commit the patch.
No objection from me.
I've pushed the patch. Thanks!
Regards,
--
Fujii Masao
On Tue, Jun 23, 2026 at 8:38 PM Tatsuo Ishii <ishii@postgresql.org> wrote:
Barring any objections, I will commit the patch.
No objection from me.
I've pushed the patch. Thanks!
Thank you so much!
--
Tatsuo Ishii
SRA OSS K.K.
English: http://www.sraoss.co.jp/index_en/
Japanese:http://www.sraoss.co.jp