> The patchset also has many small formatting issues/inconsistencies,
> maybe it would be worth to run pgindent on it?
Ran pgindent on each commit, the only thing I didn't accept was some
extra spaces before typedev names that didn't seem right.
> CREATE OR REPLACE FUNCTION log10(numeric)
> RETURNS numeric
> LANGUAGE sql
> + SUPPORT arg0_asc_slope_support
> IMMUTABLE PARALLEL SAFE STRICT COST 1
> RETURN log(10, $1);
>
> Shouldn't these additions, and generally the feature itself / how it
> works / what's the contract for functions / ... have proper
> documentation as it is user/extension facing?
I tried to improve `doc/src/sgml/xfunc.sgml` that says
---
Planner support functions have to be
written in C (although their target functions might not be), so this is
an advanced feature that relatively few people will use.
---
The same file has the documentation of some (apparently the monotonic
window function was not mentioned there). I tried to improve that a bit
by mechanically breaking it in sections adding some index terms.
At the end of the file I added a section for monotonic functions.
I couldn't find mentions of the existing support functions in the
documentation, so I am not mentioning the ones introduced here.
I moved some of the files around, implemented the nan check on
0005 Planner support.
Additionally,
0003 lets the planner ignore NULLS FIRST or NULLS LAST in a pathkey
if the target expression is known to be NOT NULL. This change is is
independent of SLOPE implementation.
I also noticed that grouping operations, such as GROUP BY, DISTINCT
or PARTITION BY, require ASC NULLS LAST, it also accepts
DESC NULLS FIRST doing a backward scan. But it will never accept
DESC NULLS LAST or ASC NULLS FIRST, this is why grouping
by a decreasing function of an index is not possible. But this seems to
touch much more code and a solution would either require comparing
the pathkey fields instead of the pointer comparison, or doubling the
search complexity by considering the unsupported variations.
Regards,
Alexandre
