diff --git a/core/modules/ccontrol/testCControl.cc b/core/modules/ccontrol/testCControl.cc index 6822d8cb0e..4290a636f6 100644 --- a/core/modules/ccontrol/testCControl.cc +++ b/core/modules/ccontrol/testCControl.cc @@ -950,6 +950,14 @@ static const std::vector ANTLR4_COMPARE_QUERIES = { }, "SELECT * FROM Filter WHERE NOT(filterId>1 AND filterId<6)" ), + + // tests XOR + Antlr4CompareQueries( + "select ra_PS, objectId from Object where ra_PS > 1 XOR ra_PS < 2;", + "select ra_PS, objectId from Object where (ra_PS > 1 and not ra_PS < 2) OR (not ra_PS > 1 and ra_PS < 2)", + [](query::SelectStmt::Ptr const & selectStatement) {}, + "SELECT ra_PS,objectId FROM Object WHERE (ra_PS>1 AND NOT ra_PS<2) OR (NOT ra_PS>1 AND ra_PS<2)" + ), }; diff --git a/core/modules/parser/QSMySqlListener.cc b/core/modules/parser/QSMySqlListener.cc index e9b94bc2d1..4622e9f947 100644 --- a/core/modules/parser/QSMySqlListener.cc +++ b/core/modules/parser/QSMySqlListener.cc @@ -536,11 +536,17 @@ class LogicalOperatorCBH : public BaseCBH { enum OperatorType { AND, OR, + XOR, }; virtual void handleLogicalOperator(OperatorType operatorType) = 0; static string OperatorTypeToStr(OperatorType operatorType) { - return operatorType == AND ? "AND" : "OR"; + switch (operatorType) { + case AND: return "AND"; break; + case OR: return "OR"; break; + case XOR: return "XOR"; break; + } + return "unrecognized operator type."; } }; @@ -2689,29 +2695,15 @@ class LogicalExpressionAdapter : void handleLogicalOperator(LogicalOperatorCBH::OperatorType operatorType) { TRACE_CALLBACK_INFO(LogicalOperatorCBH::OperatorTypeToStr(operatorType)); - switch (operatorType) { - default: - ASSERT_EXECUTION_CONDITION(false, "unhandled operator type", _ctx); - break; - - case LogicalOperatorCBH::AND: - // We capture the AndTerm into a base class so we can pass by reference into the setter. - _setLogicalOperator(make_shared()); - break; - - case LogicalOperatorCBH::OR: - // We capture the OrTerm into a base class so we can pass by reference into the setter. - _setLogicalOperator(make_shared()); - break; - } + ASSERT_EXECUTION_CONDITION(false == _logicalOperatorIsSet, + "logical operator must be set only once.", _ctx); + _logicalOperatorIsSet = true; + _logicalOperatorType = operatorType; } void handleLogicalExpression(shared_ptr const & logicalTerm, antlr4::ParserRuleContext* childCtx) override { TRACE_CALLBACK_INFO(logicalTerm); - if (_logicalOperator != nullptr && _logicalOperator->merge(*logicalTerm)) { - return; - } _terms.push_back(logicalTerm); } @@ -2725,47 +2717,74 @@ class LogicalExpressionAdapter : } void onExit() override { - ASSERT_EXECUTION_CONDITION(_logicalOperator != nullptr, "logicalOperator is not set.", _ctx); - - bool isOr = dynamic_pointer_cast(_logicalOperator) != nullptr; - for (auto term : _terms) { - if (false == _logicalOperator->merge(*term)) { - if (isOr) { - _logicalOperator->addBoolTerm(make_shared(term)); - } else { - _logicalOperator->addBoolTerm(term); + ASSERT_EXECUTION_CONDITION(_logicalOperatorIsSet, "logicalOperator is not set.", _ctx); + shared_ptr logicalTerm; + switch (_logicalOperatorType) { + case LogicalOperatorCBH::AND: { + logicalTerm = make_shared(); + for (auto term : _terms) { + if (false == logicalTerm->merge(*term)) { + logicalTerm->addBoolTerm(term); + } + } + break; + } + + case LogicalOperatorCBH::OR: { + logicalTerm = make_shared(); + for (auto term : _terms) { + if (false == logicalTerm->merge(*term)) { + logicalTerm->addBoolTerm(make_shared(term)); + } + } + break; + } + + case LogicalOperatorCBH::XOR: { + logicalTerm = make_shared(); + for (auto addTerm : _terms) { + query::BoolTerm::PtrVector terms; + for (auto otherTerm: _terms) { + if (otherTerm == addTerm) { + terms.push_back(otherTerm); + } else { + auto term = dynamic_pointer_cast(otherTerm->copySyntax()); + ASSERT_EXECUTION_CONDITION(term != nullptr, "XOR currently only works with BoolFactor", _ctx); + term->setHasNot(true); + terms.push_back(term); + } + } + query::BoolFactorTerm::PtrVector termsWithParenthesis; + termsWithParenthesis.push_back(make_shared("(")); + termsWithParenthesis.push_back(make_shared(make_shared(terms))); + termsWithParenthesis.push_back(make_shared(")")); + auto boolFactor = make_shared(termsWithParenthesis); + auto andTerm = make_shared(boolFactor); + logicalTerm->addBoolTerm(andTerm); } + break; } + + default: + ASSERT_EXECUTION_CONDITION(false, "unhandled logical operator.", _ctx); } - lockedParent()->handleLogicalExpression(_logicalOperator, _ctx); + lockedParent()->handleLogicalExpression(logicalTerm, _ctx); } string name() const override { return getTypeName(this); } private: - void _setLogicalOperator(shared_ptr const & logicalTerm) { - ASSERT_EXECUTION_CONDITION(nullptr == _logicalOperator, - "logical operator must be set only once.", _ctx); - _logicalOperator = logicalTerm; - } - friend ostream& operator<<(ostream& os, const LogicalExpressionAdapter& logicaAndlExpressionAdapter); - // a qserv restrictor fucntion can be the left side of a predicate (currently it can only be the left - // side; that is to say, it can only be the first term in the WHERE clause. If `handleQservFunctionSpec` - // is called and _leftTerm is null (as well as _rightTerm and _logicalOperator, then _leftHandled is set - // to true to indicate that the left term has been handled. This allows onExit to put only one term into - // the logicalOperator and know that it was ok (the qserv IR accepts an AndTerm with only one factor). - // This mechanism does not fully proect against qserv restrictors that may be the left side of a - // subsequent logical expression. TBD if that's really an issue. vector> _terms; - shared_ptr _logicalOperator; + LogicalOperatorCBH::OperatorType _logicalOperatorType; + bool _logicalOperatorIsSet {false}; }; ostream& operator<<(ostream& os, const LogicalExpressionAdapter& logicalExpressionAdapter) { os << "LogicalExpressionAdapter("; - os << "terms:" << util::printable(logicalExpressionAdapter._terms); + os << util::printable(logicalExpressionAdapter._terms); return os; } @@ -3277,6 +3296,8 @@ class LogicalOperatorAdapter : lockedParent()->handleLogicalOperator(LogicalOperatorCBH::AND); } else if (_ctx->OR() != nullptr || _ctx->getText() == "||") { lockedParent()->handleLogicalOperator(LogicalOperatorCBH::OR); + } else if (_ctx->XOR() != nullptr) { // just a note; there is no alt operator like e.g. && for AND + lockedParent()->handleLogicalOperator(LogicalOperatorCBH::XOR); } else { ASSERT_EXECUTION_CONDITION(false, "unhandled logical operator", _ctx); } diff --git a/core/modules/query/BoolTerm.cc b/core/modules/query/BoolTerm.cc index 4fd0480080..ce0407eb8f 100644 --- a/core/modules/query/BoolTerm.cc +++ b/core/modules/query/BoolTerm.cc @@ -309,7 +309,7 @@ bool OrTerm::merge(const BoolTerm& other) { return true; } void OrTerm::dbgPrint(std::ostream& os) const { - os << "OrTerm(terms:" << util::printable(_terms) << ")"; + os << "OrTerm(" << util::printable(_terms) << ")"; } bool OrTerm::operator==(const BoolTerm& rhs) const { auto rhsOrTerm = dynamic_cast(&rhs); @@ -332,7 +332,7 @@ bool AndTerm::merge(const BoolTerm& other) { return true; } void AndTerm::dbgPrint(std::ostream& os) const { - os << "AndTerm(terms:" << util::printable(_terms) << ")"; + os << "AndTerm(" << util::printable(_terms) << ")"; } bool AndTerm::operator==(const BoolTerm& rhs) const { auto rhsAndTerm = dynamic_cast(&rhs); @@ -348,7 +348,11 @@ std::shared_ptr BoolFactor::copySyntax() const { return bf; } void BoolFactor::dbgPrint(std::ostream& os) const { - os << "BoolFactor(terms:" << util::printable(_terms) << ", hasNot:" << _hasNot << ")"; + os << "BoolFactor(" << util::printable(_terms); + if (_hasNot) { + os << ", has NOT"; + } + os << ")"; } void UnknownTerm::dbgPrint(std::ostream& os) const { os << "UnknownTerm()"; @@ -362,7 +366,11 @@ BoolFactorTerm::Ptr PassTerm::copySyntax() const { return BoolFactorTerm::Ptr(p); } void PassTerm::dbgPrint(std::ostream& os) const { - os << "PassTerm(text:'" << _text << "')"; + os << "PassTerm('"; + if ("(" == _text) os << "LHP"; + else if (")" == _text) os << "RHP"; + else os << _text; + os << "')"; } bool PassTerm::operator==(const BoolFactorTerm& rhs) const { auto rhsPassTerm = dynamic_cast(&rhs); @@ -377,7 +385,7 @@ BoolFactorTerm::Ptr PassListTerm::copySyntax() const { return BoolFactorTerm::Ptr(p); } void PassListTerm::dbgPrint(std::ostream& os) const { - os << "PassListTerm(terms:" << util::printable(_terms) << ")"; + os << "PassListTerm(" << util::printable(_terms) << ")"; } bool PassListTerm::operator==(const BoolFactorTerm& rhs) const { auto rhsTerm = dynamic_cast(&rhs); @@ -392,7 +400,7 @@ BoolFactorTerm::Ptr BoolTermFactor::copySyntax() const { return BoolFactorTerm::Ptr(p); } void BoolTermFactor::dbgPrint(std::ostream& os) const { - os << "BoolTermFactor(term:" << _term << ")"; + os << "BoolTermFactor(" << _term << ")"; } bool BoolTermFactor::operator==(const BoolFactorTerm& rhs) const { auto rhsTerm = dynamic_cast(&rhs); diff --git a/core/modules/query/FuncExpr.cc b/core/modules/query/FuncExpr.cc index 3765447995..9a1d7fcd69 100644 --- a/core/modules/query/FuncExpr.cc +++ b/core/modules/query/FuncExpr.cc @@ -119,7 +119,7 @@ std::ostream& operator<<(std::ostream& os, FuncExpr const& fe) { os << "FuncExpr("; os << "name:" << fe._name; - os << ", params:" << util::printable(fe.params); + os << ", " << util::printable(fe.params); os << ")"; return os; } diff --git a/core/modules/query/GroupByClause.cc b/core/modules/query/GroupByClause.cc index 3a9183f625..e877cc2b76 100644 --- a/core/modules/query/GroupByClause.cc +++ b/core/modules/query/GroupByClause.cc @@ -83,7 +83,7 @@ bool GroupByTerm::operator==(const GroupByTerm& rhs) const { // GroupByClause //////////////////////////////////////////////////////////////////////// std::ostream& operator<<(std::ostream& os, GroupByClause const& c) { - os <<"GroupByClause(terms:" << util::ptrPrintable(c._terms) << ")"; + os << "GroupByClause(" << util::ptrPrintable(c._terms) << ")"; return os; } diff --git a/core/modules/query/OrderByClause.cc b/core/modules/query/OrderByClause.cc index 00ea3aad58..dcdf8c532a 100644 --- a/core/modules/query/OrderByClause.cc +++ b/core/modules/query/OrderByClause.cc @@ -140,7 +140,7 @@ bool OrderByTerm::operator==(const OrderByTerm& rhs) const { //////////////////////////////////////////////////////////////////////// std::ostream& operator<<(std::ostream& os, OrderByClause const& clause) { - os << "OrderByClause(terms:" << util::ptrPrintable(clause._terms) << ")"; + os << "OrderByClause(" << util::ptrPrintable(clause._terms) << ")"; return os; } diff --git a/core/modules/query/Predicate.cc b/core/modules/query/Predicate.cc index 2b1512f5cb..dac30247b4 100644 --- a/core/modules/query/Predicate.cc +++ b/core/modules/query/Predicate.cc @@ -246,7 +246,9 @@ BoolFactorTerm::Ptr InPredicate::clone() const { void InPredicate::dbgPrint(std::ostream& os) const { os << "InPredicate(value:" << value; os << ", cands:" << util::printable(cands); - os << ", hasNot:" << hasNot; + if (hasNot) { + os << ", has NOT"; + } os << ")"; } @@ -271,7 +273,9 @@ BoolFactorTerm::Ptr BetweenPredicate::clone() const { void BetweenPredicate::dbgPrint(std::ostream& os) const { os << "BetweenPredicate(value:" << value; - os << ", NOT:" << (hasNot ? "true" : "false"); + if (hasNot) { + os << " NOT,"; + } os << ", minValue:" << minValue; os << ", maxValue:" << maxValue; os << ")"; @@ -298,7 +302,9 @@ BoolFactorTerm::Ptr LikePredicate::clone() const { void LikePredicate::dbgPrint(std::ostream& os) const { os << "LikePredicate(value:" << value; - os << ", NOT:" << hasNot; + if (hasNot) { + os << ", NOT"; + } os << ", charValue:" << charValue; os << ")"; } @@ -322,7 +328,9 @@ BoolFactorTerm::Ptr NullPredicate::clone() const { void NullPredicate::dbgPrint(std::ostream& os) const { os << "NullPredicate(value:" << value; - os << ", hasNot:" << hasNot; + if (hasNot) { + os << ", NOT"; + } os << ")"; } diff --git a/core/modules/query/ValueExpr.cc b/core/modules/query/ValueExpr.cc index b8621e5d51..a2ddc15506 100644 --- a/core/modules/query/ValueExpr.cc +++ b/core/modules/query/ValueExpr.cc @@ -74,7 +74,7 @@ renderList(QueryTemplate& qt, ValueExprPtrVector const& vel) { //////////////////////////////////////////////////////////////////////// std::ostream& operator<<(std::ostream& os, const ValueExpr::FactorOp& factorOp) { - os << "FactorOp(op:"; + os << "FactorOp("; switch(factorOp.op) { case ValueExpr::NONE: os << "NONE"; break; case ValueExpr::UNKNOWN: os << "UNKNOWN"; break; @@ -92,7 +92,7 @@ std::ostream& operator<<(std::ostream& os, const ValueExpr::FactorOp& factorOp) case ValueExpr::BIT_XOR: os << "BIT_XOR"; break; default: os << "!!unhandled!!"; break; } - os << ", factor:" << factorOp.factor; + os << ", " << factorOp.factor; os << ")"; return os; } @@ -270,14 +270,7 @@ std::string ValueExpr::sqlFragment() const { std::ostream& operator<<(std::ostream& os, ValueExpr const& ve) { os << "ValueExpr("; - os << "alias:" << ve._alias; - os << ", isColumnRef:" << ve.isColumnRef(); - os << ", isFactor:" << ve.isFactor(); - os << ", isStar:" << ve.isStar(); - bool hasAgg = false; - qana::CheckAggregation ca(hasAgg); - os << ", hasAgg:" << hasAgg; - os << ", factorOps:"; + if (not ve._alias.empty()) os << "alias:" << ve._alias << ", "; os << util::printable(ve._factorOps); os << ")"; return os; diff --git a/core/modules/query/ValueFactor.cc b/core/modules/query/ValueFactor.cc index 05457a3116..4a62689627 100644 --- a/core/modules/query/ValueFactor.cc +++ b/core/modules/query/ValueFactor.cc @@ -136,12 +136,11 @@ ValueFactorPtr ValueFactor::clone() const{ std::ostream& operator<<(std::ostream& os, ValueFactor const& ve) { os << "ValueFactor("; - os << "type:" << ValueFactor::getTypeString(ve._type); - os << ", columnRef:" << ve._columnRef; - os << ", funcExpr:" << ve._funcExpr; - os << ", valueExpr:" << ve._valueExpr; - os << ", alias:" << ve._alias; - os << ", constVal:" << ve._constVal; + if (ve._columnRef != nullptr) os << ve._columnRef; + if (ve._funcExpr != nullptr) os << ve._funcExpr; + if (ve._valueExpr != nullptr) os << ve._valueExpr; + if (not ve._alias.empty()) os << "alias:" << ve._alias; + if (not ve._constVal.empty()) os << "constVal:" << ve._constVal; os << ")"; return os; }