| 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-09-14 21:10:01 |
| Message-ID: | CAE8JnxO9DGuxS-pP5o4iK5+Z0=DqdVXx_fTyRfSzAeMe1orPNw@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
v15.4 - Rebasing
On Tue, Sep 1, 2026 at 11:27 AM Alexandre Felipe <
o(dot)alexandre(dot)felipe(at)gmail(dot)com> wrote:
>
> Thank you again for your review Zsolt,
>
>
> On Wed, Aug 26, 2026 at 11:30 PM Zsolt Parragi <zsolt(dot)parragi(at)percona(dot)com>
> wrote:
>
>> > I had previously dropped the support to the timezone dependent
>> functions.
>> > This patchset has one additional patch where that is fixed.
>>
>> I focused on mostly this, and found a few issues in it:
>>
>> 1: session_timezone is read at plan time, so cached plans can go stale
>> and return wrong results
>>
>> SET enable_seqscan = off;
>> SET enable_bitmapscan = off;
>> CREATE TABLE pc(t timestamptz);
>> INSERT INTO pc VALUES ('2025-11-02 05:59:59.999999+00'), ('2025-11-02
>> 06:00:00+00');
>> CREATE INDEX ON pc(t);
>> PREPARE q AS SELECT t AT LOCAL AS l FROM pc ORDER BY 1;
>> EXECUTE q; -- generic plan, built under
>> Etc/UTC
>> SET TimeZone = 'America/New_York';
>> EXECUTE q; -- 01:59:59.999999 then 01:00:00
>> SELECT t AT LOCAL AS l FROM pc ORDER BY 1; -- same query, fresh plan:
>> correct
>>
>
> 0008 - "plan-cache guards" added a flag CURSOR_OPT_ONESHOT to
> cursorOptions
> that is saved as a field to PlannerGlobal, and added a field PlannerInfo
> *root to
> SupportMonotonicRequest. This way monotonic request functions can see
> information
> from the plan being created, and that in turn allows distinguishing
> whether the plan
> is going to be cached or not (not sure if I covered all the cases here).
> If !is_oneshot
> declares no monotonicity for the function forcing the plan to emit a sort
> node.
>
> 2: timezone(text, timestamp) isn't monotonic
>>
>> SET enable_seqscan = off;
>> SET enable_bitmapscan = off;
>> CREATE TABLE gap(ts timestamp);
>> INSERT INTO gap VALUES ('2026-03-08 01:59'), ('2026-03-08 02:30'),
>> ('2026-03-08 03:00');
>> CREATE INDEX ON gap(ts);
>> EXPLAIN (COSTS OFF) SELECT timezone('America/New_York', ts) FROM gap
>> ORDER BY 1;
>> SELECT timezone('America/New_York', ts) FROM gap ORDER BY 1;
>> SET enable_slope = off;
>> SELECT timezone('America/New_York', ts) FROM gap ORDER BY 1;
>>
>
> 0006 - "Timezone" implemented two monotonicity masks, one for
> UTC to TZ and one from TZ to UTC. Kept the plan-based report on
> slope.sql, and I am attaching tzbreak.sql, an attempt to reveal
> monotonicity
> violations with data.
>
> sort required | has sort | n
> ---------------+----------+-------
> maybe | f | 10129 <- monotonic, not disproven
> maybe | t | 2483 <- non-monotonic, not proven
> yes | f | 2 <- known issue [1]
> yes | t | 2934 <- proven non-monotonic
>
> The second row (maybe, t) indicates where the planner produced a sort node,
> but no example of non-monotonicity was found on the queries.
>
>
> 3: unrecognized zones results in an error during planning
>>
>> CREATE TABLE e(t timestamptz);
>> CREATE INDEX ON e(t);
>> SELECT date_trunc('day', t, 'Bogus/Zone') FROM e ORDER BY 1; --
>> ERROR: time zone "Bogus/Zone" not recognized
>>
>
> 0007 - "timezone errsave" exposed an ErrorSaveContext that can be
> passed to capture the errors instead of omitting, and updated the
> call sites to pass an extra NULL.
>
>
>> 4: inspect_monotonicity has a corner-case issue with date_trunc which
>> returns a timestamptz
>
>
> Submitted a separate patch for that [1], as in my understanding is a bug
> in date_trunc.
>
>
>> There's also an uninitialized variable warning in 0002:
>> - index_pathkeys = build_index_pathkeys(root, index,
>> -
>> BackwardScanDirection);
>>
>
> That was not incorrect but was also not obviously correct.
>
> The variable was initialized under
> + if (index_is_ordered && pathkeys_possibly_useful)
> and used under
> + if (index_is_ordered && pathkeys_possibly_useful && index_pathkeys !=
> NIL)
>
> That initialisation was selectively initializing variables under the "if".
> I simplified by initializing before the "if", it would be more elegant to
> initialise at the top but, arguably, that would be a few unnecessary
> assignments if the code returns before these variables are touched.
>
>
> [1]
> https://www.postgresql.org/message-id/flat/CAE8JnxMWMqMUfz0QruiCeHJifm55--LaBqW6-apeNJUMrJ7oig(at)mail(dot)gmail(dot)com
>
>
> Regards,
> Alexandre
>
>
| Attachment | Content-Type | Size |
|---|---|---|
| v15.4-0004-SLOPE-Planner-support.patch | application/octet-stream | 78.6 KB |
| v15.4-0002-Optimized-reverse-pathkeys.patch | application/octet-stream | 7.1 KB |
| v15.4-0001-benchmark.patch | application/octet-stream | 4.8 KB |
| v15.4-0005-SLOPE-documentation.patch | application/octet-stream | 8.3 KB |
| v15.4-0003-SLOPE-catalog-changes.patch | application/octet-stream | 94.6 KB |
| v15.4-0006-SLOPE-Timezone.patch | application/octet-stream | 54.0 KB |
| v15.4-0007-SLOPE-timezone-errsave.patch | application/octet-stream | 12.0 KB |
| v15.4-0008-SLOPE-plan-cache-guards.patch | application/octet-stream | 12.9 KB |
| From | Date | Subject | |
|---|---|---|---|
| Next Message | shihao zhong | 2026-09-14 21:48:07 | Re: Add TOAST statistics columns to pg_stat_all_tables |
| Previous Message | Masahiko Sawada | 2026-09-14 21:00:31 | Re: Optimize UUID parse using SIMD |