Skip to content

Commit

Permalink
[DAPHNE-#494] SQLParser: GROUP BY without aggregation clause (#543)
Browse files Browse the repository at this point in the history
- 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.
  • Loading branch information
tomschw authored Jun 17, 2023
1 parent 478ae7c commit c2433df
Show file tree
Hide file tree
Showing 5 changed files with 26 additions and 4 deletions.
14 changes: 12 additions & 2 deletions src/compiler/lowering/RewriteToCallKernelOpPass.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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";
Expand Down
2 changes: 1 addition & 1 deletion src/ir/daphneir/DaphneInferShapeOpInterface.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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}};
}

Expand Down
2 changes: 1 addition & 1 deletion test/api/cli/sql/SQLResultTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -34,4 +34,4 @@ const std::string dirPath = "test/api/cli/sql/";
} \
}

MAKE_TEST_CASE("group", 1)
MAKE_TEST_CASE("group", 2)
6 changes: 6 additions & 0 deletions test/api/cli/sql/group_2.daphne
Original file line number Diff line number Diff line change
@@ -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);
6 changes: 6 additions & 0 deletions test/api/cli/sql/group_2.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
Frame(5x1, [f.a:int64_t])
0
1
3
5
9

0 comments on commit c2433df

Please sign in to comment.