diff options
Diffstat (limited to 'document/src')
14 files changed, 189 insertions, 103 deletions
diff --git a/document/src/main/java/com/yahoo/document/select/rule/ComparisonNode.java b/document/src/main/java/com/yahoo/document/select/rule/ComparisonNode.java index cf7e3851d55..d6166f8fcbe 100644 --- a/document/src/main/java/com/yahoo/document/select/rule/ComparisonNode.java +++ b/document/src/main/java/com/yahoo/document/select/rule/ComparisonNode.java @@ -282,6 +282,8 @@ public class ComparisonNode implements ExpressionNode { return getAsNumber(((NumericFieldValue)value).getNumber()); } else if (value instanceof BoolFieldValue) { return ((BoolFieldValue)value).getBoolean() ? 1 : 0; + } else if (value instanceof Boolean) { + return (Boolean)value ? 1 : 0; } else { return Double.NaN; //new IllegalStateException("Term '" + value + "' (" + value.getClass() + ") does not evaluate to a number."); } diff --git a/document/src/test/java/com/yahoo/document/select/DocumentSelectorTestCase.java b/document/src/test/java/com/yahoo/document/select/DocumentSelectorTestCase.java index bf8cf07a097..38fdadb18e4 100644 --- a/document/src/test/java/com/yahoo/document/select/DocumentSelectorTestCase.java +++ b/document/src/test/java/com/yahoo/document/select/DocumentSelectorTestCase.java @@ -507,6 +507,16 @@ public class DocumentSelectorTestCase { assertEquals(Result.FALSE, evaluate("14.3 == null", documents.get(0))); assertEquals(Result.FALSE, evaluate("null = 0", documents.get(0))); + // Boolean literals in comparisons + assertEquals(Result.TRUE, evaluate("true = true", documents.get(0))); + assertEquals(Result.TRUE, evaluate("true == true", documents.get(0))); + assertEquals(Result.FALSE, evaluate("true == false", documents.get(0))); + assertEquals(Result.TRUE, evaluate("false == false", documents.get(0))); + assertEquals(Result.TRUE, evaluate("true == 1", documents.get(0))); + assertEquals(Result.FALSE, evaluate("true == 0", documents.get(0))); + assertEquals(Result.FALSE, evaluate("false == 1", documents.get(0))); + assertEquals(Result.TRUE, evaluate("false == 0", documents.get(0))); + // Field values assertEquals(Result.TRUE, evaluate("test.hint = 24", documents.get(0))); assertEquals(Result.FALSE, evaluate("test.hint = 24", documents.get(1))); @@ -748,12 +758,19 @@ public class DocumentSelectorTestCase { @Test public void boolean_fields_can_be_used_for_equality_comparisons() throws ParseException { var documents = createDocs(); - assertEquals(Result.TRUE, evaluate("test.truth == 1", documents.get(8))); // has explicit field set to true - assertEquals(Result.FALSE, evaluate("test.truth == 1", documents.get(9))); // has explicit field set to false + // Doc 8 has bool field set explicitly to true, doc 9 has field explicitly set to false + assertEquals(Result.TRUE, evaluate("test.truth == 1", documents.get(8))); + assertEquals(Result.TRUE, evaluate("test.truth == true", documents.get(8))); + assertEquals(Result.FALSE, evaluate("test.truth == 1", documents.get(9))); + assertEquals(Result.FALSE, evaluate("test.truth == true", documents.get(9))); assertEquals(Result.TRUE, evaluate("test.truth == 0", documents.get(9))); + assertEquals(Result.TRUE, evaluate("test.truth == false", documents.get(9))); // FIXME very un-intuitive behavior when nulls are implicitly returned: - assertEquals(Result.FALSE, evaluate("test.truth == 1", documents.get(0))); // Does not have field set in document - assertEquals(Result.FALSE, evaluate("test.truth == 0", documents.get(0))); // Does not have field set in document + // Doc 1 does not have the bool field set, but the implicit null value is neither true nor false + assertEquals(Result.FALSE, evaluate("test.truth == 1", documents.get(0))); + assertEquals(Result.FALSE, evaluate("test.truth == true", documents.get(0))); + assertEquals(Result.FALSE, evaluate("test.truth == 0", documents.get(0))); + assertEquals(Result.FALSE, evaluate("test.truth == false", documents.get(0))); } @Test diff --git a/document/src/tests/documentselectparsertest.cpp b/document/src/tests/documentselectparsertest.cpp index 6644fec2da0..1606cbc634d 100644 --- a/document/src/tests/documentselectparsertest.cpp +++ b/document/src/tests/documentselectparsertest.cpp @@ -637,6 +637,16 @@ TEST_F(DocumentSelectParserTest, operators_1) PARSE("14.3 == null", *_doc[0], False); PARSE("null = 0", *_doc[0], False); + // Boolean literals in comparisons + PARSE("true = true", *_doc[0], True); + PARSE("true == true", *_doc[0], True); + PARSE("true == false", *_doc[0], False); + PARSE("false == false", *_doc[0], True); + PARSE("true == 1", *_doc[0], True); + PARSE("true == 0", *_doc[0], False); + PARSE("false == 1", *_doc[0], False); + PARSE("false == 0", *_doc[0], True); + // Field values PARSE("testdoctype1.headerval = 24", *_doc[0], True); PARSE("testdoctype1.headerval = 24", *_doc[1], False); @@ -925,12 +935,19 @@ TEST_F(DocumentSelectParserTest, operators_9) TEST_F(DocumentSelectParserTest, can_use_boolean_fields_in_expressions) { createDocs(); - PARSE("testdoctype1.boolfield == 1", *_doc[11], True); // has explicit field set to true - PARSE("testdoctype1.boolfield == 1", *_doc[12], False); // has explicit field set to false + // Doc 11 has bool field set explicitly to true, doc 12 has field explicitly set to false + PARSE("testdoctype1.boolfield == 1", *_doc[11], True); + PARSE("testdoctype1.boolfield == true", *_doc[11], True); + PARSE("testdoctype1.boolfield == 1", *_doc[12], False); + PARSE("testdoctype1.boolfield == true", *_doc[12], False); PARSE("testdoctype1.boolfield == 0", *_doc[12], True); + PARSE("testdoctype1.boolfield == false", *_doc[12], True); // FIXME very un-intuitive behavior when nulls are implicitly returned: - PARSE("testdoctype1.boolfield == 1", *_doc[1], False); // Does not have field set in document - PARSE("testdoctype1.boolfield == 0", *_doc[1], False); // Does not have field set in document + // Doc 1 does not have the bool field set, but the implicit null value is neither true nor false + PARSE("testdoctype1.boolfield == 1", *_doc[1], False); + PARSE("testdoctype1.boolfield == true", *_doc[1], False); + PARSE("testdoctype1.boolfield == 0", *_doc[1], False); + PARSE("testdoctype1.boolfield == false", *_doc[1], False); } namespace { @@ -989,6 +1006,7 @@ namespace { void visitFloatValueNode(const select::FloatValueNode &) override {} void visitVariableValueNode(const select::VariableValueNode &) override {} void visitIntegerValueNode(const select::IntegerValueNode &) override {} + void visitBoolValueNode(const select::BoolValueNode &) override {} void visitCurrentTimeValueNode(const select::CurrentTimeValueNode &) override {} void visitStringValueNode(const select::StringValueNode &) override {} void visitNullValueNode(const select::NullValueNode &) override {} @@ -1392,6 +1410,9 @@ public: void visitIntegerValueNode(const select::IntegerValueNode& node) override { data << node.getValue(); } + void visitBoolValueNode(const select::BoolValueNode& node) override { + data << node.bool_value_str(); + } void visitCurrentTimeValueNode(const select::CurrentTimeValueNode&) override {} void visitStringValueNode(const select::StringValueNode& str) override { data << '"' << str.getValue() << '"'; @@ -1491,6 +1512,17 @@ TEST_F(DocumentSelectParserTest, test_ambiguous_field_spec_expression_is_handled parse_to_tree("(testdoctype1.foo) AND (testdoctype1.bar)")); } +TEST_F(DocumentSelectParserTest, test_ambiguous_bool_expression_is_handled_correctly) +{ + createDocs(); + using namespace std::string_literals; + // Bools both as high level Nodes and low level ValueNodes + EXPECT_EQ("(OR (AND true false) (== (FIELD testdoctype1 myfield) true))"s, + parse_to_tree("true and false or testdoctype1.myfield == true")); + EXPECT_EQ("(!= true false)"s, parse_to_tree("true != false")); + EXPECT_EQ("(!= true false)"s, parse_to_tree("(true) != (false)")); +} + TEST_F(DocumentSelectParserTest, special_tokens_are_allowed_as_freestanding_identifier_names) { createDocs(); EXPECT_EQ("(NOT (DOCTYPE user))", parse_to_tree("not user")); diff --git a/document/src/vespa/document/bucket/bucketselector.cpp b/document/src/vespa/document/bucket/bucketselector.cpp index 34f613bef84..b9dc6a9cfa9 100644 --- a/document/src/vespa/document/bucket/bucketselector.cpp +++ b/document/src/vespa/document/bucket/bucketselector.cpp @@ -152,6 +152,7 @@ using namespace document::select; void visitFloatValueNode(const FloatValueNode &) override {} void visitVariableValueNode(const VariableValueNode &) override {} void visitIntegerValueNode(const IntegerValueNode &) override {} + void visitBoolValueNode(const BoolValueNode &) override {} void visitCurrentTimeValueNode(const CurrentTimeValueNode &) override {} void visitStringValueNode(const StringValueNode &) override {} void visitNullValueNode(const NullValueNode &) override {} diff --git a/document/src/vespa/document/select/cloningvisitor.cpp b/document/src/vespa/document/select/cloningvisitor.cpp index 55e8c1effc1..81cb31145ec 100644 --- a/document/src/vespa/document/select/cloningvisitor.cpp +++ b/document/src/vespa/document/select/cloningvisitor.cpp @@ -10,30 +10,6 @@ namespace document::select { -const int CloningVisitor::OrPriority; -const int CloningVisitor::AndPriority; -const int CloningVisitor::NotPriority; -const int CloningVisitor::ComparePriority; -const int CloningVisitor::AddPriority; -const int CloningVisitor::SubPriority; -const int CloningVisitor::MulPriority; -const int CloningVisitor::DivPriority; -const int CloningVisitor::ModPriority; -const int CloningVisitor::DocumentTypePriority; -const int CloningVisitor::FieldValuePriority; -const int CloningVisitor::InvalidConstPriority; -const int CloningVisitor::InvalidValPriority; -const int CloningVisitor::ConstPriority; -const int CloningVisitor::FuncPriority; -const int CloningVisitor::VariablePriority; -const int CloningVisitor::FloatPriority; -const int CloningVisitor::IntegerPriority; -const int CloningVisitor::CurrentTimePriority; -const int CloningVisitor::StringPriority; -const int CloningVisitor::NullValPriority; -const int CloningVisitor::IdPriority; -const int CloningVisitor::SearchColPriority; - CloningVisitor::CloningVisitor() : _node(), _valueNode(), @@ -63,7 +39,7 @@ CloningVisitor::visitAndBranch(const And &expr) setNodeParentheses(priority); std::unique_ptr<Node> rhs(std::move(_node)); _priority = priority; - _node.reset(new And(std::move(lhs), std::move(rhs), "and")); + _node = std::make_unique<And>(std::move(lhs), std::move(rhs), "and"); }; @@ -83,7 +59,7 @@ CloningVisitor::visitOrBranch(const Or &expr) setNodeParentheses(priority); std::unique_ptr<Node> rhs(std::move(_node)); _priority = priority; - _node.reset(new Or(std::move(lhs), std::move(rhs), "or")); + _node = std::make_unique<Or>(std::move(lhs), std::move(rhs), "or"); }; @@ -96,7 +72,7 @@ CloningVisitor::visitNotBranch(const Not &expr) _resultSet.calcNot(); std::unique_ptr<Node> child(std::move(_node)); _priority = priority; - _node.reset(new Not(std::move(child), "not")); + _node = std::make_unique<Not>(std::move(child), "not"); }; @@ -116,10 +92,7 @@ CloningVisitor::visitComparison(const Compare &expr) const Operator &op(expr.getOperator()); _priority = priority; _resultSet.fill(); // should be less if const - _node.reset(new Compare(std::move(lhs), - op, - std::move(rhs), - expr.getBucketIdFactory())); + _node = std::make_unique<Compare>(std::move(lhs), op, std::move(rhs), expr.getBucketIdFactory()); }; @@ -149,7 +122,7 @@ CloningVisitor::visitFunctionValueNode(const FunctionValueNode &expr) setValueNodeParentheses(priority); std::unique_ptr<ValueNode> child(std::move(_valueNode)); _priority = priority; - _valueNode.reset(new FunctionValueNode(expr.getFunctionName(), std::move(child))); + _valueNode = std::make_unique<FunctionValueNode>(expr.getFunctionName(), std::move(child)); }; @@ -160,7 +133,7 @@ CloningVisitor::visitConstant(const Constant &expr) _priority = ConstPriority; bool val = expr.getConstantValue(); _resultSet.add(val ? Result::True : Result::False); - _node.reset(new Constant(val)); + _node = std::make_unique<Constant>(val); } @@ -171,7 +144,7 @@ CloningVisitor::visitInvalidConstant(const InvalidConstant &expr) _constVal = true; _priority = InvalidConstPriority; _resultSet.add(Result::Invalid); - _node.reset(new InvalidConstant("invalid")); + _node = std::make_unique<InvalidConstant>("invalid"); } @@ -218,7 +191,7 @@ CloningVisitor::visitFloatValueNode(const FloatValueNode &expr) void CloningVisitor::visitVariableValueNode(const VariableValueNode &expr) { - _valueNode.reset(new VariableValueNode(expr.getVariableName())); + _valueNode = std::make_unique<VariableValueNode>(expr.getVariableName()); _priority = VariablePriority; } @@ -233,6 +206,15 @@ CloningVisitor::visitIntegerValueNode(const IntegerValueNode &expr) void +CloningVisitor::visitBoolValueNode(const BoolValueNode &expr) +{ + _constVal = true; + _valueNode = expr.clone(); + _priority = BoolPriority; +} + + +void CloningVisitor::visitCurrentTimeValueNode(const CurrentTimeValueNode &expr) { _constVal = false; diff --git a/document/src/vespa/document/select/cloningvisitor.h b/document/src/vespa/document/select/cloningvisitor.h index a49f44190ef..cce171f81cb 100644 --- a/document/src/vespa/document/select/cloningvisitor.h +++ b/document/src/vespa/document/select/cloningvisitor.h @@ -20,33 +20,33 @@ protected: uint32_t _fieldNodes; ResultSet _resultSet; - static const int OrPriority = 100; - static const int AndPriority = 200; - static const int NotPriority = 300; - static const int ComparePriority = 400; - static const int AddPriority = 500; - static const int SubPriority = 500; - static const int MulPriority = 600; - static const int DivPriority = 600; - static const int ModPriority = 700; - static const int DocumentTypePriority = 1000; - static const int FieldValuePriority = 1000; - static const int InvalidConstPriority = 1000; - static const int InvalidValPriority = 1000; - static const int ConstPriority = 1000; - static const int FuncPriority = 1000; - static const int VariablePriority = 1000; - static const int FloatPriority = 1000; - static const int IntegerPriority = 1000; - static const int CurrentTimePriority = 1000; - static const int StringPriority = 1000; - static const int NullValPriority = 1000; - static const int IdPriority = 1000; - static const int SearchColPriority = 1000; + static constexpr int OrPriority = 100; + static constexpr int AndPriority = 200; + static constexpr int NotPriority = 300; + static constexpr int ComparePriority = 400; + static constexpr int AddPriority = 500; + static constexpr int SubPriority = 500; + static constexpr int MulPriority = 600; + static constexpr int DivPriority = 600; + static constexpr int ModPriority = 700; + static constexpr int DocumentTypePriority = 1000; + static constexpr int FieldValuePriority = 1000; + static constexpr int InvalidConstPriority = 1000; + static constexpr int InvalidValPriority = 1000; + static constexpr int ConstPriority = 1000; + static constexpr int FuncPriority = 1000; + static constexpr int VariablePriority = 1000; + static constexpr int FloatPriority = 1000; + static constexpr int IntegerPriority = 1000; + static constexpr int BoolPriority = 1000; + static constexpr int CurrentTimePriority = 1000; + static constexpr int StringPriority = 1000; + static constexpr int NullValPriority = 1000; + static constexpr int IdPriority = 1000; public: CloningVisitor(); - ~CloningVisitor(); + ~CloningVisitor() override; void visitAndBranch(const And &expr) override; void visitOrBranch(const Or &expr) override; @@ -62,6 +62,7 @@ public: void visitFloatValueNode(const FloatValueNode &expr) override; void visitVariableValueNode(const VariableValueNode &expr) override; void visitIntegerValueNode(const IntegerValueNode &expr) override; + void visitBoolValueNode(const BoolValueNode &expr) override; void visitCurrentTimeValueNode(const CurrentTimeValueNode &expr) override; void visitStringValueNode(const StringValueNode &expr) override; void visitNullValueNode(const NullValueNode &expr) override; @@ -77,7 +78,7 @@ public: int rhsPriority, bool rhsConstVal); void swap(CloningVisitor &rhs); - void revisit(void); + void revisit(); }; } diff --git a/document/src/vespa/document/select/gid_filter.cpp b/document/src/vespa/document/select/gid_filter.cpp index 3a3e215db68..a1954eb6bc2 100644 --- a/document/src/vespa/document/select/gid_filter.cpp +++ b/document/src/vespa/document/select/gid_filter.cpp @@ -26,6 +26,7 @@ struct NoOpVisitor : Visitor { void visitFloatValueNode(const FloatValueNode&) override {} void visitVariableValueNode(const VariableValueNode&) override {} void visitIntegerValueNode(const IntegerValueNode&) override {} + void visitBoolValueNode(const BoolValueNode&) override {} void visitCurrentTimeValueNode(const CurrentTimeValueNode&) override {} void visitStringValueNode(const StringValueNode&) override {} void visitNullValueNode(const NullValueNode&) override {} diff --git a/document/src/vespa/document/select/grammar/parser.yy b/document/src/vespa/document/select/grammar/parser.yy index 8d64b2382b5..409f8e3b29e 100644 --- a/document/src/vespa/document/select/grammar/parser.yy +++ b/document/src/vespa/document/select/grammar/parser.yy @@ -75,7 +75,7 @@ %token <string_val> ID USER GROUP SCHEME NAMESPACE SPECIFIC BUCKET GID TYPE %type <string_val> ident mangled_ident -%type <abstract_node> bool_ +%type <value_node> bool_ /* TODO 'leaf' is a bad name for something that isn't a leaf... */ %type <abstract_node> expression comparison logical_expr leaf doc_type %type <string_val> id_arg @@ -190,8 +190,8 @@ null_ ; bool_ - : TRUE { $$ = new Constant(true); } - | FALSE { $$ = new Constant(false); } + : TRUE { $$ = new BoolValueNode(true); } + | FALSE { $$ = new BoolValueNode(false); } ; number @@ -287,6 +287,7 @@ field_spec value : null_ { $$ = $1; } + | bool_ { $$ = $1; } | string { $$ = $1; } | id_spec { $$ = $1; } | variable { $$ = $1; } @@ -328,10 +329,9 @@ comparison ; leaf - : bool_ { $$ = $1; } - | comparison { $$ = $1; } + : comparison { $$ = $1; } | doc_type { $$ = $1; } - | arith_expr { /* Actually field_spec, see comment below..! */ + | arith_expr { /* Actually bool_ or field_spec, see comment below..! */ // Grammar-wise, we _do not_ accept arbitrary arith_exprs at this level. But the // selection grammar as it stands is otherwise ambiguous with LR(1) parsing. // More specifically, if we used field_spec instead of arith_expr here, the parser @@ -343,15 +343,21 @@ leaf // conflict goes away. We can then do a sneaky "run-time" type check to ensure we only // get the expected node from the rule. // It's not pretty, but it avoids an undefined grammar (which is much less pretty!). + // The same goes for boolean constants, which may be used both as higher-level (non-value) + // nodes or as value nodes to compare against. auto node = steal<ValueNode>($1); - if (dynamic_cast<FieldValueNode*>(node.get()) == nullptr) { - throw syntax_error(@$, "expected field spec, doctype, bool or comparison"); + if (auto* as_bool = dynamic_cast<BoolValueNode*>(node.get())) { + $$ = new Constant(as_bool->bool_value()); // Replace single bool node subtree with constant. + } else { + if (dynamic_cast<FieldValueNode*>(node.get()) == nullptr) { + throw syntax_error(@$, "expected field spec, doctype, bool or comparison"); + } + // Implicit rewrite to non-null comparison node + $$ = new Compare(std::move(node), + FunctionOperator::NE, + std::make_unique<NullValueNode>(), + bucket_id_factory); } - // Implicit rewrite to non-null comparison node - $$ = new Compare(std::move(node), - FunctionOperator::NE, - std::make_unique<NullValueNode>(), - bucket_id_factory); } ; diff --git a/document/src/vespa/document/select/traversingvisitor.cpp b/document/src/vespa/document/select/traversingvisitor.cpp index a0b7441903e..f7e76e6fe32 100644 --- a/document/src/vespa/document/select/traversingvisitor.cpp +++ b/document/src/vespa/document/select/traversingvisitor.cpp @@ -97,6 +97,12 @@ TraversingVisitor::visitIntegerValueNode(const IntegerValueNode &) void +TraversingVisitor::visitBoolValueNode(const BoolValueNode &) +{ +} + + +void TraversingVisitor::visitCurrentTimeValueNode(const CurrentTimeValueNode &) { } diff --git a/document/src/vespa/document/select/traversingvisitor.h b/document/src/vespa/document/select/traversingvisitor.h index be240c42537..2a7531c06f0 100644 --- a/document/src/vespa/document/select/traversingvisitor.h +++ b/document/src/vespa/document/select/traversingvisitor.h @@ -24,6 +24,7 @@ public: void visitFloatValueNode(const FloatValueNode &) override; void visitVariableValueNode(const VariableValueNode &) override; void visitIntegerValueNode(const IntegerValueNode &) override; + void visitBoolValueNode(const BoolValueNode &) override; void visitCurrentTimeValueNode(const CurrentTimeValueNode &) override; void visitStringValueNode(const StringValueNode &) override; void visitNullValueNode(const NullValueNode &) override; diff --git a/document/src/vespa/document/select/valuenode.h b/document/src/vespa/document/select/valuenode.h index 71efa73b1d8..fb86128aa2c 100644 --- a/document/src/vespa/document/select/valuenode.h +++ b/document/src/vespa/document/select/valuenode.h @@ -57,12 +57,11 @@ protected: } } - ValueNode::UP wrapParens(ValueNode* node) const { - ValueNode::UP ret(node); + std::unique_ptr<ValueNode> wrapParens(std::unique_ptr<ValueNode> node) const { if (_parentheses) { - ret->setParentheses(); + node->setParentheses(); } - return ret; + return node; } std::unique_ptr<Value> defaultTrace(std::unique_ptr<Value> val, std::ostream& out) const; diff --git a/document/src/vespa/document/select/valuenodes.cpp b/document/src/vespa/document/select/valuenodes.cpp index 1c7d47d0591..452779ca5ba 100644 --- a/document/src/vespa/document/select/valuenodes.cpp +++ b/document/src/vespa/document/select/valuenodes.cpp @@ -115,11 +115,28 @@ IntegerValueNode::print(std::ostream& out, bool verbose, if (hadParentheses()) out << ')'; } +void +BoolValueNode::visit(Visitor& visitor) const +{ + visitor.visitBoolValueNode(*this); +} + +void +BoolValueNode::print(std::ostream& out, + [[maybe_unused]] bool verbose, + [[maybe_unused]] const std::string& indent) const +{ + if (hadParentheses()) out << '('; + out << bool_value_str(); + if (hadParentheses()) out << ')'; +} + + int64_t CurrentTimeValueNode::getValue() const { struct timeval mytime; - gettimeofday(&mytime, 0); + gettimeofday(&mytime, nullptr); return mytime.tv_sec; } diff --git a/document/src/vespa/document/select/valuenodes.h b/document/src/vespa/document/select/valuenodes.h index f6fa0400d7c..0c4584e7eee 100644 --- a/document/src/vespa/document/select/valuenodes.h +++ b/document/src/vespa/document/select/valuenodes.h @@ -28,7 +28,7 @@ public: void visit(Visitor& visitor) const override; ValueNode::UP clone() const override { - return wrapParens(new InvalidValueNode(_name)); + return wrapParens(std::make_unique<InvalidValueNode>(_name)); } }; @@ -46,7 +46,7 @@ public: void visit(Visitor& visitor) const override; ValueNode::UP clone() const override { - return wrapParens(new NullValueNode()); + return wrapParens(std::make_unique<NullValueNode>()); } }; @@ -66,7 +66,7 @@ public: void visit(Visitor& visitor) const override; ValueNode::UP clone() const override { - return wrapParens(new StringValueNode(_value)); + return wrapParens(std::make_unique<StringValueNode>(_value)); } }; @@ -88,7 +88,28 @@ public: void visit(Visitor& visitor) const override; ValueNode::UP clone() const override { - return wrapParens(new IntegerValueNode(_value, _isBucketValue)); + return wrapParens(std::make_unique<IntegerValueNode>(_value, _isBucketValue)); + } +}; + +// Inherit from IntegerValueNode to be implicitly treated as an integer value by +// all code that does not explicitly know (or care) about bool values. +class BoolValueNode : public IntegerValueNode { +public: + explicit BoolValueNode(bool value) : IntegerValueNode(value, false) {} + + [[nodiscard]] bool bool_value() const noexcept { + return bool(getValue()); + } + [[nodiscard]] const char* bool_value_str() const noexcept { + return bool_value() ? "true" : "false"; + } + + void print(std::ostream& out, bool verbose, const std::string& indent) const override; + void visit(Visitor& visitor) const override; + + std::unique_ptr<ValueNode> clone() const override { + return wrapParens(std::make_unique<BoolValueNode>(bool_value())); } }; @@ -105,7 +126,7 @@ public: void visit(Visitor& visitor) const override; ValueNode::UP clone() const override { - return wrapParens(new CurrentTimeValueNode); + return wrapParens(std::make_unique<CurrentTimeValueNode>()); } }; @@ -123,7 +144,7 @@ public: void visit(Visitor& visitor) const override; ValueNode::UP clone() const override { - return wrapParens(new VariableValueNode(_value)); + return wrapParens(std::make_unique<VariableValueNode>(_value)); } }; @@ -131,7 +152,7 @@ class FloatValueNode : public ValueNode { double _value; public: - FloatValueNode(double val) : _value(val) {} + explicit FloatValueNode(double val) : _value(val) {} double getValue() const { return _value; } @@ -143,7 +164,7 @@ public: void visit(Visitor& visitor) const override; ValueNode::UP clone() const override { - return wrapParens(new FloatValueNode(_value)); + return wrapParens(std::make_unique<FloatValueNode>(_value)); } }; @@ -172,7 +193,7 @@ public: void visit(Visitor& visitor) const override; ValueNode::UP clone() const override { - return wrapParens(new FieldValueNode(_doctype, _fieldExpression)); + return wrapParens(std::make_unique<FieldValueNode>(_doctype, _fieldExpression)); } static vespalib::string extractFieldName(const vespalib::string & fieldExpression); @@ -229,10 +250,10 @@ private: ValueNode::UP clone() const override { if (_left_expr) { - return wrapParens(new FieldExprNode(std::unique_ptr<FieldExprNode>( + return wrapParens(std::make_unique<FieldExprNode>(std::unique_ptr<FieldExprNode>( static_cast<FieldExprNode*>(_left_expr->clone().release())), _right_expr)); } else { - return wrapParens(new FieldExprNode(_right_expr)); + return wrapParens(std::make_unique<FieldExprNode>(_right_expr)); } } }; @@ -261,7 +282,7 @@ public: void visit(Visitor& visitor) const override; ValueNode::UP clone() const override { - return wrapParens(new IdValueNode(_bucketIdFactory, _id, _typestring, _widthBits, _divisionBits)); + return wrapParens(std::make_unique<IdValueNode>(_bucketIdFactory, _id, _typestring, _widthBits, _divisionBits)); } int getWidthBits() const { return _widthBits; } @@ -298,7 +319,7 @@ public: void visit(Visitor& visitor) const override; ValueNode::UP clone() const override { - return wrapParens(new FunctionValueNode(_funcname, _source->clone())); + return wrapParens(std::make_unique<FunctionValueNode>(_funcname, _source->clone())); } const ValueNode& getChild() const { return *_source; } @@ -339,9 +360,7 @@ public: void visit(Visitor& visitor) const override; ValueNode::UP clone() const override { - return wrapParens(new ArithmeticValueNode(_left->clone(), - getOperatorName(), - _right->clone())); + return wrapParens(std::make_unique<ArithmeticValueNode>(_left->clone(), getOperatorName(), _right->clone())); } const ValueNode& getLeft() const { return *_left; } diff --git a/document/src/vespa/document/select/visitor.h b/document/src/vespa/document/select/visitor.h index a9b134f80fc..4664b831238 100644 --- a/document/src/vespa/document/select/visitor.h +++ b/document/src/vespa/document/select/visitor.h @@ -27,6 +27,7 @@ class InvalidConstant; class FieldValueNode; class FloatValueNode; class IntegerValueNode; +class BoolValueNode; class CurrentTimeValueNode; class StringValueNode; class NullValueNode; @@ -35,7 +36,7 @@ class VariableValueNode; class Visitor { public: - virtual ~Visitor() {} + virtual ~Visitor() = default; virtual void visitAndBranch(const And &) = 0; virtual void visitComparison(const Compare &) = 0; @@ -51,6 +52,7 @@ public: virtual void visitFloatValueNode(const FloatValueNode &) = 0; virtual void visitVariableValueNode(const VariableValueNode &) = 0; virtual void visitIntegerValueNode(const IntegerValueNode &) = 0; + virtual void visitBoolValueNode(const BoolValueNode&) = 0; virtual void visitCurrentTimeValueNode(const CurrentTimeValueNode &) = 0; virtual void visitStringValueNode(const StringValueNode &) = 0; virtual void visitNullValueNode(const NullValueNode &) = 0; |