Remove PropertyValue::Null

Reviewers: ipaljak, mferencevic

Reviewed By: ipaljak

Subscribers: pullbot

Differential Revision: https://phabricator.memgraph.io/D2332
This commit is contained in:
Teon Banek 2019-08-27 14:24:33 +02:00
parent 02fb644bdc
commit 9f4a7dcddf
18 changed files with 52 additions and 56 deletions

View File

@ -42,7 +42,7 @@ cpp<#
(property-name "std::string")
(properties "std::vector<storage::Property>")
(property-names "std::vector<std::string>")
(value "PropertyValue" :initval "PropertyValue::Null")
(value "PropertyValue")
(label "::storage::Label")
(label-name "std::string")
(check-empty :bool))
@ -65,7 +65,7 @@ in StateDeltas.")
create-edge ;; edge_id, from_vertex_id, to_vertex_id, edge_type, edge_type_name
set-property-vertex ;; vertex_id, property, property_name, property_value
set-property-edge ;; edge_id, property, property_name, property_value
;; remove property is done by setting a PropertyValue::Null
;; remove property is done by setting a PropertyValue to Null
add-label ;; vertex_id, label, label_name
remove-label ;; vertex_id, label, label_name
remove-vertex ;; vertex_id, check_empty

View File

@ -37,7 +37,7 @@ cpp<#
(property-name "std::string")
(properties "std::vector<storage::Property>")
(property-names "std::vector<std::string>")
(value "PropertyValue" :initval "PropertyValue::Null")
(value "PropertyValue")
(label "::storage::Label")
(label-name "std::string")
(check-empty :bool))
@ -61,7 +61,7 @@ in StateDeltas.")
create-edge ;; edge_id, from_vertex_id, to_vertex_id, edge_type, edge_type_name
set-property-vertex ;; vertex_id, property, property_name, property_value
set-property-edge ;; edge_id, property, property_name, property_value
;; remove property is done by setting a PropertyValue::Null
;; remove property is done by setting a PropertyValue to Null
add-label ;; vertex_id, label, label_name
remove-label ;; vertex_id, label, label_name
remove-vertex ;; vertex_id, check_empty

View File

@ -124,7 +124,7 @@ communication::bolt::Path ToBoltPath(const query::Path &path) {
PropertyValue ToPropertyValue(const Value &value) {
switch (value.type()) {
case Value::Type::Null:
return PropertyValue::Null;
return PropertyValue();
case Value::Type::Bool:
return PropertyValue(value.ValueBool());
case Value::Type::Int:

View File

@ -428,7 +428,7 @@ UniqueCursorPtr ScanAllByLabelPropertyValue::MakeCursor(
utils::MemoryResource *mem) const {
auto vertices = [this](Frame &frame, ExecutionContext &context)
-> std::optional<decltype(context.db_accessor->Vertices(
label_, property_, PropertyValue::Null, false))> {
label_, property_, PropertyValue(), false))> {
auto *db = context.db_accessor;
ExpressionEvaluator evaluator(&frame, context.symbol_table,
context.evaluation_context,

View File

@ -104,7 +104,7 @@ TypedValue::TypedValue(PropertyValue &&other, utils::MemoryResource *memory)
}
}
other = PropertyValue::Null;
other = PropertyValue();
}
TypedValue::TypedValue(const TypedValue &other)
@ -190,7 +190,7 @@ TypedValue::TypedValue(TypedValue &&other, utils::MemoryResource *memory)
TypedValue::operator PropertyValue() const {
switch (type_) {
case TypedValue::Type::Null:
return PropertyValue::Null;
return PropertyValue();
case TypedValue::Type::Bool:
return PropertyValue(bool_v);
case TypedValue::Type::Int:

View File

@ -113,14 +113,13 @@ PropertyValue &PropertyValue::operator=(PropertyValue &&other) noexcept {
}
// reset the type of other
other = PropertyValue::Null;
other.DestroyValue();
other.type_ = Type::Null;
}
return *this;
}
const PropertyValue PropertyValue::Null = PropertyValue();
void PropertyValue::DestroyValue() {
switch (type_) {
// destructor for primitive types does nothing

View File

@ -29,9 +29,6 @@ class PropertyValue {
/** A value type. Each type corresponds to exactly one C++ type */
enum class Type : unsigned { Null, String, Bool, Int, Double, List, Map };
// single static reference to Null, used whenever Null should be returned
static const PropertyValue Null;
/** Checks if the given PropertyValue::Types are comparable */
static bool AreComparableTypes(Type a, Type b) {
auto is_numeric = [](const Type t) {

View File

@ -59,7 +59,7 @@ PropertyValue PropertyValueStore::at(const Property &key) const {
auto GetValue = [&key](const auto &props) {
for (const auto &kv : props)
if (kv.first == key) return kv.second;
return PropertyValue::Null;
return PropertyValue();
};
if (key.Location() == Location::Memory) return GetValue(props_);
@ -72,7 +72,7 @@ PropertyValue PropertyValueStore::at(const Property &key) const {
DiskKey(std::to_string(version_key_), std::to_string(key.Id()));
auto serialized_prop = DiskStorage().Get(disk_key);
if (serialized_prop) return DeserializeProp(serialized_prop.value());
return PropertyValue::Null;
return PropertyValue();
}
void PropertyValueStore::set(const Property &key, const char *value) {
@ -228,7 +228,7 @@ PropertyValue PropertyValueStore::DeserializeProp(
Value dv;
if (!decoder.ReadValue(&dv)) {
DLOG(WARNING) << "Unable to read property value";
return PropertyValue::Null;
return PropertyValue();
}
return glue::ToPropertyValue(dv);
}

View File

@ -51,7 +51,7 @@ void Load(PropertyValue *value, slk::Reader *reader) {
slk::Load(&type, reader);
switch (type) {
case static_cast<uint8_t>(0):
*value = PropertyValue::Null;
*value = PropertyValue();
return;
case static_cast<uint8_t>(1): {
bool v;

View File

@ -47,9 +47,9 @@ void RecordAccessor<Vertex>::PropsErase(storage::Property key) {
auto &dba = db_accessor();
auto delta =
StateDelta::PropsSetVertex(dba.transaction_id(), gid(), key,
dba.PropertyName(key), PropertyValue::Null);
dba.PropertyName(key), PropertyValue());
auto previous_value = PropsAt(key);
update().properties_.set(key, PropertyValue::Null);
update().properties_.set(key, PropertyValue());
dba.UpdateOnRemoveProperty(key, previous_value, *this, &update());
dba.wal().Emplace(delta);
}
@ -59,8 +59,8 @@ void RecordAccessor<Edge>::PropsErase(storage::Property key) {
auto &dba = db_accessor();
auto delta =
StateDelta::PropsSetEdge(dba.transaction_id(), gid(), key,
dba.PropertyName(key), PropertyValue::Null);
update().properties_.set(key, PropertyValue::Null);
dba.PropertyName(key), PropertyValue());
update().properties_.set(key, PropertyValue());
db_accessor().wal().Emplace(delta);
}

View File

@ -47,9 +47,9 @@ void RecordAccessor<Vertex>::PropsErase(storage::Property key) {
auto &dba = db_accessor();
auto delta =
StateDelta::PropsSetVertex(dba.transaction_id(), gid(), key,
dba.PropertyName(key), PropertyValue::Null);
dba.PropertyName(key), PropertyValue());
auto previous_value = PropsAt(key);
update().properties_.set(key, PropertyValue::Null);
update().properties_.set(key, PropertyValue());
dba.UpdateOnRemoveProperty(key, previous_value, *this, &update());
dba.raft()->Emplace(delta);
}
@ -59,8 +59,8 @@ void RecordAccessor<Edge>::PropsErase(storage::Property key) {
auto &dba = db_accessor();
auto delta =
StateDelta::PropsSetEdge(dba.transaction_id(), gid(), key,
dba.PropertyName(key), PropertyValue::Null);
update().properties_.set(key, PropertyValue::Null);
dba.PropertyName(key), PropertyValue());
update().properties_.set(key, PropertyValue());
dba.raft()->Emplace(delta);
}

View File

@ -358,7 +358,7 @@ void BfsTest(Database *db, int lower_bound, int upper_bound,
input_op =
YieldEntities(dba_ptr.get(), vertices, edges, blocked_sym, nullptr);
filter_expr = IF(AND(NEQ(inner_node, blocked), NEQ(inner_edge, blocked)),
LITERAL(true), LITERAL(PropertyValue::Null));
LITERAL(true), LITERAL(PropertyValue()));
break;
case FilterLambdaType::USE_CTX:
// We only block vertex #5 and run BFS.

View File

@ -415,11 +415,11 @@ TEST_F(GraphDbAccessorIndexRange, RangeIterationCurrentState) {
TEST_F(GraphDbAccessorIndexRange, RangeInterationIncompatibleTypes) {
using std::nullopt;
// using PropertyValue::Null as a bound fails with an assertion
// using PropertyValue set to Null as a bound fails with an assertion
::testing::FLAGS_gtest_death_test_style = "threadsafe";
EXPECT_DEATH(Vertices(nullopt, Inclusive(PropertyValue::Null)),
EXPECT_DEATH(Vertices(nullopt, Inclusive(PropertyValue())),
"not a valid index bound");
EXPECT_DEATH(Vertices(Inclusive(PropertyValue::Null), nullopt),
EXPECT_DEATH(Vertices(Inclusive(PropertyValue()), nullopt),
"not a valid index bound");
std::vector<PropertyValue> incompatible_with_int{
"string", true, std::vector<PropertyValue>{1}};

View File

@ -94,10 +94,10 @@ TEST_F(PropertyValueStoreTest, AtNull) {
}
TEST_F(PropertyValueStoreTest, SetNull) {
Set(11, Location::Memory, PropertyValue::Null);
Set(11, Location::Memory, PropertyValue());
EXPECT_EQ(0, props_.size());
Set(100, Location::Disk, PropertyValue::Null);
Set(100, Location::Disk, PropertyValue());
EXPECT_EQ(0, props_.size());
}
@ -232,7 +232,7 @@ TEST_F(PropertyValueStoreTest, Size) {
TEST_F(PropertyValueStoreTest, InsertRetrieveListMemory) {
Set(0, Location::Memory, std::vector<PropertyValue>{1, true, 2.5, "something",
PropertyValue::Null});
PropertyValue()});
auto p = At(0, Location::Memory);
EXPECT_EQ(p.type(), PropertyValue::Type::List);
@ -252,7 +252,7 @@ TEST_F(PropertyValueStoreTest, InsertRetrieveListMemory) {
TEST_F(PropertyValueStoreTest, InsertRetrieveListDisk) {
Set(0, Location::Disk,
std::vector<PropertyValue>{1, true, 2.5, "something",
PropertyValue::Null});
PropertyValue()});
auto p = At(0, Location::Disk);
EXPECT_EQ(p.type(), PropertyValue::Type::List);

View File

@ -121,20 +121,20 @@ TEST_F(ExpressionEvaluatorTest, AndOperatorNull) {
{
// Null doesn't short circuit
auto *op = storage.Create<AndOperator>(
storage.Create<PrimitiveLiteral>(PropertyValue::Null),
storage.Create<PrimitiveLiteral>(PropertyValue()),
storage.Create<PrimitiveLiteral>(5));
EXPECT_THROW(Eval(op), QueryRuntimeException);
}
{
auto *op = storage.Create<AndOperator>(
storage.Create<PrimitiveLiteral>(PropertyValue::Null),
storage.Create<PrimitiveLiteral>(PropertyValue()),
storage.Create<PrimitiveLiteral>(true));
auto value = Eval(op);
EXPECT_TRUE(value.IsNull());
}
{
auto *op = storage.Create<AndOperator>(
storage.Create<PrimitiveLiteral>(PropertyValue::Null),
storage.Create<PrimitiveLiteral>(PropertyValue()),
storage.Create<PrimitiveLiteral>(false));
auto value = Eval(op);
ASSERT_TRUE(value.IsBool());
@ -295,7 +295,7 @@ TEST_F(ExpressionEvaluatorTest, InListOperator) {
}
{
auto *list_literal = storage.Create<ListLiteral>(std::vector<Expression *>{
storage.Create<PrimitiveLiteral>(PropertyValue::Null),
storage.Create<PrimitiveLiteral>(PropertyValue()),
storage.Create<PrimitiveLiteral>(2),
storage.Create<PrimitiveLiteral>("a")});
// Element doesn't exist in list with null element.
@ -308,21 +308,21 @@ TEST_F(ExpressionEvaluatorTest, InListOperator) {
// Null list.
auto *op = storage.Create<InListOperator>(
storage.Create<PrimitiveLiteral>("x"),
storage.Create<PrimitiveLiteral>(PropertyValue::Null));
storage.Create<PrimitiveLiteral>(PropertyValue()));
auto value = Eval(op);
EXPECT_TRUE(value.IsNull());
}
{
// Null literal.
auto *op = storage.Create<InListOperator>(
storage.Create<PrimitiveLiteral>(PropertyValue::Null), list_literal);
storage.Create<PrimitiveLiteral>(PropertyValue()), list_literal);
auto value = Eval(op);
EXPECT_TRUE(value.IsNull());
}
{
// Null literal, empty list.
auto *op = storage.Create<InListOperator>(
storage.Create<PrimitiveLiteral>(PropertyValue::Null),
storage.Create<PrimitiveLiteral>(PropertyValue()),
storage.Create<ListLiteral>(std::vector<Expression *>()));
auto value = Eval(op);
EXPECT_FALSE(value.ValueBool());
@ -365,7 +365,7 @@ TEST_F(ExpressionEvaluatorTest, ListIndexing) {
{
// Indexing with one operator being null.
auto *op = storage.Create<SubscriptOperator>(
storage.Create<PrimitiveLiteral>(PropertyValue::Null),
storage.Create<PrimitiveLiteral>(PropertyValue()),
storage.Create<PrimitiveLiteral>(-2));
auto value = Eval(op);
EXPECT_TRUE(value.IsNull());
@ -407,7 +407,7 @@ TEST_F(ExpressionEvaluatorTest, MapIndexing) {
{
// Indexing with Null.
auto *op = storage.Create<SubscriptOperator>(
map_literal, storage.Create<PrimitiveLiteral>(PropertyValue::Null));
map_literal, storage.Create<PrimitiveLiteral>(PropertyValue()));
auto value = Eval(op);
EXPECT_TRUE(value.IsNull());
}
@ -460,12 +460,12 @@ TEST_F(ExpressionEvaluatorTest, VertexAndEdgeIndexing) {
{
// Indexing with Null.
auto *op1 = storage.Create<SubscriptOperator>(
vertex_id, storage.Create<PrimitiveLiteral>(PropertyValue::Null));
vertex_id, storage.Create<PrimitiveLiteral>(PropertyValue()));
auto value1 = Eval(op1);
EXPECT_TRUE(value1.IsNull());
auto *op2 = storage.Create<SubscriptOperator>(
edge_id, storage.Create<PrimitiveLiteral>(PropertyValue::Null));
edge_id, storage.Create<PrimitiveLiteral>(PropertyValue()));
auto value2 = Eval(op2);
EXPECT_TRUE(value2.IsNull());
}
@ -533,7 +533,7 @@ TEST_F(ExpressionEvaluatorTest, ListSlicingOperator) {
{
// Bound of illegal type and null value bound.
auto *op = storage.Create<ListSlicingOperator>(
list_literal, storage.Create<PrimitiveLiteral>(PropertyValue::Null),
list_literal, storage.Create<PrimitiveLiteral>(PropertyValue()),
storage.Create<PrimitiveLiteral>("mirko"));
EXPECT_THROW(Eval(op), QueryRuntimeException);
}
@ -547,7 +547,7 @@ TEST_F(ExpressionEvaluatorTest, ListSlicingOperator) {
{
// Null value list with undefined upper bound.
auto *op = storage.Create<ListSlicingOperator>(
storage.Create<PrimitiveLiteral>(PropertyValue::Null),
storage.Create<PrimitiveLiteral>(PropertyValue()),
storage.Create<PrimitiveLiteral>(-2), nullptr);
auto value = Eval(op);
EXPECT_TRUE(value.IsNull());
@ -557,7 +557,7 @@ TEST_F(ExpressionEvaluatorTest, ListSlicingOperator) {
// Null value index.
auto *op = storage.Create<ListSlicingOperator>(
list_literal, storage.Create<PrimitiveLiteral>(-2),
storage.Create<PrimitiveLiteral>(PropertyValue::Null));
storage.Create<PrimitiveLiteral>(PropertyValue()));
auto value = Eval(op);
EXPECT_TRUE(value.IsNull());
;
@ -622,7 +622,7 @@ TEST_F(ExpressionEvaluatorTest, IsNullOperator) {
auto val1 = Eval(op);
ASSERT_EQ(val1.ValueBool(), false);
op = storage.Create<IsNullOperator>(
storage.Create<PrimitiveLiteral>(PropertyValue::Null));
storage.Create<PrimitiveLiteral>(PropertyValue()));
auto val2 = Eval(op);
ASSERT_EQ(val2.ValueBool(), true);
}
@ -712,7 +712,7 @@ TEST_F(ExpressionEvaluatorTest, All) {
TEST_F(ExpressionEvaluatorTest, FunctionAllNullList) {
AstStorage storage;
auto *all = ALL("x", LITERAL(PropertyValue::Null), WHERE(LITERAL(true)));
auto *all = ALL("x", LITERAL(PropertyValue()), WHERE(LITERAL(true)));
const auto x_sym = symbol_table.CreateSymbol("x", true);
all->identifier_->MapTo(x_sym);
auto value = Eval(all);
@ -756,7 +756,7 @@ TEST_F(ExpressionEvaluatorTest, FunctionSingle2) {
TEST_F(ExpressionEvaluatorTest, FunctionSingleNullList) {
AstStorage storage;
auto *single =
SINGLE("x", LITERAL(PropertyValue::Null), WHERE(LITERAL(true)));
SINGLE("x", LITERAL(PropertyValue()), WHERE(LITERAL(true)));
const auto x_sym = symbol_table.CreateSymbol("x", true);
single->identifier_->MapTo(x_sym);
auto value = Eval(single);
@ -784,7 +784,7 @@ TEST_F(ExpressionEvaluatorTest, FunctionExtract) {
AstStorage storage;
auto *ident_x = IDENT("x");
auto *extract =
EXTRACT("x", LIST(LITERAL(1), LITERAL(2), LITERAL(PropertyValue::Null)),
EXTRACT("x", LIST(LITERAL(1), LITERAL(2), LITERAL(PropertyValue())),
ADD(ident_x, LITERAL(1)));
const auto x_sym = symbol_table.CreateSymbol("x", true);
extract->identifier_->MapTo(x_sym);
@ -802,7 +802,7 @@ TEST_F(ExpressionEvaluatorTest, FunctionExtractNull) {
AstStorage storage;
auto *ident_x = IDENT("x");
auto *extract =
EXTRACT("x", LITERAL(PropertyValue::Null), ADD(ident_x, LITERAL(1)));
EXTRACT("x", LITERAL(PropertyValue()), ADD(ident_x, LITERAL(1)));
const auto x_sym = symbol_table.CreateSymbol("x", true);
extract->identifier_->MapTo(x_sym);
ident_x->MapTo(x_sym);

View File

@ -301,7 +301,7 @@ TEST(QueryPlan, AggregateGroupByValues) {
group_by_vals.emplace_back(std::vector<PropertyValue>{1});
group_by_vals.emplace_back(std::vector<PropertyValue>{1, 2});
group_by_vals.emplace_back(std::vector<PropertyValue>{2, 1});
group_by_vals.emplace_back(PropertyValue::Null);
group_by_vals.emplace_back(PropertyValue());
// should NOT result in another group because 7.0 == 7
group_by_vals.emplace_back(7.0);
// should NOT result in another group

View File

@ -115,7 +115,7 @@ TEST(QueryPlan, OrderBy) {
// contains a series of tests
// each test defines the ordering a vector of values in the desired order
auto Null = PropertyValue::Null;
auto Null = PropertyValue();
std::vector<std::pair<Ordering, std::vector<PropertyValue>>> orderable{
{Ordering::ASC, {0, 0, 0.5, 1, 2, 12.6, 42, Null, Null}},
{Ordering::ASC, {false, false, true, true, Null, Null}},

View File

@ -42,7 +42,7 @@ TEST_F(ExpressionPrettyPrinterTest, Literals) {
// [1 null "hello"]
EXPECT_EQ(ToString(LITERAL(
(std::vector<PropertyValue>{1, PropertyValue::Null, "hello"}))),
(std::vector<PropertyValue>{1, PropertyValue(), "hello"}))),
"[1, null, \"hello\"]");
// {hello: 1, there: 2}