Skip to content

Commit fcf9368

Browse files
spuchinamikishVPolka
authored
Merge fixes to 24-3-7-hotfix (#8798)
Co-authored-by: amikish <111790456+amikish@users.noreply.github.com> Co-authored-by: VPolka <39378135+VPolka@users.noreply.github.com>
1 parent f51926b commit fcf9368

File tree

16 files changed

+119
-64
lines changed

16 files changed

+119
-64
lines changed

‎ydb/core/kqp/common/simple/query_id.cpp‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
#include <google/protobuf/util/message_differencer.h>
55

66
#include <util/generic/yexception.h>
7+
#include <util/string/escape.h>
78

89
#include <memory>
910

@@ -74,4 +75,25 @@ bool TKqpQueryId::operator==(const TKqpQueryId& other) const {
7475
return true;
7576
}
7677

78+
TString TKqpQueryId::SerializeToString() const {
79+
TStringBuilder result = TStringBuilder() << "{"
80+
<< "Cluster: " << Cluster << ", "
81+
<< "Database: " << Database << ", "
82+
<< "UserSid: " << UserSid << ", "
83+
<< "Text: " << EscapeC(Text) << ", "
84+
<< "Settings: " << Settings.SerializeToString() << ", ";
85+
if (QueryParameterTypes) {
86+
result << "QueryParameterTypes: [";
87+
for (const auto& param : *QueryParameterTypes) {
88+
result << "name: " << param.first << ", type: " << param.second.ShortDebugString();
89+
}
90+
result << "], ";
91+
} else {
92+
result << "QueryParameterTypes: <empty>, ";
93+
}
94+
95+
result << "GUCSettings: " << GUCSettings.SerializeToString() << "}";
96+
return result;
97+
}
98+
7799
} // namespace NKikimr::NKqp

‎ydb/core/kqp/common/simple/query_id.h‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,8 @@ struct TKqpQueryId {
4444
GUCSettings.GetHash());
4545
return THash<decltype(tuple)>()(tuple);
4646
}
47+
48+
TString SerializeToString() const;
4749
};
4850
} // namespace NKikimr::NKqp
4951

‎ydb/core/kqp/common/simple/settings.h‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,9 @@
33
#include <ydb/core/protos/kqp.pb.h>
44
#include <ydb/public/api/protos/ydb_query.pb.h>
55

6+
#include <util/generic/string.h>
67
#include <util/str_stl.h>
8+
#include <util/string/builder.h>
79

810
#include <tuple>
911

@@ -39,6 +41,14 @@ struct TKqpQuerySettings {
3941
auto tuple = std::make_tuple(DocumentApiRestricted, IsInternalCall, QueryType, Syntax);
4042
return THash<decltype(tuple)>()(tuple);
4143
}
44+
45+
TString SerializeToString() const {
46+
TStringBuilder result = TStringBuilder() << "{"
47+
<< "DocumentApiRestricted: " << DocumentApiRestricted << ", "
48+
<< "IsInternalCall: " << IsInternalCall << ", "
49+
<< "QueryType: " << QueryType << "}";
50+
return result;
51+
}
4252
};
4353

4454
} // namespace NKikimr::NKqp

‎ydb/core/kqp/compile_service/kqp_compile_service.cpp‎

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,9 @@ class TKqpQueryCache {
4141
YQL_ENSURE(compileResult->PreparedQuery);
4242

4343
auto queryIt = QueryIndex.emplace(query, compileResult->Uid);
44+
if (!queryIt.second) {
45+
EraseByUid(compileResult->Uid);
46+
}
4447
Y_ENSURE(queryIt.second);
4548
}
4649

@@ -672,6 +675,7 @@ class TKqpCompileService : public TActorBootstrapped<TKqpCompileService> {
672675
Y_ENSURE(query.UserSid == userSid);
673676
}
674677

678+
LOG_DEBUG_S(ctx, NKikimrServices::KQP_COMPILE_SERVICE, "Try to find query by queryId, queryId: " << query.SerializeToString());
675679
auto compileResult = QueryCache.FindByQuery(query, request.KeepInCache);
676680
if (HasTempTablesNameClashes(compileResult, request.TempTablesState)) {
677681
compileResult = nullptr;
@@ -853,7 +857,7 @@ class TKqpCompileService : public TActorBootstrapped<TKqpCompileService> {
853857
try {
854858
if (compileResult->Status == Ydb::StatusIds::SUCCESS) {
855859
if (!hasTempTablesNameClashes) {
856-
UpdateQueryCache(compileResult, keepInCache, compileRequest.CompileSettings.IsQueryActionPrepare, isPerStatementExecution);
860+
UpdateQueryCache(ctx, compileResult, keepInCache, compileRequest.CompileSettings.IsQueryActionPrepare, isPerStatementExecution);
857861
}
858862

859863
if (ev->Get()->ReplayMessage && !QueryReplayBackend->IsNull()) {
@@ -935,15 +939,21 @@ class TKqpCompileService : public TActorBootstrapped<TKqpCompileService> {
935939
return compileResult->PreparedQuery->HasTempTables(tempTablesState, withSessionId);
936940
}
937941

938-
void UpdateQueryCache(TKqpCompileResult::TConstPtr compileResult, bool keepInCache, bool isQueryActionPrepare, bool isPerStatementExecution) {
942+
void UpdateQueryCache(const TActorContext& ctx, TKqpCompileResult::TConstPtr compileResult, bool keepInCache, bool isQueryActionPrepare, bool isPerStatementExecution) {
939943
if (QueryCache.FindByUid(compileResult->Uid, false)) {
940944
QueryCache.Replace(compileResult);
941945
} else if (keepInCache) {
946+
if (compileResult->Query) {
947+
LOG_DEBUG_S(ctx, NKikimrServices::KQP_COMPILE_SERVICE, "Insert query into compile cache, queryId: " << compileResult->Query->SerializeToString());
948+
if (QueryCache.FindByQuery(*compileResult->Query, keepInCache)) {
949+
LOG_ERROR_S(ctx, NKikimrServices::KQP_COMPILE_SERVICE, "Trying to insert query into compile cache when it is already there");
950+
}
951+
}
942952
if (QueryCache.Insert(compileResult, TableServiceConfig.GetEnableAstCache(), isPerStatementExecution)) {
943953
Counters->CompileQueryCacheEvicted->Inc();
944954
}
945955
if (compileResult->Query && isQueryActionPrepare) {
946-
if (InsertPreparingQuery(compileResult, true, isPerStatementExecution)) {
956+
if (InsertPreparingQuery(ctx, compileResult, true, isPerStatementExecution)) {
947957
Counters->CompileQueryCacheEvicted->Inc();
948958
};
949959
}
@@ -954,6 +964,8 @@ class TKqpCompileService : public TActorBootstrapped<TKqpCompileService> {
954964
YQL_ENSURE(queryAst.Ast);
955965
YQL_ENSURE(queryAst.Ast->IsOk());
956966
YQL_ENSURE(queryAst.Ast->Root);
967+
LOG_DEBUG_S(ctx, NKikimrServices::KQP_COMPILE_SERVICE, "Try to find query by ast, queryId: " << compileRequest.Query.SerializeToString()
968+
<< ", ast: " << queryAst.Ast->Root->ToString());
957969
auto compileResult = QueryCache.FindByAst(compileRequest.Query, *queryAst.Ast, compileRequest.CompileSettings.KeepInCache);
958970

959971
if (HasTempTablesNameClashes(compileResult, compileRequest.TempTablesState)) {
@@ -1022,7 +1034,7 @@ class TKqpCompileService : public TActorBootstrapped<TKqpCompileService> {
10221034
}
10231035

10241036
private:
1025-
bool InsertPreparingQuery(const TKqpCompileResult::TConstPtr& compileResult, bool keepInCache, bool isPerStatementExecution) {
1037+
bool InsertPreparingQuery(const TActorContext& ctx, const TKqpCompileResult::TConstPtr& compileResult, bool keepInCache, bool isPerStatementExecution) {
10261038
YQL_ENSURE(compileResult->Query);
10271039
auto query = *compileResult->Query;
10281040

@@ -1047,6 +1059,7 @@ class TKqpCompileService : public TActorBootstrapped<TKqpCompileService> {
10471059
auto newCompileResult = TKqpCompileResult::Make(CreateGuidAsString(), compileResult->Status, compileResult->Issues, compileResult->MaxReadType, std::move(query), compileResult->Ast);
10481060
newCompileResult->AllowCache = compileResult->AllowCache;
10491061
newCompileResult->PreparedQuery = compileResult->PreparedQuery;
1062+
LOG_DEBUG_S(ctx, NKikimrServices::KQP_COMPILE_SERVICE, "Insert preparing query with params, queryId: " << query.SerializeToString());
10501063
return QueryCache.Insert(newCompileResult, TableServiceConfig.GetEnableAstCache(), isPerStatementExecution);
10511064
}
10521065

‎ydb/library/yql/minikql/invoke_builtins/mkql_builtins_mul.cpp‎

Lines changed: 26 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -40,22 +40,23 @@ struct TNumMulInterval {
4040
const auto lv = static_cast<typename TOutput::TLayout>(left.template Get<typename TLeft::TLayout>());
4141
const auto rv = static_cast<typename TOutput::TLayout>(right.template Get<typename TRight::TLayout>());
4242
const auto ret = lv * rv;
43-
if (ret == 0) {
43+
if (rv == 0 || lv == 0) {
4444
return NUdf::TUnboxedValuePod(ret);
4545
}
46+
i64 i64Max = std::numeric_limits<i64>::max();
4647
if constexpr (std::is_same_v<ui64, typename TLeft::TLayout>) {
47-
if (left.Get<ui64>() > static_cast<ui64>(std::numeric_limits<i64>::max())) {
48+
if (left.Get<ui64>() >= static_cast<ui64>(i64Max)) {
4849
return NUdf::TUnboxedValuePod();
4950
}
5051
}
5152
if constexpr (std::is_same_v<ui64, typename TRight::TLayout>) {
52-
if (right.Get<ui64>() > static_cast<ui64>(std::numeric_limits<i64>::max())) {
53+
if (right.Get<ui64>() >= static_cast<ui64>(i64Max)) {
5354
return NUdf::TUnboxedValuePod();
5455
}
5556
}
56-
i64 lvAbs = (lv > 0) ? lv : -lv;
57-
i64 rvAbs = (rv > 0) ? rv : -rv;
58-
if (rvAbs != 0 && (std::numeric_limits<i64>::max() / rvAbs < lvAbs)) {
57+
auto div = i64Max / rv;
58+
auto divAbs = (div >= 0) ? div : -div;
59+
if ((lv >= 0) ? (lv > divAbs) : (lv < -divAbs)) {
5960
return NUdf::TUnboxedValuePod();
6061
}
6162
return IsBadInterval<TOutput>(ret) ? NUdf::TUnboxedValuePod() : NUdf::TUnboxedValuePod(ret);
@@ -68,39 +69,43 @@ struct TNumMulInterval {
6869
const auto bbMain = BasicBlock::Create(context, "bbMain", ctx.Func);
6970
const auto bbDone = BasicBlock::Create(context, "bbDone", ctx.Func);
7071
const auto resultType = Type::getInt128Ty(context);
71-
const auto result = PHINode::Create(resultType, 3, "result", bbDone);
72+
const auto result = PHINode::Create(resultType, 2, "result", bbDone);
7273

7374
const auto lv = GetterFor<typename TLeft::TLayout>(left, context, block);
7475
const auto lhs = StaticCast<typename TLeft::TLayout, i64>(lv, context, block);
7576
const auto rv = GetterFor<typename TRight::TLayout>(right, context, block);
7677
const auto rhs = StaticCast<typename TRight::TLayout, i64>(rv, context, block);
7778
const auto mul = BinaryOperator::CreateMul(lhs, rhs, "mul", block);
7879
const auto zero = ConstantInt::get(Type::getInt64Ty(context), 0);
79-
const auto mulZero = CmpInst::Create(Instruction::ICmp, ICmpInst::ICMP_EQ, mul, zero, "mulZero", block);
80+
const auto lhsZero = CmpInst::Create(Instruction::ICmp, ICmpInst::ICMP_EQ, lhs, zero, "lhsZero", block);
81+
const auto rhsZero = CmpInst::Create(Instruction::ICmp, ICmpInst::ICMP_EQ, rhs, zero, "rhsZero", block);
8082
const auto res = SetterFor<typename TOutput::TLayout>(mul, context, block);
8183

82-
BranchInst::Create(bbDone, bbMain, mulZero, block);
84+
BranchInst::Create(bbDone, bbMain, BinaryOperator::CreateOr(lhsZero, rhsZero, "mulZero", block), block);
8385
result->addIncoming(res, block);
8486

8587
block = bbMain;
8688

87-
const auto lhsAbs = SelectInst::Create(
88-
CmpInst::Create(Instruction::ICmp, ICmpInst::ICMP_SGT, lhs, zero, "lhsPos", block),
89-
lhs,
90-
BinaryOperator::CreateNeg(lhs, "lhsNeg", block),
91-
"lhsAbs", block);
92-
const auto rhsAbs = SelectInst::Create(
93-
CmpInst::Create(Instruction::ICmp, ICmpInst::ICMP_SGT, rhs, zero, "rhsPos", block),
94-
rhs,
95-
BinaryOperator::CreateNeg(rhs, "rhsNeg", block),
96-
"rhsAbs", block);
9789
const auto i64Max = ConstantInt::get(Type::getInt64Ty(context), std::numeric_limits<i64>::max());
98-
const auto div = BinaryOperator::CreateSDiv(i64Max, rhsAbs, "div", block);
99-
const auto mulOverflow = CmpInst::Create(Instruction::ICmp, ICmpInst::ICMP_SGT, lhsAbs, div, "mulOverflow", block);
90+
const auto div = BinaryOperator::CreateSDiv(i64Max, rhs, "div", block);
91+
const auto divAbs = SelectInst::Create(
92+
CmpInst::Create(Instruction::ICmp, ICmpInst::ICMP_SGE, div, zero, "divPos", block),
93+
div,
94+
BinaryOperator::CreateNeg(div, "divNeg", block),
95+
"divAbs", block);
96+
const auto divAbsNeg = BinaryOperator::CreateNeg(divAbs, "divAbsNeg", block);
97+
98+
const auto mulOverflow = SelectInst::Create(
99+
CmpInst::Create(Instruction::ICmp, ICmpInst::ICMP_SGE, lhs, zero, "lhsPos", block),
100+
CmpInst::Create(Instruction::ICmp, ICmpInst::ICMP_SGT, lhs, divAbs, "lhsDiv", block),
101+
CmpInst::Create(Instruction::ICmp, ICmpInst::ICMP_SLT, lhs, divAbsNeg, "lhsDivAbsNeg", block),
102+
"mulOverflow", block);
103+
100104
const auto i64Overflow = BinaryOperator::CreateOr(
101105
GenIsInt64Overflow<typename TLeft::TLayout>(lv, context, block),
102106
GenIsInt64Overflow<typename TRight::TLayout>(rv, context, block),
103107
"i64Overflow", block);
108+
104109
const auto bad = BinaryOperator::CreateOr(
105110
BinaryOperator::CreateOr(i64Overflow, mulOverflow, "overflow", block),
106111
GenIsBadInterval<TOutput>(mul, context, block),

‎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
@@ -405,9 +405,9 @@
405405
],
406406
"test.test[bigdate-arithmetic-default.txt-Debug]": [
407407
{
408-
"checksum": "968be71411b4f41aa78165913bd74333",
409-
"size": 8688,
410-
"uri": "https://{canondata_backend}/1937367/518bbcf510ad7a43c5e77746bafd21ed0e3fdc6e/resource.tar.gz#test.test_bigdate-arithmetic-default.txt-Debug_/opt.yql_patched"
408+
"checksum": "1a5cecc5f3f3d734d4eef23e12f6725b",
409+
"size": 8887,
410+
"uri": "https://{canondata_backend}/1899731/2ec8224db091f2a7362c5e4ce595bc50329b8311/resource.tar.gz#test.test_bigdate-arithmetic-default.txt-Debug_/opt.yql_patched"
411411
}
412412
],
413413
"test.test[bigdate-arithmetic-default.txt-Plan]": [

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -531,9 +531,9 @@
531531
],
532532
"test.test[bigdate-table_int_cast-default.txt-Debug]": [
533533
{
534-
"checksum": "1d672f23cc038dc42d14175121554c71",
534+
"checksum": "0f242de0900368c56d1b937487ced6cb",
535535
"size": 8617,
536-
"uri": "https://{canondata_backend}/1924537/907e79379e1e72f9d09545e57f65dee63f42dbfe/resource.tar.gz#test.test_bigdate-table_int_cast-default.txt-Debug_/opt.yql_patched"
536+
"uri": "https://{canondata_backend}/1809005/df0d5940a3b3a38ba468a035aba7ce54440f0891/resource.tar.gz#test.test_bigdate-table_int_cast-default.txt-Debug_/opt.yql_patched"
537537
}
538538
],
539539
"test.test[bigdate-table_int_cast-default.txt-Plan]": [

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -457,9 +457,9 @@
457457
],
458458
"test.test[bigdate-table_arithmetic_mul_div-default.txt-Debug]": [
459459
{
460-
"checksum": "7d3363f2bc50bd3ecf9097b47926a9e4",
460+
"checksum": "11e7ab9520da2bde231d70dbb834e16a",
461461
"size": 9840,
462-
"uri": "https://{canondata_backend}/1936842/e2f5b27b418549665a04de58de4b4e487f33c292/resource.tar.gz#test.test_bigdate-table_arithmetic_mul_div-default.txt-Debug_/opt.yql_patched"
462+
"uri": "https://{canondata_backend}/1937424/022b4c4aaf443124c76bb3e388177d9b3de00044/resource.tar.gz#test.test_bigdate-table_arithmetic_mul_div-default.txt-Debug_/opt.yql_patched"
463463
}
464464
],
465465
"test.test[bigdate-table_arithmetic_mul_div-default.txt-Plan]": [

‎ydb/library/yql/tests/sql/hybrid_file/part2/canondata/result.json‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -435,9 +435,9 @@
435435
],
436436
"test.test[bigdate-table_int_cast-default.txt-Debug]": [
437437
{
438-
"checksum": "177fc2fac903bdde7623c6ad50195b2e",
439-
"size": 22146,
440-
"uri": "https://{canondata_backend}/1784826/45c8e56b489fcd70fc011cedb88a592ff2ca27ed/resource.tar.gz#test.test_bigdate-table_int_cast-default.txt-Debug_/opt.yql_patched"
438+
"checksum": "2255e5c0c361ed4e31289580bb6a402c",
439+
"size": 18588,
440+
"uri": "https://{canondata_backend}/1775059/b57c9040709a7b012953cf170d04a292adc8d3d3/resource.tar.gz#test.test_bigdate-table_int_cast-default.txt-Debug_/opt.yql_patched"
441441
}
442442
],
443443
"test.test[bigdate-table_int_cast-default.txt-Plan]": [

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

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -477,9 +477,9 @@
477477
],
478478
"test.test[bigdate-arithmetic-default.txt-Debug]": [
479479
{
480-
"checksum": "74f20089d55fea37e0e7cdee17f0cbd8",
481-
"size": 8687,
482-
"uri": "https://{canondata_backend}/1784117/3885f0a76b64a32a48487f8866602d3fff1e416a/resource.tar.gz#test.test_bigdate-arithmetic-default.txt-Debug_/opt.yql_patched"
480+
"checksum": "c1939c3925f1c9e2d27b33692637bc61",
481+
"size": 8886,
482+
"uri": "https://{canondata_backend}/1889210/e26b9c7fc72b580fe82c1126f535456e73306c2c/resource.tar.gz#test.test_bigdate-arithmetic-default.txt-Debug_/opt.yql_patched"
483483
}
484484
],
485485
"test.test[bigdate-arithmetic-default.txt-Plan]": [
@@ -519,9 +519,9 @@
519519
],
520520
"test.test[bigdate-table_arithmetic_mul_div-default.txt-Debug]": [
521521
{
522-
"checksum": "48b58158564d3dd01c0d41b8dded6604",
523-
"size": 30153,
524-
"uri": "https://{canondata_backend}/1925842/7ff9f7f6f9fd763e965f539592431555505139a0/resource.tar.gz#test.test_bigdate-table_arithmetic_mul_div-default.txt-Debug_/opt.yql_patched"
522+
"checksum": "7f460bd132529e2aee95b0bcb5b608b7",
523+
"size": 28216,
524+
"uri": "https://{canondata_backend}/1817427/46729b354b9b15ea89f67bf14fefd2face8b402b/resource.tar.gz#test.test_bigdate-table_arithmetic_mul_div-default.txt-Debug_/opt.yql_patched"
525525
}
526526
],
527527
"test.test[bigdate-table_arithmetic_mul_div-default.txt-Plan]": [

0 commit comments

Comments
 (0)