Re: SLOPE - Planner optimizations on monotonic expressions. - Mailing list pgsql-hackers
| From | Alexandre Felipe |
|---|---|
| Subject | Re: SLOPE - Planner optimizations on monotonic expressions. |
| Date | |
| Msg-id | CAE8JnxOpFLb_=OLwG5ROgENJGZ_YD2rG2Kjv63d9zgjRjBsC4g@mail.gmail.com Whole thread |
| In response to | SLOPE - Planner optimizations on monotonic expressions. (Alexandre Felipe <o.alexandre.felipe@gmail.com>) |
| Responses |
Re: problems with toast.* reloptions
|
| List | pgsql-hackers |
Thank you for your thorough review Zsolt
On Tue, Jul 21, 2026 at 11:27 PM Zsolt Parragi <zsolt.parragi@percona.com> wrote:
Hello
> Removed prosupport for timestamptz <-> timestamp.
This is not fixed, same testcase still fails. Maybe because casts are
skipped unconditionally?
+ if ((fexpr->funcformat == COERCE_IMPLICIT_CAST ||
+ fexpr->funcformat == COERCE_EXPLICIT_CAST) &&
+ list_length(fexpr->args) == 1)
+ {
+ expr = (Expr *) linitial(fexpr->args);
+ continue;
+ }
And it seems like we have more issues with casts, there are some more
cases: timestamptz -> date (I mentioned this in my previous email),
timestamp -> time, int4 -> bool (-1 -> true, 0 -> false, 1 -> true)
Well explicit casts should not be there. I thought implicit casts would
be fine. But checking pg_cast with
SELECT
oid,
castsource::regtype, casttarget::regtype,
castfunc::regproc,
castcontext, castmethod
FROM pg_cast
oid,
castsource::regtype, casttarget::regtype,
castfunc::regproc,
castcontext, castmethod
FROM pg_cast
I see character varying => regclass, I didn't see any other such conversion.
Making it explicit now would break a few, if not many, DBA scripts.
*timestamptz -> date
I was optimistic here but you are right again. I checked tzinfo (using python
I was optimistic here but you are right again. I checked tzinfo (using python
and tzdump). From 596 time zones there are 7 for which the date moved
backward at some point.
1 America/Goose_Bay 00:00:59 -> 22:01:00, 1988
23 America/Goose_Bay 00:00:59 -> 23:01:00, 1987-2010
1 Canada/Newfoundland 00:00:59 -> 22:01:00, 1988
23 Canada/Newfoundland 00:00:59 -> 23:01:00, 1987-2010
1 America/St_Johns 00:00:59 -> 22:01:00, 1988
23 America/St_Johns 00:00:59 -> 23:01:00, 1987-2010
14 America/Moncton 00:00:59 -> 23:01:00, 1993-2006
1 Antarctica/Casey 01:59:59 -> 23:00:00, 2010-2010
1 Pacific/Guam 00:00:59 -> 23:01:00, 1969
1 Pacific/Saipan 00:00:59 -> 23:01:00, 1969
23 America/Goose_Bay 00:00:59 -> 23:01:00, 1987-2010
1 Canada/Newfoundland 00:00:59 -> 22:01:00, 1988
23 Canada/Newfoundland 00:00:59 -> 23:01:00, 1987-2010
1 America/St_Johns 00:00:59 -> 22:01:00, 1988
23 America/St_Johns 00:00:59 -> 23:01:00, 1987-2010
14 America/Moncton 00:00:59 -> 23:01:00, 1993-2006
1 Antarctica/Casey 01:59:59 -> 23:00:00, 2010-2010
1 Pacific/Guam 00:00:59 -> 23:01:00, 1969
1 Pacific/Saipan 00:00:59 -> 23:01:00, 1969
I rechecked another old issue, and realized that this was dropped
somewhere around v7, was that intentional?
> v5 aims to prevent the elimination of the sort node if the index has a
> custom sort operator family.
OK, that got lost in some refactoring, apparently I lost it squeezing
some commits on v7.2 and v7.3, the v7 and then the dropping the code
on v7.14 wasn't detected. What I submitted was v7.20, now I added that part
just after get_slope_wrt
I found one more problem with binary coercible types, the skip there
is too generic, it should be more restrictive (btree opfamilies
maybe?):
+ /* Skip RelabelType (no-op coercion) */
+ if (IsA(expr, RelabelType))
+ {
+ expr = (Expr *) ((RelabelType *) expr)->arg;
+ continue;
+ }
I think my assumptions (guesses) about RelabelType were completely
incorrect. From your feedback I thought of writing something like this
+ restype = getBaseType(((RelabelType *) expr)->resulttype);
+ expr = (Expr *) ((RelabelType *) expr)->arg;
+ if (argtype == restype)
+ continue;
+
+ arg_opclass = GetDefaultOpClass(argtype, BTREE_AM_OID);
+ res_opclass = GetDefaultOpClass(restype, BTREE_AM_OID);
+ if (arg_opclass == res_opclass)
+ continue;
+
+ if(OidIsValid(arg_opclass))
+ arg_opfamily = get_opclass_family(arg_opclass);
+
+ if(OidIsValid(res_opclass))
+ res_opfamily = get_opclass_family(res_opclass);
+
+ if (arg_opfamily == res_opfamily)
+ continue;
+
+ return MONOTONICFUNC_NONE;
But then, checking the catalog to see what could match
SELECT opfname, array_agg(oc.opcintype::regtype)
FROM pg_opclass oc
JOIN pg_am am ON am.oid = oc.opcmethod
JOIN pg_opfamily of ON of.oid = oc.opcfamily
WHERE oc.opcdefault
AND am.amname = 'btree'
GROUP BY 1
HAVING count(distinct oc.opcintype) > 1
FROM pg_opclass oc
JOIN pg_am am ON am.oid = oc.opcmethod
JOIN pg_opfamily of ON of.oid = oc.opcfamily
WHERE oc.opcdefault
AND am.amname = 'btree'
GROUP BY 1
HAVING count(distinct oc.opcintype) > 1
+------------+----------------------------+
|opfname |array_agg |
+------------+----------------------------+
|datetime_ops|{date,timestamp,timestamptz}|
|float_ops |{real,"double precision"} |
|integer_ops |{bigint,smallint,integer} |
|text_ops |{name,text} |
+------------+----------------------------+
|opfname |array_agg |
+------------+----------------------------+
|datetime_ops|{date,timestamp,timestamptz}|
|float_ops |{real,"double precision"} |
|integer_ops |{bigint,smallint,integer} |
|text_ops |{name,text} |
+------------+----------------------------+
Since we are scoping to default opfamilies I am stopping at
RelabelTypes, declaring it non-monotonic.
+ /*
+ * Case 2: f(x) after x — ascending chain. x is already
+ * in retval, so within each group of equal x values, f(x)
+ * is constant (for any deterministic f). The pathkey is
+ * redundant as a tiebreaker regardless of monotonicity.
+ */
+ if (!pathkey_is_redundant(qpk, retval))
+ retval = lappend(retval, qpk);
+ continue;
"within each group of equal x values, f(x) is constant (for any
deterministic f)" -- doesn't this also require equalimage?
Yes it is required, thank you for educating me about that, I wasn't aware
of this feature. x, f(x) pattern is very broad, and matches things beyond
intended.
There are also a few typos:
disables
Fixed.
takes
Fixed
inferred
Fixed
x1 and x2
Fixed
+ * 'nslopes' points to a MonotonicFunction array (one per argument up to
+ * nslopes). Arguments beyond nslopes are treated as MONOTONICFUNC_NONE.
slopes points to
Fixed
+
+typedef enum NUMERIC_SIGN
+{
Also nitpick, but numeric.c has a define with the same name. There's
no conflict as it's in a different file and that's a macro, but could
make searching more difficult.
Renamed to SLOPE_NUMERIC.
I also merged the 'redundancy checks' patch in the 'planner support' patch
and I changed that to use chasing pointers instead of nested loops.
doing (max(index columns, query pathkeys)) iterations instead
of ((index columns) * (query pathkeys).
I have applying and cleaning my work tree so many times that is better
to submit this before I lose something important :)
Regards,
Alexandre
Attachment
pgsql-hackers by date: