AGGREGATE: Bring up to speed.
authorBen Pfaff <blp@cs.stanford.edu>
Sun, 11 Sep 2022 19:23:10 +0000 (12:23 -0700)
committerBen Pfaff <blp@cs.stanford.edu>
Mon, 12 Sep 2022 00:23:18 +0000 (17:23 -0700)
NEWS
doc/pspp-figures/aggregate.sps
doc/transformation.texi
src/data/value.h
src/language/stats/aggregate.c
src/language/stats/aggregate.h
src/libpspp/str.c
src/libpspp/str.h
src/ui/gui/psppire-dialog-action-aggregate.c
tests/language/stats/aggregate.at

diff --git a/NEWS b/NEWS
index a034aa50af9a1253b96d5eb3294f897c7e654920..aa8e337cdaea2b169c5d0c733aa43278cc6bec9e 100644 (file)
--- a/NEWS
+++ b/NEWS
@@ -11,6 +11,12 @@ Changes after 1.6.2:
 
  * FREQUENCIES now honors the LAYERED setting on SPLIT FILE.
 
+ * AGGREGATE:
+
+   - New aggregation functions CGT, CLT, CIN, and COUT.
+
+   - Break variables are now optional.
+
  * Building from a Git repository, which previously required GIMP, now
    requires rsvg-convert from librsvg2 instead.
 
index b84620b900e29d3d9ace35133cf674239ed2a4e0..1bf1b2289102c56f691b15a8a4f842204382dc13 100644 (file)
@@ -1,10 +1,7 @@
-get file="personnel.sav".
-
-aggregate outfile=* 
-        mode = replace
-        /break= occupation
-        /occ_mean_salaray = mean (salary)
-        /occ_median_salary = median (salary)
-        /occ_std_dev_salary = sd (salary).
-
-list.
+GET FILE="personnel.sav".
+AGGREGATE OUTFILE=* MODE=REPLACE
+        /BREAK=occupation
+        /occ_mean_salary=MEAN(salary)
+        /occ_median_salary=MEDIAN(salary)
+        /occ_std_dev_salary=SD(salary).
+LIST.
index 42a9b6b2a6916d396420347024eb8c7ea8d0e06d..14d189b2dfb9eb3bec59c6fe43cedf370987a0fe 100644 (file)
@@ -32,183 +32,184 @@ as a rule.
 
 @display
 AGGREGATE
-        OUTFILE=@{*,'@var{file_name}',@var{file_handle}@} [MODE=@{REPLACE, ADDVARIABLES@}]
-        /PRESORTED
-        /DOCUMENT
-        /MISSING=COLUMNWISE
-        /BREAK=@var{var_list}
-        /@var{dest_var}['@var{label}']@dots{}=@var{agr_func}(@var{src_vars}, @var{args}@dots{})@dots{}
+        [OUTFILE=@{*,'@var{file_name}',@var{file_handle}@} [MODE=@{REPLACE,ADDVARIABLES@}]]
+        [/MISSING=COLUMNWISE]
+        [/PRESORTED]
+        [/DOCUMENT]
+        [/BREAK=@var{var_list}]
+        /@var{dest_var}['@var{label}']@dots{}=@var{agr_func}(@var{src_vars}[, @var{args}]@dots{})@dots{}
 @end display
 
 @cmd{AGGREGATE} summarizes groups of cases into single cases.
-Cases are divided into groups that have the same values for one or more
+It divides cases into groups that have the same values for one or more
 variables called @dfn{break variables}.  Several functions are available
 for summarizing case contents.
 
-The @subcmd{OUTFILE} subcommand is required and must appear first.  Specify a
-system file or portable file by file name or file
-handle (@pxref{File Handles}), or a dataset by its name
-(@pxref{Datasets}).
-The aggregated cases are written to this file.  If @samp{*} is
-specified, then the aggregated cases replace the active dataset's data.
-Use of @subcmd{OUTFILE} to write a portable file is a @pspp{} extension.
-
-If @subcmd{OUTFILE=*} is given, then the subcommand @subcmd{MODE} may also be
-specified.
-The mode subcommand has two possible values: @subcmd{ADDVARIABLES} or @subcmd{REPLACE}.
-In @subcmd{REPLACE} mode, the entire active dataset is replaced by a new dataset
+The @cmd{AGGREGATE} syntax consists of subcommands to control its
+behavior, all of which are optional, followed by one or more
+destination variable assigments, each of which uses an aggregation
+function to define how it is calculated.
+
+The @subcmd{OUTFILE} subcommand, which must be first, names the
+destination for @cmd{AGGREGATE} output.  It may name a system file by
+file name or file handle (@pxref{File Handles}), a dataset by its name
+(@pxref{Datasets}), or @samp{*} to replace the active dataset.
+@cmd{AGGREGATE} writes its output to this file.
+
+With @subcmd{OUTFILE=*} only, @code{MODE} may be specified immediately
+afterward with the value @code{ADDVARIABLES} or @code{REPLACE}:
+
+@itemize
+@item
+With @code{REPLACE}, the default, the active dataset is replaced by a new dataset
 which contains just the break variables and the destination varibles.
-In this mode, the new file contains as many cases as there are
+The new file contains as many cases as there are
 unique combinations of the break variables.
-In @subcmd{ADDVARIABLES} mode, the destination variables are appended to
+
+@item
+With @code{ADDVARIABLES}, the destination variables are added to those in
 the existing active dataset.
-Cases which have identical combinations of values in their break
-variables, receive identical values for the destination variables.
+Cases that have the same combination of values in their break
+variables receive identical values for the destination variables.
 The number of cases in the active dataset remains unchanged.
-Note that if @subcmd{ADDVARIABLES} is specified, then the data @emph{must} be
-sorted on the break variables.
+The data must be
+sorted on the break variables, that is, @code{ADDVARIABLES} implies @code{PRESORTED}
+@end itemize
+
+If @code{OUTFILE} is omitted, @cmd{AGGREGATE} acts as if
+@code{OUTFILE=* MODE=ADDVARIABLES} were specified.
 
-By default, the active dataset is sorted based on the break variables
-before aggregation takes place.  If the active dataset is already sorted
-or otherwise grouped in terms of the break variables, specify
+By default, @cmd{AGGREGATE} first sorts the data on the break variables.
+If the active dataset is already sorted
+or grouped by the break variables, specify
 @subcmd{PRESORTED} to save time.
-@subcmd{PRESORTED} is assumed if @subcmd{MODE=ADDVARIABLES} is used.
+With @subcmd{MODE=ADDVARIABLES}, the data must be pre-sorted.
 
 Specify @subcmd{DOCUMENT} to copy the documents from the active dataset into the
 aggregate file (@pxref{DOCUMENT}).  Otherwise, the aggregate file does
 not contain any documents, even if the aggregate file replaces the
 active dataset.
 
-Normally, only a single case (for @subcmd{SD} and @subcmd{SD}., two cases) need be
-non-missing in each group for the aggregate variable to be
-non-missing.  Specifying @subcmd{/MISSING=COLUMNWISE} inverts this behavior, so
-that the aggregate variable becomes missing if any aggregated value is
-missing.
+Normally, @code{AGGREGATE} produces a non-missing value whenever there
+is enough non-missing data for the aggregation function in use, that
+is, just one non-missing value or, for the @code{SD} and @code{SD.}
+aggregation functions, two non-missing values.  Specify
+@code{/MISSING=COLUMNWISE} to make @code{AGGREGATE} output a missing
+value when one or more of the input values are missing.
 
-If @subcmd{PRESORTED}, @subcmd{DOCUMENT}, or @subcmd{MISSING} are specified, they must appear
-between @subcmd{OUTFILE} and @subcmd{BREAK}.
+The @subcmd{BREAK} subcommand is optionally but usually present.  On
+@subcmd{BREAK}, list the variables used to divide the active dataset
+into groups to be summarized.
 
-At least one break variable must be specified on @subcmd{BREAK}, a
-required subcommand.  The values of these variables are used to divide
-the active dataset into groups to be summarized.  In addition, at least
-one @var{dest_var} must be specified.
+@cmd{AGGREGATE} is particular about the order of subcommands.
+@subcmd{OUTFILE} must be first, followed by @subcmd{MISSING}.
+@subcmd{PRESORTED} and @subcmd{DOCUMENT} follow @subcmd{MISSING}, in
+either order, followed by @subcmd{BREAK}, then followed by aggregation
+variable specifications.
 
-One or more sets of aggregation variables must be specified.  Each set
+At least one set of aggregation variables is required.  Each set
 comprises a list of aggregation variables, an equals sign (@samp{=}),
 the name of an aggregation function (see the list below), and a list
-of source variables in parentheses.  Some aggregation functions expect
-additional arguments following the source variable names.
+of source variables in parentheses.  A few aggregation functions do
+not accept source variables, and some aggregation functions expect
+additional arguments after the source variable names.
 
-Aggregation variables typically are created with no variable label,
-value labels, or missing values.  Their default print and write
-formats depend on the aggregation function used, with details given in
-the table below.  A variable label for an aggregation variable may be
-specified just after the variable's name in the aggregation variable
-list.
+@cmd{AGGREGATE} typically creates aggregation variables with no
+variable label, value labels, or missing values.  Their default print
+and write formats depend on the aggregation function used, with
+details given in the table below.  A variable label for an aggregation
+variable may be specified just after the variable's name in the
+aggregation variable list.
 
 Each set must have exactly as many source variables as aggregation
 variables.  Each aggregation variable receives the results of applying
 the specified aggregation function to the corresponding source
-variable.  The @subcmd{MEAN}, @subcmd{MEDIAN}, @subcmd{SD}, and @subcmd{SUM}
-aggregation functions may only be
-applied to numeric variables.  All the rest may be applied to numeric
-and string variables.
+variable.
 
-The available aggregation functions are as follows:
+The following aggregation functions may be applied only to numeric
+variables:
 
 @table @asis
-@item @subcmd{FGT(@var{var_name}, @var{value})}
-Fraction of values greater than the specified constant.  The default
-format is F5.3.
+@item @subcmd{MEAN(@var{var_name}@dots{})}
+Arithmetic mean.  Limited to numeric values.  The default format is
+F8.2.
 
-@item @subcmd{FIN(@var{var_name}, @var{low}, @var{high})}
-Fraction of values within the specified inclusive range of constants.
-The default format is F5.3.
+@item @subcmd{MEDIAN(@var{var_name}@dots{})}
+The median value.  Limited to numeric values.  The default format is F8.2.
 
-@item @subcmd{FLT(@var{var_name}, @var{value})}
-Fraction of values less than the specified constant.  The default
-format is F5.3.
+@item @subcmd{SD(@var{var_name}@dots{})}
+Standard deviation of the mean.  Limited to numeric values.  The
+default format is F8.2.
 
-@item @subcmd{FIRST(@var{var_name})}
-First non-missing value in break group.  The aggregation variable
-receives the complete dictionary information from the source variable.
-The sort performed by @cmd{AGGREGATE} (and by @cmd{SORT CASES}) is stable.
-This means that
-the first case with particular values for the break variables before
-sorting is also the first case in that break group after sorting.
+@item @subcmd{SUM(@var{var_name}@dots{})}
+Sum.  Limited to numeric values.  The default format is F8.2.
+@end table
 
-@item @subcmd{FOUT(@var{var_name}, @var{low}, @var{high})}
-Fraction of values strictly outside the specified range of constants.
-The default format is F5.3.
+These aggregation functions may be applied to numeric and string variables:
 
-@item @subcmd{LAST(@var{var_name})}
-Last non-missing value in break group.  The aggregation variable
+@table @asis
+@item @subcmd{CGT(@var{var_name}@dots{}, @var{value})}
+@itemx @subcmd{CLT(@var{var_name}@dots{}, @var{value})}
+@itemx @subcmd{CIN(@var{var_name}@dots{}, @var{low}, @var{high})}
+@itemx @subcmd{COUT(@var{var_name}@dots{}, @var{low}, @var{high})}
+Total weight of cases greater than or less than @var{value} or inside
+or outside the closed range [@var{low},@var{high}], respectively.  The
+default format is F5.3.
+
+@item @subcmd{FGT(@var{var_name}@dots{}, @var{value})}
+@itemx @subcmd{FLT(@var{var_name}@dots{}, @var{value})}
+@itemx @subcmd{FIN(@var{var_name}@dots{}, @var{low}, @var{high})}
+@itemx @subcmd{FOUT(@var{var_name}@dots{}, @var{low}, @var{high})}
+Fraction of values greater than or less than @var{value} or inside or
+outside the closed range [@var{low},@var{high}], respectively.  The
+default format is F5.3.
+
+@item @subcmd{FIRST(@var{var_name}@dots{})}
+@itemx @subcmd{LAST(@var{var_name}@dots{})}
+First or last non-missing value, respectively, in break group.  The
+aggregation variable
 receives the complete dictionary information from the source variable.
 The sort performed by @cmd{AGGREGATE} (and by @cmd{SORT CASES}) is stable.
+This means that
+the first (or last) case with particular values for the break variables before
+sorting is also the first (or last) case in that break group after sorting.
 
-@item @subcmd{MAX(@var{var_name})}
-Maximum value.  The aggregation variable receives the complete
-dictionary information from the source variable.
-
-@item @subcmd{MEAN(@var{var_name})}
-Arithmetic mean.  Limited to numeric values.  The default format is
-F8.2.
-
-@item @subcmd{MEDIAN(@var{var_name})}
-The median value.  Limited to numeric values.  The default format is F8.2.
+@item @subcmd{MIN(@var{var_name}@dots{})}
+@itemx @subcmd{MAX(@var{var_name}@dots{})}
+Minimum or maximum value, respectively.  The aggregation variable
+receives the complete dictionary information from the source variable.
 
-@item @subcmd{MIN(@var{var_name})}
-Minimum value.  The aggregation variable receives the complete
-dictionary information from the source variable.
+@item @subcmd{N(@var{var_name}@dots{})}
+@itemx @subcmd{NMISS(@var{var_name}@dots{})}
+Total weight of non-missing or missing values, respectively.  The
+default format is F7.0 if weighting is not enabled, F8.2 if it is
+(@pxref{WEIGHT}).
+
+@item @subcmd{NU(@var{var_name}@dots{})}
+@itemx @subcmd{NUMISS(@var{var_name}@dots{})}
+Count of non-missing or missing values, respectively, ignoring case
+weights.  The default format is F7.0.
+
+@item @subcmd{PGT(@var{var_name}@dots{}, @var{value})}
+@itemx @subcmd{PLT(@var{var_name}@dots{}, @var{value})}
+@itemx @subcmd{PIN(@var{var_name}@dots{}, @var{low}, @var{high})}
+@itemx @subcmd{POUT(@var{var_name}@dots{}, @var{low}, @var{high})}
+Percentage between 0 and 100 of values greater than or less than
+@var{VALUE} or inside or outside the closed range
+[@var{low},@var{high}], respectively.  The default format is F5.1.
+@end table
 
-@item @subcmd{N(@var{var_name})}
-Number of non-missing values.  The default format is F7.0 if weighting
-is not enabled, F8.2 if it is (@pxref{WEIGHT}).
+These aggregation functions do not accept source variables:
 
+@table @asis
 @item @subcmd{N}
-Number of cases aggregated to form this group.  The default format is
-F7.0 if weighting is not enabled, F8.2 if it is (@pxref{WEIGHT}).
-
-@item @subcmd{NMISS(@var{var_name})}
-Number of missing values.  The default format is F7.0 if weighting is
-not enabled, F8.2 if it is (@pxref{WEIGHT}).
-
-@item @subcmd{NU(@var{var_name})}
-Number of non-missing values.  Each case is considered to have a weight
-of 1, regardless of the current weighting variable (@pxref{WEIGHT}).
-The default format is F7.0.
+Total weight of cases aggregated to form this group.  The default
+format is F7.0 if weighting is not enabled, F8.2 if it is
+(@pxref{WEIGHT}).
 
 @item @subcmd{NU}
-Number of cases aggregated to form this group.  Each case is considered
-to have a weight of 1, regardless of the current weighting variable.
+Count of cases aggregated to form this group, ignoring case weights.
 The default format is F7.0.
-
-@item @subcmd{NUMISS(@var{var_name})}
-Number of missing values.  Each case is considered to have a weight of
-1, regardless of the current weighting variable.  The default format is F7.0.
-
-@item @subcmd{PGT(@var{var_name}, @var{value})}
-Percentage between 0 and 100 of values greater than the specified
-constant.  The default format is F5.1.
-
-@item @subcmd{PIN(@var{var_name}, @var{low}, @var{high})}
-Percentage of values within the specified inclusive range of
-constants.  The default format is F5.1.
-
-@item @subcmd{PLT(@var{var_name}, @var{value})}
-Percentage of values less than the specified constant.  The default
-format is F5.1.
-
-@item @subcmd{POUT(@var{var_name}, @var{low}, @var{high})}
-Percentage of values strictly outside the specified range of
-constants.  The default format is F5.1.
-
-@item @subcmd{SD(@var{var_name})}
-Standard deviation of the mean.  Limited to numeric values.  The
-default format is F8.2.
-
-@item @subcmd{SUM(@var{var_name})}
-Sum.  Limited to numeric values.  The default format is F8.2.
 @end table
 
 Aggregation functions compare string values in terms of internal
@@ -238,7 +239,7 @@ occupation.
 @caption {Calculating aggregated statistics from the @file{personnel.sav} file.}
 @end float
 
-Since we chose the @samp{MODE = REPLACE} option, in @ref{aggregate:res} cases
+Since we chose the @samp{MODE=REPLACE} option, in @ref{aggregate:res} cases
 for the individual persons are no longer present.  They have each been replaced
 by a single case per aggregated value.
 
@@ -251,7 +252,6 @@ Note that some values for the standard deviation are blank.
 This is because there is only one case with the respective
 occupation.
 
-
 @node AUTORECODE
 @section AUTORECODE
 @vindex AUTORECODE
index 8e624a8464b73163e9638c88c26dff5fa6a444cd..7f4399ced497e4a37b7a4a592ee35f7736e540bf 100644 (file)
@@ -23,7 +23,9 @@
 #include <stdlib.h>
 #include <stdint.h>
 #include <string.h>
-#include "xalloc.h"
+#include "libpspp/cast.h"
+#include "libpspp/str.h"
+#include "gl/xalloc.h"
 \f
 /* A numeric or string value.  The client is responsible for keeping track of
    the value's width. */
@@ -56,6 +58,8 @@ bool value_is_resizable (const union value *, int old_width, int new_width);
 bool value_needs_resize (int old_width, int new_width);
 void value_resize (union value *, int old_width, int new_width);
 
+static inline struct substring value_ss (const union value *, int width);
+
 bool value_is_spaces (const union value *, int width);
 
 static inline void value_swap (union value *, union value *);
@@ -149,4 +153,13 @@ value_swap (union value *a, union value *b)
   *b = tmp;
 }
 
+static inline struct substring
+value_ss (const union value *v, int width)
+{
+  return (struct substring) {
+    .string = CHAR_CAST (char *, v->s),
+    .length = width
+  };
+}
+
 #endif /* data/value.h */
index f351d83096815b0ab84bdd8ee54012b43b0fbacd..4edbe22f773444b70ba341bb04c0b6f03e1ca578 100644 (file)
 #define _(msgid) gettext (msgid)
 #define N_(msgid) msgid
 
-/* Argument for AGGREGATE function. */
-union agr_argument
+/* Argument for AGGREGATE function.
+
+   Only one of the members is used, so this could be a union, but it's simpler
+   to just have both. */
+struct agr_argument
   {
     double f;                           /* Numeric. */
-    char *c;                            /* Short or long string. */
+    struct substring s;                 /* String. */
   };
 
 /* Specifies how to make an aggregate variable. */
@@ -70,9 +73,9 @@ struct agr_var
     /* Collected during parsing. */
     const struct variable *src;        /* Source variable. */
     struct variable *dest;     /* Target variable. */
-    int function;              /* Function. */
+    enum agr_function function; /* Function. */
     enum mv_class exclude;      /* Classes of missing values to exclude. */
-    union agr_argument arg[2]; /* Arguments. */
+    struct agr_argument arg[2];        /* Arguments. */
 
     /* Accumulated during AGGREGATE execution. */
     double dbl;
@@ -81,38 +84,21 @@ struct agr_var
     char *string;
     bool saw_missing;
     struct moments1 *moments;
-    double cc;
 
     struct variable *subject;
     struct variable *weight;
     struct casewriter *writer;
   };
 
-
 /* Attributes of aggregation functions. */
 const struct agr_func agr_func_tab[] =
   {
-    {"SUM",     N_("Sum of values"),                         AGR_SV_YES, 0, -1,          { .type = FMT_F, .w = 8, .d = 2 }},
-    {"MEAN",   N_("Mean average"),                          AGR_SV_YES, 0, -1,          { .type = FMT_F, .w = 8, .d = 2 }},
-    {"MEDIAN", N_("Median average"),                        AGR_SV_YES, 0, -1,          { .type = FMT_F, .w = 8, .d = 2 }},
-    {"SD",      N_("Standard deviation"),                    AGR_SV_YES, 0, -1,          { .type = FMT_F, .w = 8, .d = 2 }},
-    {"MAX",     N_("Maximum value"),                         AGR_SV_YES, 0, VAL_STRING,  {-1, -1, -1}},
-    {"MIN",     N_("Minimum value"),                         AGR_SV_YES, 0, VAL_STRING,  {-1, -1, -1}},
-    {"PGT",     N_("Percentage greater than"),               AGR_SV_YES, 1, VAL_NUMERIC, { .type = FMT_F, .w = 5, .d = 1 }},
-    {"PLT",     N_("Percentage less than"),                  AGR_SV_YES, 1, VAL_NUMERIC, { .type = FMT_F, .w = 5, .d = 1 }},
-    {"PIN",     N_("Percentage included in range"),          AGR_SV_YES, 2, VAL_NUMERIC, { .type = FMT_F, .w = 5, .d = 1 }},
-    {"POUT",    N_("Percentage excluded from range"),        AGR_SV_YES, 2, VAL_NUMERIC, { .type = FMT_F, .w = 5, .d = 1 }},
-    {"FGT",     N_("Fraction greater than"),                 AGR_SV_YES, 1, VAL_NUMERIC, { .type = FMT_F, .w = 5, .d = 3 }},
-    {"FLT",     N_("Fraction less than"),                    AGR_SV_YES, 1, VAL_NUMERIC, { .type = FMT_F, .w = 5, .d = 3 }},
-    {"FIN",     N_("Fraction included in range"),            AGR_SV_YES, 2, VAL_NUMERIC, { .type = FMT_F, .w = 5, .d = 3 }},
-    {"FOUT",    N_("Fraction excluded from range"),          AGR_SV_YES, 2, VAL_NUMERIC, { .type = FMT_F, .w = 5, .d = 3 }},
-    {"N",       N_("Number of cases"),                       AGR_SV_NO,  0, VAL_NUMERIC, { .type = FMT_F, .w = 7, .d = 0 }},
-    {"NU",      N_("Number of cases (unweighted)"),          AGR_SV_OPT, 0, VAL_NUMERIC, { .type = FMT_F, .w = 7, .d = 0 }},
-    {"NMISS",   N_("Number of missing values"),              AGR_SV_YES, 0, VAL_NUMERIC, { .type = FMT_F, .w = 7, .d = 0 }},
-    {"NUMISS",  N_("Number of missing values (unweighted)"), AGR_SV_YES, 0, VAL_NUMERIC, { .type = FMT_F, .w = 7, .d = 0 }},
-    {"FIRST",   N_("First non-missing value"),               AGR_SV_YES, 0, VAL_STRING,  {-1, -1, -1}},
-    {"LAST",    N_("Last non-missing value"),                AGR_SV_YES, 0, VAL_STRING,  {-1, -1, -1}},
-    {NULL,      NULL,                                        AGR_SV_NO,  0, -1,          {-1, -1, -1}},
+#define AGRF(ENUM, NAME, DESCRIPTION, SRC_VARS, N_ARGS, ALPHA_TYPE, W, D) \
+    [ENUM] = { NAME, DESCRIPTION, SRC_VARS, N_ARGS, ALPHA_TYPE, \
+               { .type = (W) > 0 ? FMT_F : -1, .w = W, .d = D } },
+AGGREGATE_FUNCTIONS
+#undef AGRF
+    {NULL, NULL, AGR_SV_NO, 0, -1, {-1, -1, -1}},
   };
 
 /* Missing value types. */
@@ -160,52 +146,76 @@ int
 cmd_aggregate (struct lexer *lexer, struct dataset *ds)
 {
   struct dictionary *dict = dataset_dict (ds);
-  struct agr_proc agr;
+  struct agr_proc agr = {
+    .missing = ITEMWISE,
+    .src_dict = dict,
+  };
   struct file_handle *out_file = NULL;
-  struct casereader *input = NULL, *group;
-  struct casegrouper *grouper;
+  struct casereader *input = NULL;
   struct casewriter *output = NULL;
 
   bool copy_documents = false;
   bool presorted = false;
-  bool saw_direction;
-  bool ok;
-
-  memset(&agr, 0 , sizeof (agr));
-  agr.missing = ITEMWISE;
-  agr.src_dict = dict;
-  subcase_init_empty (&agr.sort);
+  int addvariables_ofs = 0;
 
   /* OUTFILE subcommand must be first. */
-  lex_match (lexer, T_SLASH);
-  if (!lex_force_match_id (lexer, "OUTFILE"))
-    goto error;
-  lex_match (lexer, T_EQUALS);
-  if (!lex_match (lexer, T_ASTERISK))
+  if (lex_match_phrase (lexer, "/OUTFILE") || lex_match_id (lexer, "OUTFILE"))
     {
-      out_file = fh_parse (lexer, FH_REF_FILE, dataset_session (ds));
-      if (out_file == NULL)
-        goto error;
+      lex_match (lexer, T_EQUALS);
+      if (!lex_match (lexer, T_ASTERISK))
+        {
+          out_file = fh_parse (lexer, FH_REF_FILE, dataset_session (ds));
+          if (out_file == NULL)
+            goto error;
+        }
+
+      if (!out_file && lex_match_id (lexer, "MODE"))
+        {
+          lex_match (lexer, T_EQUALS);
+          if (lex_match_id (lexer, "ADDVARIABLES"))
+            {
+              addvariables_ofs = lex_ofs (lexer) - 1;
+              agr.add_variables = true;
+              presorted = true;
+            }
+          else if (lex_match_id (lexer, "REPLACE"))
+            agr.add_variables = false;
+          else
+            {
+              lex_error_expecting (lexer, "ADDVARIABLES", "REPLACE");
+              goto error;
+            }
+        }
+    }
+  else
+    {
+      agr.add_variables = true;
+      presorted = true;
     }
 
-  if (out_file == NULL && lex_match_id (lexer, "MODE"))
+  if (lex_match_phrase (lexer, "/MISSING"))
     {
       lex_match (lexer, T_EQUALS);
-      if (lex_match_id (lexer, "ADDVARIABLES"))
-       {
-         agr.add_variables = true;
-
-         /* presorted is assumed in ADDVARIABLES mode */
-         presorted = true;
-       }
-      else if (lex_match_id (lexer, "REPLACE"))
-       {
-         agr.add_variables = false;
-       }
-      else
-       goto error;
+      if (!lex_match_id (lexer, "COLUMNWISE"))
+        {
+          lex_error_expecting (lexer, "COLUMNWISE");
+          goto error;
+        }
+      agr.missing = COLUMNWISE;
     }
 
+  int presorted_ofs = 0;
+  for (;;)
+    if (lex_match_phrase (lexer, "/DOCUMENT"))
+      copy_documents = true;
+    else if (lex_match_phrase (lexer, "/PRESORTED"))
+      {
+        presorted = true;
+        presorted_ofs = lex_ofs (lexer) - 1;
+      }
+    else
+      break;
+
   if (agr.add_variables)
     agr.dict = dict_clone (dict);
   else
@@ -214,53 +224,43 @@ cmd_aggregate (struct lexer *lexer, struct dataset *ds)
   dict_set_label (agr.dict, dict_get_label (dict));
   dict_set_documents (agr.dict, dict_get_documents (dict));
 
-  /* Read most of the subcommands. */
-  for (;;)
+  if (lex_match_phrase (lexer, "/BREAK"))
     {
-      lex_match (lexer, T_SLASH);
-
-      if (lex_match_id (lexer, "MISSING"))
-       {
-         lex_match (lexer, T_EQUALS);
-         if (!lex_match_id (lexer, "COLUMNWISE"))
-           {
-             lex_error_expecting (lexer, "COLUMNWISE");
-              goto error;
-           }
-         agr.missing = COLUMNWISE;
-       }
-      else if (lex_match_id (lexer, "DOCUMENT"))
-        copy_documents = true;
-      else if (lex_match_id (lexer, "PRESORTED"))
-        presorted = true;
-      else if (lex_force_match_id (lexer, "BREAK"))
-       {
-          int i;
-
-         lex_match (lexer, T_EQUALS);
-          if (!parse_sort_criteria (lexer, dict, &agr.sort, &agr.break_vars,
-                                    &saw_direction))
-            goto error;
-          agr.break_n_vars = subcase_get_n_fields (&agr.sort);
-
-         if  (! agr.add_variables)
-           for (i = 0; i < agr.break_n_vars; i++)
-             dict_clone_var_assert (agr.dict, agr.break_vars[i]);
-
-          /* BREAK must follow the options. */
-          break;
-       }
-      else
+      lex_match (lexer, T_EQUALS);
+      bool saw_direction;
+      int break_start = lex_ofs (lexer);
+      if (!parse_sort_criteria (lexer, dict, &agr.sort, &agr.break_vars,
+                                &saw_direction))
         goto error;
+      int break_end = lex_ofs (lexer) - 1;
+      agr.break_n_vars = subcase_get_n_fields (&agr.sort);
 
+      if  (! agr.add_variables)
+        for (size_t i = 0; i < agr.break_n_vars; i++)
+          dict_clone_var_assert (agr.dict, agr.break_vars[i]);
+
+      if (presorted && saw_direction)
+        {
+          lex_ofs_msg (lexer, SW, break_start, break_end,
+                       _("When the input data is presorted, specifying "
+                         "sorting directions with (A) or (D) has no effect.  "
+                         "Output data will be sorted the same way as the "
+                         "input data."));
+          if (presorted_ofs)
+            lex_ofs_msg (lexer, SN, presorted_ofs, presorted_ofs,
+                         _("The PRESORTED subcommand state that the "
+                           "input data is presorted."));
+          else if (addvariables_ofs)
+            lex_ofs_msg (lexer, SN, addvariables_ofs, addvariables_ofs,
+                         _("ADDVARIABLES implies that the input data "
+                           "is presorted."));
+          else
+            msg (SN, _("The input data must be presorted because the "
+                       "OUTFILE subcommand is not specified."));
+        }
     }
-  if (presorted && saw_direction)
-    msg (SW, _("When PRESORTED is specified, specifying sorting directions "
-               "with (A) or (D) has no effect.  Output data will be sorted "
-               "the same way as the input data."));
 
   /* Read in the aggregate functions. */
-  lex_match (lexer, T_SLASH);
   if (!parse_aggregate_functions (lexer, dict, &agr))
     goto error;
 
@@ -296,6 +296,8 @@ cmd_aggregate (struct lexer *lexer, struct dataset *ds)
       subcase_clear (&agr.sort);
     }
 
+  struct casegrouper *grouper;
+  struct casereader *group;
   for (grouper = casegrouper_create_vars (input, agr.break_vars,
                                           agr.break_n_vars);
        casegrouper_get_next_group (grouper, &group);
@@ -339,12 +341,10 @@ cmd_aggregate (struct lexer *lexer, struct dataset *ds)
   if (!casegrouper_destroy (grouper))
     goto error;
 
-  if (!proc_commit (ds))
-    {
-      input = NULL;
-      goto error;
-    }
+  bool ok = proc_commit (ds);
   input = NULL;
+  if (!ok)
+    goto error;
 
   if (out_file == NULL)
     {
@@ -377,98 +377,89 @@ error:
   return CMD_CASCADING_FAILURE;
 }
 
+static bool
+parse_agr_func_name (struct lexer *lexer, int *func_index,
+                     enum mv_class *exclude)
+{
+  if (lex_token (lexer) != T_ID)
+    {
+      lex_error (lexer, _("Syntax error expecting aggregation function."));
+      return false;
+    }
+
+  struct substring name = lex_tokss (lexer);
+  *exclude = ss_chomp_byte (&name, '.') ? MV_SYSTEM : MV_ANY;
+
+  for (const struct agr_func *f = agr_func_tab; f->name; f++)
+    if (ss_equals_case (ss_cstr (f->name), name))
+      {
+        *func_index = f - agr_func_tab;
+        lex_get (lexer);
+        return true;
+      }
+  lex_error (lexer, _("Unknown aggregation function %s."), lex_tokcstr (lexer));
+  return false;
+}
+
 /* Parse all the aggregate functions. */
 static bool
 parse_aggregate_functions (struct lexer *lexer, const struct dictionary *dict,
                           struct agr_proc *agr)
 {
+  if (!lex_force_match (lexer, T_SLASH))
+    return false;
+
+  size_t starting_n_vars = dict_get_n_vars (dict);
   size_t allocated_agr_vars = 0;
 
   /* Parse everything. */
   for (;;)
     {
-      char **dest;
-      char **dest_label;
-      size_t n_dest;
-      struct string function_name;
+      char **dest = NULL;
+      char **dest_label = NULL;
+      size_t n_vars = 0;
 
-      enum mv_class exclude;
-      const struct agr_func *function;
-      int func_index;
-
-      union agr_argument arg[2];
-
-      const struct variable **src;
-      size_t n_src;
+      struct agr_argument arg[2] = { { .f = 0 }, { .f = 0 } };
 
-      size_t i;
-
-      dest = NULL;
-      dest_label = NULL;
-      n_dest = 0;
-      src = NULL;
-      function = NULL;
-      n_src = 0;
-      arg[0].c = NULL;
-      arg[1].c = NULL;
-      ds_init_empty (&function_name);
+      const struct variable **src = NULL;
 
       /* Parse the list of target variables. */
+      int dst_start_ofs = lex_ofs (lexer);
       while (!lex_match (lexer, T_EQUALS))
        {
-         size_t n_dest_prev = n_dest;
+         size_t n_vars_prev = n_vars;
 
-         if (!parse_DATA_LIST_vars (lexer, dict, &dest, &n_dest,
+         if (!parse_DATA_LIST_vars (lexer, dict, &dest, &n_vars,
                                      (PV_APPEND | PV_SINGLE | PV_NO_SCRATCH
                                       | PV_NO_DUPLICATE)))
            goto error;
 
          /* Assign empty labels. */
-         {
-           int j;
-
-           dest_label = xnrealloc (dest_label, n_dest, sizeof *dest_label);
-           for (j = n_dest_prev; j < n_dest; j++)
-             dest_label[j] = NULL;
-         }
-
-
+          dest_label = xnrealloc (dest_label, n_vars, sizeof *dest_label);
+          for (size_t j = n_vars_prev; j < n_vars; j++)
+            dest_label[j] = NULL;
 
          if (lex_is_string (lexer))
            {
-             dest_label[n_dest - 1] = xstrdup (lex_tokcstr (lexer));
+             dest_label[n_vars - 1] = xstrdup (lex_tokcstr (lexer));
              lex_get (lexer);
            }
        }
+      int dst_end_ofs = lex_ofs (lexer) - 2;
 
       /* Get the name of the aggregation function. */
-      if (lex_token (lexer) != T_ID)
-       {
-         lex_error (lexer, _("Syntax error expecting aggregation function."));
-         goto error;
-       }
-
-      ds_assign_substring (&function_name, lex_tokss (lexer));
-      exclude = ds_chomp_byte (&function_name, '.') ? MV_SYSTEM : MV_ANY;
-
-      for (function = agr_func_tab; function->name; function++)
-       if (!c_strcasecmp (function->name, ds_cstr (&function_name)))
-         break;
-      if (NULL == function->name)
-       {
-         lex_error (lexer, _("Unknown aggregation function %s."),
-                     ds_cstr (&function_name));
-         goto error;
-       }
-      ds_destroy (&function_name);
-      func_index = function - agr_func_tab;
-      lex_get (lexer);
+      int func_index;
+      enum mv_class exclude;
+      if (!parse_agr_func_name (lexer, &func_index, &exclude))
+        goto error;
+      const struct agr_func *function = &agr_func_tab[func_index];
 
       /* Check for leading lparen. */
       if (!lex_match (lexer, T_LPAREN))
        {
          if (function->src_vars == AGR_SV_YES)
            {
+              bool ok UNUSED = lex_force_match (lexer, T_LPAREN);
              goto error;
            }
        }
@@ -476,45 +467,38 @@ parse_aggregate_functions (struct lexer *lexer, const struct dictionary *dict,
         {
          /* Parse list of source variables. */
           int pv_opts = PV_NO_SCRATCH;
-          if (func_index == SUM || func_index == MEAN || func_index == SD)
+          if (func_index == AGRF_SUM || func_index == AGRF_MEAN
+              || func_index == AGRF_MEDIAN || func_index == AGRF_SD)
             pv_opts |= PV_NUMERIC;
           else if (function->n_args)
             pv_opts |= PV_SAME_TYPE;
 
-          int vars_start_ofs = lex_ofs (lexer);
+          int src_start_ofs = lex_ofs (lexer);
+          size_t n_src;
           if (!parse_variables_const (lexer, dict, &src, &n_src, pv_opts))
             goto error;
-          int vars_end_ofs = lex_ofs (lexer) - 1;
+          int src_end_ofs = lex_ofs (lexer) - 1;
 
          /* Parse function arguments, for those functions that
             require arguments. */
           int args_start_ofs = 0;
          if (function->n_args != 0)
-           for (i = 0; i < function->n_args; i++)
+           for (size_t i = 0; i < function->n_args; i++)
              {
-               int type;
-
                lex_match (lexer, T_COMMA);
-                if (i == 0)
-                  args_start_ofs = lex_ofs (lexer);
+
+               enum val_type type;
                if (lex_is_string (lexer))
-                 {
-                   arg[i].c = recode_string (dict_get_encoding (agr->dict),
-                                              "UTF-8", lex_tokcstr (lexer),
-                                              -1);
-                   type = VAL_STRING;
-                 }
-               else if (lex_is_number (lexer))
-                 {
-                   arg[i].f = lex_tokval (lexer);
-                   type = VAL_NUMERIC;
-                 }
+                  type = VAL_STRING;
+                else if (lex_is_number (lexer))
+                  type = VAL_NUMERIC;
                 else
                   {
                    lex_error (lexer, _("Missing argument %zu to %s."),
                                i + 1, function->name);
                    goto error;
                  }
+
                if (type != var_get_type (src[0]))
                  {
                    msg (SE, _("Arguments to %s must be of same type as "
@@ -524,19 +508,27 @@ parse_aggregate_functions (struct lexer *lexer, const struct dictionary *dict,
                       {
                         lex_next_msg (lexer, SN, 0, 0,
                                       _("The argument is numeric."));
-                        lex_ofs_msg (lexer, SN, vars_start_ofs, vars_end_ofs,
+                        lex_ofs_msg (lexer, SN, src_start_ofs, src_end_ofs,
                                      _("The variables have string type."));
                       }
                     else
                       {
                         lex_next_msg (lexer, SN, 0, 0,
                                       _("The argument is a string."));
-                        lex_ofs_msg (lexer, SN, vars_start_ofs, vars_end_ofs,
+                        lex_ofs_msg (lexer, SN, src_start_ofs, src_end_ofs,
                                      _("The variables are numeric."));
                       }
                    goto error;
                  }
 
+                if (i == 0)
+                  args_start_ofs = lex_ofs (lexer);
+               if (type == VAL_NUMERIC)
+                  arg[i].f = lex_tokval (lexer);
+                else
+                  arg[i].s = recode_substring_pool (dict_get_encoding (agr->dict),
+                                                    "UTF-8", lex_tokss (lexer),
+                                                    NULL);
                lex_get (lexer);
              }
           int args_end_ofs = lex_ofs (lexer) - 1;
@@ -551,23 +543,28 @@ parse_aggregate_functions (struct lexer *lexer, const struct dictionary *dict,
             message, i.e. `AGGREGATE x=SUM(y t).' will get this
             error message when a proper message would be more
             like `unknown variable t'. */
-         if (n_src != n_dest)
+         if (n_src != n_vars)
            {
              msg (SE, _("Number of source variables (%zu) does not match "
                         "number of target variables (%zu)."),
-                   n_src, n_dest);
+                   n_src, n_vars);
+              lex_ofs_msg (lexer, SN, src_start_ofs, src_end_ofs,
+                           _("These are the source variables."));
+              lex_ofs_msg (lexer, SN, dst_start_ofs, dst_end_ofs,
+                           _("These are the target variables."));
              goto error;
            }
 
-          if ((func_index == PIN || func_index == POUT
-              || func_index == FIN || func_index == FOUT)
+          if ((func_index == AGRF_PIN || func_index == AGRF_POUT
+              || func_index == AGRF_FIN || func_index == AGRF_FOUT)
               && (var_is_numeric (src[0])
                   ? arg[0].f > arg[1].f
-                  : str_compare_rpad (arg[0].c, arg[1].c) > 0))
+                  : buf_compare_rpad (arg[0].s.string, arg[0].s.length,
+                                      arg[1].s.string, arg[1].s.length) > 0))
             {
-              union agr_argument t = arg[0];
+              struct agr_argument tmp = arg[0];
               arg[0] = arg[1];
-              arg[1] = t;
+              arg[1] = tmp;
 
               lex_ofs_msg (lexer, SW, args_start_ofs, args_end_ofs,
                            _("The value arguments passed to the %s function "
@@ -577,105 +574,79 @@ parse_aggregate_functions (struct lexer *lexer, const struct dictionary *dict,
             }
        }
 
-      /* Finally add these to the linked list of aggregation
-         variables. */
-      for (i = 0; i < n_dest; i++)
+      /* Finally add these to the aggregation variables. */
+      for (size_t i = 0; i < n_vars; i++)
        {
+          const struct variable *existing_var = dict_lookup_var (agr->dict,
+                                                                 dest[i]);
+          if (existing_var)
+            {
+              if (var_get_dict_index (existing_var) >= starting_n_vars)
+               lex_ofs_error (lexer, dst_start_ofs, dst_end_ofs,
+                               _("Duplicate target variable name %s."),
+                               dest[i]);
+              else if (agr->add_variables)
+               lex_ofs_error (lexer, dst_start_ofs, dst_end_ofs,
+                               _("Variable name %s duplicates the name of a "
+                                 "variable in the active file dictionary."),
+                               dest[i]);
+              else
+               lex_ofs_error (lexer, dst_start_ofs, dst_end_ofs,
+                               _("Variable name %s duplicates the name of a "
+                                 "break variable."), dest[i]);
+              goto error;
+            }
+
+         /* Add variable. */
           if (agr->n_agr_vars >= allocated_agr_vars)
             agr->agr_vars = x2nrealloc (agr->agr_vars, &allocated_agr_vars,
                                         sizeof *agr->agr_vars);
-         struct agr_var *v = &agr->agr_vars[agr->n_agr_vars++];
+          struct agr_var *v = &agr->agr_vars[agr->n_agr_vars++];
           *v = (struct agr_var) {
+            .exclude = exclude,
             .moments = NULL,
+            .function = func_index,
             .src = src ? src[i] : NULL,
-            .exclude = exclude,
           };
 
-         /* Create the target variable in the aggregate
-             dictionary. */
-         {
-           struct variable *destvar;
-
-           v->function = func_index;
-
-           if (src)
-             {
-               if (var_is_alpha (src[i]))
-                 {
-                   v->function |= FSTRING;
-                   v->string = xmalloc (var_get_width (src[i]));
-                 }
-
-               if (function->alpha_type == VAL_STRING)
-                 destvar = dict_clone_var_as (agr->dict, v->src, dest[i]);
-               else
-                  {
-                    assert (var_is_numeric (v->src)
-                            || function->alpha_type == VAL_NUMERIC);
-                    destvar = dict_create_var (agr->dict, dest[i], 0);
-                    if (destvar != NULL)
-                      {
-                        struct fmt_spec f;
-                        if ((func_index == N || func_index == NMISS)
-                            && dict_get_weight (dict) != NULL)
-                          f = fmt_for_output (FMT_F, 8, 2);
-                        else
-                          f = function->format;
-                        var_set_both_formats (destvar, &f);
-                      }
-                  }
-             } else {
-                struct fmt_spec f;
-               destvar = dict_create_var (agr->dict, dest[i], 0);
-               if (destvar != NULL)
-                 {
-                   if ((func_index == N || func_index == NMISS)
-                       && dict_get_weight (dict) != NULL)
-                     f = fmt_for_output (FMT_F, 8, 2);
-                   else
-                     f = function->format;
-                   var_set_both_formats (destvar, &f);
-                 }
-           }
-
-           if (!destvar)
-             {
-               msg (SE, _("Variable name %s is not unique within the "
-                          "aggregate file dictionary, which contains "
-                          "the aggregate variables and the break "
-                          "variables."),
-                    dest[i]);
-               goto error;
-             }
+         /* Create the target variable in the aggregate dictionary. */
+          if (v->src && var_is_alpha (v->src))
+            v->string = xmalloc (var_get_width (v->src));
 
-           free (dest[i]);
-           if (dest_label[i])
-              var_set_label (destvar, dest_label[i]);
-
-           v->dest = destvar;
-         }
+          if (v->src && function->alpha_type == VAL_STRING)
+            v->dest = dict_clone_var_as_assert (agr->dict, v->src, dest[i]);
+          else
+            {
+              v->dest = dict_create_var_assert (agr->dict, dest[i], 0);
+
+              struct fmt_spec f;
+              if ((func_index == AGRF_N || func_index == AGRF_NMISS)
+                  && dict_get_weight (dict) != NULL)
+                f = fmt_for_output (FMT_F, 8, 2);
+              else
+                f = function->format;
+              var_set_both_formats (v->dest, &f);
+            }
+          if (dest_label[i])
+            var_set_label (v->dest, dest_label[i]);
 
          if (v->src != NULL)
-           {
-             int j;
-
-             if (var_is_numeric (v->src))
-               for (j = 0; j < function->n_args; j++)
-                 v->arg[j].f = arg[j].f;
-             else
-               for (j = 0; j < function->n_args; j++)
-                 v->arg[j].c = xstrdup (arg[j].c);
-           }
+            for (size_t j = 0; j < function->n_args; j++)
+              v->arg[j] = (struct agr_argument) {
+                .f = arg[j].f,
+                .s = arg[j].s.string ? ss_clone (arg[j].s) : ss_empty (),
+              };
        }
 
-      if (src != NULL && var_is_alpha (src[0]))
-       for (i = 0; i < function->n_args; i++)
-         {
-           free (arg[i].c);
-           arg[i].c = NULL;
-         }
+      ss_dealloc (&arg[0].s);
+      ss_dealloc (&arg[1].s);
 
       free (src);
+      for (size_t i = 0; i < n_vars; i++)
+       {
+         free (dest[i]);
+         free (dest_label[i]);
+       }
       free (dest);
       free (dest_label);
 
@@ -690,22 +661,15 @@ parse_aggregate_functions (struct lexer *lexer, const struct dictionary *dict,
       continue;
 
     error:
-      ds_destroy (&function_name);
-      for (i = 0; i < n_dest; i++)
+      for (size_t i = 0; i < n_vars; i++)
        {
          free (dest[i]);
          free (dest_label[i]);
        }
       free (dest);
       free (dest_label);
-      free (arg[0].c);
-      free (arg[1].c);
-      if (src && n_src && var_is_alpha (src[0]))
-       for (i = 0; i < function->n_args; i++)
-         {
-           free (arg[i].c);
-           arg[i].c = NULL;
-         }
+      ss_dealloc (&arg[0].s);
+      ss_dealloc (&arg[1].s);
       free (src);
 
       return false;
@@ -722,14 +686,11 @@ agr_destroy (struct agr_proc *agr)
     {
       struct agr_var *av = &agr->agr_vars[i];
 
-      if (av->function & FSTRING)
-       {
-         size_t n_args = agr_func_tab[av->function & FUNC].n_args;
-         for (size_t i = 0; i < n_args; i++)
-           free (av->arg[i].c);
-         free (av->string);
-       }
-      else if (av->function == SD)
+      ss_dealloc (&av->arg[0].s);
+      ss_dealloc (&av->arg[1].s);
+      free (av->string);
+
+      if (av->function == AGRF_SD)
         moments1_destroy (av->moments);
 
       dict_destroy_internal_var (av->subject);
@@ -746,31 +707,55 @@ agr_destroy (struct agr_proc *agr)
 static void
 accumulate_aggregate_info (struct agr_proc *agr, const struct ccase *input)
 {
-  double weight;
   bool bad_warn = true;
-
-  weight = dict_get_case_weight (agr->src_dict, input, &bad_warn);
-
+  double weight = dict_get_case_weight (agr->src_dict, input, &bad_warn);
   for (size_t i = 0; i < agr->n_agr_vars; i++)
     {
       struct agr_var *av = &agr->agr_vars[i];
       if (av->src)
         {
+          bool is_string = var_is_alpha (av->src);
           const union value *v = case_data (input, av->src);
           int src_width = var_get_width (av->src);
+          const struct substring vs = (src_width > 0
+                                       ? value_ss (v, src_width)
+                                       : ss_empty ());
 
           if (var_is_value_missing (av->src, v) & av->exclude)
             {
               switch (av->function)
                 {
-                case NMISS:
-                case NMISS | FSTRING:
+                case AGRF_NMISS:
                   av->dbl += weight;
                   break;
-                case NUMISS:
-                case NUMISS | FSTRING:
+
+                case AGRF_NUMISS:
                   av->int1++;
                   break;
+
+                case AGRF_SUM:
+                case AGRF_MEAN:
+                case AGRF_MEDIAN:
+                case AGRF_SD:
+                case AGRF_MAX:
+                case AGRF_MIN:
+                case AGRF_PGT:
+                case AGRF_PLT:
+                case AGRF_PIN:
+                case AGRF_POUT:
+                case AGRF_FGT:
+                case AGRF_FLT:
+                case AGRF_FIN:
+                case AGRF_FOUT:
+                case AGRF_CGT:
+                case AGRF_CLT:
+                case AGRF_CIN:
+                case AGRF_COUT:
+                case AGRF_N:
+                case AGRF_NU:
+                case AGRF_FIRST:
+                case AGRF_LAST:
+                  break;
                 }
               av->saw_missing = true;
               continue;
@@ -780,147 +765,151 @@ accumulate_aggregate_info (struct agr_proc *agr, const struct ccase *input)
           av->W += weight;
           switch (av->function)
             {
-            case SUM:
+            case AGRF_SUM:
               av->dbl += v->f * weight;
               av->int1 = 1;
               break;
-            case MEAN:
+
+            case AGRF_MEAN:
               av->dbl += v->f * weight;
               break;
-            case MEDIAN:
-              {
-                double wv ;
-                struct ccase *cout;
-
-                cout = case_create (casewriter_get_proto (av->writer));
 
+            case AGRF_MEDIAN:
+              {
+                struct ccase *cout = case_create (casewriter_get_proto (av->writer));
                 *case_num_rw (cout, av->subject) = case_num (input, av->src);
-
-                wv = dict_get_case_weight (agr->src_dict, input, NULL);
-
-                *case_num_rw (cout, av->weight) = wv;
-
-                av->cc += wv;
-
+                *case_num_rw (cout, av->weight) = weight;
                 casewriter_write (av->writer, cout);
               }
               break;
-            case SD:
+
+            case AGRF_SD:
               moments1_add (av->moments, v->f, weight);
               break;
-            case MAX:
-              av->dbl = MAX (av->dbl, v->f);
-              av->int1 = 1;
-              break;
-            case MAX | FSTRING:
-              /* Need to do some kind of Unicode collation thingy here */
-              if (memcmp (av->string, v->s, src_width) < 0)
+
+            case AGRF_MAX:
+              if (!is_string)
+                av->dbl = MAX (av->dbl, v->f);
+              else if (memcmp (av->string, v->s, src_width) < 0)
                 memcpy (av->string, v->s, src_width);
               av->int1 = 1;
               break;
-            case MIN:
-              av->dbl = MIN (av->dbl, v->f);
-              av->int1 = 1;
-              break;
-            case MIN | FSTRING:
-              if (memcmp (av->string, v->s, src_width) > 0)
+
+            case AGRF_MIN:
+              if (!is_string)
+                av->dbl = MIN (av->dbl, v->f);
+              else if (memcmp (av->string, v->s, src_width) > 0)
                 memcpy (av->string, v->s, src_width);
+              av->dbl = MIN (av->dbl, v->f);
               av->int1 = 1;
               break;
-            case FGT:
-            case PGT:
-              if (v->f > av->arg[0].f)
-                av->dbl += weight;
-              break;
-            case FGT | FSTRING:
-            case PGT | FSTRING:
-              if (memcmp (av->arg[0].c, v->s, src_width) < 0)
-                av->dbl += weight;
-              break;
-            case FLT:
-            case PLT:
-              if (v->f < av->arg[0].f)
-                av->dbl += weight;
-              break;
-            case FLT | FSTRING:
-            case PLT | FSTRING:
-              if (memcmp (av->arg[0].c, v->s, src_width) > 0)
-                av->dbl += weight;
-              break;
-            case FIN:
-            case PIN:
-              if (av->arg[0].f <= v->f && v->f <= av->arg[1].f)
+
+            case AGRF_FGT:
+            case AGRF_PGT:
+            case AGRF_CGT:
+              if (is_string
+                  ? ss_compare_rpad (av->arg[0].s, vs) < 0
+                  : v->f > av->arg[0].f)
                 av->dbl += weight;
               break;
-            case FIN | FSTRING:
-            case PIN | FSTRING:
-              if (memcmp (av->arg[0].c, v->s, src_width) <= 0
-                  && memcmp (av->arg[1].c, v->s, src_width) >= 0)
+
+            case AGRF_FLT:
+            case AGRF_PLT:
+            case AGRF_CLT:
+              if (is_string
+                  ? ss_compare_rpad (av->arg[0].s, vs) > 0
+                  : v->f < av->arg[0].f)
                 av->dbl += weight;
               break;
-            case FOUT:
-            case POUT:
-              if (av->arg[0].f > v->f || v->f > av->arg[1].f)
+
+            case AGRF_FIN:
+            case AGRF_PIN:
+            case AGRF_CIN:
+              if (is_string
+                  ? (ss_compare_rpad (av->arg[0].s, vs) <= 0
+                     && ss_compare_rpad (av->arg[1].s, vs) >= 0)
+                  : av->arg[0].f <= v->f && v->f <= av->arg[1].f)
                 av->dbl += weight;
               break;
-            case FOUT | FSTRING:
-            case POUT | FSTRING:
-              if (memcmp (av->arg[0].c, v->s, src_width) > 0
-                  || memcmp (av->arg[1].c, v->s, src_width) < 0)
+
+            case AGRF_FOUT:
+            case AGRF_POUT:
+            case AGRF_COUT:
+              if (is_string
+                  ? (ss_compare_rpad (av->arg[0].s, vs) > 0
+                     || ss_compare_rpad (av->arg[1].s, vs) < 0)
+                  : av->arg[0].f > v->f || v->f > av->arg[1].f)
                 av->dbl += weight;
               break;
-            case N:
-            case N | FSTRING:
+
+            case AGRF_N:
               av->dbl += weight;
               break;
-            case NU:
-            case NU | FSTRING:
+
+            case AGRF_NU:
               av->int1++;
               break;
-            case FIRST:
-              if (av->int1 == 0)
-                {
-                  av->dbl = v->f;
-                  av->int1 = 1;
-                }
-              break;
-            case FIRST | FSTRING:
+
+            case AGRF_FIRST:
               if (av->int1 == 0)
                 {
-                  memcpy (av->string, v->s, src_width);
+                  if (is_string)
+                    memcpy (av->string, v->s, src_width);
+                  else
+                    av->dbl = v->f;
                   av->int1 = 1;
                 }
               break;
-            case LAST:
-              av->dbl = v->f;
-              av->int1 = 1;
-              break;
-            case LAST | FSTRING:
-              memcpy (av->string, v->s, src_width);
+
+            case AGRF_LAST:
+              if (is_string)
+                memcpy (av->string, v->s, src_width);
+              else
+                av->dbl = v->f;
               av->int1 = 1;
               break;
-            case NMISS:
-            case NMISS | FSTRING:
-            case NUMISS:
-            case NUMISS | FSTRING:
+
+            case AGRF_NMISS:
+            case AGRF_NUMISS:
               /* Our value is not missing or it would have been
                  caught earlier.  Nothing to do. */
               break;
-            default:
-              NOT_REACHED ();
             }
         }
       else
         {
+          av->W += weight;
           switch (av->function)
             {
-            case N:
-              av->dbl += weight;
+            case AGRF_N:
               break;
-            case NU:
+
+            case AGRF_NU:
               av->int1++;
               break;
-            default:
+
+            case AGRF_SUM:
+            case AGRF_MEAN:
+            case AGRF_MEDIAN:
+            case AGRF_SD:
+            case AGRF_MAX:
+            case AGRF_MIN:
+            case AGRF_PGT:
+            case AGRF_PLT:
+            case AGRF_PIN:
+            case AGRF_POUT:
+            case AGRF_FGT:
+            case AGRF_FLT:
+            case AGRF_FIN:
+            case AGRF_FOUT:
+            case AGRF_CGT:
+            case AGRF_CLT:
+            case AGRF_CIN:
+            case AGRF_COUT:
+            case AGRF_NMISS:
+            case AGRF_NUMISS:
+            case AGRF_FIRST:
+            case AGRF_LAST:
               NOT_REACHED ();
             }
         }
@@ -940,9 +929,8 @@ dump_aggregate_info (const struct agr_proc *agr, struct casewriter *output, cons
   else
     {
       int value_idx = 0;
-      int i;
 
-      for (i = 0; i < agr->break_n_vars; i++)
+      for (size_t i = 0; i < agr->break_n_vars; i++)
        {
          const struct variable *v = agr->break_vars[i];
          value_copy (case_data_rw_idx (c, value_idx),
@@ -959,8 +947,10 @@ dump_aggregate_info (const struct agr_proc *agr, struct casewriter *output, cons
       int width = var_get_width (av->dest);
 
       if (agr->missing == COLUMNWISE && av->saw_missing
-          && (av->function & FUNC) != N && (av->function & FUNC) != NU
-          && (av->function & FUNC) != NMISS && (av->function & FUNC) != NUMISS)
+          && av->function != AGRF_N
+          && av->function != AGRF_NU
+          && av->function != AGRF_NMISS
+          && av->function != AGRF_NUMISS)
         {
           value_set_missing (v, width);
           casewriter_destroy (av->writer);
@@ -969,17 +959,19 @@ dump_aggregate_info (const struct agr_proc *agr, struct casewriter *output, cons
 
       switch (av->function)
         {
-        case SUM:
+        case AGRF_SUM:
           v->f = av->int1 ? av->dbl : SYSMIS;
           break;
-        case MEAN:
+
+        case AGRF_MEAN:
           v->f = av->W != 0.0 ? av->dbl / av->W : SYSMIS;
           break;
-        case MEDIAN:
+
+        case AGRF_MEDIAN:
           {
             if (av->writer)
               {
-                struct percentile *median = percentile_create (0.5, av->cc);
+                struct percentile *median = percentile_create (0.5, av->W);
                 struct order_stats *os = &median->parent;
                 struct casereader *sorted_reader = casewriter_make_reader (av->writer);
                 av->writer = NULL;
@@ -995,79 +987,65 @@ dump_aggregate_info (const struct agr_proc *agr, struct casewriter *output, cons
             v->f = av->dbl;
           }
           break;
-        case SD:
+
+        case AGRF_SD:
           {
             double variance;
 
-            /* FIXME: we should use two passes. */
             moments1_calculate (av->moments, NULL, NULL, &variance,
                                 NULL, NULL);
-            if (variance != SYSMIS)
-              v->f = sqrt (variance);
-            else
-              v->f = SYSMIS;
+            v->f = variance != SYSMIS ? sqrt (variance) : SYSMIS;
           }
           break;
-        case MAX:
-        case MIN:
-          v->f = av->int1 ? av->dbl : SYSMIS;
-          break;
-        case MAX | FSTRING:
-        case MIN | FSTRING:
-          if (av->int1)
-            memcpy (v->s, av->string, width);
+
+        case AGRF_MAX:
+        case AGRF_MIN:
+        case AGRF_FIRST:
+        case AGRF_LAST:
+          if (!width)
+            v->f = av->int1 ? av->dbl : SYSMIS;
           else
-            value_set_missing (v, width);
+            {
+              if (av->int1)
+                memcpy (v->s, av->string, width);
+              else
+                value_set_missing (v, width);
+            }
           break;
-        case FGT:
-        case FGT | FSTRING:
-        case FLT:
-        case FLT | FSTRING:
-        case FIN:
-        case FIN | FSTRING:
-        case FOUT:
-        case FOUT | FSTRING:
+
+        case AGRF_FGT:
+        case AGRF_FLT:
+        case AGRF_FIN:
+        case AGRF_FOUT:
           v->f = av->W ? av->dbl / av->W : SYSMIS;
           break;
-        case PGT:
-        case PGT | FSTRING:
-        case PLT:
-        case PLT | FSTRING:
-        case PIN:
-        case PIN | FSTRING:
-        case POUT:
-        case POUT | FSTRING:
+
+        case AGRF_PGT:
+        case AGRF_PLT:
+        case AGRF_PIN:
+        case AGRF_POUT:
           v->f = av->W ? av->dbl / av->W * 100.0 : SYSMIS;
           break;
-        case N:
-        case N | FSTRING:
+
+        case AGRF_CGT:
+        case AGRF_CLT:
+        case AGRF_CIN:
+        case AGRF_COUT:
           v->f = av->dbl;
           break;
-        case NU:
-        case NU | FSTRING:
-          v->f = av->int1;
-          break;
-        case FIRST:
-        case LAST:
-          v->f = av->int1 ? av->dbl : SYSMIS;
+
+        case AGRF_N:
+          v->f = av->W;
           break;
-        case FIRST | FSTRING:
-        case LAST | FSTRING:
-          if (av->int1)
-            memcpy (v->s, av->string, width);
-          else
-            value_set_missing (v, width);
+
+        case AGRF_NU:
+        case AGRF_NUMISS:
+          v->f = av->int1;
           break;
-        case NMISS:
-        case NMISS | FSTRING:
+
+        case AGRF_NMISS:
           v->f = av->dbl;
           break;
-        case NUMISS:
-        case NUMISS | FSTRING:
-          v->f = av->int1;
-          break;
-        default:
-          NOT_REACHED ();
         }
     }
 
@@ -1084,26 +1062,27 @@ initialize_aggregate_info (struct agr_proc *agr)
       av->saw_missing = false;
       av->dbl = av->W = 0.0;
       av->int1 = 0;
+
+      int width = av->src ? var_get_width (av->src) : 0;
       switch (av->function)
        {
-       case MIN:
-         av->dbl = DBL_MAX;
-         break;
-       case MIN | FSTRING:
-         memset (av->string, 255, var_get_width (av->src));
-         break;
-       case MAX:
-         av->dbl = -DBL_MAX;
+       case AGRF_MIN:
+          if (!width)
+            av->dbl = DBL_MAX;
+          else
+            memset (av->string, 255, width);
          break;
-       case MAX | FSTRING:
-         memset (av->string, 0, var_get_width (av->src));
+
+       case AGRF_MAX:
+          if (!width)
+            av->dbl = -DBL_MAX;
+         else
+            memset (av->string, 0, width);
          break;
-       case MEDIAN:
-         {
-            struct caseproto *proto;
-            struct subcase ordering;
 
-            proto = caseproto_create ();
+       case AGRF_MEDIAN:
+         {
+            struct caseproto *proto = caseproto_create ();
             proto = caseproto_add_width (proto, 0);
             proto = caseproto_add_width (proto, 0);
 
@@ -1113,21 +1092,41 @@ initialize_aggregate_info (struct agr_proc *agr)
            if (! av->weight)
              av->weight = dict_create_internal_var (1, 0);
 
+            struct subcase ordering;
             subcase_init_var (&ordering, av->subject, SC_ASCEND);
            av->writer = sort_create_writer (&ordering, proto);
             subcase_uninit (&ordering);
             caseproto_unref (proto);
-
-           av->cc = 0;
          }
          break;
-        case SD:
+
+        case AGRF_SD:
           if (av->moments == NULL)
             av->moments = moments1_create (MOMENT_VARIANCE);
           else
             moments1_clear (av->moments);
           break;
-        default:
+
+        case AGRF_SUM:
+        case AGRF_MEAN:
+        case AGRF_PGT:
+        case AGRF_PLT:
+        case AGRF_PIN:
+        case AGRF_POUT:
+        case AGRF_FGT:
+        case AGRF_FLT:
+        case AGRF_FIN:
+        case AGRF_FOUT:
+        case AGRF_CGT:
+        case AGRF_CLT:
+        case AGRF_CIN:
+        case AGRF_COUT:
+        case AGRF_N:
+        case AGRF_NU:
+        case AGRF_NMISS:
+        case AGRF_NUMISS:
+        case AGRF_FIRST:
+        case AGRF_LAST:
           break;
        }
     }
index 8cddbcfa574120a20d326732079f3f849f070e34..293d0f589c7c7c36c3f63f1a828ab564c855dad6 100644 (file)
@@ -30,25 +30,50 @@ enum agr_src_vars
     AGR_SV_OPT
   };
 
+#define AGGREGATE_FUNCTIONS                                             \
+  AGRF(AGRF_SUM,     "SUM",     N_("Sum of values"),                         AGR_SV_YES, 0, -1,           8,  2) \
+  AGRF(AGRF_MEAN,    "MEAN",    N_("Mean average"),                          AGR_SV_YES, 0, -1,           8,  2) \
+  AGRF(AGRF_MEDIAN,  "MEDIAN",  N_("Median"),                                AGR_SV_YES, 0, -1,           8,  2) \
+  AGRF(AGRF_SD,      "SD",      N_("Standard deviation"),                    AGR_SV_YES, 0, -1,           8,  2) \
+  AGRF(AGRF_MAX,     "MAX",     N_("Maximum value"),                         AGR_SV_YES, 0, VAL_STRING,  -1, -1) \
+  AGRF(AGRF_MIN,     "MIN",     N_("Minimum value"),                         AGR_SV_YES, 0, VAL_STRING,  -1, -1) \
+  AGRF(AGRF_PGT,     "PGT",     N_("Percentage greater than"),               AGR_SV_YES, 1, VAL_NUMERIC,  5,  1) \
+  AGRF(AGRF_PLT,     "PLT",     N_("Percentage less than"),                  AGR_SV_YES, 1, VAL_NUMERIC,  5,  1) \
+  AGRF(AGRF_PIN,     "PIN",     N_("Percentage included in range"),          AGR_SV_YES, 2, VAL_NUMERIC,  5,  1) \
+  AGRF(AGRF_POUT,    "POUT",    N_("Percentage excluded from range"),        AGR_SV_YES, 2, VAL_NUMERIC,  5,  1) \
+  AGRF(AGRF_FGT,     "FGT",     N_("Fraction greater than"),                 AGR_SV_YES, 1, VAL_NUMERIC,  5,  3) \
+  AGRF(AGRF_FLT,     "FLT",     N_("Fraction less than"),                    AGR_SV_YES, 1, VAL_NUMERIC,  5,  3) \
+  AGRF(AGRF_FIN,     "FIN",     N_("Fraction included in range"),            AGR_SV_YES, 2, VAL_NUMERIC,  5,  3) \
+  AGRF(AGRF_FOUT,    "FOUT",    N_("Fraction excluded from range"),          AGR_SV_YES, 2, VAL_NUMERIC,  5,  3) \
+  AGRF(AGRF_CGT,     "CGT",     N_("Count greater than"),                    AGR_SV_YES, 1, VAL_NUMERIC,  5,  1) \
+  AGRF(AGRF_CLT,     "CLT",     N_("Count less than"),                       AGR_SV_YES, 1, VAL_NUMERIC,  5,  1) \
+  AGRF(AGRF_CIN,     "CIN",     N_("Count included in range"),               AGR_SV_YES, 2, VAL_NUMERIC,  5,  1) \
+  AGRF(AGRF_COUT,    "COUT",    N_("Count excluded from range"),             AGR_SV_YES, 2, VAL_NUMERIC,  5,  1) \
+  AGRF(AGRF_N,       "N",       N_("Number of cases"),                       AGR_SV_NO,  0, VAL_NUMERIC,  7,  0) \
+  AGRF(AGRF_NU,      "NU",      N_("Number of cases (unweighted)"),          AGR_SV_OPT, 0, VAL_NUMERIC,  7,  0) \
+  AGRF(AGRF_NMISS,   "NMISS",   N_("Number of missing values"),              AGR_SV_YES, 0, VAL_NUMERIC,  7,  0) \
+  AGRF(AGRF_NUMISS,  "NUMISS",  N_("Number of missing values (unweighted)"), AGR_SV_YES, 0, VAL_NUMERIC,  7,  0) \
+  AGRF(AGRF_FIRST,   "FIRST",   N_("First non-missing value"),               AGR_SV_YES, 0, VAL_STRING,  -1, -1) \
+  AGRF(AGRF_LAST,    "LAST",    N_("Last non-missing value"),                AGR_SV_YES, 0, VAL_STRING,  -1, -1)
+
 /* Aggregation functions. */
-enum
+enum agr_function
   {
-    SUM, MEAN, MEDIAN, SD, MAX, MIN, PGT, PLT, PIN, POUT, FGT, FLT, FIN,
-    FOUT, N, NU, NMISS, NUMISS, FIRST, LAST,
-
-    FUNC = 0x1f, /* Function mask. */
-    FSTRING = 1<<5, /* String function bit. */
+#define AGRF(ENUM, NAME, DESCRIPTION, SRC_VARS, N_ARGS, ALPHA_TYPE, W, D) \
+    ENUM,
+AGGREGATE_FUNCTIONS
+#undef AGRF
   };
 
 /* Attributes of an aggregation function. */
 struct agr_func
   {
-    const char *name;          /* Aggregation function name. */
+    const char *name;           /* Aggregation function name. */
     const char *description;    /* Translatable string describing the function. */
     enum agr_src_vars src_vars; /* Whether source variables are a parameter of the function */
     size_t n_args;              /* Number of arguments (not including src vars). */
     enum val_type alpha_type;   /* When given ALPHA arguments, output type. */
-    struct fmt_spec format;    /* Format spec if alpha_type != ALPHA. */
+    struct fmt_spec format;     /* Format spec if alpha_type != ALPHA. */
   };
 
 extern const struct agr_func agr_func_tab[];
index e851bac1f3f7cef17b35e134434be77f8a84570d..9cd6351e8d9d1dfdf67f1841b94233abad448f5e 100644 (file)
@@ -839,6 +839,15 @@ ss_compare (struct substring a, struct substring b)
   return retval;
 }
 
+/* Compares A to B and returns a strcmp()-type comparison result.  The shorter
+   string is considered to be padded with spaces to the length of the
+   longer. */
+int
+ss_compare_rpad (struct substring a, struct substring b)
+{
+  return buf_compare_rpad (a.string, a.length, b.string, b.length);
+}
+
 /* Compares A and B case-insensitively and returns a
    strcmp()-type comparison result. */
 int
index b8bb6ffca2e43077d44dae9ae26ed6ec65e0e09e..17584f8920ecea5116dde25d127b925f9242c7fb 100644 (file)
@@ -140,6 +140,7 @@ size_t ss_cspan (struct substring, struct substring stop_set);
 size_t ss_find_byte (struct substring, char);
 size_t ss_find_substring (struct substring, struct substring);
 int ss_compare (struct substring, struct substring);
+int ss_compare_rpad (struct substring, struct substring);
 int ss_compare_case (struct substring, struct substring);
 int ss_equals (struct substring, struct substring);
 int ss_equals_case (struct substring, struct substring);
index 8877de092dcc1847c108ecb63339920dafb963fc..33f6d78f5318c194d8f879e6ead506a509a560b8 100644 (file)
@@ -168,7 +168,7 @@ refresh (PsppireDialogAction *fd_)
   gtk_entry_set_text (GTK_ENTRY (agg->summary_var_name_entry), "N_BREAK");
   gtk_editable_select_region (GTK_EDITABLE (agg->summary_var_name_entry), 0, -1);
 
-  gtk_combo_box_set_active (GTK_COMBO_BOX (agg->function_combo), N);
+  gtk_combo_box_set_active (GTK_COMBO_BOX (agg->function_combo), AGRF_N);
 
   gtk_list_store_clear (PSPPIRE_ACR (agg->summary_acr)->list_store);
 
index 2b208d319829f642ebfd03bef445916028668c93..f4f0fee6650f6fd09d486e55944eeb9a4c77b38f 100644 (file)
@@ -55,8 +55,8 @@ AGGREGATE dnl
 m4_if([$1], [active], [OUTFILE=*],
       [$1], [external], [OUTFILE='aggregate.sys'],
       [outfile=aggregate]) dnl
-m4_if([$2], [presorted], [/PRESORTED]) dnl
 m4_if([$3], [columnwise], [/MISSING=COLUMNWISE])
+m4_if([$2], [presorted], [/PRESORTED]) dnl
         /DOCUMENT
         /BREAK=g
         /N = n
@@ -283,39 +283,231 @@ x,cn,y,sum,mean,median
 
 AT_CLEANUP
 
+AT_SETUP([AGGREGATE duplicate variable errors])
+AT_DATA([insert.sps], [dnl
+INSERT FILE='aggregate.sps' ERROR=IGNORE.
+])
+AT_DATA([aggregate.sps], [dnl
+DATA LIST NOTABLE LIST /x.
+AGGREGATE OUTFILE=* /BREAK=x /x=N.
+AGGREGATE OUTFILE=* MODE=ADDVARIABLES /x=N.
+AGGREGATE OUTFILE=* MODE=ADDVARIABLES /y=N /y=N.
+])
+AT_CHECK([pspp --testing-mode -O format=csv insert.sps], [1], [dnl
+"aggregate.sps:2.31: error: AGGREGATE: Variable name x duplicates the name of a break variable.
+    2 | AGGREGATE OUTFILE=* /BREAK=x /x=N.
+      |                               ^"
 
-AT_SETUP([AGGREGATE buggy duplicate variables])
-dnl Test for a bug which crashed when duplicated
-dnl variables were attempted.
-AT_DATA([dup-variables.sps],
-  [DATA LIST NOTABLE LIST /x * .
-begin data
-1
-1
-1
-1
-2
-2
-2
-3
-3
-3
-3
-3
-3
-end data.
+"aggregate.sps:3.40: error: AGGREGATE: Variable name x duplicates the name of a variable in the active file dictionary.
+    3 | AGGREGATE OUTFILE=* MODE=ADDVARIABLES /x=N.
+      |                                        ^"
 
-AGGREGATE OUTFILE=* MODE=ADDVARIABLES
-       /BREAK= x
-       /N_BREAK = N.
+"aggregate.sps:4.45: error: AGGREGATE: Duplicate target variable name y.
+    4 | AGGREGATE OUTFILE=* MODE=ADDVARIABLES /y=N /y=N.
+      |                                             ^"
+])
+AT_CLEANUP
 
-AGGREGATE OUTFILE=* MODE=ADDVARIABLES
-       /BREAK= x
-       /N_BREAK = N.
+AT_SETUP([AGGREGATE presorted warnings])
+AT_DATA([insert.sps], [dnl
+INSERT FILE='aggregate.sps' ERROR=IGNORE.
 ])
+AT_DATA([aggregate.sps], [dnl
+DATA LIST NOTABLE LIST /x.
+AGGREGATE/PRESORTED/BREAK=x(A).
+AGGREGATE/BREAK=x(A).
+AGGREGATE/OUTFILE=* MODE=ADDVARIABLES/BREAK=x(A).
+])
+AT_CHECK([pspp --testing-mode -O format=csv insert.sps], [1], [dnl
+"aggregate.sps:2.27-2.30: warning: AGGREGATE: When the input data is presorted, specifying sorting directions with (A) or (D) has no effect.  Output data will be sorted the same way as the input data.
+    2 | AGGREGATE/PRESORTED/BREAK=x(A).
+      |                           ^~~~"
+
+"aggregate.sps:2.11-2.19: note: AGGREGATE: The PRESORTED subcommand state that the input data is presorted.
+    2 | AGGREGATE/PRESORTED/BREAK=x(A).
+      |           ^~~~~~~~~"
+
+"aggregate.sps:2.31: error: AGGREGATE: Syntax error expecting `/'.
+    2 | AGGREGATE/PRESORTED/BREAK=x(A).
+      |                               ^"
+
+"aggregate.sps:3.17-3.20: warning: AGGREGATE: When the input data is presorted, specifying sorting directions with (A) or (D) has no effect.  Output data will be sorted the same way as the input data.
+    3 | AGGREGATE/BREAK=x(A).
+      |                 ^~~~"
+
+aggregate.sps:3: note: AGGREGATE: The input data must be presorted because the OUTFILE subcommand is not specified.
+
+"aggregate.sps:3.21: error: AGGREGATE: Syntax error expecting `/'.
+    3 | AGGREGATE/BREAK=x(A).
+      |                     ^"
+
+"aggregate.sps:4.45-4.48: warning: AGGREGATE: When the input data is presorted, specifying sorting directions with (A) or (D) has no effect.  Output data will be sorted the same way as the input data.
+    4 | AGGREGATE/OUTFILE=* MODE=ADDVARIABLES/BREAK=x(A).
+      |                                             ^~~~"
 
-AT_CHECK([pspp -O format=csv dup-variables.sps], [1],
-["dup-variables.sps:24: error: AGGREGATE: Variable name N_BREAK is not unique within the aggregate file dictionary, which contains the aggregate variables and the break variables."
+"aggregate.sps:4.26-4.37: note: AGGREGATE: ADDVARIABLES implies that the input data is presorted.
+    4 | AGGREGATE/OUTFILE=* MODE=ADDVARIABLES/BREAK=x(A).
+      |                          ^~~~~~~~~~~~"
+
+"aggregate.sps:4.49: error: AGGREGATE: Syntax error expecting `/'.
+    4 | AGGREGATE/OUTFILE=* MODE=ADDVARIABLES/BREAK=x(A).
+      |                                                 ^"
+])
+AT_CLEANUP
+
+AT_SETUP([AGGREGATE - subcommand syntax errors])
+AT_DATA([insert.sps], [dnl
+INSERT FILE='aggregate.sps' ERROR=IGNORE.
+])
+AT_DATA([aggregate.sps], [dnl
+DATA LIST NOTABLE LIST /x.
+AGGREGATE OUTFILE=**.
+AGGREGATE OUTFILE=* MODE=**.
+AGGREGATE /MISSING=**.
+AGGREGATE /BREAK=**.
+])
+AT_CHECK([pspp --testing-mode -O format=csv insert.sps], [1], [dnl
+"aggregate.sps:2.19-2.20: error: AGGREGATE: Syntax error expecting a file name or handle name.
+    2 | AGGREGATE OUTFILE=**.
+      |                   ^~"
+
+"aggregate.sps:3.26-3.27: error: AGGREGATE: Syntax error expecting ADDVARIABLES or REPLACE.
+    3 | AGGREGATE OUTFILE=* MODE=**.
+      |                          ^~"
+
+"aggregate.sps:4.20-4.21: error: AGGREGATE: Syntax error expecting COLUMNWISE.
+    4 | AGGREGATE /MISSING=**.
+      |                    ^~"
+
+"aggregate.sps:5.18-5.19: error: AGGREGATE: Syntax error expecting variable name.
+    5 | AGGREGATE /BREAK=**.
+      |                  ^~"
+])
+AT_CLEANUP
+
+AT_SETUP([AGGREGATE - aggregation function syntax errors])
+AT_DATA([insert.sps], [dnl
+INSERT FILE='aggregate.sps' ERROR=IGNORE.
+])
+AT_DATA([aggregate.sps], [dnl
+DATA LIST NOTABLE LIST /x (f8.2) s (a8).
+AGGREGATE **.
+AGGREGATE / **.
+AGGREGATE /y.
+AGGREGATE /y=**.
+AGGREGATE /y=xyzzy.
+AGGREGATE /y=mean.
+AGGREGATE /y=mean(**).
+AGGREGATE /y=fgt(x **).
+AGGREGATE /y=fgt(x 'xyzzy').
+AGGREGATE /y=fgt(s 1).
+AGGREGATE /y=fgt(s x).
+AGGREGATE /y=sum(s).
+AGGREGATE /y=sum(x. /* )
+AGGREGATE /y=min(x, s).
+AGGREGATE /y t=min(x).
+AGGREGATE /y=pin(x, 2, 1).
+AGGREGATE /y=mean(x)**.
 ])
+AT_CHECK([pspp --testing-mode -O format=csv insert.sps], [1], [dnl
+"aggregate.sps:2.11-2.12: error: AGGREGATE: Syntax error expecting `/'.
+    2 | AGGREGATE **.
+      |           ^~"
 
+"aggregate.sps:3.13-3.14: error: AGGREGATE: Syntax error expecting variable name.
+    3 | AGGREGATE / **.
+      |             ^~"
+
+"aggregate.sps:4.13: error: AGGREGATE: Syntax error expecting variable name.
+    4 | AGGREGATE /y.
+      |             ^"
+
+"aggregate.sps:5.14-5.15: error: AGGREGATE: Syntax error expecting aggregation function.
+    5 | AGGREGATE /y=**.
+      |              ^~"
+
+"aggregate.sps:6.14-6.18: error: AGGREGATE: Unknown aggregation function xyzzy.
+    6 | AGGREGATE /y=xyzzy.
+      |              ^~~~~"
+
+"aggregate.sps:7.18: error: AGGREGATE: Syntax error expecting `('.
+    7 | AGGREGATE /y=mean.
+      |                  ^"
+
+"aggregate.sps:8.19-8.20: error: AGGREGATE: Syntax error expecting variable name.
+    8 | AGGREGATE /y=mean(**).
+      |                   ^~"
+
+"aggregate.sps:9.20-9.21: error: AGGREGATE: Missing argument 1 to FGT.
+    9 | AGGREGATE /y=fgt(x **).
+      |                    ^~"
+
+aggregate.sps:10: error: AGGREGATE: Arguments to FGT must be of same type as source variables.
+
+"aggregate.sps:10.20-10.26: note: AGGREGATE: The argument is a string.
+   10 | AGGREGATE /y=fgt(x 'xyzzy').
+      |                    ^~~~~~~"
+
+"aggregate.sps:10.18: note: AGGREGATE: The variables are numeric.
+   10 | AGGREGATE /y=fgt(x 'xyzzy').
+      |                  ^"
+
+aggregate.sps:11: error: AGGREGATE: Arguments to FGT must be of same type as source variables.
+
+"aggregate.sps:11.20: note: AGGREGATE: The argument is numeric.
+   11 | AGGREGATE /y=fgt(s 1).
+      |                    ^"
+
+"aggregate.sps:11.18: note: AGGREGATE: The variables have string type.
+   11 | AGGREGATE /y=fgt(s 1).
+      |                  ^"
+
+"aggregate.sps:12.20: error: AGGREGATE: s and x are not the same type.  All variables in this variable list must be of the same type.  x will be omitted from the list.
+   12 | AGGREGATE /y=fgt(s x).
+      |                    ^"
+
+"aggregate.sps:12.21: error: AGGREGATE: Missing argument 1 to FGT.
+   12 | AGGREGATE /y=fgt(s x).
+      |                     ^"
+
+"aggregate.sps:13.18: warning: AGGREGATE: s is not a numeric variable.  It will not be included in the variable list.
+   13 | AGGREGATE /y=sum(s).
+      |                  ^"
+
+"aggregate.sps:14.19: error: AGGREGATE: Syntax error expecting `)'.
+   14 | AGGREGATE /y=sum(x. /* )
+      |                   ^"
+
+aggregate.sps:15: error: AGGREGATE: Number of source variables (2) does not match number of target variables (1).
+
+"aggregate.sps:15.18-15.21: note: AGGREGATE: These are the source variables.
+   15 | AGGREGATE /y=min(x, s).
+      |                  ^~~~"
+
+"aggregate.sps:15.12: note: AGGREGATE: These are the target variables.
+   15 | AGGREGATE /y=min(x, s).
+      |            ^"
+
+aggregate.sps:16: error: AGGREGATE: Number of source variables (1) does not match number of target variables (2).
+
+"aggregate.sps:16.20: note: AGGREGATE: These are the source variables.
+   16 | AGGREGATE /y t=min(x).
+      |                    ^"
+
+"aggregate.sps:16.12-16.14: note: AGGREGATE: These are the target variables.
+   16 | AGGREGATE /y t=min(x).
+      |            ^~~"
+
+"aggregate.sps:17.21-17.24: warning: AGGREGATE: The value arguments passed to the PIN function are out of order.  They will be treated as if they had been specified in the correct order.
+   17 | AGGREGATE /y=pin(x, 2, 1).
+      |                     ^~~~"
+
+"aggregate.sps:18.1-18.9: error: AGGREGATE: Syntax error expecting BEGIN.
+   18 | AGGREGATE /y=mean(x)**.
+      | ^~~~~~~~~"
+
+"aggregate.sps:18.1-18.9: error: AGGREGATE: Syntax error expecting end of command.
+   18 | AGGREGATE /y=mean(x)**.
+      | ^~~~~~~~~"
+])
 AT_CLEANUP