Обсуждение: BUG #17066: Cache lookup failed when null (iso-8859-1) is passed as anycompatiblemultirange

Поиск
Список
Период
Сортировка

BUG #17066: Cache lookup failed when null (iso-8859-1) is passed as anycompatiblemultirange

От
PG Bug reporting form
Дата:
The following bug has been logged on the website:

Bug reference:      17066
Logged by:          Alexander Lakhin
Email address:      exclusion@gmail.com
PostgreSQL version: 14beta1
Operating system:   Ubuntu 20.04
Description:

When calling the simple test function:
create function multirange_test(a anycompatiblemultirange)
  returns anycompatible as 'select lower($1);' language sql;

with the NULL argument:
SELECT multirange_test(null);

I get a somewhat unusual error:
ERROR:  cache lookup failed for type 0

The variation with the anycompatiblerange (without multi):
create function range_test(a anycompatiblerange)
  returns anycompatible as 'select lower($1);' language sql;
SELECT range_test(null);

produces:
ERROR:  could not determine polymorphic type anycompatiblerange because
input has type unknown


Greetings,

I tried to investigate the bug, but the complicated logic here completely messed up my mind...

Anyway, this patch can fix it and make the regress test happy. But I think it's better to get the author's advice - I copied this email to Alvaro, hope it doesn't offend him...


--
There is no royal road to learning.
HighGo Software Co.
Вложения

Re: BUG #17066: Cache lookup failed when null (iso-8859-1) is passed as anycompatiblemultirange

От
Alvaro Herrera
Дата:
On 2021-Jun-22, Neil Chen wrote:

> Greetings,
> 
> I tried to investigate the bug, but the complicated logic here completely
> messed up my mind...
> 
> Anyway, this patch can fix it and make the regress test happy. But I think
> it's better to get the author's advice - I copied this email to Alvaro,
> hope it doesn't offend him...

Without looking too deeply, the patch seems sensible to me.  However,
I'm CC'ing Alexander and Paul :-)

-- 
Álvaro Herrera       Valdivia, Chile



Re: BUG #17066: Cache lookup failed when null (iso-8859-1) is passed as anycompatiblemultirange

От
Alexander Korotkov
Дата:
On Tue, Jun 22, 2021 at 4:42 PM Alvaro Herrera <alvherre@2ndquadrant.com> wrote:
> On 2021-Jun-22, Neil Chen wrote:
>
> > Greetings,
> >
> > I tried to investigate the bug, but the complicated logic here completely
> > messed up my mind...
> >
> > Anyway, this patch can fix it and make the regress test happy. But I think
> > it's better to get the author's advice - I copied this email to Alvaro,
> > hope it doesn't offend him...
>
> Without looking too deeply, the patch seems sensible to me.  However,
> I'm CC'ing Alexander and Paul :-)

Thank you for pointing this out.  And thanks to Neil for the fix.

Looks good at the first glance.  I'm going to review this in the next
couple of days.

------
Regards,
Alexander Korotkov



Re: BUG #17066: Cache lookup failed when null (iso-8859-1) is passed as anycompatiblemultirange

От
Alexander Korotkov
Дата:
On Tue, Jun 22, 2021 at 7:01 PM Alexander Korotkov <aekorotkov@gmail.com> wrote:
> On Tue, Jun 22, 2021 at 4:42 PM Alvaro Herrera <alvherre@2ndquadrant.com> wrote:
> > On 2021-Jun-22, Neil Chen wrote:
> >
> > > Greetings,
> > >
> > > I tried to investigate the bug, but the complicated logic here completely
> > > messed up my mind...
> > >
> > > Anyway, this patch can fix it and make the regress test happy. But I think
> > > it's better to get the author's advice - I copied this email to Alvaro,
> > > hope it doesn't offend him...
> >
> > Without looking too deeply, the patch seems sensible to me.  However,
> > I'm CC'ing Alexander and Paul :-)
>
> Thank you for pointing this out.  And thanks to Neil for the fix.
>
> Looks good at the first glance.  I'm going to review this in the next
> couple of days.

I've reviewed enforce_generic_type_consistency() more carefully.  It
looks for me that this function requires more significant rework.  For
instance, these two functions throw errors, but it seems that both of
them should work.

create function multirange_func(r anycompatiblerange)
  returns anycompatiblemultirange as 'select multirange($1);' language sql;
create function range_func(r anycompatiblemultirange)
  returns anycompatiblerange as 'select multirange($1);' language sql;

# select multirange_func(int4range(1,10));
ERROR:  could not identify anycompatiblemultirange type

# select range_func(int4multirange(int4range(1,10)));
ERROR:  could not determine polymorphic type anycompatiblerange
because input has type unknown

So, I'm still investigating this.

------
Regards,
Alexander Korotkov



Re: BUG #17066: Cache lookup failed when null (iso-8859-1) is passed as anycompatiblemultirange

От
Alexander Korotkov
Дата:
On Wed, Jun 23, 2021 at 6:31 PM Alexander Korotkov <aekorotkov@gmail.com> wrote:
> On Tue, Jun 22, 2021 at 7:01 PM Alexander Korotkov <aekorotkov@gmail.com> wrote:
> > On Tue, Jun 22, 2021 at 4:42 PM Alvaro Herrera <alvherre@2ndquadrant.com> wrote:
> > > On 2021-Jun-22, Neil Chen wrote:
> > >
> > > > Greetings,
> > > >
> > > > I tried to investigate the bug, but the complicated logic here completely
> > > > messed up my mind...
> > > >
> > > > Anyway, this patch can fix it and make the regress test happy. But I think
> > > > it's better to get the author's advice - I copied this email to Alvaro,
> > > > hope it doesn't offend him...
> > >
> > > Without looking too deeply, the patch seems sensible to me.  However,
> > > I'm CC'ing Alexander and Paul :-)
> >
> > Thank you for pointing this out.  And thanks to Neil for the fix.
> >
> > Looks good at the first glance.  I'm going to review this in the next
> > couple of days.
>
> I've reviewed enforce_generic_type_consistency() more carefully.  It
> looks for me that this function requires more significant rework.  For
> instance, these two functions throw errors, but it seems that both of
> them should work.
>
> create function multirange_func(r anycompatiblerange)
>   returns anycompatiblemultirange as 'select multirange($1);' language sql;
> create function range_func(r anycompatiblemultirange)
>   returns anycompatiblerange as 'select multirange($1);' language sql;
>
> # select multirange_func(int4range(1,10));
> ERROR:  could not identify anycompatiblemultirange type
>
> # select range_func(int4multirange(int4range(1,10)));
> ERROR:  could not determine polymorphic type anycompatiblerange
> because input has type unknown
>
> So, I'm still investigating this.

The second function was intended to be this to be a meaningful
example.  But anyway it fails with the same error.

create function range_func(r anycompatiblemultirange)
  returns anycompatiblerange as 'select range_merge($1);' language sql;

------
Regards,
Alexander Korotkov



Re: BUG #17066: Cache lookup failed when null (iso-8859-1) is passed as anycompatiblemultirange

От
Alexander Korotkov
Дата:
On Thu, Jun 24, 2021 at 1:41 AM Alexander Korotkov <aekorotkov@gmail.com> wrote:
> On Wed, Jun 23, 2021 at 6:31 PM Alexander Korotkov <aekorotkov@gmail.com> wrote:
> > On Tue, Jun 22, 2021 at 7:01 PM Alexander Korotkov <aekorotkov@gmail.com> wrote:
> > > On Tue, Jun 22, 2021 at 4:42 PM Alvaro Herrera <alvherre@2ndquadrant.com> wrote:
> > > > On 2021-Jun-22, Neil Chen wrote:
> > > >
> > > > > Greetings,
> > > > >
> > > > > I tried to investigate the bug, but the complicated logic here completely
> > > > > messed up my mind...
> > > > >
> > > > > Anyway, this patch can fix it and make the regress test happy. But I think
> > > > > it's better to get the author's advice - I copied this email to Alvaro,
> > > > > hope it doesn't offend him...
> > > >
> > > > Without looking too deeply, the patch seems sensible to me.  However,
> > > > I'm CC'ing Alexander and Paul :-)
> > >
> > > Thank you for pointing this out.  And thanks to Neil for the fix.
> > >
> > > Looks good at the first glance.  I'm going to review this in the next
> > > couple of days.
> >
> > I've reviewed enforce_generic_type_consistency() more carefully.  It
> > looks for me that this function requires more significant rework.  For
> > instance, these two functions throw errors, but it seems that both of
> > them should work.
> >
> > create function multirange_func(r anycompatiblerange)
> >   returns anycompatiblemultirange as 'select multirange($1);' language sql;
> > create function range_func(r anycompatiblemultirange)
> >   returns anycompatiblerange as 'select multirange($1);' language sql;
> >
> > # select multirange_func(int4range(1,10));
> > ERROR:  could not identify anycompatiblemultirange type
> >
> > # select range_func(int4multirange(int4range(1,10)));
> > ERROR:  could not determine polymorphic type anycompatiblerange
> > because input has type unknown
> >
> > So, I'm still investigating this.
>
> The second function was intended to be this to be a meaningful
> example.  But anyway it fails with the same error.
>
> create function range_func(r anycompatiblemultirange)
>   returns anycompatiblerange as 'select range_merge($1);' language sql;

I've a bit stuck in understanding of
enforce_generic_type_consistency().  There is a set of "argument
declared..." errors.  But they aren't present in regression tests.
And I didn't manage to reproduce them, because a function is filtered
out before entering enforce_generic_type_consistency() and I get just
"No function matches ..." error.

Are "argument declared..." errors just internal, which users shouldn't
normally see.  Or do they happen in some circumstances?  If latter,
then I think it should be added to the regression tests.

I really appreciate a hint here.

------
Regards,
Alexander Korotkov



Alexander Korotkov <aekorotkov@gmail.com> writes:
> I really appreciate a hint here.

I think I'm to blame for most of that code originally, so I'll take
a look soon.  Been up to my neck in other stuff recently.

            regards, tom lane



Re: BUG #17066: Cache lookup failed when null (iso-8859-1) is passed as anycompatiblemultirange

От
Alexander Lakhin
Дата:
Hello,
24.06.2021 02:35, Tom Lane wrote:
> Alexander Korotkov <aekorotkov@gmail.com> writes:
>> I really appreciate a hint here.
> I think I'm to blame for most of that code originally, so I'll take
> a look soon.  Been up to my neck in other stuff recently.            
I'm not sure whether it related to the initial issue, but there is
another anomaly with the multirange types. (May be I should report it as
a distinct bug?) The query:
create table test_multirange(mr int4multirange);
select count(*) from test_multirange where mr << int4range(100,100);
produces:
ERROR:  unexpected operator 4396

while
select count(*) from test_multirange where mr << int4range(100,500);
returns a result (as the multirangetypes test shows).

Best regards,
Alexander



Re: BUG #17066: Cache lookup failed when null (iso-8859-1) is passed as anycompatiblemultirange

От
Alexander Korotkov
Дата:
On Fri, Jun 25, 2021 at 7:00 AM Alexander Lakhin <exclusion@gmail.com> wrote:
> Hello,
> 24.06.2021 02:35, Tom Lane wrote:
> > Alexander Korotkov <aekorotkov@gmail.com> writes:
> >> I really appreciate a hint here.
> > I think I'm to blame for most of that code originally, so I'll take
> > a look soon.  Been up to my neck in other stuff recently.
> I'm not sure whether it related to the initial issue, but there is
> another anomaly with the multirange types. (May be I should report it as
> a distinct bug?) The query:
> create table test_multirange(mr int4multirange);
> select count(*) from test_multirange where mr << int4range(100,100);
> produces:
> ERROR:  unexpected operator 4396
>
> while
> select count(*) from test_multirange where mr << int4range(100,500);
> returns a result (as the multirangetypes test shows).

Yep, that's a distinct bug.  It seems that I've added some missing
operators, but forgot to add some selectivity estimates for them.

I'll come with a fix soon.

------
Regards,
Alexander Korotkov



Re: BUG #17066: Cache lookup failed when null (iso-8859-1) is passed as anycompatiblemultirange

От
Alexander Korotkov
Дата:
 On Sat, Jun 26, 2021 at 12:02 AM Alexander Korotkov
<aekorotkov@gmail.com> wrote:
> On Fri, Jun 25, 2021 at 7:00 AM Alexander Lakhin <exclusion@gmail.com> wrote:
> > Hello,
> > 24.06.2021 02:35, Tom Lane wrote:
> > > Alexander Korotkov <aekorotkov@gmail.com> writes:
> > >> I really appreciate a hint here.
> > > I think I'm to blame for most of that code originally, so I'll take
> > > a look soon.  Been up to my neck in other stuff recently.
> > I'm not sure whether it related to the initial issue, but there is
> > another anomaly with the multirange types. (May be I should report it as
> > a distinct bug?) The query:
> > create table test_multirange(mr int4multirange);
> > select count(*) from test_multirange where mr << int4range(100,100);
> > produces:
> > ERROR:  unexpected operator 4396
> >
> > while
> > select count(*) from test_multirange where mr << int4range(100,500);
> > returns a result (as the multirangetypes test shows).
>
> Yep, that's a distinct bug.  It seems that I've added some missing
> operators, but forgot to add some selectivity estimates for them.
>
> I'll come with a fix soon.

The patch is attached.  It fixes some switches in calc_multirangesel()
and calc_multirangesel().  Additionally, it improves regression test
coverage for matching empty range/multirange.

I'm going to push it if no objection.

------
Regards,
Alexander Korotkov

Вложения

Re: BUG #17066: Cache lookup failed when null (iso-8859-1) is passed as anycompatiblemultirange

От
Alexander Korotkov
Дата:
On Sun, Jun 27, 2021 at 2:29 AM Alexander Korotkov <aekorotkov@gmail.com> wrote:
>  On Sat, Jun 26, 2021 at 12:02 AM Alexander Korotkov
> <aekorotkov@gmail.com> wrote:
> > On Fri, Jun 25, 2021 at 7:00 AM Alexander Lakhin <exclusion@gmail.com> wrote:
> > > Hello,
> > > 24.06.2021 02:35, Tom Lane wrote:
> > > > Alexander Korotkov <aekorotkov@gmail.com> writes:
> > > >> I really appreciate a hint here.
> > > > I think I'm to blame for most of that code originally, so I'll take
> > > > a look soon.  Been up to my neck in other stuff recently.
> > > I'm not sure whether it related to the initial issue, but there is
> > > another anomaly with the multirange types. (May be I should report it as
> > > a distinct bug?) The query:
> > > create table test_multirange(mr int4multirange);
> > > select count(*) from test_multirange where mr << int4range(100,100);
> > > produces:
> > > ERROR:  unexpected operator 4396
> > >
> > > while
> > > select count(*) from test_multirange where mr << int4range(100,500);
> > > returns a result (as the multirangetypes test shows).
> >
> > Yep, that's a distinct bug.  It seems that I've added some missing
> > operators, but forgot to add some selectivity estimates for them.
> >
> > I'll come with a fix soon.
>
> The patch is attached.  It fixes some switches in calc_multirangesel()
> and calc_multirangesel().  Additionally, it improves regression test
> coverage for matching empty range/multirange.
>
> I'm going to push it if no objection.

Pushed!

------
Regards,
Alexander Korotkov



Re: BUG #17066: Cache lookup failed when null (iso-8859-1) is passed as anycompatiblemultirange

От
Alexander Korotkov
Дата:
On Thu, Jun 24, 2021 at 2:35 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>
> Alexander Korotkov <aekorotkov@gmail.com> writes:
> > I really appreciate a hint here.
>
> I think I'm to blame for most of that code originally, so I'll take
> a look soon.  Been up to my neck in other stuff recently.

Do I understand correctly that enforce_generic_type_consistency() is
called only after check_generic_type_consistency() returned true?

If so, that means some of the checks are redundant.  Therefore, we can
replace ereport()'s with Assert()'s.

------
Regards,
Alexander Korotkov



Re: BUG #17066: Cache lookup failed when null (iso-8859-1) is passed as anycompatiblemultirange

От
Alexander Korotkov
Дата:
On Wed, Jun 30, 2021 at 1:06 AM Alexander Korotkov <aekorotkov@gmail.com> wrote:
> On Thu, Jun 24, 2021 at 2:35 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> >
> > Alexander Korotkov <aekorotkov@gmail.com> writes:
> > > I really appreciate a hint here.
> >
> > I think I'm to blame for most of that code originally, so I'll take
> > a look soon.  Been up to my neck in other stuff recently.
>
> Do I understand correctly that enforce_generic_type_consistency() is
> called only after check_generic_type_consistency() returned true?
>
> If so, that means some of the checks are redundant.  Therefore, we can
> replace ereport()'s with Assert()'s.

I got no feedback on this yet.  And 14beta3 is approaching...

I'm going to write a patch fixing enforce_generic_type_consistency()
while saving its general logic (as far as I can understand it).

------
Regards,
Alexander Korotkov



Alexander Korotkov <aekorotkov@gmail.com> writes:
> Do I understand correctly that enforce_generic_type_consistency() is
> called only after check_generic_type_consistency() returned true?
> If so, that means some of the checks are redundant.  Therefore, we can
> replace ereport()'s with Assert()'s.

They are not redundant, IIRC.  I forget the exact mechanism for
reaching them, but it likely has something to do with aggregates
or variadic functions.

In any case, apologies for taking so long to get back to this.  Here's
a proposed patch (based in part on Neil's earlier patch).

            regards, tom lane

diff --git a/src/backend/parser/parse_coerce.c b/src/backend/parser/parse_coerce.c
index 7e963b8895..199966f8a6 100644
--- a/src/backend/parser/parse_coerce.c
+++ b/src/backend/parser/parse_coerce.c
@@ -1579,8 +1579,10 @@ select_common_typmod(ParseState *pstate, List *exprs, Oid common_type)
  * 1) All arguments declared ANYELEMENT must have the same datatype.
  * 2) All arguments declared ANYARRAY must have the same datatype,
  *      which must be a varlena array type.
- * 3) All arguments declared ANYRANGE or ANYMULTIRANGE must be a range or
- *      multirange type, all derived from the same base datatype.
+ * 3) All arguments declared ANYRANGE must be the same range type.
+ *      Similarly, all arguments declared ANYMULTIRANGE must be the same
+ *      multirange type; and if both of these appear, the ANYRANGE type
+ *      must be the element type of the ANYMULTIRANGE type.
  * 4) If there are arguments of more than one of these polymorphic types,
  *      the array element type and/or range subtype must be the same as each
  *      other and the same as the ANYELEMENT type.
@@ -1605,9 +1607,11 @@ select_common_typmod(ParseState *pstate, List *exprs, Oid common_type)
  *      let us choose one over another.  Furthermore, that range's subtype
  *      must exactly match the common supertype chosen by rule 7.
  * 10) All ANYCOMPATIBLEMULTIRANGE arguments must be the exact same multirange
- *      type (after domain flattening), since we have no preference rule that would
- *      let us choose one over another.  Furthermore, that multirange's range's
- *      subtype must exactly match the common supertype chosen by rule 7.
+ *      type (after domain flattening), since we have no preference rule that
+ *      would let us choose one over another.  Furthermore, if ANYCOMPATIBLERANGE
+ *      also appears, that range type must be the multirange's element type;
+ *      otherwise, the multirange's range's subtype must exactly match the
+ *      common supertype chosen by rule 7.
  *
  * Domains over arrays match ANYARRAY, and are immediately flattened to their
  * base type.  (Thus, for example, we will consider it a match if one ANYARRAY
@@ -1616,9 +1620,9 @@ select_common_typmod(ParseState *pstate, List *exprs, Oid common_type)
  * for ANYCOMPATIBLEARRAY and ANYCOMPATIBLENONARRAY.
  *
  * Similarly, domains over ranges match ANYRANGE or ANYCOMPATIBLERANGE,
- * and are immediately flattened to their base type, and domains over
- * multiranges match ANYMULTIRANGE or ANYCOMPATIBLEMULTIRANGE and are immediately
- * flattened to their base type.
+ * and are immediately flattened to their base type.  Likewise, domains
+ * over multiranges match ANYMULTIRANGE or ANYCOMPATIBLEMULTIRANGE and are
+ * immediately flattened to their base type.
  *
  * Note that domains aren't currently considered to match ANYENUM,
  * even if their base type would match.
@@ -1760,26 +1764,7 @@ check_generic_type_consistency(const Oid *actual_arg_types,
                 anycompatible_multirange_typelem = get_multirange_range(actual_type);
                 if (!OidIsValid(anycompatible_multirange_typelem))
                     return false;    /* not a multirange type */
-
-                if (OidIsValid(anycompatible_range_typeid))
-                {
-                    /*
-                     * ANYCOMPATIBLEMULTIRANGE and ANYCOMPATIBLERANGE
-                     * arguments must match
-                     */
-                    if (anycompatible_range_typeid != anycompatible_multirange_typelem)
-                        return false;
-                }
-                else
-                {
-                    anycompatible_range_typeid = anycompatible_multirange_typelem;
-                    anycompatible_range_typelem = get_range_subtype(anycompatible_range_typeid);
-                    if (!OidIsValid(anycompatible_range_typelem))
-                        return false;    /* not a range type */
-                }
-                /* collect the subtype for common-supertype choice */
-                anycompatible_actual_types[n_anycompatible_args++] =
-                    anycompatible_range_typelem;
+                /* we'll consider the subtype below */
             }
         }
     }
@@ -1825,28 +1810,7 @@ check_generic_type_consistency(const Oid *actual_arg_types,
         }
     }

-    /* Get the element type based on the range type, if we have one */
-    if (OidIsValid(range_typeid))
-    {
-        range_typelem = get_range_subtype(range_typeid);
-        if (!OidIsValid(range_typelem))
-            return false;        /* should be a range, but isn't */
-
-        if (!OidIsValid(elem_typeid))
-        {
-            /*
-             * if we don't have an element type yet, use the one we just got
-             */
-            elem_typeid = range_typelem;
-        }
-        else if (range_typelem != elem_typeid)
-        {
-            /* otherwise, they better match */
-            return false;
-        }
-    }
-
-    /* Get the element type based on the multirange type, if we have one */
+    /* Deduce range type from multirange type, or check that they agree */
     if (OidIsValid(multirange_typeid))
     {
         Oid            multirange_typelem;
@@ -1857,9 +1821,7 @@ check_generic_type_consistency(const Oid *actual_arg_types,

         if (!OidIsValid(range_typeid))
         {
-            /*
-             * If we don't have a range type yet, use the one we just got
-             */
+            /* If we don't have a range type yet, use the one we just got */
             range_typeid = multirange_typelem;
             range_typelem = get_range_subtype(multirange_typelem);
             if (!OidIsValid(range_typelem))
@@ -1870,6 +1832,14 @@ check_generic_type_consistency(const Oid *actual_arg_types,
             /* otherwise, they better match */
             return false;
         }
+    }
+
+    /* Get the element type based on the range type, if we have one */
+    if (OidIsValid(range_typeid))
+    {
+        range_typelem = get_range_subtype(range_typeid);
+        if (!OidIsValid(range_typelem))
+            return false;        /* should be a range, but isn't */

         if (!OidIsValid(elem_typeid))
         {
@@ -1899,6 +1869,27 @@ check_generic_type_consistency(const Oid *actual_arg_types,
             return false;
     }

+    /* Deduce range type from multirange type, or check that they agree */
+    if (OidIsValid(anycompatible_multirange_typeid))
+    {
+        if (OidIsValid(anycompatible_range_typeid))
+        {
+            if (anycompatible_multirange_typelem !=
+                anycompatible_range_typeid)
+                return false;
+        }
+        else
+        {
+            anycompatible_range_typeid = anycompatible_multirange_typelem;
+            anycompatible_range_typelem = get_range_subtype(anycompatible_range_typeid);
+            if (!OidIsValid(anycompatible_range_typelem))
+                return false;    /* not a range type */
+            /* collect the subtype for common-supertype choice */
+            anycompatible_actual_types[n_anycompatible_args++] =
+                anycompatible_range_typelem;
+        }
+    }
+
     /* Check matching of ANYCOMPATIBLE-family arguments, if any */
     if (n_anycompatible_args > 0)
     {
@@ -1966,39 +1957,40 @@ check_generic_type_consistency(const Oid *actual_arg_types,
  * 2) If return type is ANYARRAY, and any argument is ANYARRAY, use the
  *      argument's actual type as the function's return type.
  * 3) Similarly, if return type is ANYRANGE or ANYMULTIRANGE, and any
- *      argument is ANYRANGE or ANYMULTIRANGE, use that argument's
- *      actual type, range type or multirange type as the function's return
+ *      argument is ANYRANGE or ANYMULTIRANGE, use that argument's actual type
+ *      (or the corresponding range or multirange type) as the function's return
  *      type.
- * 4) Otherwise, if return type is ANYMULTIRANGE, and any argument is
- *      ANYMULTIRANGE, use the argument's actual type as the function's return
- *      type. Or if any argument is ANYRANGE, use its multirange type as the
- *      function's return type.
- * 5) Otherwise, if return type is ANYELEMENT or ANYARRAY, and there is
- *      at least one ANYELEMENT, ANYARRAY, or ANYRANGE input, deduce the
- *      return type from those inputs, or throw error if we can't.
- * 6) Otherwise, if return type is ANYRANGE or ANYMULTIRANGE, throw error.
+ * 4) Otherwise, if return type is ANYELEMENT or ANYARRAY, and there is
+ *      at least one ANYELEMENT, ANYARRAY, ANYRANGE, or ANYMULTIRANGE input,
+ *      deduce the return type from those inputs, or throw error if we can't.
+ * 5) Otherwise, if return type is ANYRANGE or ANYMULTIRANGE, throw error.
  *      (We have no way to select a specific range type if the arguments don't
- *      include ANYRANGE.)
+ *      include ANYRANGE or ANYMULTIRANGE.)
+ * 6) ANYENUM is treated the same as ANYELEMENT except that if it is used
  *      (alone or in combination with plain ANYELEMENT), we add the extra
  *      condition that the ANYELEMENT type must be an enum.
- * 8) ANYNONARRAY is treated the same as ANYELEMENT except that if it is used,
+ * 7) ANYNONARRAY is treated the same as ANYELEMENT except that if it is used,
  *      we add the extra condition that the ANYELEMENT type must not be an array.
  *      (This is a no-op if used in combination with ANYARRAY or ANYENUM, but
  *      is an extra restriction if not.)
- * 9) ANYCOMPATIBLE, ANYCOMPATIBLEARRAY, ANYCOMPATIBLENONARRAY, and
- *      ANYCOMPATIBLERANGE are handled by resolving the common supertype
- *      of those arguments (or their element types/subtypes, for array and range
- *      inputs), and then coercing all those arguments to the common supertype,
- *      or the array type over the common supertype for ANYCOMPATIBLEARRAY.
- *      For ANYCOMPATIBLERANGE, there must be at least one non-UNKNOWN input,
- *      all such inputs must be the same range type, and that type's subtype
- *      must equal the common supertype.
+ * 8) ANYCOMPATIBLE, ANYCOMPATIBLEARRAY, and ANYCOMPATIBLENONARRAY are handled
+ *      by resolving the common supertype of those arguments (or their element
+ *      types, for array inputs), and then coercing all those arguments to the
+ *      common supertype, or the array type over the common supertype for
+ *      ANYCOMPATIBLEARRAY.
+ * 9) For ANYCOMPATIBLERANGE and ANYCOMPATIBLEMULTIRANGE, there must be at
+ *      least one non-UNKNOWN input matching those arguments, and all such
+ *      inputs must be the same range type (or its multirange type, as
+ *      appropriate), since we cannot deduce a range type from non-range types.
+ *      Furthermore, the range type's subtype is included while choosing the
+ *      common supertype for ANYCOMPATIBLE et al.
  *
  * Domains over arrays or ranges match ANYARRAY or ANYRANGE arguments,
  * respectively, and are immediately flattened to their base type.  (In
  * particular, if the return type is also ANYARRAY or ANYRANGE, we'll set
  * it to the base type not the domain type.)  The same is true for
- * ANYCOMPATIBLEARRAY and ANYCOMPATIBLERANGE.
+ * ANYMULTIRANGE, ANYCOMPATIBLEARRAY, ANYCOMPATIBLERANGE, and
+ * ANYCOMPATIBLEMULTIRANGE.
  *
  * When allow_poly is false, we are not expecting any of the actual_arg_types
  * to be polymorphic, and we should not return a polymorphic result type
@@ -2046,13 +2038,13 @@ enforce_generic_type_consistency(const Oid *actual_arg_types,
     Oid            anycompatible_range_typelem = InvalidOid;
     Oid            anycompatible_multirange_typeid = InvalidOid;
     Oid            anycompatible_multirange_typelem = InvalidOid;
-    Oid            range_typelem;
-    Oid            multirange_typelem;
     bool        have_anynonarray = (rettype == ANYNONARRAYOID);
     bool        have_anyenum = (rettype == ANYENUMOID);
+    bool        have_anymultirange = (rettype == ANYMULTIRANGEOID);
     bool        have_anycompatible_nonarray = (rettype == ANYCOMPATIBLENONARRAYOID);
     bool        have_anycompatible_array = (rettype == ANYCOMPATIBLEARRAYOID);
     bool        have_anycompatible_range = (rettype == ANYCOMPATIBLERANGEOID);
+    bool        have_anycompatible_multirange = (rettype == ANYCOMPATIBLEMULTIRANGEOID);
     int            n_poly_args = 0;    /* this counts all family-1 arguments */
     int            n_anycompatible_args = 0;    /* this counts only non-unknowns */
     Oid            anycompatible_actual_types[FUNC_MAX_ARGS];
@@ -2135,6 +2127,7 @@ enforce_generic_type_consistency(const Oid *actual_arg_types,
         else if (decl_type == ANYMULTIRANGEOID)
         {
             n_poly_args++;
+            have_anymultirange = true;
             if (actual_type == UNKNOWNOID)
             {
                 have_poly_unknowns = true;
@@ -2223,6 +2216,7 @@ enforce_generic_type_consistency(const Oid *actual_arg_types,
         else if (decl_type == ANYCOMPATIBLEMULTIRANGEOID)
         {
             have_poly_anycompatible = true;
+            have_anycompatible_multirange = true;
             if (actual_type == UNKNOWNOID)
                 continue;
             if (allow_poly && decl_type == actual_type)
@@ -2243,15 +2237,13 @@ enforce_generic_type_consistency(const Oid *actual_arg_types,
             {
                 anycompatible_multirange_typeid = actual_type;
                 anycompatible_multirange_typelem = get_multirange_range(actual_type);
-                anycompatible_range_typelem = get_range_subtype(anycompatible_multirange_typelem);
                 if (!OidIsValid(anycompatible_multirange_typelem))
                     ereport(ERROR,
                             (errcode(ERRCODE_DATATYPE_MISMATCH),
                              errmsg("argument declared %s is not a multirange type but type %s",
                                     "anycompatiblemultirange",
                                     format_type_be(actual_type))));
-                /* collect the subtype for common-supertype choice */
-                anycompatible_actual_types[n_anycompatible_args++] = anycompatible_range_typelem;
+                /* we'll consider the subtype below */
             }
         }
     }
@@ -2319,43 +2311,11 @@ enforce_generic_type_consistency(const Oid *actual_arg_types,
             }
         }

-        /* Get the element type based on the range type, if we have one */
-        if (OidIsValid(range_typeid))
-        {
-            range_typelem = get_range_subtype(range_typeid);
-            if (!OidIsValid(range_typelem))
-                ereport(ERROR,
-                        (errcode(ERRCODE_DATATYPE_MISMATCH),
-                         errmsg("argument declared %s is not a range type but type %s",
-                                "anyrange",
-                                format_type_be(range_typeid))));
-
-            if (!OidIsValid(elem_typeid))
-            {
-                /*
-                 * if we don't have an element type yet, use the one we just
-                 * got
-                 */
-                elem_typeid = range_typelem;
-            }
-            else if (range_typelem != elem_typeid)
-            {
-                /* otherwise, they better match */
-                ereport(ERROR,
-                        (errcode(ERRCODE_DATATYPE_MISMATCH),
-                         errmsg("argument declared %s is not consistent with argument declared %s",
-                                "anyrange", "anyelement"),
-                         errdetail("%s versus %s",
-                                   format_type_be(range_typeid),
-                                   format_type_be(elem_typeid))));
-            }
-        }
-        else
-            range_typelem = InvalidOid;
-
-        /* Get the element type based on the multirange type, if we have one */
+        /* Deduce range type from multirange type, or vice versa */
         if (OidIsValid(multirange_typeid))
         {
+            Oid            multirange_typelem;
+
             multirange_typelem = get_multirange_range(multirange_typeid);
             if (!OidIsValid(multirange_typelem))
                 ereport(ERROR,
@@ -2366,11 +2326,8 @@ enforce_generic_type_consistency(const Oid *actual_arg_types,

             if (!OidIsValid(range_typeid))
             {
-                /*
-                 * If we don't have a range type yet, use the one we just got
-                 */
+                /* if we don't have a range type yet, use the one we just got */
                 range_typeid = multirange_typelem;
-                range_typelem = get_range_subtype(range_typeid);
             }
             else if (multirange_typelem != range_typeid)
             {
@@ -2383,6 +2340,25 @@ enforce_generic_type_consistency(const Oid *actual_arg_types,
                                    format_type_be(multirange_typeid),
                                    format_type_be(range_typeid))));
             }
+        }
+        else if (have_anymultirange && OidIsValid(range_typeid))
+        {
+            multirange_typeid = get_range_multirange(range_typeid);
+            /* We'll complain below if that didn't work */
+        }
+
+        /* Get the element type based on the range type, if we have one */
+        if (OidIsValid(range_typeid))
+        {
+            Oid            range_typelem;
+
+            range_typelem = get_range_subtype(range_typeid);
+            if (!OidIsValid(range_typelem))
+                ereport(ERROR,
+                        (errcode(ERRCODE_DATATYPE_MISMATCH),
+                         errmsg("argument declared %s is not a range type but type %s",
+                                "anyrange",
+                                format_type_be(range_typeid))));

             if (!OidIsValid(elem_typeid))
             {
@@ -2398,14 +2374,12 @@ enforce_generic_type_consistency(const Oid *actual_arg_types,
                 ereport(ERROR,
                         (errcode(ERRCODE_DATATYPE_MISMATCH),
                          errmsg("argument declared %s is not consistent with argument declared %s",
-                                "anymultirange", "anyelement"),
+                                "anyrange", "anyelement"),
                          errdetail("%s versus %s",
-                                   format_type_be(multirange_typeid),
+                                   format_type_be(range_typeid),
                                    format_type_be(elem_typeid))));
             }
         }
-        else
-            multirange_typelem = InvalidOid;

         if (!OidIsValid(elem_typeid))
         {
@@ -2456,6 +2430,46 @@ enforce_generic_type_consistency(const Oid *actual_arg_types,
     /* Check matching of family-2 polymorphic arguments, if any */
     if (have_poly_anycompatible)
     {
+        /* Deduce range type from multirange type, or vice versa */
+        if (OidIsValid(anycompatible_multirange_typeid))
+        {
+            if (OidIsValid(anycompatible_range_typeid))
+            {
+                if (anycompatible_multirange_typelem !=
+                    anycompatible_range_typeid)
+                    ereport(ERROR,
+                            (errcode(ERRCODE_DATATYPE_MISMATCH),
+                             errmsg("argument declared %s is not consistent with argument declared %s",
+                                    "anycompatiblemultirange",
+                                    "anycompatiblerange"),
+                             errdetail("%s versus %s",
+                                       format_type_be(anycompatible_multirange_typeid),
+                                       format_type_be(anycompatible_range_typeid))));
+            }
+            else
+            {
+                anycompatible_range_typeid = anycompatible_multirange_typelem;
+                anycompatible_range_typelem = get_range_subtype(anycompatible_range_typeid);
+                if (!OidIsValid(anycompatible_range_typelem))
+                    ereport(ERROR,
+                            (errcode(ERRCODE_DATATYPE_MISMATCH),
+                             errmsg("argument declared %s is not a multirange type but type %s",
+                                    "anycompatiblemultirange",
+                                    format_type_be(anycompatible_multirange_typeid))));
+                /* this enables element type matching check below */
+                have_anycompatible_range = true;
+                /* collect the subtype for common-supertype choice */
+                anycompatible_actual_types[n_anycompatible_args++] =
+                    anycompatible_range_typelem;
+            }
+        }
+        else if (have_anycompatible_multirange &&
+                 OidIsValid(anycompatible_range_typeid))
+        {
+            anycompatible_multirange_typeid = get_range_multirange(anycompatible_range_typeid);
+            /* We'll complain below if that didn't work */
+        }
+
         if (n_anycompatible_args > 0)
         {
             anycompatible_typeid =
@@ -2494,6 +2508,27 @@ enforce_generic_type_consistency(const Oid *actual_arg_types,
                                     format_type_be(anycompatible_typeid))));
             }

+            if (have_anycompatible_multirange)
+            {
+                /* we can't infer a multirange type from the others */
+                if (!OidIsValid(anycompatible_multirange_typeid))
+                    ereport(ERROR,
+                            (errcode(ERRCODE_DATATYPE_MISMATCH),
+                             errmsg("could not determine polymorphic type %s because input has type %s",
+                                    "anycompatiblemultirange", "unknown")));
+
+                /*
+                 * the anycompatible type must exactly match the multirange
+                 * element type
+                 */
+                if (anycompatible_range_typelem != anycompatible_typeid)
+                    ereport(ERROR,
+                            (errcode(ERRCODE_DATATYPE_MISMATCH),
+                             errmsg("anycompatiblemultirange type %s does not match anycompatible type %s",
+                                    format_type_be(anycompatible_multirange_typeid),
+                                    format_type_be(anycompatible_typeid))));
+            }
+
             if (have_anycompatible_nonarray)
             {
                 /*
@@ -2522,7 +2557,7 @@ enforce_generic_type_consistency(const Oid *actual_arg_types,
                  * Only way to get here is if all the family-2 polymorphic
                  * arguments have UNKNOWN inputs.  Resolve to TEXT as
                  * select_common_type() would do.  That doesn't license us to
-                 * use TEXTRANGE, though.
+                 * use TEXTRANGE or TEXTMULTIRANGE, though.
                  */
                 anycompatible_typeid = TEXTOID;
                 anycompatible_array_typeid = TEXTARRAYOID;
@@ -2531,6 +2566,11 @@ enforce_generic_type_consistency(const Oid *actual_arg_types,
                             (errcode(ERRCODE_DATATYPE_MISMATCH),
                              errmsg("could not determine polymorphic type %s because input has type %s",
                                     "anycompatiblerange", "unknown")));
+                if (have_anycompatible_multirange)
+                    ereport(ERROR,
+                            (errcode(ERRCODE_DATATYPE_MISMATCH),
+                             errmsg("could not determine polymorphic type %s because input has type %s",
+                                    "anycompatiblemultirange", "unknown")));
             }
         }

@@ -2602,10 +2642,11 @@ enforce_generic_type_consistency(const Oid *actual_arg_types,
             {
                 if (!OidIsValid(multirange_typeid))
                 {
+                    /* we can't infer a multirange type from the others */
                     ereport(ERROR,
-                            (errcode(ERRCODE_UNDEFINED_OBJECT),
-                             errmsg("could not find multirange type for data type %s",
-                                    format_type_be(elem_typeid))));
+                            (errcode(ERRCODE_DATATYPE_MISMATCH),
+                             errmsg("could not determine polymorphic type %s because input has type %s",
+                                    "anymultirange", "unknown")));
                 }
                 declared_arg_types[j] = multirange_typeid;
             }
@@ -2640,24 +2681,20 @@ enforce_generic_type_consistency(const Oid *actual_arg_types,
         if (!OidIsValid(range_typeid))
             ereport(ERROR,
                     (errcode(ERRCODE_DATATYPE_MISMATCH),
-                     errmsg("could not determine polymorphic type %s because input has type %s",
-                            "anyrange", "unknown")));
+                     errmsg_internal("could not determine polymorphic type %s because input has type %s",
+                                     "anyrange", "unknown")));
         return range_typeid;
     }

     /* if we return ANYMULTIRANGE use the appropriate argument type */
     if (rettype == ANYMULTIRANGEOID)
     {
+        /* this error is unreachable if the function signature is valid: */
         if (!OidIsValid(multirange_typeid))
-        {
-            if (OidIsValid(range_typeid))
-                multirange_typeid = get_range_multirange(range_typeid);
-            else
-                ereport(ERROR,
-                        (errcode(ERRCODE_UNDEFINED_OBJECT),
-                         errmsg("could not find multirange type for data type %s",
-                                format_type_be(elem_typeid))));
-        }
+            ereport(ERROR,
+                    (errcode(ERRCODE_DATATYPE_MISMATCH),
+                     errmsg_internal("could not determine polymorphic type %s because input has type %s",
+                                     "anymultirange", "unknown")));
         return multirange_typeid;
     }

@@ -2777,7 +2814,7 @@ check_valid_polymorphic_signature(Oid ret_type,
                 return NULL;    /* OK */
         }
         /* Keep this list in sync with IsPolymorphicTypeFamily2! */
-        return psprintf(_("A result of type %s requires at least one input of type anycompatible, anycompatiblearray,
anycompatiblenonarray,or anycompatiblerange."), 
+        return psprintf(_("A result of type %s requires at least one input of type anycompatible, anycompatiblearray,
anycompatiblenonarray,anycompatiblerange, or anycompatiblemultirange."), 
                         format_type_be(ret_type));
     }
     else
@@ -2917,7 +2954,7 @@ IsBinaryCoercible(Oid srctype, Oid targettype)
         if (type_is_range(srctype))
             return true;

-    /* Also accept any multirange type as coercible to ANMULTIYRANGE */
+    /* Also, any multirange type is coercible to ANY[COMPATIBLE]MULTIRANGE */
     if (targettype == ANYMULTIRANGEOID || targettype == ANYCOMPATIBLEMULTIRANGEOID)
         if (type_is_multirange(srctype))
             return true;
diff --git a/src/test/regress/expected/polymorphism.out b/src/test/regress/expected/polymorphism.out
index 772345584f..1cd558d668 100644
--- a/src/test/regress/expected/polymorphism.out
+++ b/src/test/regress/expected/polymorphism.out
@@ -179,6 +179,72 @@ LINE 2:   from polyf(11, array[1, 2.2], 42, 34.5);
 HINT:  No function matches the given name and argument types. You might need to add explicit type casts.
 drop function polyf(a anyelement, b anyarray,
                     c anycompatible, d anycompatible);
+create function polyf(anyrange) returns anymultirange
+as 'select multirange($1);' language sql;
+select polyf(int4range(1,10));
+  polyf
+----------
+ {[1,10)}
+(1 row)
+
+select polyf(null);
+ERROR:  could not determine polymorphic type because input has type unknown
+drop function polyf(anyrange);
+create function polyf(anymultirange) returns anyelement
+as 'select lower($1);' language sql;
+select polyf(int4multirange(int4range(1,10), int4range(20,30)));
+ polyf
+-------
+     1
+(1 row)
+
+select polyf(null);
+ERROR:  could not determine polymorphic type because input has type unknown
+drop function polyf(anymultirange);
+create function polyf(anycompatiblerange) returns anycompatiblemultirange
+as 'select multirange($1);' language sql;
+select polyf(int4range(1,10));
+  polyf
+----------
+ {[1,10)}
+(1 row)
+
+select polyf(null);
+ERROR:  could not determine polymorphic type anycompatiblerange because input has type unknown
+drop function polyf(anycompatiblerange);
+create function polyf(anymultirange) returns anyrange
+as 'select range_merge($1);' language sql;
+select polyf(int4multirange(int4range(1,10), int4range(20,30)));
+ polyf
+--------
+ [1,30)
+(1 row)
+
+select polyf(null);
+ERROR:  could not determine polymorphic type because input has type unknown
+drop function polyf(anymultirange);
+create function polyf(anycompatiblemultirange) returns anycompatiblerange
+as 'select range_merge($1);' language sql;
+select polyf(int4multirange(int4range(1,10), int4range(20,30)));
+ polyf
+--------
+ [1,30)
+(1 row)
+
+select polyf(null);
+ERROR:  could not determine polymorphic type anycompatiblerange because input has type unknown
+drop function polyf(anycompatiblemultirange);
+create function polyf(anycompatiblemultirange) returns anycompatible
+as 'select lower($1);' language sql;
+select polyf(int4multirange(int4range(1,10), int4range(20,30)));
+ polyf
+-------
+     1
+(1 row)
+
+select polyf(null);
+ERROR:  could not determine polymorphic type anycompatiblemultirange because input has type unknown
+drop function polyf(anycompatiblemultirange);
 --
 -- Polymorphic aggregate tests
 --
@@ -1914,7 +1980,7 @@ LINE 1: select x, pg_typeof(x) from anyctest(11.2, multirange(int4ra...
                                     ^
 HINT:  No function matches the given name and argument types. You might need to add explicit type casts.
 select x, pg_typeof(x) from anyctest(11.2, '{[4,7)}') x;  -- fail
-ERROR:  could not identify anycompatiblemultirange type
+ERROR:  could not determine polymorphic type anycompatiblemultirange because input has type unknown
 drop function anyctest(anycompatible, anycompatiblemultirange);
 create function anyctest(anycompatiblemultirange, anycompatiblemultirange)
 returns anycompatible as $$
diff --git a/src/test/regress/expected/rangefuncs.out b/src/test/regress/expected/rangefuncs.out
index 5bbd5f6c0b..cafca1f9ae 100644
--- a/src/test/regress/expected/rangefuncs.out
+++ b/src/test/regress/expected/rangefuncs.out
@@ -1609,7 +1609,7 @@ DROP FUNCTION dup(f1 anycompatiblerange);
 CREATE FUNCTION bad (f1 anyarray, out f2 anycompatible, out f3 anycompatiblearray)
 AS 'select $1, array[$1,$1]' LANGUAGE sql;
 ERROR:  cannot determine result data type
-DETAIL:  A result of type anycompatible requires at least one input of type anycompatible, anycompatiblearray,
anycompatiblenonarray,or anycompatiblerange. 
+DETAIL:  A result of type anycompatible requires at least one input of type anycompatible, anycompatiblearray,
anycompatiblenonarray,anycompatiblerange, or anycompatiblemultirange. 
 --
 -- table functions
 --
diff --git a/src/test/regress/sql/polymorphism.sql b/src/test/regress/sql/polymorphism.sql
index 891b023ada..fa57db6559 100644
--- a/src/test/regress/sql/polymorphism.sql
+++ b/src/test/regress/sql/polymorphism.sql
@@ -126,6 +126,54 @@ select x, pg_typeof(x), y, pg_typeof(y)
 drop function polyf(a anyelement, b anyarray,
                     c anycompatible, d anycompatible);

+create function polyf(anyrange) returns anymultirange
+as 'select multirange($1);' language sql;
+
+select polyf(int4range(1,10));
+select polyf(null);
+
+drop function polyf(anyrange);
+
+create function polyf(anymultirange) returns anyelement
+as 'select lower($1);' language sql;
+
+select polyf(int4multirange(int4range(1,10), int4range(20,30)));
+select polyf(null);
+
+drop function polyf(anymultirange);
+
+create function polyf(anycompatiblerange) returns anycompatiblemultirange
+as 'select multirange($1);' language sql;
+
+select polyf(int4range(1,10));
+select polyf(null);
+
+drop function polyf(anycompatiblerange);
+
+create function polyf(anymultirange) returns anyrange
+as 'select range_merge($1);' language sql;
+
+select polyf(int4multirange(int4range(1,10), int4range(20,30)));
+select polyf(null);
+
+drop function polyf(anymultirange);
+
+create function polyf(anycompatiblemultirange) returns anycompatiblerange
+as 'select range_merge($1);' language sql;
+
+select polyf(int4multirange(int4range(1,10), int4range(20,30)));
+select polyf(null);
+
+drop function polyf(anycompatiblemultirange);
+
+create function polyf(anycompatiblemultirange) returns anycompatible
+as 'select lower($1);' language sql;
+
+select polyf(int4multirange(int4range(1,10), int4range(20,30)));
+select polyf(null);
+
+drop function polyf(anycompatiblemultirange);
+

 --
 -- Polymorphic aggregate tests

Re: BUG #17066: Cache lookup failed when null (iso-8859-1) is passed as anycompatiblemultirange

От
Alexander Korotkov
Дата:
On Wed, Jul 21, 2021 at 12:54 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Alexander Korotkov <aekorotkov@gmail.com> writes:
> > Do I understand correctly that enforce_generic_type_consistency() is
> > called only after check_generic_type_consistency() returned true?
> > If so, that means some of the checks are redundant.  Therefore, we can
> > replace ereport()'s with Assert()'s.
>
> They are not redundant, IIRC.  I forget the exact mechanism for
> reaching them, but it likely has something to do with aggregates
> or variadic functions.

If checks aren't redundant, there should be cases when they don't
pass.  It would be nice to identify these cases and add them to the
regression tests.  I didn't manage to do this yet.

> In any case, apologies for taking so long to get back to this.  Here's
> a proposed patch (based in part on Neil's earlier patch).

Thank you!  I'll review this and come back to you.

------
Regards,
Alexander Korotkov



Alexander Korotkov <aekorotkov@gmail.com> writes:
> On Wed, Jul 21, 2021 at 12:54 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> Alexander Korotkov <aekorotkov@gmail.com> writes:
>>> Do I understand correctly that enforce_generic_type_consistency() is
>>> called only after check_generic_type_consistency() returned true?
>>> If so, that means some of the checks are redundant.  Therefore, we can
>>> replace ereport()'s with Assert()'s.

>> They are not redundant, IIRC.  I forget the exact mechanism for
>> reaching them, but it likely has something to do with aggregates
>> or variadic functions.

> If checks aren't redundant, there should be cases when they don't
> pass.  It would be nice to identify these cases and add them to the
> regression tests.  I didn't manage to do this yet.

Hmm ... whether or not there are edge cases where those errors are
reachable, it's certainly true that the mainline case doesn't reach
them:

regression=# create function myadd(anyelement, anyelement) returns anyelement
as 'select $1 + $2' language sql;
CREATE FUNCTION
regression=# select myadd(1, 2.3);
ERROR:  function myadd(integer, numeric) does not exist
LINE 1: select myadd(1, 2.3);
               ^
HINT:  No function matches the given name and argument types. You might need to add explicit type casts.

That's too bad, because IMO it'd be way more helpful to say
  ERROR:  arguments declared "anyelement" are not all alike
  DETAIL:  integer versus numeric
which is what enforce_generic_type_consistency would say if it
were reached.  Similarly, the other error cases in that code
are far more specific and thus more helpful than simply reporting
that there's no matching function.

I'm tempted to propose that, if there is only one possible match
but check_generic_type_consistency rejects it, then
function/operator lookup should return that OID anyway, allowing
enforce_generic_type_consistency to throw the appropriate error.
This would obviously not help when there are multiple polymorphic
functions having the same name and number of arguments, but that
strikes me as a very unusual corner case.

            regards, tom lane



Re: BUG #17066: Cache lookup failed when null (iso-8859-1) is passed as anycompatiblemultirange

От
Alexander Korotkov
Дата:
On Wed, Jul 21, 2021 at 6:03 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>
> Alexander Korotkov <aekorotkov@gmail.com> writes:
> > On Wed, Jul 21, 2021 at 12:54 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> >> Alexander Korotkov <aekorotkov@gmail.com> writes:
> >>> Do I understand correctly that enforce_generic_type_consistency() is
> >>> called only after check_generic_type_consistency() returned true?
> >>> If so, that means some of the checks are redundant.  Therefore, we can
> >>> replace ereport()'s with Assert()'s.
>
> >> They are not redundant, IIRC.  I forget the exact mechanism for
> >> reaching them, but it likely has something to do with aggregates
> >> or variadic functions.
>
> > If checks aren't redundant, there should be cases when they don't
> > pass.  It would be nice to identify these cases and add them to the
> > regression tests.  I didn't manage to do this yet.
>
> Hmm ... whether or not there are edge cases where those errors are
> reachable, it's certainly true that the mainline case doesn't reach
> them:
>
> regression=# create function myadd(anyelement, anyelement) returns anyelement
> as 'select $1 + $2' language sql;
> CREATE FUNCTION
> regression=# select myadd(1, 2.3);
> ERROR:  function myadd(integer, numeric) does not exist
> LINE 1: select myadd(1, 2.3);
>                ^
> HINT:  No function matches the given name and argument types. You might need to add explicit type casts.
>
> That's too bad, because IMO it'd be way more helpful to say
>   ERROR:  arguments declared "anyelement" are not all alike
>   DETAIL:  integer versus numeric
> which is what enforce_generic_type_consistency would say if it
> were reached.  Similarly, the other error cases in that code
> are far more specific and thus more helpful than simply reporting
> that there's no matching function.
>
> I'm tempted to propose that, if there is only one possible match
> but check_generic_type_consistency rejects it, then
> function/operator lookup should return that OID anyway, allowing
> enforce_generic_type_consistency to throw the appropriate error.
> This would obviously not help when there are multiple polymorphic
> functions having the same name and number of arguments, but that
> strikes me as a very unusual corner case.

I spend some time thinking about this.  I'm actually not sure this
approach is really correct.  If there is only one polymorphic
candidate, it's still possible that the user means non-polymorphic
function with exactly matching arguments, which is simply doesn't
exist.

------
Regards,
Alexander Korotkov



Re: BUG #17066: Cache lookup failed when null (iso-8859-1) is passed as anycompatiblemultirange

От
Alexander Korotkov
Дата:
On Wed, Jul 21, 2021 at 12:54 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Alexander Korotkov <aekorotkov@gmail.com> writes:
> > Do I understand correctly that enforce_generic_type_consistency() is
> > called only after check_generic_type_consistency() returned true?
> > If so, that means some of the checks are redundant.  Therefore, we can
> > replace ereport()'s with Assert()'s.
>
> They are not redundant, IIRC.  I forget the exact mechanism for
> reaching them, but it likely has something to do with aggregates
> or variadic functions.
>
> In any case, apologies for taking so long to get back to this.  Here's
> a proposed patch (based in part on Neil's earlier patch).

This patch looks good to me.  Besides fixing the particular bug
report, it seems that the situation of mismatching range and
multirange types in arguments is now handled correctly.

------
Regards,
Alexander Korotkov



Alexander Korotkov <aekorotkov@gmail.com> writes:
> On Wed, Jul 21, 2021 at 12:54 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> In any case, apologies for taking so long to get back to this.  Here's
>> a proposed patch (based in part on Neil's earlier patch).

> This patch looks good to me.  Besides fixing the particular bug
> report, it seems that the situation of mismatching range and
> multirange types in arguments is now handled correctly.

Thanks for reviewing!  I'll push it later today.

            regards, tom lane



Alexander Korotkov <aekorotkov@gmail.com> writes:
> On Wed, Jul 21, 2021 at 6:03 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> That's too bad, because IMO it'd be way more helpful to say
>> ERROR:  arguments declared "anyelement" are not all alike
>> DETAIL:  integer versus numeric
>> which is what enforce_generic_type_consistency would say if it
>> were reached.  Similarly, the other error cases in that code
>> are far more specific and thus more helpful than simply reporting
>> that there's no matching function.
>> 
>> I'm tempted to propose that, if there is only one possible match
>> but check_generic_type_consistency rejects it, then
>> function/operator lookup should return that OID anyway, allowing
>> enforce_generic_type_consistency to throw the appropriate error.
>> This would obviously not help when there are multiple polymorphic
>> functions having the same name and number of arguments, but that
>> strikes me as a very unusual corner case.

> I spend some time thinking about this.  I'm actually not sure this
> approach is really correct.  If there is only one polymorphic
> candidate, it's still possible that the user means non-polymorphic
> function with exactly matching arguments, which is simply doesn't
> exist.

I don't particularly buy that reasoning.  Certainly the true cause of
the error could be that the user mistyped the function name, or meant
to refer to something that's not in the search_path, or forgot to load
the function into this particular database, etc etc.  But we have
to act on the basis of the information we have, and that is the
function(s) we see.  If we let possibilities like these paralyze us,
we'll never be able to issue useful error messages at all.

I don't deny that what I'm proposing above is a bit weird and
non-orthogonal; there may be a better way to do it.  But the
existing code structure where check_generic_type_consistency
silently returns a boolean just isn't very conducive to giving
a good error message.  We have a lot more information available
to give, if we choose to give it.

Possibly we should think in terms of rewriting
enforce_generic_type_consistency's messages so that they are
errdetail() messages with a common primary message that's still
some variation of "there's no matching function".

            regards, tom lane



Re: BUG #17066: Cache lookup failed when null (iso-8859-1) is passed as anycompatiblemultirange

От
Alexander Korotkov
Дата:
On Tue, Jul 27, 2021 at 10:43 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Alexander Korotkov <aekorotkov@gmail.com> writes:
> > On Wed, Jul 21, 2021 at 6:03 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> >> That's too bad, because IMO it'd be way more helpful to say
> >> ERROR:  arguments declared "anyelement" are not all alike
> >> DETAIL:  integer versus numeric
> >> which is what enforce_generic_type_consistency would say if it
> >> were reached.  Similarly, the other error cases in that code
> >> are far more specific and thus more helpful than simply reporting
> >> that there's no matching function.
> >>
> >> I'm tempted to propose that, if there is only one possible match
> >> but check_generic_type_consistency rejects it, then
> >> function/operator lookup should return that OID anyway, allowing
> >> enforce_generic_type_consistency to throw the appropriate error.
> >> This would obviously not help when there are multiple polymorphic
> >> functions having the same name and number of arguments, but that
> >> strikes me as a very unusual corner case.
>
> > I spend some time thinking about this.  I'm actually not sure this
> > approach is really correct.  If there is only one polymorphic
> > candidate, it's still possible that the user means non-polymorphic
> > function with exactly matching arguments, which is simply doesn't
> > exist.
>
> I don't particularly buy that reasoning.  Certainly the true cause of
> the error could be that the user mistyped the function name, or meant
> to refer to something that's not in the search_path, or forgot to load
> the function into this particular database, etc etc.  But we have
> to act on the basis of the information we have, and that is the
> function(s) we see.  If we let possibilities like these paralyze us,
> we'll never be able to issue useful error messages at all.
>
> I don't deny that what I'm proposing above is a bit weird and
> non-orthogonal; there may be a better way to do it.  But the
> existing code structure where check_generic_type_consistency
> silently returns a boolean just isn't very conducive to giving
> a good error message.  We have a lot more information available
> to give, if we choose to give it.
>
> Possibly we should think in terms of rewriting
> enforce_generic_type_consistency's messages so that they are
> errdetail() messages with a common primary message that's still
> some variation of "there's no matching function".

That's an interesting idea!  If the primary message is still  "there's
no matching function", I feel absolutely comfortable about putting
information about "closest match" into detail.

------
Regards,
Alexander Korotkov