| 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-01 10:27:22 |
| Message-ID: | CAE8JnxOo0UGSzMf6Cw6bvSFv3E4XwhLvrwbmsJGU1_6d8mpguA@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
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.
Regards,
Alexandre
| Attachment | Content-Type | Size |
|---|---|---|
| tzbreak.sql | application/octet-stream | 6.6 KB |
| v15.3-0002-Optimized-reverse-pathkeys.patch | application/octet-stream | 7.1 KB |
| v15.3-0003-SLOPE-catalog-changes.patch | application/octet-stream | 94.6 KB |
| v15.3-0001-benchmark.patch | application/octet-stream | 4.8 KB |
| v15.3-0004-SLOPE-Planner-support.patch | application/octet-stream | 78.7 KB |
| v15.3-0005-SLOPE-documentation.patch | application/octet-stream | 8.3 KB |
| v15.3-0007-SLOPE-timezone-errsave.patch | application/octet-stream | 12.0 KB |
| v15.3-0006-SLOPE-Timezone.patch | application/octet-stream | 54.0 KB |
| v15.3-0008-SLOPE-plan-cache-guards.patch | application/octet-stream | 12.9 KB |
| From | Date | Subject | |
|---|---|---|---|
| Next Message | shveta malik | 2026-09-01 10:32:46 | Re: Proposal: Conflict log history table for Logical Replication |
| Previous Message | Antonin Houska | 2026-09-01 10:10:44 | Re: REPACK (CONCURRENTLY) fails when replica identity index is dropped |