public inbox for [email protected]  

help / color / mirror / Atom feed

From: Tatsuo Ishii <[email protected]>
To: [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: Wed, 17 Sep 2025 18:18:51 +0900 (JST)
Message-ID: <[email protected]> (raw)
In-Reply-To: <[email protected]>
References: <CAGMVOduZ+7=NR7zW5cLfSRghZAf0yzb3tRceKAXkfg+f-xAZog@mail.gmail.com>
	<[email protected]>
	<[email protected]>

> Overall LGTM. Just a few small comments:

> 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?

Good suggestion. Will fix.

> 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.

If we change the data type for ignore_nulls in WindowFunc, we may also
want to change it elsewhere (FuncCall, WindowObjectData,
WindowStatePerFuncData) for consistency?

> 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”.

Good catch. Will fix.

> 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.

I think procform is used in the following elog(ERROR, ...).

Best regards,
--
Tatsuo Ishii
SRA OSS K.K.
English: http://www.sraoss.co.jp/index_en/
Japanese:http://www.sraoss.co.jp

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