Skip to content

refactor: Change leafCallToSubfieldFilter API to return an optional pair #15555

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.

Sign up for GitHub

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

Closed
mbasmanova wants to merge 1 commit into from
Closed

refactor: Change leafCallToSubfieldFilter API to return an optional pair #15555

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions velox/connectors/hive/HiveConnectorUtil.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -890,10 +890,10 @@ core::TypedExprPtr extractFiltersFromRemainingFilter(
}
common::Filter* oldFilter = nullptr;
try {
common::Subfield subfield;
if (auto filter = exec::ExprToSubfieldFilterParser::getInstance()
->leafCallToSubfieldFilter(
*call, subfield, evaluator, negated)) {
if (auto subfieldAndFilter =
exec::ExprToSubfieldFilterParser::getInstance()
->leafCallToSubfieldFilter(*call, evaluator, negated)) {
auto& [subfield, filter] = subfieldAndFilter.value();
if (auto it = filters.find(subfield); it != filters.end()) {
oldFilter = it->second.get();
filter = filter->mergeWith(oldFilter);
Expand Down
9 changes: 5 additions & 4 deletions velox/dwio/common/MetadataFilter.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -208,13 +208,14 @@ std::unique_ptr<MetadataFilter::Node> MetadataFilter::Node::fromExpression(
return fromExpression(*call->inputs()[0], evaluator, !negated);
}
try {
Subfield subfield;
auto filter =
auto subfieldAndFilter =
exec::ExprToSubfieldFilterParser::getInstance()
->leafCallToSubfieldFilter(*call, subfield, evaluator, negated);
if (!filter) {
->leafCallToSubfieldFilter(*call, evaluator, negated);
if (!subfieldAndFilter.has_value()) {
return nullptr;
}

auto& [subfield, filter] = subfieldAndFilter.value();
VELOX_CHECK(
subfield.valid(),
"Invalid subfield from expression: {}",
Expand Down
74 changes: 48 additions & 26 deletions velox/expression/ExprToSubfieldFilter.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -471,67 +471,90 @@ std::unique_ptr<common::Filter> ExprToSubfieldFilterParser::makeOrFilter(
return orFilter(std::move(a), std::move(b));
}

std::unique_ptr<common::Filter>
namespace {
std::optional<std::pair<common::Subfield, std::unique_ptr<common::Filter>>>
combine(common::Subfield& subfield, std::unique_ptr<common::Filter>& filter) {
if (filter != nullptr) {
return std::make_pair(std::move(subfield), std::move(filter));
}

return std::nullopt;
}
} // namespace

std::optional<std::pair<common::Subfield, std::unique_ptr<common::Filter>>>
PrestoExprToSubfieldFilterParser::leafCallToSubfieldFilter(
const core::CallTypedExpr& call,
common::Subfield& subfield,
core::ExpressionEvaluator* evaluator,
bool negated) {
if (call.inputs().empty()) {
return nullptr;
return std::nullopt;
}

const auto* leftSide = call.inputs()[0].get();

common::Subfield subfield;
if (call.name() == "eq") {
if (toSubfield(leftSide, subfield)) {
return negated ? makeNotEqualFilter(call.inputs()[1], evaluator)
: makeEqualFilter(call.inputs()[1], evaluator);
auto filter = negated ? makeNotEqualFilter(call.inputs()[1], evaluator)
: makeEqualFilter(call.inputs()[1], evaluator);

return combine(subfield, filter);
}
} else if (call.name() == "neq") {
if (toSubfield(leftSide, subfield)) {
return negated ? makeEqualFilter(call.inputs()[1], evaluator)
: makeNotEqualFilter(call.inputs()[1], evaluator);
auto filter = negated ? makeEqualFilter(call.inputs()[1], evaluator)
: makeNotEqualFilter(call.inputs()[1], evaluator);
return combine(subfield, filter);
}
} else if (call.name() == "lte") {
if (toSubfield(leftSide, subfield)) {
return negated ? makeGreaterThanFilter(call.inputs()[1], evaluator)
: makeLessThanOrEqualFilter(call.inputs()[1], evaluator);
auto filter = negated
? makeGreaterThanFilter(call.inputs()[1], evaluator)
: makeLessThanOrEqualFilter(call.inputs()[1], evaluator);
return combine(subfield, filter);
}
} else if (call.name() == "lt") {
if (toSubfield(leftSide, subfield)) {
return negated ? makeGreaterThanOrEqualFilter(call.inputs()[1], evaluator)
: makeLessThanFilter(call.inputs()[1], evaluator);
auto filter = negated
? makeGreaterThanOrEqualFilter(call.inputs()[1], evaluator)
: makeLessThanFilter(call.inputs()[1], evaluator);
return combine(subfield, filter);
}
} else if (call.name() == "gte") {
if (toSubfield(leftSide, subfield)) {
return negated
auto filter = negated
? makeLessThanFilter(call.inputs()[1], evaluator)
: makeGreaterThanOrEqualFilter(call.inputs()[1], evaluator);
return combine(subfield, filter);
}
} else if (call.name() == "gt") {
if (toSubfield(leftSide, subfield)) {
return negated ? makeLessThanOrEqualFilter(call.inputs()[1], evaluator)
: makeGreaterThanFilter(call.inputs()[1], evaluator);
auto filter = negated
? makeLessThanOrEqualFilter(call.inputs()[1], evaluator)
: makeGreaterThanFilter(call.inputs()[1], evaluator);
return combine(subfield, filter);
}
} else if (call.name() == "between") {
if (toSubfield(leftSide, subfield)) {
return makeBetweenFilter(
auto filter = makeBetweenFilter(
call.inputs()[1], call.inputs()[2], evaluator, negated);
return combine(subfield, filter);
}
} else if (call.name() == "in") {
if (toSubfield(leftSide, subfield)) {
return makeInFilter(call.inputs()[1], evaluator, negated);
auto filter = makeInFilter(call.inputs()[1], evaluator, negated);
return combine(subfield, filter);
}
} else if (call.name() == "is_null") {
if (toSubfield(leftSide, subfield)) {
if (negated) {
return isNotNull();
return std::make_pair(std::move(subfield), isNotNull());
}
return isNull();
return std::make_pair(std::move(subfield), isNull());
}
}
return nullptr;
return std::nullopt;
}

std::pair<common::Subfield, std::unique_ptr<common::Filter>>
Expand All @@ -550,19 +573,18 @@ PrestoExprToSubfieldFilterParser::toSubfieldFilter(
makeOrFilter(std::move(left.second), std::move(right.second))};
}

common::Subfield subfield;
std::unique_ptr<common::Filter> filter;
if (call->name() == "not") {
const auto& input = call->inputs()[0];
if (input->isCallKind();
auto* inner = input->asUnchecked<core::CallTypedExpr>()) {
filter = leafCallToSubfieldFilter(*inner, subfield, evaluator, true);
if (auto result = leafCallToSubfieldFilter(*inner, evaluator, true)) {
return std::move(result.value());
}
}
} else {
filter = leafCallToSubfieldFilter(*call, subfield, evaluator, false);
}
if (filter) {
return std::make_pair(std::move(subfield), std::move(filter));
if (auto result = leafCallToSubfieldFilter(*call, evaluator, false)) {
return std::move(result.value());
}
}
}
VELOX_UNSUPPORTED(
Expand Down
24 changes: 11 additions & 13 deletions velox/expression/ExprToSubfieldFilter.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -443,22 +443,20 @@ class ExprToSubfieldFilterParser {
core::ExpressionEvaluator*) = 0;

/// Analyzes 'call' expression to determine if it can be expressed as a
/// subfield filter. Returns the filter and sets 'subfield' output argument if
/// so. Otherwise, returns nullptr. If 'negated' is true, considers the
/// negation of 'call' expressions (not(call)). It is possible that 'call'
/// expression can be represented as subfield filter, but its negation cannot.
///
/// TODO Make this and toSubfieldFilter APIs consistent. Both should not throw
/// and return std::optional pair of filter and subfield.
virtual std::unique_ptr<common::Filter> leafCallToSubfieldFilter(
/// subfield filter. Returns the subfield and filter if so. Otherwise, returns
/// std::nullopt. If 'negated' is true, considers the negation of 'call'
/// expressions (not(call)). It is possible that 'call' expression can be
/// represented as subfield filter, but its negation cannot.
virtual std::optional<
std::pair<common::Subfield, std::unique_ptr<common::Filter>>>
leafCallToSubfieldFilter(
const core::CallTypedExpr& call,
common::Subfield& subfield,
core::ExpressionEvaluator* evaluator,
bool negated = false) = 0;

protected:
// Converts an expression into a subfield. Returns false if the expression is
// not a valid field expression.
// Converts an expression into a subfield. Returns false if the expression
// is not a valid field expression.
static bool toSubfield(
const core::ITypedExpr* field,
common::Subfield& subfield);
Expand Down Expand Up @@ -523,9 +521,9 @@ class PrestoExprToSubfieldFilterParser : public ExprToSubfieldFilterParser {
const core::TypedExprPtr& expr,
core::ExpressionEvaluator* evaluator) override;

std::unique_ptr<common::Filter> leafCallToSubfieldFilter(
std::optional<std::pair<common::Subfield, std::unique_ptr<common::Filter>>>
leafCallToSubfieldFilter(
const core::CallTypedExpr& call,
common::Subfield& subfield,
core::ExpressionEvaluator* evaluator,
bool negated = false) override;
};
Expand Down
46 changes: 26 additions & 20 deletions velox/expression/tests/ExprToSubfieldFilterTest.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,11 +68,13 @@ class ExprToSubfieldFilterTest : public testing::Test {

std::pair<common::Subfield, std::unique_ptr<common::Filter>>
leafCallToSubfieldFilter(const core::CallTypedExprPtr& call) {
Subfield subfield;
auto filter =
ExprToSubfieldFilterParser::getInstance()->leafCallToSubfieldFilter(
*call, subfield, evaluator());
return {std::move(subfield), std::move(filter)};
if (auto result =
ExprToSubfieldFilterParser::getInstance()->leafCallToSubfieldFilter(
*call, evaluator())) {
return std::move(result.value());
}

return std::make_pair(common::Subfield(), nullptr);
}

std::pair<common::Subfield, std::unique_ptr<common::Filter>> toSubfieldFilter(
Expand Down Expand Up @@ -319,49 +321,53 @@ class CustomExprToSubfieldFilterParser : public ExprToSubfieldFilterParser {
std::move(left.first),
makeOrFilter(std::move(left.second), std::move(right.second))};
}
common::Subfield subfield;
std::unique_ptr<common::Filter> filter;
if (call->name() == "not") {
if (auto* inner =
call->inputs()[0]->asUnchecked<core::CallTypedExpr>()) {
filter = leafCallToSubfieldFilter(*inner, subfield, evaluator, true);
if (auto result = leafCallToSubfieldFilter(*inner, evaluator, true)) {
return std::move(result.value());
}
}
} else {
filter = leafCallToSubfieldFilter(*call, subfield, evaluator, false);
}
if (filter) {
return std::make_pair(std::move(subfield), std::move(filter));
if (auto result = leafCallToSubfieldFilter(*call, evaluator, false)) {
return std::move(result.value());
}
}
}
VELOX_UNSUPPORTED(
"Unsupported expression for range filter: {}", expr->toString());
}

std::unique_ptr<common::Filter> leafCallToSubfieldFilter(
std::optional<std::pair<common::Subfield, std::unique_ptr<common::Filter>>>
leafCallToSubfieldFilter(
const core::CallTypedExpr& call,
common::Subfield& subfield,
core::ExpressionEvaluator* evaluator,
bool negated) override {
if (call.inputs().empty()) {
return nullptr;
return std::nullopt;
}

const auto* leftSide = call.inputs()[0].get();

common::Subfield subfield;
if (call.name() == "custom_eq") {
if (toSubfield(leftSide, subfield)) {
return negated ? makeNotEqualFilter(call.inputs()[1], evaluator)
: makeEqualFilter(call.inputs()[1], evaluator);
auto filter = negated ? makeNotEqualFilter(call.inputs()[1], evaluator)
: makeEqualFilter(call.inputs()[1], evaluator);
if (filter != nullptr) {
return std::make_pair(std::move(subfield), std::move(filter));
}
return std::nullopt;
}
} else if (call.name() == "is_null") {
if (toSubfield(call.inputs()[0].get(), subfield)) {
if (negated) {
return isNotNull();
return std::make_pair(std::move(subfield), isNotNull());
}
return isNull();
return std::make_pair(std::move(subfield), isNull());
}
}
return nullptr;
return std::nullopt;
}
};

Expand Down
Loading