agora inbox for [email protected]  
help / color / mirror / Atom feed
[PATCH 3/9] Silence 'mixed declarations and code' compiler warning
6+ messages / 3 participants
[nested] [flat]

* [PATCH 3/9] Silence 'mixed declarations and code' compiler warning
@ 2019-09-26 20:42 Alvaro Herrera <[email protected]>
  0 siblings, 0 replies; 6+ messages in thread

From: Alvaro Herrera @ 2019-09-26 20:42 UTC (permalink / raw)

---
 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


--G4iJoqBmSsgzjUCe
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
 filename="0004-Silence-mixed-declarations-and-code-compiler-warning.patch"



^ permalink  raw  reply  [nested|flat] 6+ messages in thread

* [PATCH 4/9] Silence 'mixed declarations and code' compiler warnings
@ 2019-09-26 20:49 Alvaro Herrera <[email protected]>
  0 siblings, 0 replies; 6+ messages in thread

From: Alvaro Herrera @ 2019-09-26 20:49 UTC (permalink / raw)

---
 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


--G4iJoqBmSsgzjUCe
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
 filename="0005-Silence-mixed-declarations-and-code-compiler-warning.patch"



^ permalink  raw  reply  [nested|flat] 6+ messages in thread

* [PATCH 5/9] Silence 'mixed declarations and code' compiler warnings
@ 2019-09-26 20:51 Alvaro Herrera <[email protected]>
  0 siblings, 0 replies; 6+ messages in thread

From: Alvaro Herrera @ 2019-09-26 20:51 UTC (permalink / raw)

---
 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


--G4iJoqBmSsgzjUCe
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
 filename="0006-silence-variable-set-but-not-used-compiler-warning.patch"



^ permalink  raw  reply  [nested|flat] 6+ messages in thread

* Re: Stability of queryid in minor versions
@ 2024-04-14 23:47 Peter Geoghegan <[email protected]>
  2024-04-15 01:00 ` Re: Stability of queryid in minor versions David Rowley <[email protected]>
  0 siblings, 1 reply; 6+ messages in thread

From: Peter Geoghegan @ 2024-04-14 23:47 UTC (permalink / raw)
  To: David Rowley <[email protected]>; +Cc: PostgreSQL Developers <[email protected]>

On Sun, Apr 14, 2024 at 7:20 PM David Rowley <[email protected]> wrote:
> It's the "underlying server version" that I think needs some
> clarification. It's unclear if the minor version must match or just
> the major version number. The preceding paragraph does mention:
>
> "Furthermore, it is not safe to assume that queryid will be stable
> across major versions of PostgreSQL."
>
> but not stable across *major* versions does *not* mean stable across
> *minor* versions. The reader is just left guessing if that's true.

Technically we don't promise that WAL records won't change in minor
versions. In fact, the docs specifically state that the format of any
WAL record might change, and that users should upgrade standbys first
on general principle (though I imagine few do). We try hard to avoid
changing the format of WAL records in point releases, of course, but
strictly speaking there is no guarantee. This situation seems similar
(though much lower stakes) to me. Query normalization isn't perfect --
there's a trade-off.

> Maybe the paragraph starting with "Consumers of" can detail the
> reasons queryid might be unstable and the following paragraph can
> describe the scenario for when the queryid can generally assumed to be
> stable.

I think that it would be reasonable to say that we strive to not break
the format in point releases. Fundamentally, if pg_stat_statements
sees a hard queryid format change (e.g. due to a major version
upgrade), then pg_stat_statements throws away the accumulated query
stats without being asked to.

-- 
Peter Geoghegan






^ permalink  raw  reply  [nested|flat] 6+ messages in thread

* Re: Stability of queryid in minor versions
  2024-04-14 23:47 Re: Stability of queryid in minor versions Peter Geoghegan <[email protected]>
@ 2024-04-15 01:00 ` David Rowley <[email protected]>
  2024-04-15 01:11   ` Re: Stability of queryid in minor versions Peter Geoghegan <[email protected]>
  0 siblings, 1 reply; 6+ messages in thread

From: David Rowley @ 2024-04-15 01:00 UTC (permalink / raw)
  To: Peter Geoghegan <[email protected]>; +Cc: PostgreSQL Developers <[email protected]>

On Mon, 15 Apr 2024 at 11:47, Peter Geoghegan <[email protected]> wrote:
>
> On Sun, Apr 14, 2024 at 7:20 PM David Rowley <[email protected]> wrote:
> > It's the "underlying server version" that I think needs some
> > clarification. It's unclear if the minor version must match or just
> > the major version number. The preceding paragraph does mention:
> >
> > "Furthermore, it is not safe to assume that queryid will be stable
> > across major versions of PostgreSQL."
> >
> > but not stable across *major* versions does *not* mean stable across
> > *minor* versions. The reader is just left guessing if that's true.
>
> Technically we don't promise that WAL records won't change in minor
> versions. In fact, the docs specifically state that the format of any
> WAL record might change, and that users should upgrade standbys first
> on general principle (though I imagine few do). We try hard to avoid
> changing the format of WAL records in point releases, of course, but
> strictly speaking there is no guarantee. This situation seems similar
> (though much lower stakes) to me. Query normalization isn't perfect --
> there's a trade-off.

set compute_query_id = 'on';
explain (costs off, verbose) select oid from pg_class;
                           QUERY PLAN
-----------------------------------------------------------------
 Index Only Scan using pg_class_oid_index on pg_catalog.pg_class
   Output: oid
 Query Identifier: -8748805461085747951
(3 rows)

As far as I understand query ID; it's based on the parse nodes and
values in the system catalogue tables and is calculated on the local
server. Computed values are susceptible to variations in hash values
calculated by different CPU architectures.

Where does WAL fit into this? And why would a WAL format change the
computed value?

David






^ permalink  raw  reply  [nested|flat] 6+ messages in thread

* Re: Stability of queryid in minor versions
  2024-04-14 23:47 Re: Stability of queryid in minor versions Peter Geoghegan <[email protected]>
  2024-04-15 01:00 ` Re: Stability of queryid in minor versions David Rowley <[email protected]>
@ 2024-04-15 01:11   ` Peter Geoghegan <[email protected]>
  0 siblings, 0 replies; 6+ messages in thread

From: Peter Geoghegan @ 2024-04-15 01:11 UTC (permalink / raw)
  To: David Rowley <[email protected]>; +Cc: PostgreSQL Developers <[email protected]>

On Sun, Apr 14, 2024 at 9:01 PM David Rowley <[email protected]> wrote:
> On Mon, 15 Apr 2024 at 11:47, Peter Geoghegan <[email protected]> wrote:
> > Technically we don't promise that WAL records won't change in minor
> > versions. In fact, the docs specifically state that the format of any
> > WAL record might change, and that users should upgrade standbys first
> > on general principle (though I imagine few do). We try hard to avoid
> > changing the format of WAL records in point releases, of course, but
> > strictly speaking there is no guarantee. This situation seems similar
> > (though much lower stakes) to me. Query normalization isn't perfect --
> > there's a trade-off.

> Where does WAL fit into this? And why would a WAL format change the
> computed value?

It doesn't. I just compared the two situations, which seem analogous.

-- 
Peter Geoghegan






^ permalink  raw  reply  [nested|flat] 6+ messages in thread


end of thread, other threads:[~2024-04-15 01:11 UTC | newest]

Thread overview: 6+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2019-09-26 20:42 [PATCH 3/9] Silence 'mixed declarations and code' compiler warning Alvaro Herrera <[email protected]>
2019-09-26 20:49 [PATCH 4/9] Silence 'mixed declarations and code' compiler warnings Alvaro Herrera <[email protected]>
2019-09-26 20:51 [PATCH 5/9] Silence 'mixed declarations and code' compiler warnings Alvaro Herrera <[email protected]>
2024-04-14 23:47 Re: Stability of queryid in minor versions Peter Geoghegan <[email protected]>
2024-04-15 01:00 ` Re: Stability of queryid in minor versions David Rowley <[email protected]>
2024-04-15 01:11   ` Re: Stability of queryid in minor versions Peter Geoghegan <[email protected]>

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox