From 65a7d38146f513c82bf2ab27c8597a8c09427a05 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ingo=20M=C3=BCller?= Date: Fri, 27 Sep 2024 21:15:53 +0200 Subject: [PATCH] feat: change grouping expressions in AggregateRel to references (#706) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit BREAKING CHANGE: This PR changes the definition of grouping sets in `AggregateRel` to consist of references into a list of grouping expressions instead of consisting of expressions directly. With the previous definition, consumers had to deduplicate the expressions in the grouping sets in order to execute the query or even derive the output schema (which is problematic, as explained below). With this change, the responsibility of deduplicating expressions is now on the producer. Concretely, consumers are now expected to be simpler: The list of grouping expressions immediately provides the information needed to derive the output schema and the list of grouping sets explicitly and unambiguously provides the equality of grouping expressions. Producers now have to specify the grouping sets explicitly. If their internal representation of grouping sets consists of full grouping expressions (rather than references), then they must deduplicate these expressions according to their internal notion of expression equality in order to produce grouping sets consisting of references to these deduplicated expressions. If the previous format is desired, it can be obtained from the new format by (1) deduplicating the grouping expressions (according to the previously applicable definition of expression equality), (2) re-establishing the duplicates using the emit clause, and (3) "dereferencing" the references in the grouping sets, i.e., by replacing each reference in the grouping sets with the expression it refers to. The previous version was problematic because it required the *consumers* to deduplicate the expressions from the grouping sets. This, in turn, requires to parse and understand 100% of these expression even in cases where that understanding is otherwise optional, which is in opposition to the general philosophy of allowing for simple-minded consumers. The new version avoids that problem and, thus, allows consumers to be simpler. The issues are discussed in even more detail in #700. --------- Signed-off-by: Ingo Müller Co-authored-by: Weston Pace --- proto/substrait/algebra.proto | 18 +++++++++++++++--- site/docs/relations/logical_relations.md | 6 +++--- 2 files changed, 18 insertions(+), 6 deletions(-) diff --git a/proto/substrait/algebra.proto b/proto/substrait/algebra.proto index eed7bcc79..5c4b9c8ac 100644 --- a/proto/substrait/algebra.proto +++ b/proto/substrait/algebra.proto @@ -244,18 +244,30 @@ message AggregateRel { // Input of the aggregation Rel input = 2; - // A list of one or more grouping expression sets that the aggregation measures should be calculated for. - // Required if there are no measures. + // A list of zero or more grouping sets that the aggregation measures should + // be calculated for. There must be at least one grouping set if there are no + // measures (but it can be the empty grouping set). repeated Grouping groupings = 3; // A list of one or more aggregate expressions along with an optional filter. // Required if there are no groupings. repeated Measure measures = 4; + // A list of zero or more grouping expressions that grouping sets (i.e., + // `Grouping` messages in the `groupings` field) can reference. Each + // expression in this list must be referred to by at least one + // `Grouping.expression_references`. + repeated Expression grouping_expressions = 5; + substrait.extensions.AdvancedExtension advanced_extension = 10; message Grouping { - repeated Expression grouping_expressions = 1; + // Deprecated in favor of `expression_references` below. + repeated Expression grouping_expressions = 1 [deprecated = true]; + + // A list of zero or more references to grouping expressions, i.e., indices + // into the `grouping_expression` list. + repeated uint32 expression_references = 2; } message Measure { diff --git a/site/docs/relations/logical_relations.md b/site/docs/relations/logical_relations.md index d71b57d2f..4d6a1f9d3 100644 --- a/site/docs/relations/logical_relations.md +++ b/site/docs/relations/logical_relations.md @@ -345,13 +345,13 @@ The aggregate operation groups input data on one or more sets of grouping keys, | Inputs | 1 | | Outputs | 1 | | Property Maintenance | Maintains distribution if all distribution fields are contained in every grouping set. No orderedness guaranteed. | -| Direct Output Order | The list of distinct columns from each grouping set (ordered by their first appearance) followed by the list of measures in declaration order, followed by an `i32` describing the associated particular grouping set the value is derived from (if applicable). | +| Direct Output Order | The list of grouping expressions in declaration order followed by the list of measures in declaration order, followed by an `i32` describing the associated particular grouping set the value is derived from (if applicable). | In its simplest form, an aggregation has only measures. In this case, all records are folded into one, and a column is returned for each aggregate expression in the measures list. -Grouping sets can be used for finer-grained control over which records are folded. Within a grouping set, two records will be folded together if and only if each expressions in the grouping set yields the same value for each. The values returned by the grouping sets will be returned as columns to the left of the columns for the aggregate expressions. If a grouping set contains no grouping expressions, all rows will be folded for that grouping set. +Grouping sets can be used for finer-grained control over which records are folded. A grouping set consists of zero or more references to the list of grouping expressions. Within a grouping set, two records will be folded together if and only if they have the same values for each of the expressions in the grouping set. The values returned by the grouping expressions will be returned as columns to the left of the columns for the aggregate expressions. Each of the grouping expressions must occur in at least one of the grouping sets. If a grouping set contains no grouping expressions, all rows will be folded for that grouping set. (Having a single grouping set with no grouping expressions is thus equivalent to not having any grouping sets.) -It's possible to specify multiple grouping sets in a single aggregate operation. The grouping sets behave more or less independently, with each returned record belonging to one of the grouping sets. The values for the grouping expression columns that are not part of the grouping set for a particular record will be set to null. Two grouping expressions will be returned using the same column if the protobuf messages describing the expressions are equal. The columns for grouping expressions that do *not* appear in *all* grouping sets will be nullable (regardless of the nullability of the type returned by the grouping expression) to accomodate the null insertion. +It is possible to specify multiple grouping sets in a single aggregate operation. The grouping sets behave more or less independently, with each returned record belonging to one of the grouping sets. The values for the grouping expression columns that are not part of the grouping set for a particular record will be set to null. The columns for grouping expressions that do *not* appear in *all* grouping sets will be nullable (regardless of the nullability of the type returned by the grouping expression) to accomodate the null insertion. To further disambiguate which record belongs to which grouping set, an aggregate relation with more than one grouping set receives an extra `i32` column on the right-hand side. The value of this field will be the zero-based index of the grouping set that yielded the record.