pg.ddx.io pgsql-hackers@postgresql.org mailing list archive help / color / mirror / Atom feed From: Alvaro Herrera <alvherre@2ndquadrant.com>
To: Paul A Jungwirth <pj@illuminatedcomputing.com>
Cc: Jeff Davis <pgsql@j-davis.com>
Cc: Pgsql Hackers <pgsql-hackers@postgresql.org>
Subject: Re: range_agg
Date: Thu, 26 Sep 2019 18:13:49 -0300
Message-ID: <20190926211349.GA3375@alvherre.pgsql> (raw )
In-Reply-To: <CA+renyWOSQ5P-=KCqti4WN1hdS9h9O5kFSvXqodq49XaR43HkA@mail.gmail.com >
Hello Paul, I've started to review this patch. Here's a few minor
things I ran across -- mostly compiler warnings (is my compiler too
ancient?). You don't have to agree with every fix -- feel free to use
different fixes if you have them. Also, feel free to squash them onto
whatever commit you like (I think they all belong onto 0001 except the
last which seems to be for 0002).
Did you not push your latest version to your github repo? I pulled from
there and branch 'multirange' does not seem to match what you posted.
--
Álvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Attachments:
[text/x-diff] 0001-Remove-unused-variable.patch (1.1K, ../20190926211349.GA3375@alvherre.pgsql/2-0001-Remove-unused-variable.patch)
download | inline diff:
From 6d849beafea1e4129f044f8dd038933d6ea72b54 Mon Sep 17 00:00:00 2001
From: Alvaro Herrera <alvherre@alvh.no-ip.org>
Date: Thu, 26 Sep 2019 16:56:26 -0300
Subject: [PATCH 1/9] Remove unused variable
---
src/backend/catalog/pg_type.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/src/backend/catalog/pg_type.c b/src/backend/catalog/pg_type.c
index 08552850c8..ea5b161c4d 100644
--- a/src/backend/catalog/pg_type.c
+++ b/src/backend/catalog/pg_type.c
@@ -876,7 +876,6 @@ makeMultirangeTypeName(const char *rangeTypeName, Oid typeNamespace)
char mltrng[NAMEDATALEN];
char *mltrngunique = (char *) palloc(NAMEDATALEN);
int namelen = strlen(rangeTypeName);
- int rangelen;
char *rangestr;
int rangeoffset;
int underscores;
@@ -892,7 +891,6 @@ makeMultirangeTypeName(const char *rangeTypeName, Oid typeNamespace)
if (rangestr)
{
rangeoffset = rangestr - rangeTypeName;
- rangelen = strlen(rangestr);
strlcpy(mltrng + rangeoffset, "multi", NAMEDATALEN - rangeoffset);
strlcpy(mltrng + rangeoffset + 5, rangestr, NAMEDATALEN - rangeoffset - 5);
namelen += 5;
--
2.17.1
[text/x-diff] 0002-Silence-compiler-warning.patch (777B, ../20190926211349.GA3375@alvherre.pgsql/3-0002-Silence-compiler-warning.patch)
download | inline diff:
From 63bd9723a45c33cccd11704817fd954af5abaf31 Mon Sep 17 00:00:00 2001
From: Alvaro Herrera <alvherre@alvh.no-ip.org>
Date: Thu, 26 Sep 2019 17:39:54 -0300
Subject: [PATCH 2/9] Silence compiler warning
---
src/backend/parser/parse_coerce.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/src/backend/parser/parse_coerce.c b/src/backend/parser/parse_coerce.c
index f0b1f6f831..aab2348952 100644
--- a/src/backend/parser/parse_coerce.c
+++ b/src/backend/parser/parse_coerce.c
@@ -1919,6 +1919,8 @@ enforce_generic_type_consistency(const Oid *actual_arg_types,
format_type_be(elem_typeid))));
}
}
+ else
+ range_typelem = InvalidOid;
/* Get the range type based on the multirange type, if we have one */
if (OidIsValid(multirange_typeid))
--
2.17.1
[text/x-diff] 0003-Silence-mixed-declarations-and-code-compiler-warning.patch (1.4K, ../20190926211349.GA3375@alvherre.pgsql/4-0003-Silence-mixed-declarations-and-code-compiler-warning.patch)
download | inline diff:
From c4e4b1a97136f7a2047f8be6608cac0782dd6d12 Mon Sep 17 00:00:00 2001
From: Alvaro Herrera <alvherre@alvh.no-ip.org>
Date: Thu, 26 Sep 2019 17:42:22 -0300
Subject: [PATCH 3/9] Silence 'mixed declarations and code' compiler warning
---
src/backend/commands/typecmds.c | 8 ++------
1 file changed, 2 insertions(+), 6 deletions(-)
diff --git a/src/backend/commands/typecmds.c b/src/backend/commands/typecmds.c
index 14a6857062..26ed3e4c76 100644
--- a/src/backend/commands/typecmds.c
+++ b/src/backend/commands/typecmds.c
@@ -1782,14 +1782,10 @@ makeMultirangeConstructors(const char *name, Oid namespace,
static const char *const prosrc[2] = {"multirange_constructor0",
"multirange_constructor1"};
static const int pronargs[2] = {0, 1};
-
- Oid constructorArgTypes[0];
+ Oid constructorArgTypes = rangeArrayOid;
ObjectAddress myself,
referenced;
int i;
-
- constructorArgTypes[0] = rangeArrayOid;
-
Datum allParamTypes[1] = {ObjectIdGetDatum(rangeArrayOid)};
ArrayType *allParameterTypes = construct_array(allParamTypes, 1, OIDOID,
sizeof(Oid), true, 'i');
@@ -1808,7 +1804,7 @@ makeMultirangeConstructors(const char *name, Oid namespace,
{
oidvector *constructorArgTypesVector;
- constructorArgTypesVector = buildoidvector(constructorArgTypes,
+ constructorArgTypesVector = buildoidvector(&constructorArgTypes,
pronargs[i]);
myself = ProcedureCreate(name, /* name: same as multirange type */
--
2.17.1
[text/x-diff] 0004-Silence-mixed-declarations-and-code-compiler-warning.patch (6.5K, ../20190926211349.GA3375@alvherre.pgsql/5-0004-Silence-mixed-declarations-and-code-compiler-warning.patch)
download | inline diff:
From af414286417d4fddd078c7ffac087482b3ba959d Mon Sep 17 00:00:00 2001
From: Alvaro Herrera <alvherre@alvh.no-ip.org>
Date: Thu, 26 Sep 2019 17:49:19 -0300
Subject: [PATCH 4/9] Silence 'mixed declarations and code' compiler warnings
---
src/backend/utils/fmgr/funcapi.c | 98 +++++++++++++++++++-------------
1 file changed, 59 insertions(+), 39 deletions(-)
diff --git a/src/backend/utils/fmgr/funcapi.c b/src/backend/utils/fmgr/funcapi.c
index e6e82fda63..701664bd3b 100644
--- a/src/backend/utils/fmgr/funcapi.c
+++ b/src/backend/utils/fmgr/funcapi.c
@@ -541,18 +541,21 @@ resolve_polymorphic_tupdesc(TupleDesc tupdesc, oidvector *declared_args,
if (OidIsValid(anymultirange_type))
{
- Oid rngtype = resolve_generic_type(ANYRANGEOID,
- anymultirange_type,
- ANYMULTIRANGEOID);
+ Oid rngtype;
+ Oid subtype;
+
+ rngtype = resolve_generic_type(ANYRANGEOID,
+ anymultirange_type,
+ ANYMULTIRANGEOID);
/* check for inconsistent range and multirange results */
if (OidIsValid(anyrange_type) && anyrange_type != rngtype)
return false;
- anyrange_type = rngtype;
- Oid subtype = resolve_generic_type(ANYELEMENTOID,
- anyrange_type,
- ANYRANGEOID);
+ anyrange_type = rngtype;
+ subtype = resolve_generic_type(ANYELEMENTOID,
+ anyrange_type,
+ ANYRANGEOID);
/* check for inconsistent array and multirange results */
if (OidIsValid(anyelement_type) && anyelement_type != subtype)
@@ -596,18 +599,21 @@ resolve_polymorphic_tupdesc(TupleDesc tupdesc, oidvector *declared_args,
{
if (OidIsValid(anymultirange_type))
{
- Oid rngtype = resolve_generic_type(ANYRANGEOID,
- anymultirange_type,
- ANYMULTIRANGEOID);
+ Oid rngtype;
+ Oid subtype;
+
+ rngtype = resolve_generic_type(ANYRANGEOID,
+ anymultirange_type,
+ ANYMULTIRANGEOID);
/* check for inconsistent range and multirange results */
if (OidIsValid(anyrange_type) && anyrange_type != rngtype)
return false;
anyrange_type = rngtype;
- Oid subtype = resolve_generic_type(ANYELEMENTOID,
- anyrange_type,
- ANYRANGEOID);
+ subtype = resolve_generic_type(ANYELEMENTOID,
+ anyrange_type,
+ ANYRANGEOID);
/* check for inconsistent array and multirange results */
if (OidIsValid(anyelement_type) && anyelement_type != subtype)
@@ -628,21 +634,25 @@ resolve_polymorphic_tupdesc(TupleDesc tupdesc, oidvector *declared_args,
{
if (OidIsValid(anyrange_type))
{
- Oid subtype = resolve_generic_type(ANYELEMENTOID,
- anyrange_type,
- ANYRANGEOID);
+ Oid subtype;
+ Oid mltrngtype;
+ Oid rngtype;
+
+ subtype = resolve_generic_type(ANYELEMENTOID,
+ anyrange_type,
+ ANYRANGEOID);
/* check for inconsistent array and range results */
if (OidIsValid(anyelement_type) && anyelement_type != subtype)
return false;
anyelement_type = subtype;
- Oid mltrngtype = resolve_generic_type(ANYMULTIRANGEOID,
- anyrange_type,
- ANYRANGEOID);
+ mltrngtype = resolve_generic_type(ANYMULTIRANGEOID,
+ anyrange_type,
+ ANYRANGEOID);
/* check for inconsistent range and multirange results */
- Oid rngtype = get_multirange_subtype(mltrngtype);
+ rngtype = get_multirange_subtype(mltrngtype);
if (OidIsValid(anyrange_type) && anyrange_type != rngtype)
return false;
@@ -868,18 +878,21 @@ resolve_polymorphic_argtypes(int numargs, Oid *argtypes, char *argmodes,
if (OidIsValid(anymultirange_type))
{
- Oid rngtype = resolve_generic_type(ANYRANGEOID,
- anymultirange_type,
- ANYMULTIRANGEOID);
+ Oid rngtype;
+ Oid subtype;
+
+ rngtype = resolve_generic_type(ANYRANGEOID,
+ anymultirange_type,
+ ANYMULTIRANGEOID);
/* check for inconsistent range and multirange results */
if (OidIsValid(anyrange_type) && anyrange_type != rngtype)
return false;
anyrange_type = rngtype;
- Oid subtype = resolve_generic_type(ANYELEMENTOID,
- anyrange_type,
- ANYRANGEOID);
+ subtype = resolve_generic_type(ANYELEMENTOID,
+ anyrange_type,
+ ANYRANGEOID);
/* check for inconsistent array and multirange results */
if (OidIsValid(anyelement_type) && anyelement_type != subtype)
@@ -923,18 +936,21 @@ resolve_polymorphic_argtypes(int numargs, Oid *argtypes, char *argmodes,
{
if (OidIsValid(anymultirange_type))
{
- Oid rngtype = resolve_generic_type(ANYRANGEOID,
- anymultirange_type,
- ANYMULTIRANGEOID);
+ Oid rngtype;
+ Oid subtype;
+
+ rngtype = resolve_generic_type(ANYRANGEOID,
+ anymultirange_type,
+ ANYMULTIRANGEOID);
/* check for inconsistent range and multirange results */
if (OidIsValid(anyrange_type) && anyrange_type != rngtype)
return false;
anyrange_type = rngtype;
- Oid subtype = resolve_generic_type(ANYELEMENTOID,
- anyrange_type,
- ANYRANGEOID);
+ subtype = resolve_generic_type(ANYELEMENTOID,
+ anyrange_type,
+ ANYRANGEOID);
/* check for inconsistent array and multirange results */
if (OidIsValid(anyelement_type) && anyelement_type != subtype)
@@ -955,21 +971,25 @@ resolve_polymorphic_argtypes(int numargs, Oid *argtypes, char *argmodes,
{
if (OidIsValid(anyrange_type))
{
- Oid subtype = resolve_generic_type(ANYELEMENTOID,
- anyrange_type,
- ANYRANGEOID);
+ Oid subtype;
+ Oid mltrngtype;
+ Oid rngtype;
+
+ subtype = resolve_generic_type(ANYELEMENTOID,
+ anyrange_type,
+ ANYRANGEOID);
/* check for inconsistent array and range results */
if (OidIsValid(anyelement_type) && anyelement_type != subtype)
return false;
anyelement_type = subtype;
- Oid mltrngtype = resolve_generic_type(ANYMULTIRANGEOID,
- anyrange_type,
- ANYRANGEOID);
+ mltrngtype = resolve_generic_type(ANYMULTIRANGEOID,
+ anyrange_type,
+ ANYRANGEOID);
/* check for inconsistent range and multirange results */
- Oid rngtype = get_multirange_subtype(mltrngtype);
+ rngtype = get_multirange_subtype(mltrngtype);
if (OidIsValid(anyrange_type) && anyrange_type != rngtype)
return false;
--
2.17.1
[text/x-diff] 0005-Silence-mixed-declarations-and-code-compiler-warning.patch (2.4K, ../20190926211349.GA3375@alvherre.pgsql/6-0005-Silence-mixed-declarations-and-code-compiler-warning.patch)
download | inline diff:
From 43c82181760ad982cb6958817f34cb80f39ffd8c Mon Sep 17 00:00:00 2001
From: Alvaro Herrera <alvherre@alvh.no-ip.org>
Date: Thu, 26 Sep 2019 17:51:02 -0300
Subject: [PATCH 5/9] Silence 'mixed declarations and code' compiler warnings
---
src/backend/utils/adt/multirangetypes.c | 25 ++++++++++++++-----------
1 file changed, 14 insertions(+), 11 deletions(-)
diff --git a/src/backend/utils/adt/multirangetypes.c b/src/backend/utils/adt/multirangetypes.c
index c509994796..359b78d056 100644
--- a/src/backend/utils/adt/multirangetypes.c
+++ b/src/backend/utils/adt/multirangetypes.c
@@ -351,9 +351,12 @@ multirange_send(PG_FUNCTION_ARGS)
for (i = 0; i < range_count; i++)
{
Datum range = RangeTypePGetDatum(ranges[i]);
+ uint32 range_len;
+ char *range_data;
+
range = PointerGetDatum(SendFunctionCall(&cache->proc, range));
- uint32 range_len = VARSIZE(range) - VARHDRSZ;
- char *range_data = VARDATA(range);
+ range_len = VARSIZE(range) - VARHDRSZ;
+ *range_data = VARDATA(range);
pq_sendint32(buf, range_len);
pq_sendbytes(buf, range_data, range_len);
@@ -2039,12 +2042,12 @@ bool
range_before_multirange_internal(TypeCacheEntry *typcache, RangeType *r,
MultirangeType * mr)
{
- if (RangeIsEmpty(r) || MultirangeIsEmpty(mr))
- return false;
-
int32 range_count;
RangeType **ranges;
+ if (RangeIsEmpty(r) || MultirangeIsEmpty(mr))
+ return false;
+
multirange_deserialize(mr, &range_count, &ranges);
return range_before_internal(typcache->rngtype, r, ranges[0]);
@@ -2054,14 +2057,14 @@ bool
multirange_before_multirange_internal(TypeCacheEntry *typcache, MultirangeType * mr1,
MultirangeType * mr2)
{
- if (MultirangeIsEmpty(mr1) || MultirangeIsEmpty(mr2))
- return false;
-
int32 range_count1;
int32 range_count2;
RangeType **ranges1;
RangeType **ranges2;
+ if (MultirangeIsEmpty(mr1) || MultirangeIsEmpty(mr2))
+ return false;
+
multirange_deserialize(mr1, &range_count1, &ranges1);
multirange_deserialize(mr2, &range_count2, &ranges2);
@@ -2074,12 +2077,12 @@ bool
range_after_multirange_internal(TypeCacheEntry *typcache, RangeType *r,
MultirangeType * mr)
{
- if (RangeIsEmpty(r) || MultirangeIsEmpty(mr))
- return false;
-
int32 range_count;
RangeType **ranges;
+ if (RangeIsEmpty(r) || MultirangeIsEmpty(mr))
+ return false;
+
multirange_deserialize(mr, &range_count, &ranges);
return range_after_internal(typcache->rngtype, r, ranges[range_count - 1]);
--
2.17.1
[text/x-diff] 0006-silence-variable-set-but-not-used-compiler-warning.patch (1.1K, ../20190926211349.GA3375@alvherre.pgsql/7-0006-silence-variable-set-but-not-used-compiler-warning.patch)
download | inline diff:
From a735dadde028785020b71e8aac40fc717f6c765c Mon Sep 17 00:00:00 2001
From: Alvaro Herrera <alvherre@alvh.no-ip.org>
Date: Thu, 26 Sep 2019 17:51:33 -0300
Subject: [PATCH 6/9] silence 'variable set but not used' compiler warning
---
src/backend/utils/adt/multirangetypes.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/src/backend/utils/adt/multirangetypes.c b/src/backend/utils/adt/multirangetypes.c
index 359b78d056..cea7e7f97e 100644
--- a/src/backend/utils/adt/multirangetypes.c
+++ b/src/backend/utils/adt/multirangetypes.c
@@ -91,7 +91,6 @@ multirange_in(PG_FUNCTION_ARGS)
Oid mltrngtypoid = PG_GETARG_OID(1);
Oid typmod = PG_GETARG_INT32(2);
TypeCacheEntry *rangetyp;
- Oid rngtypoid;
int32 ranges_seen = 0;
int32 range_count = 0;
int32 range_capacity = 8;
@@ -107,7 +106,6 @@ multirange_in(PG_FUNCTION_ARGS)
cache = get_multirange_io_data(fcinfo, mltrngtypoid, IOFunc_input);
rangetyp = cache->typcache->rngtype;
- rngtypoid = rangetyp->type_id;
/* consume whitespace */
while (*ptr != '\0' && isspace((unsigned char) *ptr))
--
2.17.1
[text/x-diff] 0007-Fix-uninitialized-variable-warning.patch (798B, ../20190926211349.GA3375@alvherre.pgsql/8-0007-Fix-uninitialized-variable-warning.patch)
download | inline diff:
From 670f80824c92dd9b36f823110182339dd8fc85b8 Mon Sep 17 00:00:00 2001
From: Alvaro Herrera <alvherre@alvh.no-ip.org>
Date: Thu, 26 Sep 2019 18:00:29 -0300
Subject: [PATCH 7/9] Fix uninitialized variable warning
---
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 cea7e7f97e..ba7c2a23ae 100644
--- a/src/backend/utils/adt/multirangetypes.c
+++ b/src/backend/utils/adt/multirangetypes.c
@@ -100,7 +100,7 @@ multirange_in(PG_FUNCTION_ARGS)
MultirangeType *ret;
MultirangeParseState parse_state;
const char *ptr = input_str;
- const char *range_str;
+ const char *range_str = NULL;
int32 range_str_len;
char *range_str_copy;
--
2.17.1
[text/x-diff] 0008-Fix-wrong-type-assignment-warning.patch (855B, ../20190926211349.GA3375@alvherre.pgsql/9-0008-Fix-wrong-type-assignment-warning.patch)
download | inline diff:
From edc5171168c804fe6127cb0ce51c5fad9a6cbb46 Mon Sep 17 00:00:00 2001
From: Alvaro Herrera <alvherre@alvh.no-ip.org>
Date: Thu, 26 Sep 2019 18:00:44 -0300
Subject: [PATCH 8/9] Fix wrong-type assignment warning
---
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 ba7c2a23ae..c85f5dc90d 100644
--- a/src/backend/utils/adt/multirangetypes.c
+++ b/src/backend/utils/adt/multirangetypes.c
@@ -354,7 +354,7 @@ multirange_send(PG_FUNCTION_ARGS)
range = PointerGetDatum(SendFunctionCall(&cache->proc, range));
range_len = VARSIZE(range) - VARHDRSZ;
- *range_data = VARDATA(range);
+ range_data = VARDATA(range);
pq_sendint32(buf, range_len);
pq_sendbytes(buf, range_data, range_len);
--
2.17.1
[text/x-diff] 0009-Fix-crash-in-multirange_intersect_multirange_interna.patch (1.3K, ../20190926211349.GA3375@alvherre.pgsql/10-0009-Fix-crash-in-multirange_intersect_multirange_interna.patch)
download | inline diff:
From 1479908cd0df9cc77968dbb0def484f59ad9d64c Mon Sep 17 00:00:00 2001
From: Alvaro Herrera <alvherre@alvh.no-ip.org>
Date: Thu, 26 Sep 2019 18:01:04 -0300
Subject: [PATCH 9/9] Fix crash in multirange_intersect_multirange_internal
---
src/backend/utils/adt/multirangetypes.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/src/backend/utils/adt/multirangetypes.c b/src/backend/utils/adt/multirangetypes.c
index c85f5dc90d..7717acb3b1 100644
--- a/src/backend/utils/adt/multirangetypes.c
+++ b/src/backend/utils/adt/multirangetypes.c
@@ -1123,7 +1123,8 @@ multirange_intersect_multirange(PG_FUNCTION_ARGS)
MultirangeType *
multirange_intersect_multirange_internal(Oid mltrngtypoid, TypeCacheEntry *rangetyp,
- int32 range_count1, RangeType **ranges1, int32 range_count2, RangeType **ranges2)
+ int32 range_count1, RangeType **ranges1,
+ int32 range_count2, RangeType **ranges2)
{
RangeType *r1;
RangeType *r2;
@@ -1132,6 +1133,9 @@ multirange_intersect_multirange_internal(Oid mltrngtypoid, TypeCacheEntry *range
int32 i1;
int32 i2;
+ if (range_count1 == 0 || range_count2 == 0)
+ return make_multirange(mltrngtypoid, rangetyp, 0, NULL);
+
/*
* Worst case is a stitching pattern like this:
*
--
2.17.1
view thread (152+ messages) latest in thread
Message-ID: <20190926211349.GA3375@alvherre.pgsql>
Permalink: ../20190926211349.GA3375@alvherre.pgsql/
Also on: postgresql.org/message-id/20190926211349.GA3375@alvherre.pgsql
copy link · copy postgr.es · copy git am
reply Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: alvherre@2ndquadrant.com, pj@illuminatedcomputing.com, pgsql@j-davis.com
Subject: Re: range_agg
In-Reply-To: <20190926211349.GA3375@alvherre.pgsql>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox