| From: | Alexandre Felipe <o(dot)alexandre(dot)felipe(at)gmail(dot)com> |
|---|---|
| To: | Zsolt Parragi <zsolt(dot)parragi(at)percona(dot)com> |
| Cc: | pgsql-hackers(at)lists(dot)postgresql(dot)org |
| Subject: | Re: SLOPE - Planner optimizations on monotonic expressions. |
| Date: | 2026-07-16 19:51:33 |
| Message-ID: | CAE8JnxP9nnjQBwaEMFk+vS85E7PuveEEPGos5r3S2d4qcCCsyA@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
> 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
On Fri, Jul 3, 2026 at 9:21 PM Zsolt Parragi <zsolt(dot)parragi(at)percona(dot)com>
wrote:
> Thanks for the updated version, I didn't find more correctness issues
> with the patch in v9.
>
> > Is there a written standard/recommendation for these, maybe
> > /*
> > * arg0_asc_slope_support
> > *
> > * Prosupport: f(x, ...) is monotonically increasing in x.
> > */
>
> I think most of the function comment's don't mention the function name
> at all, but those that do follow this approach with a blank line.
> At some places I also see a
>
> /* functionname: some description
> *
> * more description
> */
>
> pattern.
>
>
> 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?
>
> Another similar question is if the feature should have an enable_
> toggle similar to others, enable_slope?
>
>
>
| Attachment | Content-Type | Size |
|---|---|---|
| v10-0002-Optimized-reverse-pathkeys.patch | application/octet-stream | 6.3 KB |
| v10-0004-SLOPE-catalog-changes.patch | application/octet-stream | 95.8 KB |
| v10-0001-benchmark.patch | application/octet-stream | 4.8 KB |
| v10-0005-SLOPE-Planner-support.patch | application/octet-stream | 57.4 KB |
| v10-0003-Order-by-NOT-NULL.patch | application/octet-stream | 8.6 KB |
| v10-0006-SLOPE-documentation.patch | application/octet-stream | 7.5 KB |
| v10-0007-SLOPE-redundancy-checks.patch | application/octet-stream | 13.7 KB |
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Melanie Plageman | 2026-07-16 19:59:12 | Re: pg19b1: TRAP: failed Assert("pgstat_bktype_io_stats_valid(bktype_shstats, MyBackendType)") |
| Previous Message | Zsolt Parragi | 2026-07-16 19:50:57 | Re: pg_hosts: Add pg_hosts_file_rules() |