| 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 |
| 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) |