Hi, On Fri, Oct 11, 2024 at 05:43:04PM -0700, Masahiko Sawada wrote: > Hi, > > On Thu, Oct 10, 2024 at 10:37 PM Bertrand Drouvot > <bertranddrouvot...@gmail.com> wrote: > > > > Hi hackers, > > > > While working on [1], I noticed that we missed using > > deconstruct_array_builtin() > > in 062a8444242. > > > > Indeed, d746021de1 added construct_array_builtin and > > deconstruct_array_builtin > > but , later on, 062a8444242 made use of deconstruct_array for TEXTOID. > > > > Please find attached a tiny patch to add the $SUBJECT. > > > > That does not fix any issues, it just removes this unnecessary hardcoded > > parameters related to TEXTOID passed to deconstruct_array. > > +1 for better consistency and simplicity. > > > > > A quick check: > > > > $ git grep construct_array | grep OID | grep -v builtin > p> src/backend/catalog/pg_publication.c: > deconstruct_array(arr, TEXTOID, -1, false, TYPALIGN_INT, > > src/backend/utils/fmgr/funcapi.c: * For the OID and char arrays, we > > don't need to use deconstruct_array() > > > > shows that this is the "only" miss since d746021de1, so I still don't think > > it > > has to be more "complicated" than it is currently (as already mentioned by > > Peter > > in [2]). > > It seems that src/backend/utils/adt/float.c file still has functions > that can use [de]construct_array_builtin(), for example > float8_combine(). I think it's an oversight of d746021de1.
Thanks for looking at it. Good catch, please find attached v2 taking care of those. I looked at the remaining [de]construct_array() usages and that looks ok to me. Please note that v1 has been committed (099c572d33). Regards, -- Bertrand Drouvot PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services: https://aws.amazon.com
>From 7227bc20a7d3863200129c13750089f642032332 Mon Sep 17 00:00:00 2001 From: Bertrand Drouvot <bertranddrouvot...@gmail.com> Date: Sun, 13 Oct 2024 16:14:01 +0000 Subject: [PATCH v2] construct_array_builtin usage for FLOAT8OID Commit d746021de1 introduced use of construct_array_builtin for built-in types instead of construct_array but forgot some replacements linked to FLOAT8OID. --- src/backend/utils/adt/arrayfuncs.c | 6 ++++++ src/backend/utils/adt/float.c | 20 +++++--------------- 2 files changed, 11 insertions(+), 15 deletions(-) 100.0% src/backend/utils/adt/ diff --git a/src/backend/utils/adt/arrayfuncs.c b/src/backend/utils/adt/arrayfuncs.c index e5c7e57a5d..1640d83885 100644 --- a/src/backend/utils/adt/arrayfuncs.c +++ b/src/backend/utils/adt/arrayfuncs.c @@ -3404,6 +3404,12 @@ construct_array_builtin(Datum *elems, int nelems, Oid elmtype) elmalign = TYPALIGN_INT; break; + case FLOAT8OID: + elmlen = sizeof(float8); + elmbyval = FLOAT8PASSBYVAL; + elmalign = TYPALIGN_DOUBLE; + break; + case INT2OID: elmlen = sizeof(int16); elmbyval = true; diff --git a/src/backend/utils/adt/float.c b/src/backend/utils/adt/float.c index f709c21e1f..6fa6ffb51f 100644 --- a/src/backend/utils/adt/float.c +++ b/src/backend/utils/adt/float.c @@ -2946,9 +2946,7 @@ float8_combine(PG_FUNCTION_ARGS) transdatums[1] = Float8GetDatumFast(Sx); transdatums[2] = Float8GetDatumFast(Sxx); - result = construct_array(transdatums, 3, - FLOAT8OID, - sizeof(float8), FLOAT8PASSBYVAL, TYPALIGN_DOUBLE); + result = construct_array_builtin(transdatums, 3, FLOAT8OID); PG_RETURN_ARRAYTYPE_P(result); } @@ -3029,9 +3027,7 @@ float8_accum(PG_FUNCTION_ARGS) transdatums[1] = Float8GetDatumFast(Sx); transdatums[2] = Float8GetDatumFast(Sxx); - result = construct_array(transdatums, 3, - FLOAT8OID, - sizeof(float8), FLOAT8PASSBYVAL, TYPALIGN_DOUBLE); + result = construct_array_builtin(transdatums, 3, FLOAT8OID); PG_RETURN_ARRAYTYPE_P(result); } @@ -3114,9 +3110,7 @@ float4_accum(PG_FUNCTION_ARGS) transdatums[1] = Float8GetDatumFast(Sx); transdatums[2] = Float8GetDatumFast(Sxx); - result = construct_array(transdatums, 3, - FLOAT8OID, - sizeof(float8), FLOAT8PASSBYVAL, TYPALIGN_DOUBLE); + result = construct_array_builtin(transdatums, 3, FLOAT8OID); PG_RETURN_ARRAYTYPE_P(result); } @@ -3359,9 +3353,7 @@ float8_regr_accum(PG_FUNCTION_ARGS) transdatums[4] = Float8GetDatumFast(Syy); transdatums[5] = Float8GetDatumFast(Sxy); - result = construct_array(transdatums, 6, - FLOAT8OID, - sizeof(float8), FLOAT8PASSBYVAL, TYPALIGN_DOUBLE); + result = construct_array_builtin(transdatums, 6, FLOAT8OID); PG_RETURN_ARRAYTYPE_P(result); } @@ -3500,9 +3492,7 @@ float8_regr_combine(PG_FUNCTION_ARGS) transdatums[4] = Float8GetDatumFast(Syy); transdatums[5] = Float8GetDatumFast(Sxy); - result = construct_array(transdatums, 6, - FLOAT8OID, - sizeof(float8), FLOAT8PASSBYVAL, TYPALIGN_DOUBLE); + result = construct_array_builtin(transdatums, 6, FLOAT8OID); PG_RETURN_ARRAYTYPE_P(result); } -- 2.34.1
