From c2433df00cfc8fb17c8ae5fbce8df1a5f1bda448 Mon Sep 17 00:00:00 2001
From: Tom Schwarzburg <tomschwarzburg@googlemail.com>
Date: Sat, 17 Jun 2023 15:59:04 +0200
Subject: [PATCH] [DAPHNE-#494] SQLParser: GROUP BY without aggregation clause
 (#543)

- Add simple test which checks if the SQL grouping works without an aggregation function.
- Implement a workaround to allow the use of the group operation without aggregations.
- Fixed shape inference for GroupOp.
- Hardcode the addition to the callee in the kernel lowering as a workaround for grouping without aggregation.
- Closes #494.
---
 .../lowering/RewriteToCallKernelOpPass.cpp         | 14 ++++++++++++--
 src/ir/daphneir/DaphneInferShapeOpInterface.cpp    |  2 +-
 test/api/cli/sql/SQLResultTest.cpp                 |  2 +-
 test/api/cli/sql/group_2.daphne                    |  6 ++++++
 test/api/cli/sql/group_2.txt                       |  6 ++++++
 5 files changed, 26 insertions(+), 4 deletions(-)
 create mode 100644 test/api/cli/sql/group_2.daphne
 create mode 100644 test/api/cli/sql/group_2.txt

diff --git a/src/compiler/lowering/RewriteToCallKernelOpPass.cpp b/src/compiler/lowering/RewriteToCallKernelOpPass.cpp
index 7bd9b58fe..2379b1ad8 100644
--- a/src/compiler/lowering/RewriteToCallKernelOpPass.cpp
+++ b/src/compiler/lowering/RewriteToCallKernelOpPass.cpp
@@ -92,7 +92,7 @@ namespace
             }
             if(auto concreteOp = llvm::dyn_cast<daphne::GroupOp>(op)) {
                 auto idxAndLen = concreteOp.getODSOperandIndexAndLength(index);
-                static bool isVariadic[] = {false, true, true, true};
+                static bool isVariadic[] = {false, true, true};
                 return std::make_tuple(
                         idxAndLen.first,
                         idxAndLen.second,
@@ -214,8 +214,18 @@ namespace
                     const unsigned idx = std::get<0>(odsOpInfo);
                     const unsigned len = std::get<1>(odsOpInfo);
                     const bool isVariadic = std::get<2>(odsOpInfo);
+                    
+                    // TODO The group operation currently expects at least four inputs due to the
+                    // expectation of a aggregation. To make the group operation possible without aggregations,
+                    // we have to use this workaround to create the correct name and skip the creation
+                    // of the variadic pack ops. Should be changed when reworking the lowering to kernels.
+                    if(llvm::dyn_cast<daphne::GroupOp>(op) && idx >= operandTypes.size()) {
+                        callee << "__char_variadic__size_t";
+                        continue;
+                    } else {
+                        callee << "__" << CompilerUtils::mlirTypeToCppTypeName(operandTypes[idx], generalizeInputTypes);
+                    }
 
-                    callee << "__" << CompilerUtils::mlirTypeToCppTypeName(operandTypes[idx], generalizeInputTypes);
                     if(isVariadic) {
                         // Variadic operand.
                         callee << "_variadic__size_t";
diff --git a/src/ir/daphneir/DaphneInferShapeOpInterface.cpp b/src/ir/daphneir/DaphneInferShapeOpInterface.cpp
index 021da2486..1a3f90569 100644
--- a/src/ir/daphneir/DaphneInferShapeOpInterface.cpp
+++ b/src/ir/daphneir/DaphneInferShapeOpInterface.cpp
@@ -184,7 +184,7 @@ std::vector<std::pair<ssize_t, ssize_t>> daphne::GroupJoinOp::inferShape() {
 std::vector<std::pair<ssize_t, ssize_t>> daphne::GroupOp::inferShape() {
     // We don't know the exact number of groups here.
     const size_t numRows = -1;
-    const size_t numCols = inferNumColsFromArgs(getKeyCol()) + inferNumColsFromArgs(getAggCol());
+    const size_t numCols = getKeyCol().size() + getAggCol().size();
     return {{numRows, numCols}};
 }
 
diff --git a/test/api/cli/sql/SQLResultTest.cpp b/test/api/cli/sql/SQLResultTest.cpp
index ee773a844..31ec2f45e 100644
--- a/test/api/cli/sql/SQLResultTest.cpp
+++ b/test/api/cli/sql/SQLResultTest.cpp
@@ -34,4 +34,4 @@ const std::string dirPath = "test/api/cli/sql/";
         } \
     }
 
-MAKE_TEST_CASE("group", 1)
\ No newline at end of file
+MAKE_TEST_CASE("group", 2)
\ No newline at end of file
diff --git a/test/api/cli/sql/group_2.daphne b/test/api/cli/sql/group_2.daphne
new file mode 100644
index 000000000..75878486b
--- /dev/null
+++ b/test/api/cli/sql/group_2.daphne
@@ -0,0 +1,6 @@
+# GROUP BY without aggregation.
+
+f = createFrame([0, 1, 1, 3, 1, 5, 1, 3, 0, 9], "a");
+registerView("f", f);
+res = sql("SELECT f.a FROM f GROUP BY f.a;");
+print(res);
\ No newline at end of file
diff --git a/test/api/cli/sql/group_2.txt b/test/api/cli/sql/group_2.txt
new file mode 100644
index 000000000..96df9fd03
--- /dev/null
+++ b/test/api/cli/sql/group_2.txt
@@ -0,0 +1,6 @@
+Frame(5x1, [f.a:int64_t])
+0
+1
+3
+5
+9