From 33a833d18764816a4d3c22e72e5b8f23faba6eae Mon Sep 17 00:00:00 2001 From: Marco Bubke Date: Thu, 14 May 2020 12:50:34 +0200 Subject: [PATCH] Sqlite: Add null value So we can distingish between a null value and zero or an empty string. Change-Id: I9122fdafdf85cf04dcf8bca7bf294be9b28ee251 Reviewed-by: Tim Jenssen --- src/libs/sqlite/sqlitebasestatement.cpp | 12 ++- src/libs/sqlite/sqlitebasestatement.h | 1 + src/libs/sqlite/sqlitevalue.h | 32 +++++++- tests/unit/unittest/sqlitestatement-test.cpp | 78 +++++++++++++++----- tests/unit/unittest/sqlitevalue-test.cpp | 71 +++++++++++++++++- 5 files changed, 167 insertions(+), 27 deletions(-) diff --git a/src/libs/sqlite/sqlitebasestatement.cpp b/src/libs/sqlite/sqlitebasestatement.cpp index 85a6db2ddcd..3be7f75bbec 100644 --- a/src/libs/sqlite/sqlitebasestatement.cpp +++ b/src/libs/sqlite/sqlitebasestatement.cpp @@ -158,6 +158,13 @@ Utils::SmallStringVector BaseStatement::columnNames() const return columnNames; } +void BaseStatement::bind(int index, NullValue) +{ + int resultCode = sqlite3_bind_null(m_compiledStatement.get(), index); + if (resultCode != SQLITE_OK) + checkForBindingError(resultCode); +} + void BaseStatement::bind(int index, int value) { int resultCode = sqlite3_bind_int(m_compiledStatement.get(), index, value); @@ -529,6 +536,8 @@ ValueView BaseStatement::fetchValueView(int column) const { int dataType = sqlite3_column_type(m_compiledStatement.get(), column); switch (dataType) { + case SQLITE_NULL: + return ValueView::create(NullValue{}); case SQLITE_INTEGER: return ValueView::create(fetchLongLongValue(column)); case SQLITE_FLOAT: @@ -536,11 +545,10 @@ ValueView BaseStatement::fetchValueView(int column) const case SQLITE3_TEXT: return ValueView::create(fetchValue(column)); case SQLITE_BLOB: - case SQLITE_NULL: break; } - return ValueView::create(0LL); + return ValueView::create(NullValue{}); } } // namespace Sqlite diff --git a/src/libs/sqlite/sqlitebasestatement.h b/src/libs/sqlite/sqlitebasestatement.h index aa40e0c6ea2..c63cfc37d5f 100644 --- a/src/libs/sqlite/sqlitebasestatement.h +++ b/src/libs/sqlite/sqlitebasestatement.h @@ -74,6 +74,7 @@ public: int columnCount() const; Utils::SmallStringVector columnNames() const; + void bind(int index, NullValue); void bind(int index, int fetchValue); void bind(int index, long long fetchValue); void bind(int index, double fetchValue); diff --git a/src/libs/sqlite/sqlitevalue.h b/src/libs/sqlite/sqlitevalue.h index ca576421777..e3b6485b5f1 100644 --- a/src/libs/sqlite/sqlitevalue.h +++ b/src/libs/sqlite/sqlitevalue.h @@ -34,13 +34,22 @@ namespace Sqlite { -enum class ValueType : unsigned char { Integer, Float, String }; +enum class ValueType : unsigned char { Null, Integer, Float, String }; + +class NullValue +{ + friend bool operator==(NullValue, NullValue) { return false; } +}; template class ValueBase { public: - using VariantType = Utils::variant; + using VariantType = Utils::variant; + + ValueBase() = default; + + explicit ValueBase(NullValue) {} explicit ValueBase(VariantType &&value) : value(value) @@ -70,6 +79,8 @@ public: {} + bool isNull() const { return value.index() == 0; } + long long toInteger() const { return Utils::get(value); } double toFloat() const { return Utils::get(value); } @@ -93,6 +104,8 @@ public: return {}; } + friend bool operator==(const ValueBase &first, nullptr_t) { return first.isNull(); } + friend bool operator==(const ValueBase &first, long long second) { auto maybeInteger = Utils::get_if(&first.value); @@ -171,10 +184,12 @@ public: { switch (value.index()) { case 0: - return ValueType::Integer; + return ValueType::Null; case 1: - return ValueType::Float; + return ValueType::Integer; case 2: + return ValueType::Float; + case 3: return ValueType::String; } @@ -206,6 +221,10 @@ class Value : public ValueBase public: using Base::Base; + Value() = default; + + explicit Value(NullValue) {} + explicit Value(ValueView view) : ValueBase(convert(view)) {} @@ -265,6 +284,9 @@ public: private: static Base::VariantType convert(const QVariant &value) { + if (value.isNull()) + return VariantType{NullValue{}}; + switch (value.type()) { case QVariant::Int: return VariantType{static_cast(value.toInt())}; @@ -284,6 +306,8 @@ private: static Base::VariantType convert(ValueView view) { switch (view.type()) { + case ValueType::Null: + return VariantType(NullValue{}); case ValueType::Integer: return VariantType{view.toInteger()}; case ValueType::Float: diff --git a/tests/unit/unittest/sqlitestatement-test.cpp b/tests/unit/unittest/sqlitestatement-test.cpp index c7da3971e4f..6775d9e1685 100644 --- a/tests/unit/unittest/sqlitestatement-test.cpp +++ b/tests/unit/unittest/sqlitestatement-test.cpp @@ -66,11 +66,33 @@ MATCHER_P3(HasValues, value1, value2, rowid, && statement.fetchSmallStringViewValue(1) == value2; } +MATCHER_P(HasNullValues, rowid, std::string(negation ? "isn't null" : "is null")) +{ + Database &database = arg.database(); + + SqliteTestStatement statement("SELECT name, number FROM test WHERE rowid=?", database); + statement.bind(1, rowid); + + statement.next(); + + return statement.fetchValueView(0).isNull() && statement.fetchValueView(1).isNull(); +} + class SqliteStatement : public ::testing::Test { protected: - void SetUp() override; - void TearDown() override; + void SetUp() override + { + database.execute("CREATE TABLE test(name TEXT UNIQUE, number NUMERIC, value NUMERIC)"); + database.execute("INSERT INTO test VALUES ('bar', 'blah', 1)"); + database.execute("INSERT INTO test VALUES ('foo', 23.3, 2)"); + database.execute("INSERT INTO test VALUES ('poo', 40, 3)"); + } + void TearDown() override + { + if (database.isOpen()) + database.close(); + } protected: Database database{":memory:", Sqlite::JournalMode::Memory}; @@ -210,13 +232,24 @@ TEST_F(SqliteStatement, ColumnNames) ASSERT_THAT(columnNames, ElementsAre("name", "number")); } +TEST_F(SqliteStatement, BindNull) +{ + database.execute("INSERT INTO test VALUES (NULL, 323, 344)"); + SqliteTestStatement statement("SELECT name, number FROM test WHERE name IS ?", database); + + statement.bind(1, Sqlite::NullValue{}); + statement.next(); + + ASSERT_TRUE(statement.fetchValueView(0).isNull()); + ASSERT_THAT(statement.fetchValue(1), 323); +} + TEST_F(SqliteStatement, BindString) { SqliteTestStatement statement("SELECT name, number FROM test WHERE name=?", database); statement.bind(1, "foo"); - statement.next(); ASSERT_THAT(statement.fetchSmallStringViewValue(0), "foo"); @@ -314,6 +347,16 @@ TEST_F(SqliteStatement, BindValues) ASSERT_THAT(statement, HasValues("see", "7.23", 1)); } +TEST_F(SqliteStatement, BindNullValues) +{ + SqliteTestStatement statement("UPDATE test SET name=?, number=? WHERE rowid=?", database); + + statement.bindValues(Sqlite::NullValue{}, Sqlite::Value{}, 1); + statement.execute(); + + ASSERT_THAT(statement, HasNullValues(1)); +} + TEST_F(SqliteStatement, WriteValues) { WriteStatement statement("UPDATE test SET name=?, number=? WHERE rowid=?", database); @@ -323,6 +366,15 @@ TEST_F(SqliteStatement, WriteValues) ASSERT_THAT(statement, HasValues("see", "7.23", 1)); } +TEST_F(SqliteStatement, WriteNullValues) +{ + WriteStatement statement("UPDATE test SET name=?, number=? WHERE rowid=?", database); + + statement.write(Sqlite::NullValue{}, Sqlite::Value{}, 1); + + ASSERT_THAT(statement, HasNullValues(1)); +} + TEST_F(SqliteStatement, WriteSqliteValues) { WriteStatement statement("UPDATE test SET name=?, number=? WHERE rowid=?", database); @@ -407,10 +459,11 @@ public: TEST_F(SqliteStatement, GetSingleSqliteValuesWithoutArguments) { ReadStatement statement("SELECT number FROM test", database); + database.execute("INSERT INTO test VALUES (NULL, NULL, NULL)"); std::vector values = statement.values(3); - ASSERT_THAT(values, ElementsAre(Eq("blah"), Eq(23.3), Eq(40))); + ASSERT_THAT(values, ElementsAre(Eq("blah"), Eq(23.3), Eq(40), IsNull())); } TEST_F(SqliteStatement, GetStructValuesWithoutArguments) @@ -710,19 +763,4 @@ TEST_F(SqliteStatement, ResetIfExecuteThrowsException) ASSERT_ANY_THROW(mockStatement.execute()); } - -void SqliteStatement::SetUp() -{ - database.execute("CREATE TABLE test(name TEXT UNIQUE, number NUMERIC, value NUMERIC)"); - database.execute("INSERT INTO test VALUES ('bar', 'blah', 1)"); - database.execute("INSERT INTO test VALUES ('foo', 23.3, 2)"); - database.execute("INSERT INTO test VALUES ('poo', 40, 3)"); -} - -void SqliteStatement::TearDown() -{ - if (database.isOpen()) - database.close(); -} - -} +} // namespace diff --git a/tests/unit/unittest/sqlitevalue-test.cpp b/tests/unit/unittest/sqlitevalue-test.cpp index e9cc5c9e408..ab4d628ec8f 100644 --- a/tests/unit/unittest/sqlitevalue-test.cpp +++ b/tests/unit/unittest/sqlitevalue-test.cpp @@ -29,6 +29,20 @@ namespace { +TEST(SqliteValue, ConstructDefault) +{ + Sqlite::Value value{}; + + ASSERT_TRUE(value.isNull()); +} + +TEST(SqliteValue, ConstructNullValue) +{ + Sqlite::Value value{Sqlite::NullValue{}}; + + ASSERT_TRUE(value.isNull()); +} + TEST(SqliteValue, ConstructLongLong) { Sqlite::Value value{1LL}; @@ -36,7 +50,7 @@ TEST(SqliteValue, ConstructLongLong) ASSERT_THAT(value.toInteger(), Eq(1LL)); } -TEST(SqliteValue, Construct) +TEST(SqliteValue, ConstructInteger) { Sqlite::Value value{1}; @@ -71,6 +85,15 @@ TEST(SqliteValue, ConstructStringFromQString) ASSERT_THAT(value.toStringView(), Eq("foo")); } +TEST(SqliteValue, ConstructNullFromNullQVariant) +{ + QVariant variant{}; + + Sqlite::Value value{variant}; + + ASSERT_TRUE(value.isNull()); +} + TEST(SqliteValue, ConstructStringFromIntQVariant) { QVariant variant{1}; @@ -116,6 +139,15 @@ TEST(SqliteValue, ConstructStringFromStringQVariant) ASSERT_THAT(value.toStringView(), Eq("foo")); } +TEST(SqliteValue, ConvertToNullQVariant) +{ + Sqlite::Value value{}; + + auto variant = QVariant{value}; + + ASSERT_TRUE(variant.isNull()); +} + TEST(SqliteValue, ConvertToStringQVariant) { Sqlite::Value value{"foo"}; @@ -192,6 +224,13 @@ TEST(SqliteValue, IntegerAndFloatAreNotEquals) ASSERT_FALSE(isEqual); } +TEST(SqliteValue, NullValuesNeverEqual) +{ + bool isEqual = Sqlite::Value{} == Sqlite::Value{}; + + ASSERT_FALSE(isEqual); +} + TEST(SqliteValue, IntegerValuesAreEquals) { bool isEqual = Sqlite::Value{1} == Sqlite::Value{1}; @@ -248,6 +287,13 @@ TEST(SqliteValue, IntegersAreUnequalInverse) ASSERT_TRUE(isUnequal); } +TEST(SqliteValue, NullType) +{ + auto type = Sqlite::Value{}.type(); + + ASSERT_THAT(type, Sqlite::ValueType::Null); +} + TEST(SqliteValue, IntegerType) { auto type = Sqlite::Value{1}.type(); @@ -269,6 +315,20 @@ TEST(SqliteValue, StringType) ASSERT_THAT(type, Sqlite::ValueType::String); } +TEST(SqliteValue, NullValueAndValueViewAreNotEqual) +{ + bool isEqual = Sqlite::ValueView::create(Sqlite::NullValue{}) == Sqlite::Value{}; + + ASSERT_FALSE(isEqual); +} + +TEST(SqliteValue, NullValueViewAndValueAreNotEqual) +{ + bool isEqual = Sqlite::Value{} == Sqlite::ValueView::create(Sqlite::NullValue{}); + + ASSERT_FALSE(isEqual); +} + TEST(SqliteValue, StringValueAndValueViewEquals) { bool isEqual = Sqlite::ValueView::create("foo") == Sqlite::Value{"foo"}; @@ -318,6 +378,15 @@ TEST(SqliteValue, StringValueAndIntergerValueViewAreNotEqual) ASSERT_FALSE(isEqual); } +TEST(SqliteValue, ConvertNullValueViewIntoValue) +{ + auto view = Sqlite::ValueView::create(Sqlite::NullValue{}); + + Sqlite::Value value{view}; + + ASSERT_TRUE(value.isNull()); +} + TEST(SqliteValue, ConvertStringValueViewIntoValue) { auto view = Sqlite::ValueView::create("foo");