Re: Add RESPECT/IGNORE NULLS and FROM FIRST/LAST options

> 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