From 7132b3a1c18c05c983f237bcf21acbefed563175 Mon Sep 17 00:00:00 2001 From: Carlos O'Ryan Date: Tue, 19 Nov 2019 11:01:17 -0500 Subject: [PATCH 1/2] bug: fix year formatting in spanner::Date The year is required to be 0-padded to 4 digits, the month and date padding seems to be optional. Introduce a new integration test to verify data types can be written and read back. --- .../spanner/integration_tests/CMakeLists.txt | 1 + .../data_types_integration_test.cc | 124 ++++++++++++++++++ .../spanner_client_integration_tests.bzl | 1 + google/cloud/spanner/internal/date.cc | 2 +- google/cloud/spanner/internal/date_test.cc | 3 + .../spanner/testing/database_environment.cc | 10 +- 6 files changed, 138 insertions(+), 3 deletions(-) create mode 100644 google/cloud/spanner/integration_tests/data_types_integration_test.cc diff --git a/google/cloud/spanner/integration_tests/CMakeLists.txt b/google/cloud/spanner/integration_tests/CMakeLists.txt index 872a05b5..4ecda99d 100644 --- a/google/cloud/spanner/integration_tests/CMakeLists.txt +++ b/google/cloud/spanner/integration_tests/CMakeLists.txt @@ -30,6 +30,7 @@ function (spanner_client_define_integration_tests) set(spanner_client_integration_tests # cmake-format: sortable client_integration_test.cc + data_types_integration_test.cc database_admin_integration_test.cc client_stress_test.cc instance_admin_integration_test.cc diff --git a/google/cloud/spanner/integration_tests/data_types_integration_test.cc b/google/cloud/spanner/integration_tests/data_types_integration_test.cc new file mode 100644 index 00000000..5cc929a4 --- /dev/null +++ b/google/cloud/spanner/integration_tests/data_types_integration_test.cc @@ -0,0 +1,124 @@ +// Copyright 2019 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#include "google/cloud/spanner/client.h" +#include "google/cloud/spanner/database.h" +#include "google/cloud/spanner/database_admin_client.h" +#include "google/cloud/spanner/mutations.h" +#include "google/cloud/spanner/testing/database_environment.h" +#include "google/cloud/internal/getenv.h" +#include "google/cloud/testing_util/assert_ok.h" +#include "google/cloud/testing_util/init_google_mock.h" +#include + +namespace google { +namespace cloud { +namespace spanner { +inline namespace SPANNER_CLIENT_NS { +namespace { + +class DataTypeIntegrationTest : public ::testing::Test { + public: + static void SetUpTestSuite() { + std::cout << "Creating additional DataTypes table " << std::flush; + client_ = google::cloud::internal::make_unique( + MakeConnection(spanner_testing::DatabaseEnvironment::GetDatabase())); + spanner::DatabaseAdminClient admin_client; + auto database_future = admin_client.UpdateDatabase( + spanner_testing::DatabaseEnvironment::GetDatabase(), + {R"sql(CREATE TABLE DataTypes ( + Id INT64 NOT NULL, + BoolValue BOOL, + DateValue DATE, + ) PRIMARY KEY (Id))sql"}); + int i = 0; + int const timeout = 120; + while (++i < timeout) { + auto status = database_future.wait_for(std::chrono::seconds(1)); + if (status == std::future_status::ready) break; + std::cout << '.' << std::flush; + } + if (i >= timeout) { + std::cout << "TIMEOUT\n"; + FAIL(); + } + auto database = database_future.get(); + ASSERT_STATUS_OK(database); + std::cout << "DONE\n"; + } + + void SetUp() override { + auto commit_result = client_->Commit([](Transaction const&) { + return Mutations{ + MakeDeleteMutation("Singers", KeySet::All()), + MakeDeleteMutation("DataTypes", KeySet::All()), + }; + }); + EXPECT_STATUS_OK(commit_result); + } + + static void TearDownTestSuite() { client_ = nullptr; } + + protected: + static std::unique_ptr client_; +}; + +std::unique_ptr DataTypeIntegrationTest::client_; + +TEST_F(DataTypeIntegrationTest, ReadWriteDate) { + auto& client = *client_; + using RowType = std::tuple; + RowType read_back; + auto commit_result = client_->Commit( + [&client, &read_back](Transaction const& txn) -> StatusOr { + auto result = client.ExecuteDml( + txn, + SqlStatement( + "INSERT INTO DataTypes (Id, DateValue, StringValue) " + "VALUES(@id, @date, @event)", + {{"id", Value("ReadWriteDate-1")}, + {"date", Value(Date(161, 3, 8))}, + {"event", Value("Marcus Aurelius ascends to the throne")}})); + if (!result) return std::move(result).status(); + + auto reader = client.ExecuteQuery( + txn, SqlStatement("SELECT Id, DateValue, StringValue FROM DataTypes" + " WHERE Id = @id", + {{"id", Value("ReadWriteDate-1")}})); + + auto row = GetCurrentRow(StreamOf(reader)); + if (!row) return std::move(row).status(); + read_back = *std::move(row); + return Mutations{}; + }); + + ASSERT_TRUE(commit_result.ok()); + EXPECT_EQ("ReadWriteDate-1", std::get<0>(read_back)); + EXPECT_EQ(Date(161, 3, 8), std::get<1>(read_back)); + EXPECT_EQ("Marcus Aurelius ascends to the throne", std::get<2>(read_back)); +} + +} // namespace +} // namespace SPANNER_CLIENT_NS +} // namespace spanner +} // namespace cloud +} // namespace google + +int main(int argc, char* argv[]) { + ::google::cloud::testing_util::InitGoogleMock(argc, argv); + (void)::testing::AddGlobalTestEnvironment( + new google::cloud::spanner_testing::DatabaseEnvironment()); + + return RUN_ALL_TESTS(); +} diff --git a/google/cloud/spanner/integration_tests/spanner_client_integration_tests.bzl b/google/cloud/spanner/integration_tests/spanner_client_integration_tests.bzl index 6b55c0ce..5751b054 100644 --- a/google/cloud/spanner/integration_tests/spanner_client_integration_tests.bzl +++ b/google/cloud/spanner/integration_tests/spanner_client_integration_tests.bzl @@ -18,6 +18,7 @@ spanner_client_integration_tests = [ "client_integration_test.cc", + "data_types_integration_test.cc", "database_admin_integration_test.cc", "client_stress_test.cc", "instance_admin_integration_test.cc", diff --git a/google/cloud/spanner/internal/date.cc b/google/cloud/spanner/internal/date.cc index 291de6f6..4365ae42 100644 --- a/google/cloud/spanner/internal/date.cc +++ b/google/cloud/spanner/internal/date.cc @@ -25,7 +25,7 @@ namespace internal { std::string DateToString(Date d) { std::array buf; - std::snprintf(buf.data(), buf.size(), "%" PRId64 "-%02d-%02d", d.year(), + std::snprintf(buf.data(), buf.size(), "%04" PRId64 "-%02d-%02d", d.year(), d.month(), d.day()); return std::string(buf.data()); } diff --git a/google/cloud/spanner/internal/date_test.cc b/google/cloud/spanner/internal/date_test.cc index 74d59744..0f5eb1c6 100644 --- a/google/cloud/spanner/internal/date_test.cc +++ b/google/cloud/spanner/internal/date_test.cc @@ -24,6 +24,9 @@ namespace { TEST(Date, DateToString) { EXPECT_EQ("2019-06-21", DateToString(Date(2019, 6, 21))); + EXPECT_EQ("1066-10-14", DateToString(Date(1066, 10, 14))); + EXPECT_EQ("0865-03-21", DateToString(Date(865, 3, 21))); + EXPECT_EQ("0014-08-19", DateToString(Date(14, 8, 19))); } TEST(Date, DateFromString) { diff --git a/google/cloud/spanner/testing/database_environment.cc b/google/cloud/spanner/testing/database_environment.cc index defc2746..56208a3c 100644 --- a/google/cloud/spanner/testing/database_environment.cc +++ b/google/cloud/spanner/testing/database_environment.cc @@ -44,11 +44,17 @@ void DatabaseEnvironment::SetUp() { std::cout << "Creating database and table " << std::flush; spanner::DatabaseAdminClient admin_client; auto database_future = - admin_client.CreateDatabase(*db_, {R"""(CREATE TABLE Singers ( + admin_client.CreateDatabase(*db_, {R"sql(CREATE TABLE Singers ( SingerId INT64 NOT NULL, FirstName STRING(1024), LastName STRING(1024) - ) PRIMARY KEY (SingerId))"""}); + ) PRIMARY KEY (SingerId))sql", + R"sql(CREATE TABLE DataTypes ( + Id STRING(256) NOT NULL, + BoolValue BOOL, + DateValue DATE, + StringValue STRING(1024), + ) PRIMARY KEY (Id))sql"}); int i = 0; int const timeout = 120; while (++i < timeout) { From d72269046a4d11b49076ea5ba7288329991bc392 Mon Sep 17 00:00:00 2001 From: Carlos O'Ryan Date: Tue, 19 Nov 2019 12:08:06 -0500 Subject: [PATCH 2/2] Only create DataTypes table once. --- .../data_types_integration_test.cc | 23 ------------------- 1 file changed, 23 deletions(-) diff --git a/google/cloud/spanner/integration_tests/data_types_integration_test.cc b/google/cloud/spanner/integration_tests/data_types_integration_test.cc index 5cc929a4..7607b46b 100644 --- a/google/cloud/spanner/integration_tests/data_types_integration_test.cc +++ b/google/cloud/spanner/integration_tests/data_types_integration_test.cc @@ -31,31 +31,8 @@ namespace { class DataTypeIntegrationTest : public ::testing::Test { public: static void SetUpTestSuite() { - std::cout << "Creating additional DataTypes table " << std::flush; client_ = google::cloud::internal::make_unique( MakeConnection(spanner_testing::DatabaseEnvironment::GetDatabase())); - spanner::DatabaseAdminClient admin_client; - auto database_future = admin_client.UpdateDatabase( - spanner_testing::DatabaseEnvironment::GetDatabase(), - {R"sql(CREATE TABLE DataTypes ( - Id INT64 NOT NULL, - BoolValue BOOL, - DateValue DATE, - ) PRIMARY KEY (Id))sql"}); - int i = 0; - int const timeout = 120; - while (++i < timeout) { - auto status = database_future.wait_for(std::chrono::seconds(1)); - if (status == std::future_status::ready) break; - std::cout << '.' << std::flush; - } - if (i >= timeout) { - std::cout << "TIMEOUT\n"; - FAIL(); - } - auto database = database_future.get(); - ASSERT_STATUS_OK(database); - std::cout << "DONE\n"; } void SetUp() override {