| From: | Alexandre Felipe <o(dot)alexandre(dot)felipe(at)gmail(dot)com> |
|---|---|
| To: | Corey Huinker <corey(dot)huinker(at)gmail(dot)com> |
| Cc: | pgsql-hackers(at)postgresql(dot)org |
| Subject: | Re: SLOPE - Planner optimizations on monotonic expressions. |
| Date: | 2026-03-25 14:47:58 |
| Message-ID: | CAE8JnxP=2_eOEMvoq0KHopUHfv=oRbesbL1p48qo-G853v525A@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
Thank you for your review Corey,
On Wed, Mar 25, 2026 at 5:18 AM Corey Huinker <corey(dot)huinker(at)gmail(dot)com>
wrote:
> +explain (costs off, verbose)
>> +select date_trunc('month', ts), count(*)
>> +from src
>> +group by 1;
>> + QUERY PLAN
>> +------------------------------------------------------
>> + GroupAggregate
>> + Output: (date_trunc('month'::text, ts)), count(*)
>> + Group Key: date_trunc('month'::text, src.ts)
>> + -> Index Only Scan using src_ts_idx on public.src
>> + Output: date_trunc('month'::text, ts)
>> +(5 rows)
>>
>
> That's a good one.
>
I am glad you noted, I waited for a while for this moment :)
> +/* Slope type for representing monotonicity */
> +typedef int8 Slope;
> +#define SLOPE_ANY 0 /* 0b00 - unknown/either (safe default) */
> +#define SLOPE_ASC 1 /* 0b01 - ascending (descending blocked) */
> +#define SLOPE_DESC 2 /* 0b10 - descending (ascending blocked) */
> +#define SLOPE_CONST 3 /* 0b11 - constant (both blocked) */
>
> The MonotonicFunction enum seems like a good pattern to follow here.
>
Learning about the existence of that now
typedef enum MonotonicFunction
{
MONOTONICFUNC_NONE = 0,
MONOTONICFUNC_INCREASING = (1 << 0),
MONOTONICFUNC_DECREASING = (1 << 1),
MONOTONICFUNC_BOTH = MONOTONICFUNC_INCREASING | MONOTONICFUNC_DECREASING,
} MonotonicFunction;
So, BOTH means that the function is both increasing and decreasing, thus it
is constant.
Simply replacing the SLOPE_* by the corresponding MONOTONICFUNC_* does the
job.
But now using an enum we will have 4 bytes per argument.
Nitpick: it's a slope sign (+/-) rather than a slope itself, which to me
> implies a scalar. I can't think of a good singular word for it, either.
>
Solved if we use MonotonicFunction I guess. But the word slope isn't always
used to refer to derivatives.
And here we are mostly dealing with functions that are constant or
discontinuous, i.e. they never have a non-zero real derivative.
+ * If the result is SLOPE_ASC or SLOPE_DESC, *underlying_expr is set to the
> + * by checking the slopes of the function arguments and the expression
> + * passed combined as follows:
>
> "is set to the by" - seems like you left out a word here.
>
rewording like this
* The contribution of each argument to the final slope of the function
* determined by the slope of the function with respect to an argument
* and the slope of the underlying expression expression passed to it
* as follows:
> Patches 0002-0003 would have to get committed at the same time, but I see
> why you separated them for clarity.
>
>
0002 is missing the catversion bump but that's fine at this early stage.
>
changed catversion this time
So, how would this work with a function like left() with a positive 2nd
> param (assuming specified collation matches the index)?
>
That would require a separate prosupport function, it would have to do some
extra work, checking if the second argument is a positive constant at
planning time.
I'm happy to do that if there is interest, but I would keep after this.
> I'm also curious if NULLS FIRST/LAST will throw this off.
>
All that this will do is, when considering index scans, check if the
requested order
matches the index order. And whether we have to do a backwards or forwards
scan.
+ if (mono_decreasing)
+ {
+ reverse_sort = !reverse_sort;
+ nulls_first = !nulls_first;
+ }
I think this covers all the cases under consideration.
Added more test cases at the end of 0003.
Is there a specialized nulls first/last sort or does it use a generic sort?
Regards,
Alexandre
| Attachment | Content-Type | Size |
|---|---|---|
| v3-0001-SLOPE-Analysis-Machinery.patch | application/octet-stream | 14.7 KB |
| v3-0003-SLOPE-Tests.patch | application/octet-stream | 28.1 KB |
| v3-0002-SLOPE-Builtin-support.patch | application/octet-stream | 53.3 KB |
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Yura Sokolov | 2026-03-25 14:51:30 | BM_IO_ERROR flag is lost in TerminateBufferIO due to order of operations in UnlockBufHdrExt |
| Previous Message | Matthias van de Meent | 2026-03-25 14:46:08 | Re: SQL-level pg_datum_image_equal |