Fix #565: Review Time_Agg in group by / group except

src/vtlengine/AST/ASTConstructorModules/Expr.py

@@ -1,6 +1,6 @@
 import re
 from copy import copy
-from typing import Any, Optional
+from typing import Any, List, Optional, Tuple
 
 from antlr4.tree.Tree import TerminalNodeImpl
 
@@ -1653,8 +1653,11 @@ def visitAggrClause(self, ctx: Parser.AggrClauseContext):
     def visitGroupingClause(self, ctx: Parser.GroupingClauseContext):
         """
         groupingClause:
-            GROUP op=(BY | EXCEPT) componentID (COMMA componentID)*     # groupByOrExcept
-            | GROUP ALL exprComponent                                   # groupAll
+            GROUP op=(BY | EXCEPT) componentID (COMMA componentID)*
+                ( TIME_AGG LPAREN STRING_CONSTANT
+                  (COMMA delim=(FIRST|LAST))? RPAREN )?
+            | GROUP ALL ( TIME_AGG LPAREN STRING_CONSTANT
+                  (COMMA delim=(FIRST|LAST))? RPAREN )?
           ;
         """
         if isinstance(ctx, Parser.GroupByOrExceptContext):
@@ -1706,6 +1709,22 @@ def visitHavingClause(self, ctx: Parser.HavingClauseContext):
             op=op_node, children=None, params=param_nodes, **extract_token_info(ctx)
         ), expr
 
+    @staticmethod
+    def _extract_time_agg_tokens(
+        ctx_list: List[Any],
+    ) -> Tuple[Optional[str], Optional[str]]:
+        """Extract TIME_AGG parameters (period_to, conf) from parse tree children."""
+        period_to: Optional[str] = None
+        conf: Optional[str] = None
+        for child in ctx_list:
+            if isinstance(child, TerminalNodeImpl):
+                token = child.getSymbol()
+                if token.type == Parser.STRING_CONSTANT:
+                    period_to = token.text[1:-1]
+                elif token.type in (Parser.FIRST, Parser.LAST):
+                    conf = token.text
+        return period_to, conf
+
     def visitGroupByOrExcept(self, ctx: Parser.GroupByOrExceptContext):
         ctx_list = list(ctx.getChildren())
 
@@ -1714,11 +1733,29 @@ def visitGroupByOrExcept(self, ctx: Parser.GroupByOrExceptContext):
 
         op_node = token_left + " " + token_right
 
-        children_nodes = [
-            Terminals().visitComponentID(identifier)
-            for identifier in ctx_list
-            if isinstance(identifier, Parser.ComponentIDContext)
-        ]
+        children_nodes: List[Any] = []
+        has_time_agg = False
+
+        for child in ctx_list:
+            if isinstance(child, Parser.ComponentIDContext):
+                children_nodes.append(Terminals().visitComponentID(child))
+            elif isinstance(child, TerminalNodeImpl) and child.getSymbol().type == Parser.TIME_AGG:
+                has_time_agg = True
+
+        if has_time_agg:
+            period_to, conf = self._extract_time_agg_tokens(ctx_list)
+            if period_to is None:
+                raise NotImplementedError
+            children_nodes.append(
+                TimeAggregation(
+                    op="time_agg",
+                    operand=None,
+                    period_to=period_to,
+                    period_from=None,
+                    conf=conf,
+                    **extract_token_info(ctx),
+                )
+            )
 
         return op_node, children_nodes
 
@@ -1734,34 +1771,14 @@ def visitGroupAll(self, ctx: Parser.GroupAllContext):
 
         # Check if TIME_AGG is present (more than just GROUP ALL)
         if len(ctx_list) > 2:
-            period_to = None
-            period_from = None
-            operand_node = None
-            conf = None
-
-            for child in ctx_list:
-                if isinstance(child, TerminalNodeImpl):
-                    token = child.getSymbol()
-                    if token.type == Parser.STRING_CONSTANT:
-                        if period_to is None:
-                            period_to = token.text[1:-1]
-                        else:
-                            period_from = token.text[1:-1]
-                    elif token.type in [Parser.FIRST, Parser.LAST]:
-                        conf = token.text
-                elif isinstance(child, Parser.OptionalExprContext):
-                    operand_node = self.visitOptionalExpr(child)
-                    if isinstance(operand_node, ID):
-                        operand_node = None
-                    elif isinstance(operand_node, Identifier):
-                        operand_node = VarID(value=operand_node.value, **extract_token_info(child))
+            period_to, conf = self._extract_time_agg_tokens(ctx_list)
 
             children_nodes = [
                 TimeAggregation(
                     op="time_agg",
-                    operand=operand_node,
+                    operand=None,
                     period_to=period_to,
-                    period_from=period_from,
+                    period_from=None,
                     conf=conf,
                     **extract_token_info(ctx),
                 )

src/vtlengine/Interpreter/__init__.py

@@ -492,13 +492,35 @@ def visit_UnaryOp(self, node: AST.UnaryOp) -> None:
             return ROLE_SETTER_MAPPING[node.op].analyze(operand, data_size)
         return UNARY_MAPPING[node.op].analyze(operand)
 
-    def visit_Aggregation(self, node: AST.Aggregation) -> None:
-        # Having takes precedence as it is lower in the AST
+    @staticmethod
+    def _apply_time_agg_grouping(
+        operand: Dataset,
+        groupings: List[Any],
+        grouping_op: Optional[str],
+    ) -> List[Any]:
+        """Extract TimeAggregation DataComponent from groupings and merge into operand."""
+        time_comp = None
+        regular_groupings: List[Any] = []
+        for g in groupings:
+            if isinstance(g, DataComponent):
+                time_comp = g
+            else:
+                regular_groupings.append(g)
+        if time_comp is not None:
+            if operand.data is not None and time_comp.data is not None and len(time_comp.data) > 0:
+                operand.data = operand.data.copy()
+                operand.data[time_comp.name] = time_comp.data
+            if grouping_op != "group except":
+                regular_groupings.append(time_comp.name)
+        return regular_groupings
+
+    def _resolve_aggregation_operand(self, node: AST.Aggregation) -> Any:
+        """Resolve the operand for an aggregation node."""
         if self.is_from_having:
             if node.operand is not None:
                 self.visit(node.operand)
-            operand = self.aggregation_dataset
-        elif self.is_from_regular_aggregation and self.regular_aggregation_dataset is not None:
+            return self.aggregation_dataset
+        if self.is_from_regular_aggregation and self.regular_aggregation_dataset is not None:
             operand = self.regular_aggregation_dataset
             if node.operand is not None and operand is not None:
                 op_comp: DataComponent = self.visit(node.operand)
@@ -520,9 +542,12 @@ def visit_Aggregation(self, node: AST.Aggregation) -> None:
                     data_to_keep[op_comp.name] = op_comp.data
                 else:
                     data_to_keep = None
-                operand = Dataset(name=operand.name, components=comps_to_keep, data=data_to_keep)
-        else:
-            operand = self.visit(node.operand)
+                return Dataset(name=operand.name, components=comps_to_keep, data=data_to_keep)
+            return operand
+        return self.visit(node.operand)
+
+    def visit_Aggregation(self, node: AST.Aggregation) -> None:
+        operand = self._resolve_aggregation_operand(node)
 
         if not isinstance(operand, Dataset):
             raise SemanticError("2-3-4", op=node.op, comp="dataset")
@@ -538,7 +563,8 @@ def visit_Aggregation(self, node: AST.Aggregation) -> None:
         having = None
         grouping_op = node.grouping_op
         if node.grouping is not None:
-            if grouping_op == "group all":
+            has_time_agg = any(isinstance(x, AST.TimeAggregation) for x in node.grouping)
+            if grouping_op == "group all" or has_time_agg:
                 data = None if self.only_semantic else copy(operand.data)
                 self.aggregation_dataset = Dataset(
                     name=operand.name, components=operand.components, data=data
@@ -548,17 +574,8 @@ def visit_Aggregation(self, node: AST.Aggregation) -> None:
             for x in node.grouping:
                 groupings.append(self.visit(x))
             self.is_from_grouping = False
-            if grouping_op == "group all":
-                comp_grouped = groupings[0]
-                if (
-                    operand.data is not None
-                    and comp_grouped.data is not None
-                    and len(comp_grouped.data) > 0
-                ):
-                    # Deep copy the data to avoid modifying the original dataset
-                    operand.data = operand.data.copy()
-                    operand.data[comp_grouped.name] = comp_grouped.data
-                groupings = [comp_grouped.name]
+            if grouping_op == "group all" or has_time_agg:
+                groupings = self._apply_time_agg_grouping(operand, groupings, grouping_op)
                 self.aggregation_dataset = None
             if node.having_clause is not None:
                 self.aggregation_dataset = Dataset(

tests/Additional/data/DataSet/input/7-30-DS_1.csv

@@ -0,0 +1,8 @@
+Id_1,Id_2,Me_1
+2010Q1,A,20
+2010Q2,A,20
+2010Q3,A,20
+2010Q1,B,50
+2010Q2,B,50
+2010Q1,C,10
+2010Q2,C,10

tests/Additional/data/DataSet/input/7-31-DS_1.csv

@@ -0,0 +1,9 @@
+Id_1,Id_2,Me_1
+2010-10-22,A,20
+2010-09-15,A,30
+2010-08-27,A,50
+2010-07-14,A,80
+2010-06-18,B,100
+2010-05-04,B,20
+2010-04-20,B,30
+2010-03-12,B,40

tests/Additional/data/DataSet/input/7-32-DS_1.csv

@@ -0,0 +1,8 @@
+Id_1,Id_2,Me_1
+2010-03-15,A,10
+2010-07-20,A,30
+2010-11-05,A,20
+2011-02-10,A,40
+2011-08-25,A,60
+2010-04-12,B,50
+2010-09-30,B,70

tests/Additional/data/DataSet/input/7-33-DS_1.csv

@@ -0,0 +1,7 @@
+Id_1,Id_2,Me_1,Me_2
+2010Q1,A,10,100
+2010Q2,A,20,200
+2010Q3,A,30,300
+2011Q1,A,40,400
+2011Q2,B,50,500
+2011Q3,B,60,600

tests/Additional/data/DataSet/input/7-34-DS_1.csv

@@ -0,0 +1,6 @@
+Id_1,Id_2,Id_3,Me_1
+2010Q1,A,X,10
+2010Q2,A,X,20
+2010Q1,A,Y,30
+2010Q2,B,X,40
+2010Q3,B,Y,50

tests/Additional/data/DataSet/input/7-35-DS_1.csv

@@ -0,0 +1,7 @@
+Id_1,Id_2,Id_3,Me_1
+2010-03-15,A,X,10
+2010-06-20,A,Y,20
+2010-09-10,B,X,30
+2011-01-05,A,X,40
+2011-04-18,B,Y,50
+2011-07-22,B,X,60

tests/Additional/data/DataSet/output/7-30-DS_r.csv

@@ -0,0 +1,4 @@
+Id_1,Id_2,Me_1
+2010,A,60
+2010,B,100
+2010,C,20

tests/Additional/data/DataSet/output/7-31-DS_r.csv

@@ -0,0 +1,2 @@
+Id_1,Me_1
+2010-12-31,370

tests/Additional/data/DataSet/output/7-32-DS_r.csv

@@ -0,0 +1,4 @@
+Id_1,Id_2,Me_1
+2010-01-01,A,60
+2011-01-01,A,100
+2010-01-01,B,120

tests/Additional/data/DataSet/output/7-33-DS_r.csv

@@ -0,0 +1,4 @@
+Id_1,Id_2,Me_1,Me_2
+2010,A,60,600
+2011,A,40,400
+2011,B,110,1100

tests/Additional/data/DataSet/output/7-34-DS_r.csv

@@ -0,0 +1,3 @@
+Id_1,Id_2,Me_1
+2010,A,60
+2010,B,90

tests/Additional/data/DataSet/output/7-35-DS_r.csv

@@ -0,0 +1,3 @@
+Id_1,Me_1
+2010-12-31,60
+2011-12-31,150

tests/Additional/data/DataStructure/input/7-30-DS_1.json

@@ -0,0 +1,27 @@
+{
+  "datasets": [
+    {
+      "name": "DS_1",
+      "DataStructure": [
+        {
+          "name": "Id_1",
+          "role": "Identifier",
+          "type": "Time_Period",
+          "nullable": false
+        },
+        {
+          "name": "Id_2",
+          "role": "Identifier",
+          "type": "String",
+          "nullable": false
+        },
+        {
+          "name": "Me_1",
+          "role": "Measure",
+          "type": "Integer",
+          "nullable": true
+        }
+      ]
+    }
+  ]
+}

tests/Additional/data/DataStructure/input/7-31-DS_1.json

@@ -0,0 +1,27 @@
+{
+  "datasets": [
+    {
+      "name": "DS_1",
+      "DataStructure": [
+        {
+          "name": "Id_1",
+          "role": "Identifier",
+          "type": "Date",
+          "nullable": false
+        },
+        {
+          "name": "Id_2",
+          "role": "Identifier",
+          "type": "String",
+          "nullable": false
+        },
+        {
+          "name": "Me_1",
+          "role": "Measure",
+          "type": "Integer",
+          "nullable": true
+        }
+      ]
+    }
+  ]
+}

tests/Additional/data/DataStructure/input/7-32-DS_1.json

@@ -0,0 +1,27 @@
+{
+  "datasets": [
+    {
+      "name": "DS_1",
+      "DataStructure": [
+        {
+          "name": "Id_1",
+          "role": "Identifier",
+          "type": "Date",
+          "nullable": false
+        },
+        {
+          "name": "Id_2",
+          "role": "Identifier",
+          "type": "String",
+          "nullable": false
+        },
+        {
+          "name": "Me_1",
+          "role": "Measure",
+          "type": "Integer",
+          "nullable": true
+        }
+      ]
+    }
+  ]
+}

tests/Additional/data/DataStructure/input/7-33-DS_1.json

@@ -0,0 +1,33 @@
+{
+  "datasets": [
+    {
+      "name": "DS_1",
+      "DataStructure": [
+        {
+          "name": "Id_1",
+          "role": "Identifier",
+          "type": "Time_Period",
+          "nullable": false
+        },
+        {
+          "name": "Id_2",
+          "role": "Identifier",
+          "type": "String",
+          "nullable": false
+        },
+        {
+          "name": "Me_1",
+          "role": "Measure",
+          "type": "Integer",
+          "nullable": true
+        },
+        {
+          "name": "Me_2",
+          "role": "Measure",
+          "type": "Integer",
+          "nullable": true
+        }
+      ]
+    }
+  ]
+}