On 2020-Mar-19, Paul A Jungwirth wrote:
> On Thu, Mar 19, 2020 at 1:43 PM Paul A Jungwirth
> <p...@illuminatedcomputing.com> wrote:
> > On Thu, Mar 19, 2020 at 1:42 PM Alvaro Herrera <alvhe...@2ndquadrant.com>
> > wrote:
> > > There's been another flurry of commits in the polymorphic types area.
> > > Can you please rebase again?
> >
> > I noticed that too. :-) I'm about halfway through a rebase right now.
> > I can probably finish it up tonight.
>
> Here is that patch. I should probably add an anycompatiblemultirange
> type now too? I'll get started on that tomorrow.
Thanks for the new version. Here's a few minor adjustments while I
continue to read through it.
Thinking about the on-disk representation, can we do better than putting
the contained ranges in long-varlena format, including padding; also we
include the type OID with each element. Sounds wasteful. A more
compact representation might be to allow short varlenas and doing away
with the alignment padding, put the the type OID just once. This is
important because we cannot change it later.
I'm also wondering if multirange_in() is the right strategy. Would it
be sensible to give each range to range_parse or range_parse_bounde, so
that it determines where each range starts and ends? Then that function
doesn't have to worry about each quote and escape, duplicating range
parsing code. (This will probably require changing signature of the
rangetypes.c function, and exporting it; for example have
range_parse_bound allow bound_str to be NULL and in that case don't mess
with the StringInfo and just return the end position of the parsed
bound.)
--
Álvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
>From bd8b814426bef6f5a620351557cdc953ba0ae22e Mon Sep 17 00:00:00 2001
From: Alvaro Herrera <alvhe...@alvh.no-ip.org>
Date: Mon, 23 Mar 2020 18:50:02 -0300
Subject: [PATCH 1/5] Fix typo
---
src/backend/utils/adt/multirangetypes.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/backend/utils/adt/multirangetypes.c b/src/backend/utils/adt/multirangetypes.c
index bd3cf3dc93..6b3cde8eca 100644
--- a/src/backend/utils/adt/multirangetypes.c
+++ b/src/backend/utils/adt/multirangetypes.c
@@ -79,7 +79,7 @@ static int32 multirange_canonicalize(TypeCacheEntry *rangetyp, int32 input_range
* to range_in, but we have to detect quoting and backslash-escaping
* which can happen for range bounds.
* Backslashes can escape something inside or outside a quoted string,
- * and a quoted string can escape quote marks either either backslashes
+ * and a quoted string can escape quote marks with either backslashes
* or double double-quotes.
*/
Datum
--
2.20.1
>From afd017faab3d6aff3ca8618d95a6f9c918ab8cca Mon Sep 17 00:00:00 2001
From: Alvaro Herrera <alvhe...@alvh.no-ip.org>
Date: Mon, 23 Mar 2020 18:50:19 -0300
Subject: [PATCH 2/5] Remove trailing useless ;
---
src/backend/utils/adt/multirangetypes.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/backend/utils/adt/multirangetypes.c b/src/backend/utils/adt/multirangetypes.c
index 6b3cde8eca..6e9cf77651 100644
--- a/src/backend/utils/adt/multirangetypes.c
+++ b/src/backend/utils/adt/multirangetypes.c
@@ -131,7 +131,7 @@ multirange_in(PG_FUNCTION_ARGS)
input_str),
errdetail("Unexpected end of input.")));
- /* skip whitespace */ ;
+ /* skip whitespace */
if (isspace((unsigned char) ch))
continue;
--
2.20.1
>From d7c1ea171de018d025ab5de57f572a9353b02fdf Mon Sep 17 00:00:00 2001
From: Alvaro Herrera <alvhe...@alvh.no-ip.org>
Date: Mon, 23 Mar 2020 18:50:40 -0300
Subject: [PATCH 3/5] reduce palloc+strlcpy to pnstrdup
---
src/backend/utils/adt/multirangetypes.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/src/backend/utils/adt/multirangetypes.c b/src/backend/utils/adt/multirangetypes.c
index 6e9cf77651..5b57416cfd 100644
--- a/src/backend/utils/adt/multirangetypes.c
+++ b/src/backend/utils/adt/multirangetypes.c
@@ -167,9 +167,8 @@ multirange_in(PG_FUNCTION_ARGS)
parse_state = MULTIRANGE_IN_RANGE_ESCAPED;
else if (ch == ']' || ch == ')')
{
- range_str_len = ptr - range_str + 2;
- range_str_copy = palloc0(range_str_len);
- strlcpy(range_str_copy, range_str, range_str_len);
+ range_str_len = ptr - range_str + 1;
+ range_str_copy = pnstrdup(range_str, range_str_len);
if (range_capacity == range_count)
{
range_capacity *= 2;
--
2.20.1
>From 7698f57d1f5ab870c89a84da7bea24bef241a458 Mon Sep 17 00:00:00 2001
From: Alvaro Herrera <alvhe...@alvh.no-ip.org>
Date: Mon, 23 Mar 2020 18:50:59 -0300
Subject: [PATCH 4/5] silence compiler warning
---
src/backend/utils/fmgr/funcapi.c | 13 ++++++++-----
1 file changed, 8 insertions(+), 5 deletions(-)
diff --git a/src/backend/utils/fmgr/funcapi.c b/src/backend/utils/fmgr/funcapi.c
index ab24359981..3bfefcf48a 100644
--- a/src/backend/utils/fmgr/funcapi.c
+++ b/src/backend/utils/fmgr/funcapi.c
@@ -508,10 +508,13 @@ resolve_anyelement_from_others(polymorphic_actuals *actuals)
else if (OidIsValid(actuals->anymultirange_type))
{
/* Use the element type based on the multirange type */
- Oid multirange_base_type = getBaseType(actuals->anymultirange_type);
- Oid multirange_typelem =
- get_range_multirange_subtype(multirange_base_type);
+ Oid multirange_base_type;
+ Oid multirange_typelem;
+ Oid range_base_type;
+ Oid range_typelem;
+ multirange_base_type = getBaseType(actuals->anymultirange_type);
+ multirange_typelem = get_range_multirange_subtype(multirange_base_type);
if (!OidIsValid(multirange_typelem))
ereport(ERROR,
(errcode(ERRCODE_DATATYPE_MISMATCH),
@@ -519,8 +522,8 @@ resolve_anyelement_from_others(polymorphic_actuals *actuals)
"anymultirange",
format_type_be(multirange_base_type))));
- Oid range_base_type = getBaseType(multirange_typelem);
- Oid range_typelem = get_range_subtype(range_base_type);
+ range_base_type = getBaseType(multirange_typelem);
+ range_typelem = get_range_subtype(range_base_type);
if (!OidIsValid(range_typelem))
ereport(ERROR,
--
2.20.1
>From b70eb4fcff3819a3671f21164319750a5a174083 Mon Sep 17 00:00:00 2001
From: Alvaro Herrera <alvhe...@alvh.no-ip.org>
Date: Mon, 23 Mar 2020 18:58:14 -0300
Subject: [PATCH 5/5] rename get_range_multirange_subtype to
get_multirange_range
---
src/backend/parser/parse_coerce.c | 4 ++--
src/backend/utils/cache/lsyscache.c | 8 ++++----
src/backend/utils/cache/typcache.c | 2 +-
src/backend/utils/fmgr/funcapi.c | 4 ++--
src/include/utils/lsyscache.h | 2 +-
5 files changed, 10 insertions(+), 10 deletions(-)
diff --git a/src/backend/parser/parse_coerce.c b/src/backend/parser/parse_coerce.c
index fb8c3e5cb0..91f80a0656 100644
--- a/src/backend/parser/parse_coerce.c
+++ b/src/backend/parser/parse_coerce.c
@@ -1757,7 +1757,7 @@ check_generic_type_consistency(const Oid *actual_arg_types,
{
Oid multirange_typelem;
- multirange_typelem = get_range_multirange_subtype(multirange_typeid);
+ multirange_typelem = get_multirange_range(multirange_typeid);
if (!OidIsValid(multirange_typelem))
return false; /* should be a multirange, but isn't */
@@ -2222,7 +2222,7 @@ enforce_generic_type_consistency(const Oid *actual_arg_types,
/* Get the element type based on the multirange type, if we have one */
if (OidIsValid(multirange_typeid))
{
- multirange_typelem = get_range_multirange_subtype(multirange_typeid);
+ multirange_typelem = get_multirange_range(multirange_typeid);
if (!OidIsValid(multirange_typelem))
ereport(ERROR,
(errcode(ERRCODE_DATATYPE_MISMATCH),
diff --git a/src/backend/utils/cache/lsyscache.c b/src/backend/utils/cache/lsyscache.c
index c6c079c623..8d773d40c4 100644
--- a/src/backend/utils/cache/lsyscache.c
+++ b/src/backend/utils/cache/lsyscache.c
@@ -3238,13 +3238,13 @@ get_range_multirange(Oid rangeOid)
}
/*
- * get_range_multirange_subtype
- * Returns the subtype of a given multirange type
+ * get_multirange_range
+ * Returns the range type of a given multirange
*
- * Returns InvalidOid if the type is not a multirange type.
+ * Returns InvalidOid if the type is not a multirange.
*/
Oid
-get_range_multirange_subtype(Oid multirangeOid)
+get_multirange_range(Oid multirangeOid)
{
HeapTuple tp;
diff --git a/src/backend/utils/cache/typcache.c b/src/backend/utils/cache/typcache.c
index 160846cf10..f1d3cb97d0 100644
--- a/src/backend/utils/cache/typcache.c
+++ b/src/backend/utils/cache/typcache.c
@@ -964,7 +964,7 @@ load_multirangetype_info(TypeCacheEntry *typentry)
{
Oid rangetypeOid;
- rangetypeOid = get_range_multirange_subtype(typentry->type_id);
+ rangetypeOid = get_multirange_range(typentry->type_id);
if (!OidIsValid(rangetypeOid))
elog(ERROR, "cache lookup failed for multirange type %u",
typentry->type_id);
diff --git a/src/backend/utils/fmgr/funcapi.c b/src/backend/utils/fmgr/funcapi.c
index 3bfefcf48a..ce09d0d128 100644
--- a/src/backend/utils/fmgr/funcapi.c
+++ b/src/backend/utils/fmgr/funcapi.c
@@ -514,7 +514,7 @@ resolve_anyelement_from_others(polymorphic_actuals *actuals)
Oid range_typelem;
multirange_base_type = getBaseType(actuals->anymultirange_type);
- multirange_typelem = get_range_multirange_subtype(multirange_base_type);
+ multirange_typelem = get_multirange_range(multirange_base_type);
if (!OidIsValid(multirange_typelem))
ereport(ERROR,
(errcode(ERRCODE_DATATYPE_MISMATCH),
@@ -579,7 +579,7 @@ resolve_anyrange_from_others(polymorphic_actuals *actuals)
/* Use the element type based on the multirange type */
Oid multirange_base_type = getBaseType(actuals->anymultirange_type);
Oid multirange_typelem =
- get_range_multirange_subtype(multirange_base_type);
+ get_multirange_range(multirange_base_type);
if (!OidIsValid(multirange_typelem))
ereport(ERROR,
diff --git a/src/include/utils/lsyscache.h b/src/include/utils/lsyscache.h
index 80410dcd04..e361a7943b 100644
--- a/src/include/utils/lsyscache.h
+++ b/src/include/utils/lsyscache.h
@@ -183,7 +183,7 @@ extern char *get_namespace_name_or_temp(Oid nspid);
extern Oid get_range_subtype(Oid rangeOid);
extern Oid get_range_collation(Oid rangeOid);
extern Oid get_range_multirange(Oid rangeOid);
-extern Oid get_range_multirange_subtype(Oid multirangeOid);
+extern Oid get_multirange_range(Oid multirangeOid);
extern Oid get_index_column_opclass(Oid index_oid, int attno);
extern bool get_index_isreplident(Oid index_oid);
extern bool get_index_isvalid(Oid index_oid);
--
2.20.1
