Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add PG.OID type #13127

Merged
merged 19 commits into from
Nov 21, 2023
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -292,6 +292,18 @@ TEST_F(PgDataTypeIntegrationTest, WriteReadNumeric) {
EXPECT_THAT(result, IsOkAndHolds(UnorderedElementsAreArray(data)));
}

TEST_F(PgDataTypeIntegrationTest, WriteReadPgOid) {
if (UsingEmulator()) GTEST_SKIP() << "emulator does not support PG.OID";

std::vector<PgNumeric> const data = {
devbww marked this conversation as resolved.
Show resolved Hide resolved
PgOid("0"), //
PgOid("42"), //
PgOid("999"), //
devbww marked this conversation as resolved.
Show resolved Hide resolved
};
auto result = WriteReadData(*client_, data, "PgOidValue");
devbww marked this conversation as resolved.
Show resolved Hide resolved
EXPECT_THAT(result, IsOkAndHolds(UnorderedElementsAreArray(data)));
}

TEST_F(DataTypeIntegrationTest, WriteReadArrayBool) {
std::vector<std::vector<bool>> const data = {
std::vector<bool>{},
Expand Down
43 changes: 43 additions & 0 deletions google/cloud/spanner/internal/connection_impl_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1069,6 +1069,49 @@ TEST(ConnectionImplTest, ExecuteQueryNumericParameter) {
}
}

TEST(ConnectionImplTest, ExecuteQueryPgOidResult) {
auto mock = std::make_shared<spanner_testing::MockSpannerStub>();
auto db = spanner::Database("placeholder_project", "placeholder_instance",
"placeholder_database_id");
EXPECT_CALL(*mock, BatchCreateSessions(_, HasDatabase(db)))
.WillOnce(Return(MakeSessionsResponse({"test-session-name"})));
auto constexpr kText = R"pb(
metadata: {
row_type: {
fields: {
name: "ColumnA",
type: { code: INT64 type_annotation: PG_OID }
}
fields: {
name: "ColumnB",
type: { code: INT64 type_annotation: PG_OID }
}
}
}
values: { string_value: "42" }
values: { null_value: NULL_VALUE }
values: { string_value: "0" }
values: { string_value: "999" }
)pb";
EXPECT_CALL(*mock, ExecuteStreamingSql)
.WillOnce(Return(ByMove(MakeReader<PartialResultSet>({kText}))));

auto conn = MakeConnectionImpl(db, mock);
internal::OptionsSpan span(MakeLimitedTimeOptions());
auto rows = conn->ExecuteQuery(
{MakeSingleUseTransaction(spanner::Transaction::ReadOnlyOptions()),
spanner::SqlStatement("SELECT * FROM Table")});

using RowType = std::tuple<spanner::PgOid, absl::optional<spanner::PgOid>>;
auto stream = spanner::StreamOf<RowType>(rows);
auto actual = std::vector<StatusOr<RowType>>{stream.begin(), stream.end()};
EXPECT_THAT(
actual,
ElementsAre(
IsOkAndHolds(RowType(spanner::PgOid("42"), absl::nullopt)),
IsOkAndHolds(RowType(spanner::PgOid("0"), spanner::PgOid("999")))));
}

/// @test Verify implicit "begin transaction" in ExecuteQuery() works.
TEST(ConnectionImplTest, ExecuteQueryImplicitBeginTransaction) {
auto mock = std::make_shared<spanner_testing::MockSpannerStub>();
Expand Down
73 changes: 73 additions & 0 deletions google/cloud/spanner/oid.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
// Copyright 2023 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
//
// https://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.

#ifndef GOOGLE_CLOUD_CPP_GOOGLE_CLOUD_SPANNER_OID_H
#define GOOGLE_CLOUD_CPP_GOOGLE_CLOUD_SPANNER_OID_H


devbww marked this conversation as resolved.
Show resolved Hide resolved
#include "google/cloud/spanner/version.h"
#include <ostream>
#include <string>
thiagotnunes marked this conversation as resolved.
Show resolved Hide resolved

namespace google {
namespace cloud {
namespace spanner {
GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_BEGIN

class PgOid {
devbww marked this conversation as resolved.
Show resolved Hide resolved
public:
devbww marked this conversation as resolved.
Show resolved Hide resolved
/// @name Regular value type, supporting copy, assign, move.
///@{
PgOid(PgOid const&) = default;
PgOid& operator=(PgOid const&) = default;
PgOid(PgOid&&) = default;
PgOid& operator=(PgOid&&) = default;
///@}

/**
* Construction from an oid string. Note that there is no check here that the
thiagotnunes marked this conversation as resolved.
Show resolved Hide resolved
* argument string is indeed well-formatted. Error detection will be delayed
* until the value is passed to Spanner.
*/
explicit PgOid(std::string value) : rep_(value) {}
devbww marked this conversation as resolved.
Show resolved Hide resolved

/// @name Conversion to a JSON-formatted string.
thiagotnunes marked this conversation as resolved.
Show resolved Hide resolved
///@{
explicit operator std::string() const& { return rep_; }
explicit operator std::string() && { return std::move(rep_); }
///@}

private:
std::string rep_;
devbww marked this conversation as resolved.
Show resolved Hide resolved
};

/// @name Relational operators
///@{
inline bool operator==(PgOid const& lhs, PgOid const& rhs) {
return std::string(lhs) == std::string(rhs);
}
inline bool operator!=(PgOid const& lhs, PgOid const& rhs) { return !(lhs == rhs); }
devbww marked this conversation as resolved.
Show resolved Hide resolved
///@}

/// Outputs an Oid formatted string to the provided stream.
thiagotnunes marked this conversation as resolved.
Show resolved Hide resolved
inline std::ostream& operator<<(std::ostream& os, PgOid const& oid) {
return os << std::string(oid);
devbww marked this conversation as resolved.
Show resolved Hide resolved
}

GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_END
} // namespace spanner
} // namespace cloud
} // namespace google

#endif // GOOGLE_CLOUD_CPP_GOOGLE_CLOUD_SPANNER_OID_H
27 changes: 27 additions & 0 deletions google/cloud/spanner/value.cc
Original file line number Diff line number Diff line change
Expand Up @@ -259,6 +259,12 @@ bool Value::TypeProtoIs(PgNumeric const&,
google::spanner::v1::TypeAnnotationCode::PG_NUMERIC;
}

bool Value::TypeProtoIs(PgOid const&, google::spanner::v1::Type const& type) {
return type.code() == google::spanner::v1::TypeCode::INT64 &&
type.type_annotation() ==
google::spanner::v1::TypeAnnotationCode::PG_OID;
}

//
// Value::MakeTypeProto
//
Expand Down Expand Up @@ -323,6 +329,13 @@ google::spanner::v1::Type Value::MakeTypeProto(PgNumeric const&) {
return t;
}

google::spanner::v1::Type Value::MakeTypeProto(PgOid const&) {
google::spanner::v1::Type t;
t.set_code(google::spanner::v1::TypeCode::INT64);
t.set_type_annotation(google::spanner::v1::TypeAnnotationCode::PG_OID);
return t;
}

google::spanner::v1::Type Value::MakeTypeProto(Timestamp) {
google::spanner::v1::Type t;
t.set_code(google::spanner::v1::TypeCode::TIMESTAMP);
Expand Down Expand Up @@ -413,6 +426,12 @@ google::protobuf::Value Value::MakeValueProto(PgNumeric n) {
return v;
}

google::protobuf::Value Value::MakeValueProto(PgOid n) {
google::protobuf::Value v;
v.set_string_value(std::string(std::move(n)));
thiagotnunes marked this conversation as resolved.
Show resolved Hide resolved
return v;
}

google::protobuf::Value Value::MakeValueProto(Timestamp ts) {
google::protobuf::Value v;
v.set_string_value(spanner_internal::TimestampToRFC3339(ts));
Expand Down Expand Up @@ -562,6 +581,14 @@ StatusOr<PgNumeric> Value::GetValue(PgNumeric const&,
return *decoded;
}

StatusOr<PgOid> Value::GetValue(PgOid const&, google::protobuf::Value const& pv,
google::spanner::v1::Type const&) {
if (pv.kind_case() != google::protobuf::Value::kStringValue) {
return Status(StatusCode::kUnknown, "missing OID");
}
return PgOid(pv.string_value());
}

StatusOr<Timestamp> Value::GetValue(Timestamp,
google::protobuf::Value const& pv,
google::spanner::v1::Type const&) {
Expand Down
9 changes: 9 additions & 0 deletions google/cloud/spanner/value.h
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
#include "google/cloud/spanner/internal/tuple_utils.h"
#include "google/cloud/spanner/json.h"
#include "google/cloud/spanner/numeric.h"
#include "google/cloud/spanner/oid.h"
#include "google/cloud/spanner/timestamp.h"
#include "google/cloud/spanner/version.h"
#include "google/cloud/internal/throw_delegate.h"
Expand Down Expand Up @@ -68,6 +69,7 @@ GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_BEGIN
* JSONB | `google::cloud::spanner::JsonB`
* NUMERIC | `google::cloud::spanner::Numeric`
* NUMERIC(PG) | `google::cloud::spanner::PgNumeric`
* OID(PG) | `google::cloud::spanner::PgOid`
* TIMESTAMP | `google::cloud::spanner::Timestamp`
* DATE | `absl::CivilDay`
* ARRAY | `std::vector<T>` // [1]
Expand Down Expand Up @@ -201,6 +203,8 @@ class Value {
/// @copydoc Value(bool)
explicit Value(PgNumeric v) : Value(PrivateConstructor{}, std::move(v)) {}
/// @copydoc Value(bool)
explicit Value(PgOid v) : Value(PrivateConstructor{}, std::move(v)) {}
/// @copydoc Value(bool)
explicit Value(Timestamp v) : Value(PrivateConstructor{}, std::move(v)) {}
/// @copydoc Value(bool)
explicit Value(CommitTimestamp v)
Expand Down Expand Up @@ -372,6 +376,7 @@ class Value {
static bool TypeProtoIs(JsonB const&, google::spanner::v1::Type const&);
static bool TypeProtoIs(Numeric const&, google::spanner::v1::Type const&);
static bool TypeProtoIs(PgNumeric const&, google::spanner::v1::Type const&);
static bool TypeProtoIs(PgOid const&, google::spanner::v1::Type const&);
template <typename T>
static bool TypeProtoIs(absl::optional<T>,
google::spanner::v1::Type const& type) {
Expand Down Expand Up @@ -421,6 +426,7 @@ class Value {
static google::spanner::v1::Type MakeTypeProto(JsonB const&);
static google::spanner::v1::Type MakeTypeProto(Numeric const&);
static google::spanner::v1::Type MakeTypeProto(PgNumeric const&);
static google::spanner::v1::Type MakeTypeProto(PgOid const&);
static google::spanner::v1::Type MakeTypeProto(Timestamp);
static google::spanner::v1::Type MakeTypeProto(CommitTimestamp);
static google::spanner::v1::Type MakeTypeProto(absl::CivilDay);
Expand Down Expand Up @@ -484,6 +490,7 @@ class Value {
static google::protobuf::Value MakeValueProto(JsonB j);
static google::protobuf::Value MakeValueProto(Numeric n);
static google::protobuf::Value MakeValueProto(PgNumeric n);
static google::protobuf::Value MakeValueProto(PgOid n);
static google::protobuf::Value MakeValueProto(Timestamp ts);
static google::protobuf::Value MakeValueProto(CommitTimestamp ts);
static google::protobuf::Value MakeValueProto(absl::CivilDay d);
Expand Down Expand Up @@ -555,6 +562,8 @@ class Value {
static StatusOr<PgNumeric> GetValue(PgNumeric const&,
google::protobuf::Value const&,
google::spanner::v1::Type const&);
static StatusOr<PgOid> GetValue(PgOid const&, google::protobuf::Value const&,
google::spanner::v1::Type const&);
static StatusOr<Timestamp> GetValue(Timestamp, google::protobuf::Value const&,
google::spanner::v1::Type const&);
static StatusOr<CommitTimestamp> GetValue(CommitTimestamp,
Expand Down
38 changes: 38 additions & 0 deletions google/cloud/spanner/value_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -247,6 +247,7 @@ TEST(Value, Equality) {
{Value(JsonB("42")), Value(JsonB("true"))},
{Value(MakeNumeric(0).value()), Value(MakeNumeric(1).value())},
{Value(MakePgNumeric(0).value()), Value(MakePgNumeric(1).value())},
{Value(PgOid("200")), Value(PgOid("999"))},
{Value(absl::CivilDay(1970, 1, 1)), Value(absl::CivilDay(2020, 3, 15))},
{Value(std::vector<double>{1.2, 3.4}),
Value(std::vector<double>{4.5, 6.7})},
Expand Down Expand Up @@ -753,6 +754,21 @@ TEST(Value, ProtoConversionNumeric) {
}
}

TEST(Value, ProtoConversionPgOid) {
for (auto const& x : std::vector<PgOid>{
PgOid("0"),
PgOid("200"),
PgOid("999"),
}) {
Value const v(x);
auto const p = spanner_internal::ToProto(v);
EXPECT_EQ(v, spanner_internal::FromProto(p.first, p.second));
EXPECT_EQ(p.first.code(), google::spanner::v1::TypeCode::INT64);
EXPECT_EQ(p.first.type_annotation(), google::spanner::v1::TypeAnnotationCode::PG_OID);
devbww marked this conversation as resolved.
Show resolved Hide resolved
EXPECT_EQ(std::string(x), p.second.string_value());
devbww marked this conversation as resolved.
Show resolved Hide resolved
}
}

TEST(Value, ProtoConversionTimestamp) {
for (auto ts : TestTimes()) {
Value const v(ts);
Expand Down Expand Up @@ -975,6 +991,21 @@ TEST(Value, GetBadNumeric) {
EXPECT_THAT(v.get<std::int64_t>(), Not(IsOk()));
}

TEST(Value, GetBadPgOid) {
Value v(PgOid("1"));
ClearProtoKind(v);
EXPECT_THAT(v.get<std::string>(), Not(IsOk()));

SetProtoKind(v, google::protobuf::NULL_VALUE);
EXPECT_THAT(v.get<PgOid>(), Not(IsOk()));

SetProtoKind(v, true);
EXPECT_THAT(v.get<PgOid>(), Not(IsOk()));

SetProtoKind(v, 0.0);
EXPECT_THAT(v.get<PgOid>(), Not(IsOk()));
}

TEST(Value, GetBadInt) {
Value v(42);
ClearProtoKind(v);
Expand Down Expand Up @@ -1164,6 +1195,7 @@ TEST(Value, OutputStream) {
{Value(JsonB("true")), "true", normal},
{Value(MakeNumeric(1234567890).value()), "1234567890", normal},
{Value(MakePgNumeric(1234567890).value()), "1234567890", normal},
{Value(PgOid("1234567890")), "1234567890", normal},
{Value(absl::CivilDay()), "1970-01-01", normal},
{Value(Timestamp()), "1970-01-01T00:00:00Z", normal},

Expand All @@ -1189,6 +1221,7 @@ TEST(Value, OutputStream) {
{MakeNullValue<Json>(), "NULL", normal},
{MakeNullValue<JsonB>(), "NULL", normal},
{MakeNullValue<Numeric>(), "NULL", normal},
{MakeNullValue<PgOid>(), "NULL", normal},
{MakeNullValue<absl::CivilDay>(), "NULL", normal},
{MakeNullValue<Timestamp>(), "NULL", normal},

Expand Down Expand Up @@ -1343,6 +1376,11 @@ TEST(Value, OutputStreamMatchesT) {
StreamMatchesValueStream(MakePgNumeric(42).value());
StreamMatchesValueStream(MakePgNumeric("NaN").value());

// PgOid
StreamMatchesValueStream(PgOid("999"));
StreamMatchesValueStream(PgOid("-1"));
StreamMatchesValueStream(PgOid("0"));

// Date
StreamMatchesValueStream(absl::CivilDay(1, 1, 1));
StreamMatchesValueStream(absl::CivilDay());
Expand Down