Skip to content

Commit 521f690

Browse files
ydbotydbot
authored
stable-25-1-4: add validation for different precisions (#26774)
2 parents 229bb23 + af901b2 commit 521f690

File tree

4 files changed

+224
-2
lines changed

4 files changed

+224
-2
lines changed

‎ydb/core/kqp/ut/query/kqp_query_ut.cpp‎

Lines changed: 140 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,146 @@ Y_UNIT_TEST_SUITE(KqpQuery) {
7474
UNIT_ASSERT_VALUES_EQUAL(counters.RecompileRequestGet()->Val(), 1);
7575
}
7676

77+
Y_UNIT_TEST_TWIN(DecimalOutOfPrecisionBulk, EnableParameterizedDecimal) {
78+
TKikimrSettings serverSettings;
79+
serverSettings.FeatureFlags.SetEnableParameterizedDecimal(EnableParameterizedDecimal);
80+
serverSettings.WithSampleTables = false;
81+
82+
TKikimrRunner kikimr(serverSettings);
83+
auto client = kikimr.GetQueryClient();
84+
85+
{
86+
auto ddlResult = client.ExecuteQuery(R"(
87+
CREATE TABLE DecTest (
88+
Key Int32 NOT NULL,
89+
Value Decimal(22, 9) NOT NULL,
90+
PRIMARY KEY (Key)
91+
);
92+
)", NYdb::NQuery::TTxControl::NoTx()).ExtractValueSync();
93+
UNIT_ASSERT_C(ddlResult.IsSuccess(), ddlResult.GetIssues().ToString());
94+
}
95+
96+
// 10000000000000 in Decimal(35, 9), invalid for Decimal(22, 9)
97+
Ydb::Value value;
98+
value.set_low_128(1864712049423024128);
99+
value.set_high_128(542);
100+
auto invalidValue = TDecimalValue(value, NYdb::TDecimalType(22, 9));
101+
102+
{
103+
auto db = kikimr.GetTableClient();
104+
auto session = db.CreateSession().GetValueSync().GetSession();
105+
106+
NYdb::TValueBuilder rows;
107+
rows.BeginList();
108+
rows.AddListItem()
109+
.BeginStruct()
110+
.AddMember("Key").Int32(1)
111+
.AddMember("Value").Decimal(TDecimalValue("10", 22, 9))
112+
.EndStruct();
113+
rows.AddListItem()
114+
.BeginStruct()
115+
.AddMember("Key").Int32(2)
116+
.AddMember("Value").Decimal(invalidValue)
117+
.EndStruct();
118+
rows.AddListItem()
119+
.BeginStruct()
120+
.AddMember("Key").Int32(3)
121+
.AddMember("Value").Decimal(TDecimalValue("10000000000000", 22, 9))
122+
.EndStruct();
123+
rows.EndList();
124+
125+
auto resultUpsert = db.BulkUpsert("/Root/DecTest", rows.Build()).GetValueSync();
126+
// TODO: Plan A, upsert should fail as provided value is invalid for given type.
127+
UNIT_ASSERT_C(!resultUpsert.IsSuccess(), resultUpsert.GetIssues().ToString());
128+
129+
auto tableYson = ReadTableToYson(session, "/Root/DecTest");
130+
// TODO: Plan B, value for key 2 should be inf, as provided value is out of range
131+
// for given type.
132+
CompareYson(R"([])", tableYson);
133+
}
134+
}
135+
136+
Y_UNIT_TEST_QUAD(DecimalOutOfPrecision, UseOltpSink, EnableParameterizedDecimal) {
137+
TKikimrSettings serverSettings;
138+
serverSettings.AppConfig.MutableTableServiceConfig()->SetEnableOltpSink(UseOltpSink);
139+
serverSettings.FeatureFlags.SetEnableParameterizedDecimal(EnableParameterizedDecimal);
140+
serverSettings.WithSampleTables = false;
141+
142+
TKikimrRunner kikimr(serverSettings);
143+
auto client = kikimr.GetQueryClient();
144+
145+
{
146+
auto ddlResult = client.ExecuteQuery(Sprintf(R"(
147+
CREATE TABLE DecTest (
148+
Key Int32 NOT NULL,
149+
Value Decimal(22, 9),
150+
%s
151+
PRIMARY KEY (Key)
152+
);
153+
)", EnableParameterizedDecimal ? " ValueLarge Decimal(35, 9), " : ""), NYdb::NQuery::TTxControl::NoTx()).ExtractValueSync();
154+
UNIT_ASSERT_C(ddlResult.IsSuccess(), ddlResult.GetIssues().ToString());
155+
}
156+
157+
// 10000000000000 in Decimal(35, 9), invalid for Decimal(22, 9)
158+
Ydb::Value value;
159+
value.set_low_128(1864712049423024128);
160+
value.set_high_128(542);
161+
auto invalidValue = TDecimalValue(value, NYdb::TDecimalType(22, 9));
162+
163+
auto validValue = TDecimalValue(value, NYdb::TDecimalType(35, 9));
164+
165+
{
166+
auto params = TParamsBuilder()
167+
.AddParam("$value").Decimal(invalidValue).Build()
168+
.Build();
169+
170+
auto writeResult = client.ExecuteQuery(R"(
171+
UPSERT INTO DecTest (Key, Value) VALUES
172+
(1, CAST(10 AS Decimal(22,9))),
173+
(2, $value),
174+
(3, $value - CAST(1 AS Decimal(22,9)));
175+
)", NYdb::NQuery::TTxControl::BeginTx().CommitTx(), params).ExtractValueSync();
176+
177+
// TODO: Plan A, query should fail as provided value is invalid for given type.
178+
UNIT_ASSERT_C(!writeResult.IsSuccess(), writeResult.GetIssues().ToString());
179+
UNIT_ASSERT_EQUAL_C(writeResult.GetStatus(), EStatus::BAD_REQUEST, writeResult.GetIssues().ToString());
180+
181+
// TODO: Plan B, value for key 2 should be inf, as provided value is out of range
182+
// for given type.
183+
auto session = kikimr.GetTableClient().CreateSession().GetValueSync().GetSession();
184+
auto tableYson = ReadTableToYson(session, "/Root/DecTest");
185+
CompareYson(R"([])", tableYson);
186+
187+
if (EnableParameterizedDecimal)
188+
{
189+
auto paramsValid = TParamsBuilder()
190+
.AddParam("$value").Decimal(validValue).Build()
191+
.Build();
192+
auto writeResult = client.ExecuteQuery(R"(
193+
UPSERT INTO DecTest (Key, ValueLarge) VALUES
194+
(2, $value);
195+
)", NYdb::NQuery::TTxControl::BeginTx().CommitTx(), paramsValid).ExtractValueSync();
196+
197+
// TODO: Plan A, query should fail as provided value is invalid for given type.
198+
UNIT_ASSERT_C(writeResult.IsSuccess(), writeResult.GetIssues().ToString());
199+
UNIT_ASSERT_EQUAL_C(writeResult.GetStatus(), EStatus::SUCCESS, writeResult.GetIssues().ToString());
200+
201+
auto tableYson = ReadTableToYson(session, "/Root/DecTest");
202+
CompareYson(R"([[[2];#;["10000000000000"]]])", tableYson);
203+
}
204+
205+
if (EnableParameterizedDecimal)
206+
{
207+
auto writeResult = client.ExecuteQuery(R"(
208+
UPSERT INTO DecTest (Key, Value) SELECT Key, ValueLarge as Value FROM DecTest;
209+
)", NYdb::NQuery::TTxControl::BeginTx().CommitTx()).ExtractValueSync();
210+
// TODO: Plan A, query should fail as provided value is invalid for given type.
211+
UNIT_ASSERT_C(!writeResult.IsSuccess(), writeResult.GetIssues().ToString());
212+
UNIT_ASSERT_EQUAL_C(writeResult.GetStatus(), EStatus::GENERIC_ERROR, writeResult.GetIssues().ToString());
213+
}
214+
}
215+
}
216+
77217
Y_UNIT_TEST(QueryCache) {
78218
TKikimrRunner kikimr;
79219
auto db = kikimr.GetTableClient();

‎ydb/core/ydb_convert/ydb_convert.cpp‎

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
#include <ydb/library/ydb_issue/issue_helpers.h>
66
#include <ydb/core/protos/table_stats.pb.h>
77
#include <ydb/core/protos/subdomains.pb.h>
8+
#include <ydb/library/mkql_proto/mkql_proto.h>
89

910
#include <ydb-cpp-sdk/client/value/value.h>
1011

@@ -1292,7 +1293,21 @@ bool CellFromProtoVal(const NScheme::TTypeInfo& type, i32 typmod, const Ydb::Val
12921293
}
12931294
break;
12941295
}
1295-
case NScheme::NTypeIds::Decimal :
1296+
case NScheme::NTypeIds::Decimal : {
1297+
1298+
std::pair<ui64,ui64>& valInPool = *valueDataPool.Allocate<std::pair<ui64,ui64> >();
1299+
valInPool.first = val.low_128();
1300+
valInPool.second = val.high_128();
1301+
ui8 precision = type.GetDecimalType().GetPrecision();
1302+
auto validate = NYql::NDecimal::FromHalfs(val.low_128(), val.high_128());
1303+
if (!NKikimr::NMiniKQL::IsValidDecimal(precision, validate)) {
1304+
err = "Invalid decimal value";
1305+
return false;
1306+
}
1307+
1308+
c = TCell((const char*)&valInPool, sizeof(valInPool));
1309+
break;
1310+
}
12961311
case NScheme::NTypeIds::Uuid : {
12971312
std::pair<ui64,ui64>& valInPool = *valueDataPool.Allocate<std::pair<ui64,ui64> >();
12981313
valInPool.first = val.low_128();

‎ydb/library/mkql_proto/mkql_proto.cpp‎

Lines changed: 67 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,45 @@ namespace NKikimr::NMiniKQL {
2020

2121
namespace {
2222

23+
static constexpr std::array<std::pair<NYql::NDecimal::TInt128, NYql::NDecimal::TInt128>, NYql::NDecimal::MaxPrecision + 1> DecimalBounds = {
24+
NYql::NDecimal::GetBounds<0>(),
25+
NYql::NDecimal::GetBounds<1>(),
26+
NYql::NDecimal::GetBounds<2>(),
27+
NYql::NDecimal::GetBounds<3>(),
28+
NYql::NDecimal::GetBounds<4>(),
29+
NYql::NDecimal::GetBounds<5>(),
30+
NYql::NDecimal::GetBounds<6>(),
31+
NYql::NDecimal::GetBounds<7>(),
32+
NYql::NDecimal::GetBounds<8>(),
33+
NYql::NDecimal::GetBounds<9>(),
34+
NYql::NDecimal::GetBounds<10>(),
35+
NYql::NDecimal::GetBounds<11>(),
36+
NYql::NDecimal::GetBounds<12>(),
37+
NYql::NDecimal::GetBounds<13>(),
38+
NYql::NDecimal::GetBounds<14>(),
39+
NYql::NDecimal::GetBounds<15>(),
40+
NYql::NDecimal::GetBounds<16>(),
41+
NYql::NDecimal::GetBounds<17>(),
42+
NYql::NDecimal::GetBounds<18>(),
43+
NYql::NDecimal::GetBounds<19>(),
44+
NYql::NDecimal::GetBounds<20>(),
45+
NYql::NDecimal::GetBounds<21>(),
46+
NYql::NDecimal::GetBounds<22>(),
47+
NYql::NDecimal::GetBounds<23>(),
48+
NYql::NDecimal::GetBounds<24>(),
49+
NYql::NDecimal::GetBounds<25>(),
50+
NYql::NDecimal::GetBounds<26>(),
51+
NYql::NDecimal::GetBounds<27>(),
52+
NYql::NDecimal::GetBounds<28>(),
53+
NYql::NDecimal::GetBounds<29>(),
54+
NYql::NDecimal::GetBounds<30>(),
55+
NYql::NDecimal::GetBounds<31>(),
56+
NYql::NDecimal::GetBounds<32>(),
57+
NYql::NDecimal::GetBounds<33>(),
58+
NYql::NDecimal::GetBounds<34>(),
59+
NYql::NDecimal::GetBounds<35>(),
60+
};
61+
2362
void ExportTypeToProtoImpl(TType* type, NKikimrMiniKQL::TType& res, const TVector<ui32>* columnOrder = nullptr);
2463

2564
Y_FORCE_INLINE void HandleKindDataExport(const TType* type, const NUdf::TUnboxedValuePod& value, Ydb::Value& res) {
@@ -1628,7 +1667,17 @@ Y_FORCE_INLINE NUdf::TUnboxedValue KindDataImport(const TType* type, const Ydb::
16281667
return MakeString(value.bytes_value());
16291668
}
16301669
case NUdf::TDataType<NUdf::TDecimal>::Id: {
1631-
return NUdf::TUnboxedValuePod(NYql::NDecimal::FromHalfs(value.low_128(), value.high_128()));
1670+
auto data = NYql::NDecimal::FromHalfs(value.low_128(), value.high_128());
1671+
auto dataType = static_cast<const TDataType*>(type);
1672+
auto schemeType = dataType->GetSchemeType();
1673+
Y_ENSURE(schemeType == NYql::NProto::TypeIds::Decimal, "Expected decimal type, but found " << schemeType);
1674+
auto decimalType = static_cast<const TDataDecimalType *>(dataType);
1675+
auto params = decimalType->GetParams();
1676+
ui8 precision = params.first;
1677+
if (!IsValidDecimal(precision, data)) {
1678+
throw yexception() << "Invalid Decimal value for precision: " << precision;
1679+
}
1680+
return NUdf::TUnboxedValuePod(data);
16321681
}
16331682
default: {
16341683
throw yexception() << "Unsupported data type: " << dataType->GetSchemeType();
@@ -1794,6 +1843,23 @@ NUdf::TUnboxedValue TProtoImporter::ImportValueFromProto(const TType* type, cons
17941843
}
17951844
}
17961845

1846+
bool IsValidDecimal(ui8 precision, NYql::NDecimal::TInt128 v) {
1847+
if (NYql::NDecimal::IsError(v))
1848+
return false;
1849+
1850+
if (NYql::NDecimal::IsNan(v))
1851+
return true;
1852+
1853+
if (NYql::NDecimal::IsInf(v))
1854+
return true;
1855+
1856+
if (precision >= DecimalBounds.size())
1857+
return false;
1858+
1859+
const auto& db = DecimalBounds[precision];
1860+
return v > db.first && v < db.second;
1861+
}
1862+
17971863
NUdf::TUnboxedValue TProtoImporter::ImportValueFromProto(const TType* type, const NKikimrMiniKQL::TValue& value, const THolderFactory& factory) {
17981864
switch (type->GetKind()) {
17991865
case TType::EKind::Void:

‎ydb/library/mkql_proto/mkql_proto.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ void ExportValueToProto(TType* type, const NUdf::TUnboxedValuePod& value, Ydb::V
2020

2121

2222
TType* ImportTypeFromProto(const NKikimrMiniKQL::TType& type, const TTypeEnvironment& env);
23+
bool IsValidDecimal(ui8 precision, NYql::NDecimal::TInt128 v);
2324

2425
std::pair<TType*, NUdf::TUnboxedValue> ImportValueFromProto(const Ydb::Type& type, const Ydb::Value& value,
2526
const TTypeEnvironment& env, const THolderFactory& factory);

0 commit comments

Comments
 (0)