Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[DAPHNE-#494] SQLParser: Group by without aggregation clause not working #543

Merged
merged 7 commits into from
Jun 17, 2023
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