Skip to content

Commit

Permalink
reorged match validator, added a new orderby validator.
Browse files Browse the repository at this point in the history
  • Loading branch information
@xtcyclist
xtcyclist committed Dec 2, 2022
1 parent 9983c2a commit 6bc61a6
Show file tree
Hide file tree
Showing 3 changed files with 107 additions and 62 deletions.
147 changes: 85 additions & 62 deletions src/graph/validator/MatchValidator.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -463,64 +463,12 @@ Status MatchValidator::validateReturn(MatchReturn *ret,
}
}
DCHECK(!columns->empty());
bool preValidatedOrderBy = false;
if (ret->orderFactors() != nullptr && !retClauseCtx.yield->hasAgg_ && !ret->isDistinct() &&
!ret->returnItems()->allNamedAliases() && !queryParts.empty()) {
if (ret->orderFactors() != nullptr) {
auto orderByCtx = getContext<OrderByClauseContext>();
// if the columns required are not already in the projection columns, they may be found
// in in the columns before the projection
YieldColumns *allQueryPartsColumns = nullptr;
std::vector<std::string> inputColList;
inputColList.reserve(columns->columns().size());
for (auto *col : columns->columns()) {
inputColList.emplace_back(col->name());
}
std::unordered_map<std::string, size_t> inputColIndices;
for (auto i = 0u; i < inputColList.size(); i++) {
if (!inputColIndices.emplace(inputColList[i], i).second) {
return Status::SemanticError("Duplicated columns not allowed: %s", inputColList[i].c_str());
}
}
for (auto &factor : ret->orderFactors()->factors()) {
auto factorExpr = factor->expr();
if (ExpressionUtils::isEvaluableExpr(factorExpr, qctx_)) continue;
if (factorExpr->kind() != Expression::Kind::kLabel) {
return Status::SemanticError("Only column name can be used as sort item");
}
auto &name = static_cast<const LabelExpression *>(factor->expr())->name();
auto iter = inputColIndices.find(name);
if (iter == inputColIndices.end()) {
// some column is not found
bool found = false;
if (allQueryPartsColumns == nullptr) {
allQueryPartsColumns = retClauseCtx.qctx->objPool()->makeAndAdd<YieldColumns>();
auto status = buildColumnsForAllNamedAliases(queryParts, allQueryPartsColumns);
if (!status.ok()) {
return status;
}
}
for (auto &col : allQueryPartsColumns->columns()) {
if (col->name().compare(name) != 0) {
continue;
} else {
col->setVisible(false);
columns->addColumn(col->clone().release());
found = true;
orderByCtx->indexedOrderFactors.emplace_back(columns->size() - 1, factor->orderType());
break;
}
}
if (!found) {
return Status::SemanticError("Column `%s' not found", name.c_str());
}
} else {
// iter found
orderByCtx->indexedOrderFactors.emplace_back(iter->second, factor->orderType());
}
}
NG_RETURN_IF_ERROR(validateOrderBy(ret, retClauseCtx, columns, *orderByCtx, queryParts));
retClauseCtx.order = std::move(orderByCtx);
preValidatedOrderBy = true;
}

// finished producing columns for further validations
retClauseCtx.yield->yieldColumns = columns;

Expand All @@ -533,13 +481,14 @@ Status MatchValidator::validateReturn(MatchReturn *ret,
NG_RETURN_IF_ERROR(validatePagination(ret->skip(), ret->limit(), *paginationCtx));
retClauseCtx.pagination = std::move(paginationCtx);

if (!preValidatedOrderBy && ret->orderFactors() != nullptr) {
auto orderByCtx = getContext<OrderByClauseContext>();
NG_RETURN_IF_ERROR(
validateOrderBy(ret->orderFactors(), retClauseCtx.yield->yieldColumns, *orderByCtx));
retClauseCtx.order = std::move(orderByCtx);
}

/*
if (!preValidatedOrderBy && ret->orderFactors() != nullptr) {
auto orderByCtx = getContext<OrderByClauseContext>();
NG_RETURN_IF_ERROR(
validateOrderBy(ret->orderFactors(), retClauseCtx.yield->yieldColumns, *orderByCtx));
retClauseCtx.order = std::move(orderByCtx);
}
*/
return Status::OK();
}

Expand Down Expand Up @@ -890,6 +839,80 @@ Status MatchValidator::validateOrderBy(const OrderFactors *factors,
return Status::OK();
}

Status MatchValidator::validateOrderBy(MatchReturn *ret,
ReturnClauseContext &retClauseCtx,
YieldColumns *yieldColumns,
OrderByClauseContext &orderByCtx,
const std::vector<QueryPart> &queryParts) const {
const OrderFactors *factors = ret->orderFactors();
bool needsAndOkToExpandScope = !retClauseCtx.yield->hasAgg_ && !ret->isDistinct() &&
!ret->returnItems()->allNamedAliases() && !queryParts.empty();

// if the columns required are not already in the projection columns, they may be found
// in in the columns before the projection
std::vector<std::string> inputColList;
inputColList.reserve(yieldColumns->columns().size());
for (auto *col : yieldColumns->columns()) {
inputColList.emplace_back(col->name());
}
std::unordered_map<std::string, size_t> inputColIndices;
for (auto i = 0u; i < inputColList.size(); i++) {
if (!inputColIndices.emplace(inputColList[i], i).second) {
return Status::SemanticError("Duplicated columns not allowed: %s", inputColList[i].c_str());
}
}
YieldColumns *allQueryPartsColumns = nullptr;
for (auto &factor : factors->factors()) {
auto factorExpr = factor->expr();
if (ExpressionUtils::isEvaluableExpr(factorExpr, qctx_)) continue;
if (factorExpr->kind() != Expression::Kind::kLabel) {
return Status::SemanticError("Only column name can be used as sort item");
}
auto &name = static_cast<const LabelExpression *>(factor->expr())->name();
auto iter = inputColIndices.find(name);
if (iter == inputColIndices.end()) {
// some column is not found
if (!needsAndOkToExpandScope) {
return Status::SemanticError("Column `%s' not found", name.c_str());
} else {
// expand the scope to columns before the projection
// and see whether the required column could be found
bool found = false;
if (allQueryPartsColumns == nullptr) {
// expand
allQueryPartsColumns = retClauseCtx.qctx->objPool()->makeAndAdd<YieldColumns>();
auto status = buildColumnsForAllNamedAliases(queryParts, allQueryPartsColumns);
if (!status.ok()) {
return status;
}
}
for (auto &col : allQueryPartsColumns->columns()) {
// check column
if (col->name().compare(name) != 0) {
continue;
} else {
col->setVisible(false);
yieldColumns->addColumn(col->clone().release());
found = true;
orderByCtx.indexedOrderFactors.emplace_back(yieldColumns->size() - 1,
factor->orderType());
break;
}
}
if (!found) {
return Status::SemanticError("Column `%s' not found", name.c_str());
}
}
} else {
// iter found
orderByCtx.indexedOrderFactors.emplace_back(iter->second, factor->orderType());
}
}
// retClauseCtx.order = std::move(orderByCtx);

return Status::OK();
}

// Validate group by and fill group by context.
Status MatchValidator::validateGroup(YieldClauseContext &yieldCtx) {
auto cols = yieldCtx.yieldColumns->columns();
Expand Down
6 changes: 6 additions & 0 deletions src/graph/validator/MatchValidator.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,12 @@ class MatchValidator final : public Validator {
const YieldColumns *yieldColumns,
OrderByClauseContext &orderByCtx) const;

Status validateOrderBy(MatchReturn *ret,
ReturnClauseContext &retClauseCtx,
YieldColumns *yieldColumns,
OrderByClauseContext &orderByCtx,
const std::vector<QueryPart> &queryParts) const;

Status validateGroup(YieldClauseContext &yieldCtx);

Status validateYield(YieldClauseContext &yieldCtx);
Expand Down
16 changes: 16 additions & 0 deletions tests/tck/features/match/OrderBy.feature
View file
Original file line number Diff line number Diff line change
Expand Up @@ -121,3 +121,19 @@ Feature: OrderBy in Match
| "Tim Duncan" | 2 |
| "Tim Duncan" | 2 |
| "Manu Ginobili" | 2 |
When executing query:
"""
match (v:player)-->(v1:player)
with v, v1, count(v) as a0
where a0 > 1
return *
order by a0 desc
"""
Then the result should be, in order:
| v | v1 | a0 |
| ("Tony Parker" :player{age: 36, name: "Tony Parker"}) | ("LaMarcus Aldridge" :player{age: 33, name: "LaMarcus Aldridge"}) | 2 |
| ("Tony Parker" :player{age: 36, name: "Tony Parker"}) | ("Tim Duncan" :player{age: 42, name: "Tim Duncan"} :bachelor{name: "Tim Duncan", speciality: "psychology"}) | 2 |
| ("Tony Parker" :player{age: 36, name: "Tony Parker"}) | ("Manu Ginobili" :player{age: 41, name: "Manu Ginobili"}) | 2 |
| ("Tim Duncan" :player{age: 42, name: "Tim Duncan"} :bachelor{name: "Tim Duncan", speciality: "psychology"}) | ("Tony Parker" :player{age: 36, name: "Tony Parker"}) | 2 |
| ("Tim Duncan" :player{age: 42, name: "Tim Duncan"} :bachelor{name: "Tim Duncan", speciality: "psychology"}) | ("Manu Ginobili" :player{age: 41, name: "Manu Ginobili"}) | 2 |
| ("Manu Ginobili" :player{age: 41, name: "Manu Ginobili"}) | ("Tim Duncan" :player{age: 42, name: "Tim Duncan"} :bachelor{name: "Tim Duncan", speciality: "psychology"}) | 2 |

0 comments on commit 6bc61a6

Please sign in to comment.