Re: SLOPE - Planner optimizations on monotonic expressions.

From: Alexandre Felipe <o(dot)alexandre(dot)felipe(at)gmail(dot)com>
To: Zsolt Parragi <zsolt(dot)parragi(at)percona(dot)com>
Cc: Corey Huinker <corey(dot)huinker(at)gmail(dot)com>, pgsql-hackers(at)postgresql(dot)org
Subject: Re: SLOPE - Planner optimizations on monotonic expressions.
Date: 2026-03-25 23:34:40
Message-ID: CAE8JnxMq5mrfPAUfc_WwTozd622+SkbcLY8hHGAtqEUE4OZpkw@mail.gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Wed, Mar 25, 2026 at 8:57 PM Zsolt Parragi <zsolt(dot)parragi(at)percona(dot)com>
wrote:

> Hello
>
> - proname => 'time_pl_interval', prorettype => 'time',
> - proargtypes => 'time interval', prosrc => 'time_pl_interval' },
> + proname => 'time_pl_interval', prosupport => 'addition_slope_support',
> + prorettype => 'time', proargtypes => 'time interval',
> + prosrc => 'time_pl_interval' },
>
> I think this is incorrect, time can wrap around and isn't monotonic.
>
> CREATE TABLE time_wrap_test (t time PRIMARY KEY);
> INSERT INTO time_wrap_test VALUES
> ('20:00'), ('21:00'), ('22:00'), ('23:00'), ('23:30'),
> ('00:00'), ('01:00'), ('02:00'), ('03:00'), ('04:00');
> SELECT t, t + interval '3 hours' AS t_plus_3h
> FROM time_wrap_test
> ORDER BY t + interval '3 hours';
>

Good catch,
Can you think of any other type that would wrap,
int2, int4, int8, money, float4, float8, date, timestamp, timestamptz,
interval
all raise some error.
time and timetz wrap around.

@@ -4820,21 +4820,17 @@
prosrc => 'width_bucket_numeric' },

{ oid => '1747',
- proname => 'time_pl_interval', prosupport => 'addition_slope_support',
- prorettype => 'time', proargtypes => 'time interval',
- prosrc => 'time_pl_interval' },
+ proname => 'time_pl_interval', prorettype => 'time',
+ proargtypes => 'time interval', prosrc => 'time_pl_interval' },
{ oid => '1748',
- proname => 'time_mi_interval', prosupport => 'diff_slope_support',
- prorettype => 'time', proargtypes => 'time interval',
- prosrc => 'time_mi_interval' },
+ proname => 'time_mi_interval', prorettype => 'time',
+ proargtypes => 'time interval', prosrc => 'time_mi_interval' },
{ oid => '1749',
- proname => 'timetz_pl_interval', prosupport => 'addition_slope_support',
- prorettype => 'timetz', proargtypes => 'timetz interval',
- prosrc => 'timetz_pl_interval' },
+ proname => 'timetz_pl_interval', prorettype => 'timetz',
+ proargtypes => 'timetz interval', prosrc => 'timetz_pl_interval' },
{ oid => '1750',
- proname => 'timetz_mi_interval', prosupport => 'diff_slope_support',
- prorettype => 'timetz', proargtypes => 'timetz interval',
- prosrc => 'timetz_mi_interval' },
+ proname => 'timetz_mi_interval', prorettype => 'timetz',
+ proargtypes => 'timetz interval', prosrc => 'timetz_mi_interval' },

{ oid => '1764', descr => 'increment by one',
proname => 'numeric_inc', prorettype => 'numeric', proargtypes =>
'numeric',

> + /* Check each index on this relation */
> + foreach(lc, rel->indexlist)
> + {
> + IndexOptInfo *index = (IndexOptInfo *) lfirst(lc);
>
> Isn't a sortopfamily check missing from this?
>

You are probably right but I am confused here.
We could simply skip this analysis entirely if the index has a custom
operator family.
Do you think this would be enough?

> Added one line to .gitignore file because I like to keep
> > data related to this in the project workarea, e.g.
> > ".install", ".dbdata", ".patches", and I don't want those
>
> +
> +# ignore hidden files
> +.*
>
> This doesn't seem related to the patch.
>

Yes, it is not specific for this patch, just annoying that it doesn't
ignore hidden files.

PS.
The v3 patch failed on the CI checks, I was testing without assertions.
The problem was that v2-0001 had a
+ sortkey = mono_var;
modifying the sort key from the query, and this caused an assertion to fail.
v4 keeps the sort key untouched but has checks
ec_member_is_monotonic_in
in both pathkey_is_monotonic_of to determine if the available order is
useful,
and again in build_index_pathkeys that creates a trivial equivalence class
to be used in pathkey_is_monotonic_of with the index key if it doesn't
belong to one already.
Maybe this can be improved later, but it is likely better than a broken
build.

Regards,
Alexandre

Attachment Content-Type Size
v4-0001-SLOPE-Analysis-Machinery.patch application/octet-stream 19.2 KB
v4-0002-SLOPE-Builtin-support.patch application/octet-stream 51.9 KB
v4-0003-SLOPE-Tests.patch application/octet-stream 28.2 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2026-03-25 23:40:47 Re: Add uuid_to_base32hex() and base32hex_to_uuid() built-in functions
Previous Message Tomas Vondra 2026-03-25 23:29:03 Re: eliminate xl_heap_visible to reduce WAL (and eventually set VM on-access)