From 716ea626a88ac510523ab3af5bc779d78eeced58 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Mon, 25 Sep 2017 11:55:24 -0400 Subject: [PATCH] Make construct_[md_]array return a valid empty array for zero-size input. If construct_array() or construct_md_array() were given a dimension of zero, they'd produce an array that contains no elements but has positive dimension. This violates a general expectation that empty arrays should have ndims = 0; in particular, while arrays like this print as empty, they don't compare equal to other empty arrays. Up to now we've expected callers to avoid making such calls and instead be careful to call construct_empty_array() if there would be no elements. But this has always been an easily missed case, and we've repeatedly had to fix callers to do it right. In bug #14826, Erwin Brandstetter pointed out yet another such oversight, in ts_lexize(); and a bit of examination of other call sites found at least two more with similar issues. So let's fix the problem centrally and permanently by changing these two functions to construct a proper zero-D empty array whenever the array would be empty. This renders a few explicit calls of construct_empty_array() redundant, but the only such place I found that really seemed worth changing was in ExecEvalArrayExpr(). Although this fixes some very old bugs, no back-patch: the problem is pretty minor and the risk of changing behavior seems to outweigh the benefit in stable branches. Discussion: https://postgr.es/m/20170923125723.1448.39412@wrigleys.postgresql.org Discussion: https://postgr.es/m/20570.1506198383@sss.pgh.pa.us --- src/backend/executor/execExprInterp.c | 8 -------- src/backend/utils/adt/arrayfuncs.c | 10 ++++++---- src/test/regress/expected/tsearch.out | 11 +++++++++++ src/test/regress/sql/tsearch.sql | 4 ++++ 4 files changed, 21 insertions(+), 12 deletions(-) diff --git a/src/backend/executor/execExprInterp.c b/src/backend/executor/execExprInterp.c index bd8a15d6c3..09abd46dda 100644 --- a/src/backend/executor/execExprInterp.c +++ b/src/backend/executor/execExprInterp.c @@ -2131,14 +2131,6 @@ ExecEvalArrayExpr(ExprState *state, ExprEvalStep *op) Datum *dvalues = op->d.arrayexpr.elemvalues; bool *dnulls = op->d.arrayexpr.elemnulls; - /* Shouldn't happen here, but if length is 0, return empty array */ - if (nelems == 0) - { - *op->resvalue = - PointerGetDatum(construct_empty_array(element_type)); - return; - } - /* setup for 1-D array of the given length */ ndims = 1; dims[0] = nelems; diff --git a/src/backend/utils/adt/arrayfuncs.c b/src/backend/utils/adt/arrayfuncs.c index e4101c9af0..d1f2fe7d95 100644 --- a/src/backend/utils/adt/arrayfuncs.c +++ b/src/backend/utils/adt/arrayfuncs.c @@ -3297,6 +3297,7 @@ array_map(FunctionCallInfo fcinfo, Oid retType, ArrayMapState *amstate) * * A palloc'd 1-D array object is constructed and returned. Note that * elem values will be copied into the object even if pass-by-ref type. + * Also note the result will be 0-D not 1-D if nelems = 0. * * NOTE: it would be cleaner to look up the elmlen/elmbval/elmalign info * from the system catalogs, given the elmtype. However, the caller is @@ -3331,6 +3332,7 @@ construct_array(Datum *elems, int nelems, * * A palloc'd ndims-D array object is constructed and returned. Note that * elem values will be copied into the object even if pass-by-ref type. + * Also note the result will be 0-D not ndims-D if any dims[i] = 0. * * NOTE: it would be cleaner to look up the elmlen/elmbval/elmalign info * from the system catalogs, given the elmtype. However, the caller is @@ -3362,12 +3364,12 @@ construct_md_array(Datum *elems, errmsg("number of array dimensions (%d) exceeds the maximum allowed (%d)", ndims, MAXDIM))); - /* fast track for empty array */ - if (ndims == 0) - return construct_empty_array(elmtype); - nelems = ArrayGetNItems(ndims, dims); + /* if ndims <= 0 or any dims[i] == 0, return empty array */ + if (nelems <= 0) + return construct_empty_array(elmtype); + /* compute required space */ nbytes = 0; hasnulls = false; diff --git a/src/test/regress/expected/tsearch.out b/src/test/regress/expected/tsearch.out index b2fc9e207e..d63fb12f1d 100644 --- a/src/test/regress/expected/tsearch.out +++ b/src/test/regress/expected/tsearch.out @@ -618,6 +618,17 @@ SELECT * from ts_debug('english', '5aew.werc.ewr:8100/?xx'); url_path | URL path | /?xx | {simple} | simple | {/?xx} (3 rows) +SELECT token, alias, + dictionaries, dictionaries is null as dnull, array_dims(dictionaries) as ddims, + lexemes, lexemes is null as lnull, array_dims(lexemes) as ldims +from ts_debug('english', 'a title'); + token | alias | dictionaries | dnull | ddims | lexemes | lnull | ldims +-------+-----------+----------------+-------+-------+---------+-------+------- + a | asciiword | {english_stem} | f | [1:1] | {} | f | + | blank | {} | f | | | t | + title | asciiword | {english_stem} | f | [1:1] | {titl} | f | [1:1] +(3 rows) + -- to_tsquery SELECT to_tsquery('english', 'qwe & sKies '); to_tsquery diff --git a/src/test/regress/sql/tsearch.sql b/src/test/regress/sql/tsearch.sql index e4b21f8f18..1c8520b3e9 100644 --- a/src/test/regress/sql/tsearch.sql +++ b/src/test/regress/sql/tsearch.sql @@ -145,6 +145,10 @@ SELECT * from ts_debug('english', 'http://www.harewoodsolutions.co.uk/press.aspx SELECT * from ts_debug('english', 'http://aew.wer0c.ewr/id?ad=qwe&dw'); SELECT * from ts_debug('english', 'http://5aew.werc.ewr:8100/?'); SELECT * from ts_debug('english', '5aew.werc.ewr:8100/?xx'); +SELECT token, alias, + dictionaries, dictionaries is null as dnull, array_dims(dictionaries) as ddims, + lexemes, lexemes is null as lnull, array_dims(lexemes) as ldims +from ts_debug('english', 'a title'); -- to_tsquery -- 2.40.0