public inbox for [email protected]  

help / color / mirror / Atom feed

From: Chao Li <[email protected]>
To: Tatsuo Ishii <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Subject: Re: Add RESPECT/IGNORE NULLS and FROM FIRST/LAST options
Date: Mon, 15 Sep 2025 16:08:43 +0800
Message-ID: <[email protected]> (raw)
In-Reply-To: <[email protected]>
References: <[email protected]>
	<[email protected]>
	<CAGMVOduZ+7=NR7zW5cLfSRghZAf0yzb3tRceKAXkfg+f-xAZog@mail.gmail.com>
	<[email protected]>

Overall LGTM. Just a few small comments:

> On Sep 12, 2025, at 17:53, Tatsuo Ishii <[email protected]> wrote:
> 
> 
> Comments/suggestions are welcome.
> --
> Tatsuo Ishii
> SRA OSS K.K.
> English: http://www.sraoss.co.jp/index_en/
> Japanese:http://www.sraoss.co.jp
> 


1 - 0001
```
--- a/src/backend/parser/parse_func.c
+++ b/src/backend/parser/parse_func.c
@@ -98,6 +98,7 @@ ParseFuncOrColumn(ParseState *pstate, List *funcname, List *fargs,
 	bool		agg_star = (fn ? fn->agg_star : false);
 	bool		agg_distinct = (fn ? fn->agg_distinct : false);
 	bool		func_variadic = (fn ? fn->func_variadic : false);
+	int			ignore_nulls = (fn ? fn->ignore_nulls : 0);
```

Should we use the constant NO_NULLTREATMENT here for 0?

2 - 0001
```
--- a/src/include/nodes/primnodes.h
+++ b/src/include/nodes/primnodes.h
@@ -579,6 +579,17 @@ typedef struct GroupingFunc
  * Collation information is irrelevant for the query jumbling, as is the
  * internal state information of the node like "winstar" and "winagg".
  */
+
+/*
+ * Null Treatment options. If specified, initially set to PARSER_IGNORE_NULLS
+ * which is then converted to IGNORE_NULLS if the window function allows the
+ * null treatment clause.
+ */
+#define NO_NULLTREATMENT 0
+#define PARSER_IGNORE_NULLS 1
+#define PARSER_RESPECT_NULLS 2
+#define IGNORE_NULLS 3
+
 typedef struct WindowFunc
 {
 	Expr		xpr;
@@ -602,6 +613,8 @@ typedef struct WindowFunc
 	bool		winstar pg_node_attr(query_jumble_ignore);
 	/* is function a simple aggregate? */
 	bool		winagg pg_node_attr(query_jumble_ignore);
+	/* ignore nulls. One of the Null Treatment options */
+	int			ignore_nulls;
```

Maybe we can use “uint8” type for “ignore_nulls”. Because the previous two are both of type “bool”, an uint8 will just fit to the padding bytes, so that new field won’t add extra memory to the structure.

3 - 0004
```
 			winobj->markpos = -1;
 			winobj->seekpos = -1;
+
+			/* reset null map */
+			if (perfuncstate->winobj->ignore_nulls == IGNORE_NULLS)
+				memset(perfuncstate->winobj->notnull_info, 0,
+					   NN_POS_TO_BYTES(perfuncstate->winobj->num_notnull_info));
 		}
```
Where in “if” and “memset()”, we can just use “winobj”.

4 - 0004
```
+		if (!HeapTupleIsValid(proctup))
+			elog(ERROR, "cache lookup failed for function %u", funcid);
+		procform = (Form_pg_proc) GETSTRUCT(proctup);
+		elog(ERROR, "function %s does not allow RESPECT/IGNORE NULLS",
+			 NameStr(procform->proname));
```

“Procform” is assigned but not used.

Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/

view thread (91+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
  Subject: Re: Add RESPECT/IGNORE NULLS and FROM FIRST/LAST options
  In-Reply-To: <[email protected]>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox