From f755a152d4e3e6f913c6b7b6afe1785b1171c1cb Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Mon, 21 May 2018 11:41:42 -0400 Subject: [PATCH] Improve spelling of new FINALFUNC_MODIFY aggregate attribute. I'd used SHARABLE as a value originally, but Peter Eisentraut points out that dictionaries agree that SHAREABLE is the preferred spelling. Run around and change that before it's too late. Discussion: https://postgr.es/m/d2e1afd4-659c-50d6-1b20-7cfd3675e909@2ndquadrant.com --- doc/src/sgml/ref/create_aggregate.sgml | 19 ++++++++-------- doc/src/sgml/xaggr.sgml | 4 ++-- src/backend/commands/aggregatecmds.c | 6 ++--- src/backend/executor/nodeAgg.c | 22 +++++++++---------- src/bin/pg_dump/pg_dump.c | 8 +++---- src/bin/pg_dump/t/002_pg_dump.pl | 4 ++-- src/include/catalog/pg_aggregate.h | 2 +- src/include/executor/nodeAgg.h | 4 ++-- .../regress/expected/create_aggregate.out | 2 +- src/test/regress/sql/create_aggregate.sql | 2 +- 10 files changed, 37 insertions(+), 36 deletions(-) diff --git a/doc/src/sgml/ref/create_aggregate.sgml b/doc/src/sgml/ref/create_aggregate.sgml index a4aaae876e..b8cd2e7af9 100644 --- a/doc/src/sgml/ref/create_aggregate.sgml +++ b/doc/src/sgml/ref/create_aggregate.sgml @@ -27,7 +27,7 @@ CREATE AGGREGATE name ( [ state_data_size ] [ , FINALFUNC = ffunc ] [ , FINALFUNC_EXTRA ] - [ , FINALFUNC_MODIFY = { READ_ONLY | SHARABLE | READ_WRITE } ] + [ , FINALFUNC_MODIFY = { READ_ONLY | SHAREABLE | READ_WRITE } ] [ , COMBINEFUNC = combinefunc ] [ , SERIALFUNC = serialfunc ] [ , DESERIALFUNC = deserialfunc ] @@ -38,7 +38,7 @@ CREATE AGGREGATE name ( [ mstate_data_size ] [ , MFINALFUNC = mffunc ] [ , MFINALFUNC_EXTRA ] - [ , MFINALFUNC_MODIFY = { READ_ONLY | SHARABLE | READ_WRITE } ] + [ , MFINALFUNC_MODIFY = { READ_ONLY | SHAREABLE | READ_WRITE } ] [ , MINITCOND = minitial_condition ] [ , SORTOP = sort_operator ] [ , PARALLEL = { SAFE | RESTRICTED | UNSAFE } ] @@ -51,7 +51,7 @@ CREATE AGGREGATE name ( [ [ state_data_size ] [ , FINALFUNC = ffunc ] [ , FINALFUNC_EXTRA ] - [ , FINALFUNC_MODIFY = { READ_ONLY | SHARABLE | READ_WRITE } ] + [ , FINALFUNC_MODIFY = { READ_ONLY | SHAREABLE | READ_WRITE } ] [ , INITCOND = initial_condition ] [ , PARALLEL = { SAFE | RESTRICTED | UNSAFE } ] [ , HYPOTHETICAL ] @@ -66,7 +66,7 @@ CREATE AGGREGATE name ( [ , SSPACE = state_data_size ] [ , FINALFUNC = ffunc ] [ , FINALFUNC_EXTRA ] - [ , FINALFUNC_MODIFY = { READ_ONLY | SHARABLE | READ_WRITE } ] + [ , FINALFUNC_MODIFY = { READ_ONLY | SHAREABLE | READ_WRITE } ] [ , COMBINEFUNC = combinefunc ] [ , SERIALFUNC = serialfunc ] [ , DESERIALFUNC = deserialfunc ] @@ -77,7 +77,7 @@ CREATE AGGREGATE name ( [ , MSSPACE = mstate_data_size ] [ , MFINALFUNC = mffunc ] [ , MFINALFUNC_EXTRA ] - [ , MFINALFUNC_MODIFY = { READ_ONLY | SHARABLE | READ_WRITE } ] + [ , MFINALFUNC_MODIFY = { READ_ONLY | SHAREABLE | READ_WRITE } ] [ , MINITCOND = minitial_condition ] [ , SORTOP = sort_operator ] ) @@ -419,7 +419,7 @@ SELECT col FROM tab ORDER BY col USING sortop LIMIT 1; - FINALFUNC_MODIFY = { READ_ONLY | SHARABLE | READ_WRITE } + FINALFUNC_MODIFY = { READ_ONLY | SHAREABLE | READ_WRITE } This option specifies whether the final function is a pure function @@ -585,7 +585,7 @@ SELECT col FROM tab ORDER BY col USING sortop LIMIT 1; - MFINALFUNC_MODIFY = { READ_ONLY | SHARABLE | READ_WRITE } + MFINALFUNC_MODIFY = { READ_ONLY | SHAREABLE | READ_WRITE } This option is like FINALFUNC_MODIFY, but it describes @@ -678,12 +678,13 @@ SELECT col FROM tab ORDER BY col USING sortop LIMIT 1; Likewise, while an aggregate final function is normally expected not to modify its input values, sometimes it is impractical to avoid modifying the transition-state argument. Such behavior must be declared using - the FINALFUNC_MODIFY parameter. The READ_WRITE + the FINALFUNC_MODIFY parameter. + The READ_WRITE value indicates that the final function modifies the transition state in unspecified ways. This value prevents use of the aggregate as a window function, and it also prevents merging of transition states for aggregate calls that share the same input values and transition functions. - The SHARABLE value indicates that the transition function + The SHAREABLE value indicates that the transition function cannot be applied after the final function, but multiple final-function calls can be performed on the ending transition state value. This value prevents use of the aggregate as a window function, but it allows merging diff --git a/doc/src/sgml/xaggr.sgml b/doc/src/sgml/xaggr.sgml index 1514e5c388..4155b01ece 100644 --- a/doc/src/sgml/xaggr.sgml +++ b/doc/src/sgml/xaggr.sgml @@ -491,8 +491,8 @@ SELECT percentile_disc(0.5) WITHIN GROUP (ORDER BY income) FROM households; to continue adding input rows by executing the transition function again later. This means the final function is not READ_ONLY; it must be declared in - as READ_WRITE, or as SHARABLE if it's - possible for additional final-function calls to make use of the + as READ_WRITE, or as SHAREABLE if + it's possible for additional final-function calls to make use of the already-sorted state. diff --git a/src/backend/commands/aggregatecmds.c b/src/backend/commands/aggregatecmds.c index a1fd871d31..877f658ce7 100644 --- a/src/backend/commands/aggregatecmds.c +++ b/src/backend/commands/aggregatecmds.c @@ -477,13 +477,13 @@ extractModify(DefElem *defel) if (strcmp(val, "read_only") == 0) return AGGMODIFY_READ_ONLY; - if (strcmp(val, "sharable") == 0) - return AGGMODIFY_SHARABLE; + if (strcmp(val, "shareable") == 0) + return AGGMODIFY_SHAREABLE; if (strcmp(val, "read_write") == 0) return AGGMODIFY_READ_WRITE; ereport(ERROR, (errcode(ERRCODE_SYNTAX_ERROR), - errmsg("parameter \"%s\" must be READ_ONLY, SHARABLE, or READ_WRITE", + errmsg("parameter \"%s\" must be READ_ONLY, SHAREABLE, or READ_WRITE", defel->defname))); return 0; /* keep compiler quiet */ } diff --git a/src/backend/executor/nodeAgg.c b/src/backend/executor/nodeAgg.c index a27c292d4c..0fe0c22c1e 100644 --- a/src/backend/executor/nodeAgg.c +++ b/src/backend/executor/nodeAgg.c @@ -288,7 +288,7 @@ static void build_pertrans_for_aggref(AggStatePerTrans pertrans, static int find_compatible_peragg(Aggref *newagg, AggState *aggstate, int lastaggno, List **same_input_transnos); static int find_compatible_pertrans(AggState *aggstate, Aggref *newagg, - bool sharable, + bool shareable, Oid aggtransfn, Oid aggtranstype, Oid aggserialfn, Oid aggdeserialfn, Datum initValue, bool initValueIsNull, @@ -2522,7 +2522,7 @@ ExecInitAgg(Agg *node, EState *estate, int eflags) AclResult aclresult; Oid transfn_oid, finalfn_oid; - bool sharable; + bool shareable; Oid serialfn_oid, deserialfn_oid; Expr *finalfnexpr; @@ -2597,12 +2597,12 @@ ExecInitAgg(Agg *node, EState *estate, int eflags) /* * If finalfn is marked read-write, we can't share transition states; - * but it is okay to share states for AGGMODIFY_SHARABLE aggs. Also, + * but it is okay to share states for AGGMODIFY_SHAREABLE aggs. Also, * if we're not executing the finalfn here, we can share regardless. */ - sharable = (aggform->aggfinalmodify != AGGMODIFY_READ_WRITE) || + shareable = (aggform->aggfinalmodify != AGGMODIFY_READ_WRITE) || (finalfn_oid == InvalidOid); - peragg->sharable = sharable; + peragg->shareable = shareable; serialfn_oid = InvalidOid; deserialfn_oid = InvalidOid; @@ -2746,12 +2746,12 @@ ExecInitAgg(Agg *node, EState *estate, int eflags) * 2. Build working state for invoking the transition function, or * look up previously initialized working state, if we can share it. * - * find_compatible_peragg() already collected a list of sharable + * find_compatible_peragg() already collected a list of shareable * per-Trans's with the same inputs. Check if any of them have the * same transition function and initial value. */ existing_transno = find_compatible_pertrans(aggstate, aggref, - sharable, + shareable, transfn_oid, aggtranstype, serialfn_oid, deserialfn_oid, initValue, initValueIsNull, @@ -3170,7 +3170,7 @@ GetAggInitVal(Datum textInitVal, Oid transtype) * with this one, with the same input parameters. If no compatible aggregate * can be found, returns -1. * - * As a side-effect, this also collects a list of existing, sharable per-Trans + * As a side-effect, this also collects a list of existing, shareable per-Trans * structs with matching inputs. If no identical Aggref is found, the list is * passed later to find_compatible_pertrans, to see if we can at least reuse * the state value of another aggregate. @@ -3237,7 +3237,7 @@ find_compatible_peragg(Aggref *newagg, AggState *aggstate, * we might report a transno more than once. find_compatible_pertrans * is cheap enough that it's not worth spending cycles to avoid that.) */ - if (peragg->sharable) + if (peragg->shareable) *same_input_transnos = lappend_int(*same_input_transnos, peragg->transno); } @@ -3254,7 +3254,7 @@ find_compatible_peragg(Aggref *newagg, AggState *aggstate, * verified to match.) */ static int -find_compatible_pertrans(AggState *aggstate, Aggref *newagg, bool sharable, +find_compatible_pertrans(AggState *aggstate, Aggref *newagg, bool shareable, Oid aggtransfn, Oid aggtranstype, Oid aggserialfn, Oid aggdeserialfn, Datum initValue, bool initValueIsNull, @@ -3263,7 +3263,7 @@ find_compatible_pertrans(AggState *aggstate, Aggref *newagg, bool sharable, ListCell *lc; /* If this aggregate can't share transition states, give up */ - if (!sharable) + if (!shareable) return -1; foreach(lc, transnos) diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c index d6ceb72c05..f8fbcdad5b 100644 --- a/src/bin/pg_dump/pg_dump.c +++ b/src/bin/pg_dump/pg_dump.c @@ -13833,8 +13833,8 @@ dumpAgg(Archive *fout, AggInfo *agginfo) case AGGMODIFY_READ_ONLY: appendPQExpBufferStr(details, ",\n FINALFUNC_MODIFY = READ_ONLY"); break; - case AGGMODIFY_SHARABLE: - appendPQExpBufferStr(details, ",\n FINALFUNC_MODIFY = SHARABLE"); + case AGGMODIFY_SHAREABLE: + appendPQExpBufferStr(details, ",\n FINALFUNC_MODIFY = SHAREABLE"); break; case AGGMODIFY_READ_WRITE: appendPQExpBufferStr(details, ",\n FINALFUNC_MODIFY = READ_WRITE"); @@ -13889,8 +13889,8 @@ dumpAgg(Archive *fout, AggInfo *agginfo) case AGGMODIFY_READ_ONLY: appendPQExpBufferStr(details, ",\n MFINALFUNC_MODIFY = READ_ONLY"); break; - case AGGMODIFY_SHARABLE: - appendPQExpBufferStr(details, ",\n MFINALFUNC_MODIFY = SHARABLE"); + case AGGMODIFY_SHAREABLE: + appendPQExpBufferStr(details, ",\n MFINALFUNC_MODIFY = SHAREABLE"); break; case AGGMODIFY_READ_WRITE: appendPQExpBufferStr(details, ",\n MFINALFUNC_MODIFY = READ_WRITE"); diff --git a/src/bin/pg_dump/t/002_pg_dump.pl b/src/bin/pg_dump/t/002_pg_dump.pl index fe036b57ee..7eee870259 100644 --- a/src/bin/pg_dump/t/002_pg_dump.pl +++ b/src/bin/pg_dump/t/002_pg_dump.pl @@ -1431,7 +1431,7 @@ my %tests = ( basetype = int4, stype = _int8, finalfunc = int8_avg, - finalfunc_modify = sharable, + finalfunc_modify = shareable, initcond1 = \'{0,0}\' );', regexp => qr/^ @@ -1440,7 +1440,7 @@ my %tests = ( \n\s+\QSTYPE = bigint[],\E \n\s+\QINITCOND = '{0,0}',\E \n\s+\QFINALFUNC = int8_avg,\E - \n\s+\QFINALFUNC_MODIFY = SHARABLE\E + \n\s+\QFINALFUNC_MODIFY = SHAREABLE\E \n\);/xm, like => { %full_runs, diff --git a/src/include/catalog/pg_aggregate.h b/src/include/catalog/pg_aggregate.h index 72dd581e7a..bcae93f5e1 100644 --- a/src/include/catalog/pg_aggregate.h +++ b/src/include/catalog/pg_aggregate.h @@ -134,7 +134,7 @@ typedef FormData_pg_aggregate *Form_pg_aggregate; * transfn cannot be applied anymore after the first finalfn call. */ #define AGGMODIFY_READ_ONLY 'r' -#define AGGMODIFY_SHARABLE 's' +#define AGGMODIFY_SHAREABLE 's' #define AGGMODIFY_READ_WRITE 'w' #endif /* EXPOSE_TO_CLIENT_CODE */ diff --git a/src/include/executor/nodeAgg.h b/src/include/executor/nodeAgg.h index ab55be8f11..8fb8c8fe80 100644 --- a/src/include/executor/nodeAgg.h +++ b/src/include/executor/nodeAgg.h @@ -216,10 +216,10 @@ typedef struct AggStatePerAggData bool resulttypeByVal; /* - * "sharable" is false if this agg cannot share state values with other + * "shareable" is false if this agg cannot share state values with other * aggregates because the final function is read-write. */ - bool sharable; + bool shareable; } AggStatePerAggData; /* diff --git a/src/test/regress/expected/create_aggregate.out b/src/test/regress/expected/create_aggregate.out index b9b7fbcc9e..3d92084e13 100644 --- a/src/test/regress/expected/create_aggregate.out +++ b/src/test/regress/expected/create_aggregate.out @@ -148,7 +148,7 @@ CREATE AGGREGATE myavg (numeric) serialfunc = numeric_avg_serialize, deserialfunc = numeric_avg_deserialize, combinefunc = numeric_avg_combine, - finalfunc_modify = sharable -- just to test a non-default setting + finalfunc_modify = shareable -- just to test a non-default setting ); -- Ensure all these functions made it into the catalog SELECT aggfnoid, aggtransfn, aggcombinefn, aggtranstype::regtype, diff --git a/src/test/regress/sql/create_aggregate.sql b/src/test/regress/sql/create_aggregate.sql index 590ca9a624..cb6552e2d6 100644 --- a/src/test/regress/sql/create_aggregate.sql +++ b/src/test/regress/sql/create_aggregate.sql @@ -163,7 +163,7 @@ CREATE AGGREGATE myavg (numeric) serialfunc = numeric_avg_serialize, deserialfunc = numeric_avg_deserialize, combinefunc = numeric_avg_combine, - finalfunc_modify = sharable -- just to test a non-default setting + finalfunc_modify = shareable -- just to test a non-default setting ); -- Ensure all these functions made it into the catalog -- 2.40.0