Skip to content

Commit

Permalink
feat(spanner): add spanner::Value support for TypeCode::FLOAT32 (#13862)
Browse files Browse the repository at this point in the history 
Represented as a `float` in C++, so no need for a wrapper class.
  • Loading branch information
@devbww
devbww authored Mar 28, 2024
1 parent a882bc8 commit 8af53cf
Show file tree
Hide file tree
Showing 3 changed files with 173 additions and 1 deletion.
48 changes: 48 additions & 0 deletions google/cloud/spanner/value.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,9 @@ std::ostream& StreamHelper(std::ostream& os, // NOLINT(misc-no-recursion)
return os
<< spanner_internal::FromProto(t, v).get<std::int64_t>().value();

case google::spanner::v1::TypeCode::FLOAT32:
return os << spanner_internal::FromProto(t, v).get<float>().value();

case google::spanner::v1::TypeCode::FLOAT64:
return os << spanner_internal::FromProto(t, v).get<double>().value();

Expand Down Expand Up @@ -244,6 +247,10 @@ bool Value::TypeProtoIs(std::int64_t, google::spanner::v1::Type const& type) {
return type.code() == google::spanner::v1::TypeCode::INT64;
}

bool Value::TypeProtoIs(float, google::spanner::v1::Type const& type) {
return type.code() == google::spanner::v1::TypeCode::FLOAT32;
}

bool Value::TypeProtoIs(double, google::spanner::v1::Type const& type) {
return type.code() == google::spanner::v1::TypeCode::FLOAT64;
}
Expand Down Expand Up @@ -317,6 +324,12 @@ google::spanner::v1::Type Value::MakeTypeProto(std::int64_t) {
return t;
}

google::spanner::v1::Type Value::MakeTypeProto(float) {
google::spanner::v1::Type t;
t.set_code(google::spanner::v1::TypeCode::FLOAT32);
return t;
}

google::spanner::v1::Type Value::MakeTypeProto(double) {
google::spanner::v1::Type t;
t.set_code(google::spanner::v1::TypeCode::FLOAT64);
Expand Down Expand Up @@ -414,6 +427,20 @@ google::protobuf::Value Value::MakeValueProto(std::int64_t i) {
return v;
}

google::protobuf::Value Value::MakeValueProto(float f) {
google::protobuf::Value v;
if (std::isnan(f)) {
v.set_string_value("NaN");
} else if (std::isinf(f)) {
v.set_string_value(f < 0 ? "-Infinity" : "Infinity");
} else {
// A widening conversion (i.e., a floating-point promotion), but
// that's OK as the standard guarantees that the value is unchanged.
v.set_number_value(f);
}
return v;
}

google::protobuf::Value Value::MakeValueProto(double d) {
google::protobuf::Value v;
if (std::isnan(d)) {
Expand Down Expand Up @@ -535,6 +562,27 @@ StatusOr<std::int64_t> Value::GetValue(std::int64_t,
return x;
}

StatusOr<float> Value::GetValue(float, google::protobuf::Value const& pv,
google::spanner::v1::Type const&) {
if (pv.kind_case() == google::protobuf::Value::kNumberValue) {
// A narrowing conversion, but that's OK. If the value originated
// as a float, then the conversion through double is required to
// produce the same value (and we already assume that a double value
// if preserved over the wire). If the value originated as a double
// then we're simply doing the requested narrowing.
return static_cast<float>(pv.number_value());
}
if (pv.kind_case() != google::protobuf::Value::kStringValue) {
return Status(StatusCode::kUnknown, "missing FLOAT32");
}
std::string const& s = pv.string_value();
auto const inf = std::numeric_limits<float>::infinity();
if (s == "-Infinity") return -inf;
if (s == "Infinity") return inf;
if (s == "NaN") return std::nanf("");
return Status(StatusCode::kUnknown, "bad FLOAT32 data: \"" + s + "\"");
}

StatusOr<double> Value::GetValue(double, google::protobuf::Value const& pv,
google::spanner::v1::Type const&) {
if (pv.kind_case() == google::protobuf::Value::kNumberValue) {
Expand Down
8 changes: 8 additions & 0 deletions google/cloud/spanner/value.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_BEGIN
* ------------ | ------------
* BOOL | `bool`
* INT64 | `std::int64_t`
* FLOAT32 | `float`
* FLOAT64 | `double`
* STRING | `std::string`
* BYTES | `google::cloud::spanner::Bytes`
Expand Down Expand Up @@ -194,6 +195,8 @@ class Value {
/// @copydoc Value(bool)
explicit Value(std::int64_t v) : Value(PrivateConstructor{}, v) {}
/// @copydoc Value(bool)
explicit Value(float v) : Value(PrivateConstructor{}, v) {}
/// @copydoc Value(bool)
explicit Value(double v) : Value(PrivateConstructor{}, v) {}
/// @copydoc Value(bool)
explicit Value(std::string v) : Value(PrivateConstructor{}, std::move(v)) {}
Expand Down Expand Up @@ -378,6 +381,7 @@ class Value {
// by the given `Type` proto.
static bool TypeProtoIs(bool, google::spanner::v1::Type const&);
static bool TypeProtoIs(std::int64_t, google::spanner::v1::Type const&);
static bool TypeProtoIs(float, google::spanner::v1::Type const&);
static bool TypeProtoIs(double, google::spanner::v1::Type const&);
static bool TypeProtoIs(Timestamp, google::spanner::v1::Type const&);
static bool TypeProtoIs(CommitTimestamp, google::spanner::v1::Type const&);
Expand Down Expand Up @@ -443,6 +447,7 @@ class Value {
// argument type is the tag, the argument value is ignored.
static google::spanner::v1::Type MakeTypeProto(bool);
static google::spanner::v1::Type MakeTypeProto(std::int64_t);
static google::spanner::v1::Type MakeTypeProto(float);
static google::spanner::v1::Type MakeTypeProto(double);
static google::spanner::v1::Type MakeTypeProto(std::string const&);
static google::spanner::v1::Type MakeTypeProto(Bytes const&);
Expand Down Expand Up @@ -521,6 +526,7 @@ class Value {
// https://github.com/googleapis/googleapis/blob/master/google/spanner/v1/type.proto
static google::protobuf::Value MakeValueProto(bool b);
static google::protobuf::Value MakeValueProto(std::int64_t i);
static google::protobuf::Value MakeValueProto(float f);
static google::protobuf::Value MakeValueProto(double d);
static google::protobuf::Value MakeValueProto(std::string s);
static google::protobuf::Value MakeValueProto(Bytes b);
Expand Down Expand Up @@ -590,6 +596,8 @@ class Value {
static StatusOr<std::int64_t> GetValue(std::int64_t,
google::protobuf::Value const&,
google::spanner::v1::Type const&);
static StatusOr<float> GetValue(float, google::protobuf::Value const&,
google::spanner::v1::Type const&);
static StatusOr<double> GetValue(double, google::protobuf::Value const&,
google::spanner::v1::Type const&);
static StatusOr<std::string> GetValue(std::string const&,
Expand Down
118 changes: 117 additions & 1 deletion google/cloud/spanner/value_test.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,7 @@ TEST(Value, BasicSemantics) {

// Note: We skip testing the NaN case here because NaN always compares not
// equal, even with itself. So NaN is handled in a separate test.
static_assert(std::numeric_limits<double>::has_infinity, "");
auto const inf = std::numeric_limits<double>::infinity();
for (auto x : {-inf, -1.0, -0.5, 0.0, 0.5, 1.0, inf}) {
SCOPED_TRACE("Testing: double " + std::to_string(x));
Expand All @@ -156,6 +157,19 @@ TEST(Value, BasicSemantics) {
TestBasicSemantics(v);
}

// Note: We skip testing the NaN case here because NaN always compares not
// equal, even with itself. So NaN is handled in a separate test.
static_assert(std::numeric_limits<float>::has_infinity, "");
auto const inff = std::numeric_limits<float>::infinity();
for (auto x : {-inff, -1.0F, -0.5F, 0.0F, 0.5F, 1.0F, inff}) {
SCOPED_TRACE("Testing: float " + std::to_string(x));
TestBasicSemantics(x);
TestBasicSemantics(std::vector<float>(5, x));
std::vector<absl::optional<float>> v(5, x);
v.resize(10);
TestBasicSemantics(v);
}

for (auto const& x :
std::vector<std::string>{"", "f", "foo", "12345678901234567"}) {
SCOPED_TRACE("Testing: std::string " + std::string(x));
Expand Down Expand Up @@ -467,6 +481,9 @@ TEST(Value, ConstructionFromLiterals) {
Value v_int64(42);
EXPECT_EQ(42, *v_int64.get<std::int64_t>());

Value v_float64(1.5);
EXPECT_EQ(1.5, *v_float64.get<double>());

Value v_string("hello");
EXPECT_EQ("hello", *v_string.get<std::string>());

Expand Down Expand Up @@ -518,6 +535,7 @@ TEST(Value, MixingTypes) {
TEST(Value, SpannerArray) {
using ArrayInt64 = std::vector<std::int64_t>;
using ArrayDouble = std::vector<double>;
using ArrayFloat = std::vector<float>;

ArrayInt64 const empty = {};
Value const ve(empty);
Expand All @@ -541,6 +559,14 @@ TEST(Value, SpannerArray) {
ASSERT_STATUS_OK(vd.get<ArrayDouble>());
EXPECT_EQ(ad, *vd.get<ArrayDouble>());

ArrayFloat const af = {1.0, 2.0, 3.0};
Value const vf(af);
EXPECT_EQ(vf, vf);
EXPECT_NE(vi, vf);
EXPECT_THAT(vf.get<ArrayInt64>(), Not(IsOk()));
ASSERT_STATUS_OK(vf.get<ArrayFloat>());
EXPECT_EQ(af, *vf.get<ArrayFloat>());

Value const null_vi = MakeNullValue<ArrayInt64>();
EXPECT_EQ(null_vi, null_vi);
EXPECT_NE(null_vi, vi);
Expand All @@ -555,6 +581,14 @@ TEST(Value, SpannerArray) {
EXPECT_NE(null_vd, vi);
EXPECT_THAT(null_vd.get<ArrayDouble>(), Not(IsOk()));
EXPECT_THAT(null_vd.get<ArrayInt64>(), Not(IsOk()));

Value const null_vf = MakeNullValue<ArrayFloat>();
EXPECT_EQ(null_vf, null_vf);
EXPECT_NE(null_vf, null_vi);
EXPECT_NE(null_vf, vf);
EXPECT_NE(null_vf, vi);
EXPECT_THAT(null_vf.get<ArrayFloat>(), Not(IsOk()));
EXPECT_THAT(null_vf.get<ArrayInt64>(), Not(IsOk()));
}

TEST(Value, SpannerStruct) {
Expand Down Expand Up @@ -729,6 +763,39 @@ TEST(Value, ProtoConversionFloat64) {
EXPECT_EQ("NaN", p.second.string_value());
}

TEST(Value, ProtoConversionFloat32) {
for (auto x : {-1.0F, -0.5F, 0.0F, 0.5F, 1.0F}) {
Value const v(x);
auto const p = spanner_internal::ToProto(v);
EXPECT_EQ(v, spanner_internal::FromProto(p.first, p.second));
EXPECT_EQ(google::spanner::v1::TypeCode::FLOAT32, p.first.code());
EXPECT_EQ(x, p.second.number_value());
}

// Tests special cases
auto const inf = std::numeric_limits<float>::infinity();
Value v(inf);
auto p = spanner_internal::ToProto(v);
EXPECT_EQ(v, spanner_internal::FromProto(p.first, p.second));
EXPECT_EQ(google::spanner::v1::TypeCode::FLOAT32, p.first.code());
EXPECT_EQ("Infinity", p.second.string_value());

v = Value(-inf);
p = spanner_internal::ToProto(v);
EXPECT_EQ(v, spanner_internal::FromProto(p.first, p.second));
EXPECT_EQ(google::spanner::v1::TypeCode::FLOAT32, p.first.code());
EXPECT_EQ("-Infinity", p.second.string_value());

auto const nan = std::nanf("NaN");
v = Value(nan);
p = spanner_internal::ToProto(v);
// Note: Unlike IEEE 754, Spanner NaN values are considered equal
// (for easy sorting), so spanner::Value behaves the same way.
EXPECT_EQ(v, spanner_internal::FromProto(p.first, p.second));
EXPECT_EQ(google::spanner::v1::TypeCode::FLOAT32, p.first.code());
EXPECT_EQ("NaN", p.second.string_value());
}

TEST(Value, ProtoConversionString) {
for (auto const& x :
std::vector<std::string>{"", "f", "foo", "12345678901234567890"}) {
Expand Down Expand Up @@ -931,6 +998,12 @@ void SetProtoKind(Value& v, double x) {
v = spanner_internal::FromProto(p.first, p.second);
}

void SetProtoKind(Value& v, float x) {
auto p = spanner_internal::ToProto(v);
p.second.set_number_value(x);
v = spanner_internal::FromProto(p.first, p.second);
}

void SetProtoKind(Value& v, char const* x) {
auto p = spanner_internal::ToProto(v);
p.second.set_string_value(x);
Expand Down Expand Up @@ -960,6 +1033,9 @@ TEST(Value, GetBadBool) {
SetProtoKind(v, 0.0);
EXPECT_THAT(v.get<bool>(), Not(IsOk()));

SetProtoKind(v, 0.0F);
EXPECT_THAT(v.get<bool>(), Not(IsOk()));

SetProtoKind(v, "hello");
EXPECT_THAT(v.get<bool>(), Not(IsOk()));
}
Expand All @@ -979,6 +1055,21 @@ TEST(Value, GetBadDouble) {
EXPECT_THAT(v.get<double>(), Not(IsOk()));
}

TEST(Value, GetBadFloat) {
Value v(0.0F);
ClearProtoKind(v);
EXPECT_THAT(v.get<float>(), Not(IsOk()));

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

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

SetProtoKind(v, "bad string");
EXPECT_THAT(v.get<float>(), Not(IsOk()));
}

TEST(Value, GetBadString) {
Value v("hello");
ClearProtoKind(v);
Expand Down Expand Up @@ -1053,6 +1144,9 @@ TEST(Value, GetBadNumeric) {
SetProtoKind(v, 0.0);
EXPECT_THAT(v.get<std::string>(), Not(IsOk()));

SetProtoKind(v, 0.0F);
EXPECT_THAT(v.get<std::string>(), Not(IsOk()));

SetProtoKind(v, "");
EXPECT_THAT(v.get<std::int64_t>(), Not(IsOk()));

Expand Down Expand Up @@ -1296,6 +1390,7 @@ TEST(Value, OutputStream) {
};

auto const inf = std::numeric_limits<double>::infinity();
auto const inff = std::numeric_limits<float>::infinity();
auto const singer =
MakeSinger(1, "1817-05-25", "French", testing::Genre::FOLK);

Expand All @@ -1316,6 +1411,10 @@ TEST(Value, OutputStream) {
{Value(42.0), "42.00", float4},
{Value(inf), "inf", normal},
{Value(-inf), "-inf", normal},
{Value(42.0F), "42", normal},
{Value(42.0F), "42.00", float4},
{Value(inff), "inf", normal},
{Value(-inff), "-inf", normal},
{Value(""), "", normal},
{Value("foo"), "foo", normal},
{Value("NULL"), "NULL", normal},
Expand Down Expand Up @@ -1353,6 +1452,7 @@ TEST(Value, OutputStream) {
{MakeNullValue<bool>(), "NULL", normal},
{MakeNullValue<std::int64_t>(), "NULL", normal},
{MakeNullValue<double>(), "NULL", normal},
{MakeNullValue<float>(), "NULL", normal},
{MakeNullValue<std::string>(), "NULL", normal},
{MakeNullValue<Bytes>(), "NULL", normal},
{MakeNullValue<Json>(), "NULL", normal},
Expand All @@ -1371,6 +1471,8 @@ TEST(Value, OutputStream) {
{Value(std::vector<std::int64_t>{10, 11}), "[a, b]", hex},
{Value(std::vector<double>{1.0, 2.0}), "[1, 2]", normal},
{Value(std::vector<double>{1.0, 2.0}), "[1.000, 2.000]", float4},
{Value(std::vector<float>{1.0F, 2.0F}), "[1, 2]", normal},
{Value(std::vector<float>{1.0F, 2.0F}), "[1.000, 2.000]", float4},
{Value(std::vector<std::string>{"a", "b"}), R"(["a", "b"])", normal},
{Value(std::vector<Bytes>{2}), R"([B"", B""])", normal},
{Value(std::vector<Json>{2}), R"([null, null])", normal},
Expand All @@ -1396,6 +1498,7 @@ TEST(Value, OutputStream) {
{MakeNullValue<std::vector<bool>>(), "NULL", normal},
{MakeNullValue<std::vector<std::int64_t>>(), "NULL", normal},
{MakeNullValue<std::vector<double>>(), "NULL", normal},
{MakeNullValue<std::vector<float>>(), "NULL", normal},
{MakeNullValue<std::vector<std::string>>(), "NULL", normal},
{MakeNullValue<std::vector<Bytes>>(), "NULL", normal},
{MakeNullValue<std::vector<Json>>(), "NULL", normal},
Expand Down Expand Up @@ -1443,7 +1546,7 @@ TEST(Value, OutputStream) {
// Tests null structs
{MakeNullValue<std::tuple<bool>>(), "NULL", normal},
{MakeNullValue<std::tuple<bool, std::int64_t>>(), "NULL", normal},
{MakeNullValue<std::tuple<bool, std::string>>(), "NULL", normal},
{MakeNullValue<std::tuple<float, std::string>>(), "NULL", normal},
{MakeNullValue<std::tuple<double, Bytes, Timestamp>>(), "NULL", normal},
{MakeNullValue<std::tuple<Numeric, absl::CivilDay>>(), "NULL", normal},
{MakeNullValue<std::tuple<Json, std::vector<bool>>>(), "NULL", normal},
Expand All @@ -1461,6 +1564,12 @@ TEST(Value, OutputStream) {
std::stringstream ss;
ss << Value(std::nan(""));
EXPECT_THAT(ss.str(), HasSubstr("nan"));

// `float std::nanf("")` is a special case because the output conversion
// is implementation defined. So, we just look for a "nan" substring.
std::stringstream ssf;
ssf << Value(std::nanf(""));
EXPECT_THAT(ssf.str(), HasSubstr("nan"));
}

// Ensures that the following expressions produce the same output.
Expand Down Expand Up @@ -1494,6 +1603,13 @@ TEST(Value, OutputStreamMatchesT) {
StreamMatchesValueStream(std::numeric_limits<double>::infinity());
StreamMatchesValueStream(-std::numeric_limits<double>::infinity());

// float
StreamMatchesValueStream(0.0F);
StreamMatchesValueStream(3.14F);
StreamMatchesValueStream(std::nanf("NaN"));
StreamMatchesValueStream(std::numeric_limits<float>::infinity());
StreamMatchesValueStream(-std::numeric_limits<float>::infinity());

// std::string
StreamMatchesValueStream("");
StreamMatchesValueStream("foo");
Expand Down

0 comments on commit 8af53cf

Please sign in to comment.