Skip to content

Commit 8a1f080

Browse files
nepalnepal
authored
[YQL-18098] Fix type annotation of Aggregate with non-null default value in AggregationTraits (#4215)
1 parent 82b202c commit 8a1f080

File tree

31 files changed

+385
-102
lines changed

31 files changed

+385
-102
lines changed

‎ydb/library/yql/core/type_ann/type_ann_list.cpp‎

Lines changed: 48 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -614,16 +614,37 @@ namespace {
614614
return IGraphTransformer::TStatus::Error;
615615
}
616616

617-
auto finishItemType = RemoveOptionalType(&finishType);
618-
if (finishItemType->GetKind() != ETypeAnnotationKind::Null) {
619-
auto arg = ctx.Expr.NewArgument(defaultValue->Pos(), "arg");
620-
auto status = TrySilentConvertTo(arg, *defaultValue->GetTypeAnn(), finishType, ctx.Expr);
621-
if (status == IGraphTransformer::TStatus::Error) {
622-
ctx.Expr.AddError(TIssue(ctx.Expr.GetPosition(defaultValue->Pos()),
623-
TStringBuilder() << "Uncompatible types of default value and " << finishName
624-
<< " : " << *defaultValue->GetTypeAnn() << " != " << finishType));
625-
return status;
626-
}
617+
// defaultValue should have such type that expression (<value-of-finishType> ?? defaultValue) is the correct one
618+
const auto leftType = &finishType;
619+
const auto rightType = defaultValue->GetTypeAnn();
620+
621+
auto leftItemType = leftType;
622+
if (leftType->GetKind() == ETypeAnnotationKind::Optional) {
623+
leftItemType = leftType->Cast<TOptionalExprType>()->GetItemType();
624+
}
625+
626+
auto rightItemType = rightType;
627+
if (leftType->GetKind() != ETypeAnnotationKind::Optional &&
628+
rightType->GetKind() == ETypeAnnotationKind::Optional) {
629+
rightItemType = rightType->Cast<TOptionalExprType>()->GetItemType();
630+
}
631+
632+
TExprNode::TPtr arg1 = ctx.Expr.NewArgument(input->Pos(), "arg1");
633+
TExprNode::TPtr arg2 = defaultValue;
634+
auto convertedArg2 = arg2;
635+
const TTypeAnnotationNode* commonItemType = nullptr;
636+
auto status = SilentInferCommonType(convertedArg2, *rightItemType, arg1, *leftItemType, ctx.Expr, commonItemType,
637+
TConvertFlags().Set(NConvertFlags::AllowUnsafeConvert));
638+
if (status == IGraphTransformer::TStatus::Error) {
639+
ctx.Expr.AddError(TIssue(ctx.Expr.GetPosition(defaultValue->Pos()),
640+
TStringBuilder() << "Uncompatible types of default value and " << finishName
641+
<< " : " << *defaultValue->GetTypeAnn() << " vs " << finishType));
642+
return status;
643+
}
644+
645+
if (arg2 != convertedArg2) {
646+
output = ctx.Expr.ChangeChild(*input, defaultValueIndex, std::move(convertedArg2));
647+
return IGraphTransformer::TStatus::Repeat;
627648
}
628649
}
629650

@@ -4983,35 +5004,26 @@ namespace {
49835004
!item->IsOptionalOrNull() ? ctx.Expr.MakeType<TOptionalExprType>(item) : item));
49845005
}
49855006
} else {
4986-
const TTypeAnnotationNode* defValType;
4987-
bool isDefNull;
4988-
if (isTraits) {
4989-
auto defVal = child->Child(1)->Child(7);
4990-
isDefNull = defVal->IsCallable("Null");
4991-
defValType = defVal->GetTypeAnn();
4992-
} else {
4993-
auto name = child->Child(1)->Child(0)->Content();
4994-
if (name == "count" || name == "count_all") {
4995-
isDefNull = false;
4996-
defValType = ctx.Expr.MakeType<TDataExprType>(EDataSlot::Uint64);
5007+
if (input->Child(1)->ChildrenSize() == 0 && !isHopping) {
5008+
if (isTraits) {
5009+
// need to apply default value
5010+
auto defVal = child->Child(1)->ChildPtr(7);
5011+
if (!defVal->IsCallable("Null")) {
5012+
finishType = defVal->GetTypeAnn();
5013+
} else if (!finishType->IsOptionalOrNull()) {
5014+
finishType = ctx.Expr.MakeType<TOptionalExprType>(finishType);
5015+
}
49975016
} else {
4998-
isDefNull = true;
4999-
defValType = ctx.Expr.MakeType<TNullExprType>();
5000-
}
5001-
}
5002-
5003-
if (isDefNull && !isOptional && !isHopping && input->Child(1)->ChildrenSize() == 0) {
5004-
if (finishType->GetKind() != ETypeAnnotationKind::Null &&
5005-
finishType->GetKind() != ETypeAnnotationKind::Pg) {
5006-
finishType = ctx.Expr.MakeType<TOptionalExprType>(finishType);
5017+
auto name = child->Child(1)->Child(0)->Content();
5018+
if ((name == "count" || name == "count_all")) {
5019+
if (isOptional) {
5020+
finishType = finishType->Cast<TOptionalExprType>()->GetItemType();
5021+
}
5022+
} else if (!finishType->IsOptionalOrNull()) {
5023+
finishType = ctx.Expr.MakeType<TOptionalExprType>(finishType);
5024+
}
50075025
}
5008-
} else if (!isDefNull && defValType->GetKind() != ETypeAnnotationKind::Optional
5009-
&& finishType->GetKind() == ETypeAnnotationKind::Optional) {
5010-
finishType = finishType->Cast<TOptionalExprType>()->GetItemType();
5011-
} else if (!isDefNull && finishType->GetKind() == ETypeAnnotationKind::Null && input->Child(1)->ChildrenSize() == 0) {
5012-
finishType = defValType;
50135026
}
5014-
50155027
rowColumns.push_back(ctx.Expr.MakeType<TItemExprType>(child->Head().Content(), finishType));
50165028
}
50175029
} else if (suffix == "Combine" || suffix == "CombineState" || suffix == "MergeState") {

‎ydb/library/yql/tests/s-expressions/yt_native_file/part3/canondata/result.json‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6255,9 +6255,9 @@
62556255
],
62566256
"test.test[Udf-TopFreq-Debug]": [
62576257
{
6258-
"checksum": "0165fc4b55d06481e09e222eeb2ae12f",
6259-
"size": 5573,
6260-
"uri": "https://{canondata_backend}/1937429/ee1dc97fdb7ce980999482311c9b3c21628733a4/resource.tar.gz#test.test_Udf-TopFreq-Debug_/opt.yql"
6258+
"checksum": "02807cf360c2310bd107a9df22d1cf6c",
6259+
"size": 5603,
6260+
"uri": "https://{canondata_backend}/1889210/5c38be307a1bcc56586511ac7eac4e75066ff5f6/resource.tar.gz#test.test_Udf-TopFreq-Debug_/opt.yql"
62616261
}
62626262
],
62636263
"test.test[Udf-TopFreq-Plan]": [

‎ydb/library/yql/tests/sql/dq_file/part0/canondata/result.json‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3277,9 +3277,9 @@
32773277
],
32783278
"test.test[window-generic/aggregations_mixed--Debug]": [
32793279
{
3280-
"checksum": "e3e237f950d5e6e5b8970dfa289bb409",
3281-
"size": 7046,
3282-
"uri": "https://{canondata_backend}/1942100/a4c7af0cfa6de91d520eb74fb8af1e64b13ecf0a/resource.tar.gz#test.test_window-generic_aggregations_mixed--Debug_/opt.yql_patched"
3280+
"checksum": "2ada042808b40662c30bf5a81533d767",
3281+
"size": 7148,
3282+
"uri": "https://{canondata_backend}/1130705/7dd3fbb351b6ea2ab8b0736da59f105c9241fd78/resource.tar.gz#test.test_window-generic_aggregations_mixed--Debug_/opt.yql_patched"
32833283
}
32843284
],
32853285
"test.test[window-generic/aggregations_mixed--Plan]": [

‎ydb/library/yql/tests/sql/dq_file/part10/canondata/result.json‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -190,9 +190,9 @@
190190
],
191191
"test.test[aggr_factory-bottom-default.txt-Debug]": [
192192
{
193-
"checksum": "feb82529e74c587eafff41b1bc7f0a5d",
194-
"size": 7377,
195-
"uri": "https://{canondata_backend}/1031349/6c70521322fc43f752ef6b89f8667fefd006af8b/resource.tar.gz#test.test_aggr_factory-bottom-default.txt-Debug_/opt.yql_patched"
193+
"checksum": "b1258866995ee0c237856f6fcd19cd80",
194+
"size": 7385,
195+
"uri": "https://{canondata_backend}/1809005/a1fd498781ad8b3801c9a55d0bd7d614dc85bb93/resource.tar.gz#test.test_aggr_factory-bottom-default.txt-Debug_/opt.yql_patched"
196196
}
197197
],
198198
"test.test[aggr_factory-bottom-default.txt-Plan]": [

‎ydb/library/yql/tests/sql/dq_file/part12/canondata/result.json‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -177,9 +177,9 @@
177177
],
178178
"test.test[aggr_factory-mode-default.txt-Debug]": [
179179
{
180-
"checksum": "251729805f597fafb1a3d50b19792bb0",
181-
"size": 8480,
182-
"uri": "https://{canondata_backend}/1942173/50b4ae48e906d86b27ee0b68ed5a08b5ad6bf50e/resource.tar.gz#test.test_aggr_factory-mode-default.txt-Debug_/opt.yql_patched"
180+
"checksum": "b4a62a7a804be70149ab082c37b922ca",
181+
"size": 8464,
182+
"uri": "https://{canondata_backend}/1130705/ef53df0087abe70482ce7b2a8f02692fa114faa8/resource.tar.gz#test.test_aggr_factory-mode-default.txt-Debug_/opt.yql_patched"
183183
}
184184
],
185185
"test.test[aggr_factory-mode-default.txt-Plan]": [

‎ydb/library/yql/tests/sql/dq_file/part15/canondata/result.json‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -281,9 +281,9 @@
281281
],
282282
"test.test[aggr_factory-top-default.txt-Debug]": [
283283
{
284-
"checksum": "a3ee40a7333056081eadeb60d9839797",
285-
"size": 7376,
286-
"uri": "https://{canondata_backend}/1925842/a4b71373097359ba466e2713f3de746df8a53ab1/resource.tar.gz#test.test_aggr_factory-top-default.txt-Debug_/opt.yql_patched"
284+
"checksum": "31b55bd53878eb318566a97877e62fc0",
285+
"size": 7384,
286+
"uri": "https://{canondata_backend}/1903280/46af5dd813e524044963c92a5f2ce6250ce88cd1/resource.tar.gz#test.test_aggr_factory-top-default.txt-Debug_/opt.yql_patched"
287287
}
288288
],
289289
"test.test[aggr_factory-top-default.txt-Plan]": [

‎ydb/library/yql/tests/sql/dq_file/part16/canondata/result.json‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2819,9 +2819,9 @@
28192819
],
28202820
"test.test[window-lagging/aggregations--Debug]": [
28212821
{
2822-
"checksum": "5219b7bb2217832aaea18c7e2c690a65",
2823-
"size": 6819,
2824-
"uri": "https://{canondata_backend}/1599023/6ea95a71ae6e3995d639ef495d263a106e521882/resource.tar.gz#test.test_window-lagging_aggregations--Debug_/opt.yql_patched"
2822+
"checksum": "4669056e08b2ee4de4de8f0522ddc9d8",
2823+
"size": 6877,
2824+
"uri": "https://{canondata_backend}/1809005/6204dd19a10bd9bc701faf558e7ce789c91e22ff/resource.tar.gz#test.test_window-lagging_aggregations--Debug_/opt.yql_patched"
28252825
}
28262826
],
28272827
"test.test[window-lagging/aggregations--Plan]": [

‎ydb/library/yql/tests/sql/dq_file/part3/canondata/result.json‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2578,9 +2578,9 @@
25782578
],
25792579
"test.test[window-generic/aggregations_before_current--Debug]": [
25802580
{
2581-
"checksum": "9c85297b4224c3080023af43abfe8a40",
2582-
"size": 7203,
2583-
"uri": "https://{canondata_backend}/1031349/d86a7eaf6f5bc2cdaedba52c0890601b8cc1d981/resource.tar.gz#test.test_window-generic_aggregations_before_current--Debug_/opt.yql_patched"
2581+
"checksum": "7cbd7e87b710e5169c98f1ed5cedd003",
2582+
"size": 7259,
2583+
"uri": "https://{canondata_backend}/1809005/b5fd98a03b02a29344a248fc7017d3e834d02658/resource.tar.gz#test.test_window-generic_aggregations_before_current--Debug_/opt.yql_patched"
25842584
}
25852585
],
25862586
"test.test[window-generic/aggregations_before_current--Plan]": [

‎ydb/library/yql/tests/sql/dq_file/part5/canondata/result.json‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2934,9 +2934,9 @@
29342934
],
29352935
"test.test[window-generic/aggregations_after_current--Debug]": [
29362936
{
2937-
"checksum": "5f9f9e4572a3a2d1918a6bd7495d1058",
2938-
"size": 7233,
2939-
"uri": "https://{canondata_backend}/1937001/3df1bf80f5738c3f0205526961db8957f75fdaea/resource.tar.gz#test.test_window-generic_aggregations_after_current--Debug_/opt.yql_patched"
2937+
"checksum": "2e87e119d176d86d6373aee6b8515655",
2938+
"size": 7289,
2939+
"uri": "https://{canondata_backend}/1903280/0db3cff29ce26bdeebe43f7c9bad950e873a4e5d/resource.tar.gz#test.test_window-generic_aggregations_after_current--Debug_/opt.yql_patched"
29402940
}
29412941
],
29422942
"test.test[window-generic/aggregations_after_current--Plan]": [

‎ydb/library/yql/tests/sql/dq_file/part7/canondata/result.json‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -175,6 +175,28 @@
175175
}
176176
],
177177
"test.test[agg_apply-min-default.txt-Results]": [],
178+
"test.test[aggr_factory-def_value_with_keys-default.txt-Analyze]": [
179+
{
180+
"checksum": "b4dd508a329723c74293d80f0278c705",
181+
"size": 505,
182+
"uri": "https://{canondata_backend}/1889210/25bb12516fb50fd6341f375d4bd251cc1316e0aa/resource.tar.gz#test.test_aggr_factory-def_value_with_keys-default.txt-Analyze_/plan.txt"
183+
}
184+
],
185+
"test.test[aggr_factory-def_value_with_keys-default.txt-Debug]": [
186+
{
187+
"checksum": "2ab470351943b187ad5610f599b37e0d",
188+
"size": 1480,
189+
"uri": "https://{canondata_backend}/1889210/25bb12516fb50fd6341f375d4bd251cc1316e0aa/resource.tar.gz#test.test_aggr_factory-def_value_with_keys-default.txt-Debug_/opt.yql_patched"
190+
}
191+
],
192+
"test.test[aggr_factory-def_value_with_keys-default.txt-Plan]": [
193+
{
194+
"checksum": "b4dd508a329723c74293d80f0278c705",
195+
"size": 505,
196+
"uri": "https://{canondata_backend}/1889210/25bb12516fb50fd6341f375d4bd251cc1316e0aa/resource.tar.gz#test.test_aggr_factory-def_value_with_keys-default.txt-Plan_/plan.txt"
197+
}
198+
],
199+
"test.test[aggr_factory-def_value_with_keys-default.txt-Results]": [],
178200
"test.test[aggregate-aggregate_key_column-default.txt-Analyze]": [
179201
{
180202
"checksum": "e447076882c3384f28df0db8089de406",

0 commit comments

Comments
 (0)