Skip to content

Commit f191f9e

Browse files
authored
[stable-25-3-1] speed up the TValue::HasValue method (#28067)
LOGBROKER-10046
2 parents 17d2a79 + 9485344 commit f191f9e

File tree

5 files changed

+68
-17
lines changed

5 files changed

+68
-17
lines changed
Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
2+
#include <ydb/public/lib/value/value.h>
3+
#include <ydb/library/mkql_proto/protos/minikql.pb.h>
4+
5+
#include <library/cpp/testing/gtest/gtest.h>
6+
7+
TEST(TValue, FieldsCount) {
8+
ASSERT_EQ(NKikimrMiniKQL::TValue::GetDescriptor()->field_count(), 18) << "update the NKikimr::NClient::TValue wrapper to support new fields";
9+
}

ydb/public/lib/value/ut/ya.make

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
GTEST()
2+
3+
SIZE(SMALL)
4+
5+
PEERDIR(
6+
ydb/public/lib/value
7+
)
8+
9+
SRCS(
10+
value_ut.cpp
11+
)
12+
13+
END()

ydb/public/lib/value/value.cpp

Lines changed: 40 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,10 +4,14 @@
44

55
#include <library/cpp/string_utils/base64/base64.h>
66

7+
#include <google/protobuf/text_format.h>
8+
79
#include <util/charset/utf8.h>
10+
#include <util/string/builder.h>
811
#include <util/string/cast.h>
912
#include <util/string/escape.h>
1013
#include <util/string/printf.h>
14+
#include <util/stream/output.h>
1115
#include <ydb/public/sdk/cpp/include/ydb-cpp-sdk/client/value/value.h>
1216

1317
namespace NKikimr {
@@ -28,14 +32,33 @@ TValue TValue::Create(const NKikimrMiniKQL::TResult& result) {
2832
return TValue::Create(result.GetValue(), result.GetType());
2933
}
3034

35+
static bool ValueProtobufHasEmptyPayload(const NKikimrMiniKQL::TValue& value) {
36+
bool emptyPayload = true;
37+
emptyPayload &= (value.value_value_case() == value.VALUE_VALUE_NOT_SET);
38+
emptyPayload &= value.GetList().empty();
39+
emptyPayload &= value.GetTuple().empty();
40+
emptyPayload &= value.GetStruct().empty();
41+
emptyPayload &= value.GetDict().empty();
42+
emptyPayload &= !value.HasHi128();
43+
emptyPayload &= !value.HasVariantIndex();
44+
// Non-empty unknown fields are a weird case, as they are not accessible through the wrapper and can only be viewed in the debug dump.
45+
// If the value is not considered empty, the wrapper will return a non-empty value, for which no accessor can return a meaningful value.
46+
constexpr bool checkUnknownFields = false;
47+
if (checkUnknownFields) {
48+
emptyPayload = emptyPayload && value.unknown_fields().empty();
49+
}
50+
return emptyPayload;
51+
}
52+
3153
bool TValue::HaveValue() const {
3254
return !IsNull();
3355
}
3456

3557
bool TValue::IsNull() const {
3658
if (&Value == &Null)
3759
return true;
38-
return Value.ByteSize() == 0;
60+
61+
return ValueProtobufHasEmptyPayload(Value);
3962
}
4063

4164
TValue TValue::operator [](const char* name) const {
@@ -108,6 +131,21 @@ TVector<TString> TValue::GetMembersNames() const {
108131
return members;
109132
}
110133

134+
TString TValue::DumpToString() const {
135+
TStringBuilder dump;
136+
TString res;
137+
::google::protobuf::TextFormat::PrintToString(Type, &res);
138+
dump << "Type:" << Endl << res << Endl;
139+
::google::protobuf::TextFormat::PrintToString(Value, &res);
140+
dump << "Value:" << Endl << res << Endl;
141+
return std::move(dump);
142+
}
143+
144+
void TValue::DumpValue() const {
145+
Cerr << DumpToString();
146+
}
147+
148+
111149
TWriteValue TWriteValue::Create(NKikimrMiniKQL::TValue& value, NKikimrMiniKQL::TType& type) {
112150
return TWriteValue(value, type);
113151
}
@@ -453,7 +491,7 @@ TString TValue::GetDataText() const {
453491
case NScheme::NTypeIds::Datetime64:
454492
case NScheme::NTypeIds::Timestamp64:
455493
case NScheme::NTypeIds::Interval64:
456-
return ToString(Value.GetInt64());
494+
return ToString(Value.GetInt64());
457495
case NScheme::NTypeIds::JsonDocument:
458496
return "\"<JsonDocument>\"";
459497
case NScheme::NTypeIds::Uuid:

ydb/public/lib/value/value.h

Lines changed: 2 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,6 @@
44
#include <ydb/public/lib/scheme_types/scheme_type_id.h>
55

66
#include <util/generic/vector.h>
7-
#include <util/string/builder.h>
8-
9-
#include <google/protobuf/text_format.h>
107

118
namespace NKikimr {
129
namespace NClient {
@@ -106,19 +103,9 @@ class TValue {
106103
// returns member index by name
107104
int GetMemberIndex(TStringBuf name) const;
108105

109-
TString DumpToString() const {
110-
TStringBuilder dump;
111-
TString res;
112-
::google::protobuf::TextFormat::PrintToString(Type, &res);
113-
dump << "Type:" << Endl << res << Endl;
114-
::google::protobuf::TextFormat::PrintToString(Value, &res);
115-
dump << "Value:" << Endl << res << Endl;
116-
return std::move(dump);
117-
}
106+
TString DumpToString() const;
118107

119-
void DumpValue() const {
120-
Cerr << DumpToString();
121-
}
108+
void DumpValue() const; // dump value to stderr
122109

123110
const NKikimrMiniKQL::TType& GetType() const { return Type; };
124111
const NKikimrMiniKQL::TValue& GetValue() const { return Value; };

ydb/public/lib/value/ya.make

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,3 +14,7 @@ PEERDIR(
1414
)
1515

1616
END()
17+
18+
RECURSE_FOR_TESTS(
19+
ut
20+
)

0 commit comments

Comments
 (0)