Skip to content

Commit f97111f

Browse files
shnikdshnikd
committed
Fixes
1 parent 399be5e commit f97111f

11 files changed

+115
-119
lines changed

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

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,4 +2,21 @@
22

33
namespace NKikimr::NKqp {
44

5+
THashMap<TString, TKqpTempTablesState::TTempTableInfo>::const_iterator
6+
TKqpTempTablesState::FindInfo(const std::string_view& path, bool withSessionId) const {
7+
if (!withSessionId) {
8+
return TempTables.find(path);
9+
}
10+
11+
if (path.size() < SessionId.size()) {
12+
return TempTables.end();
13+
}
14+
size_t pos = path.size() - SessionId.size();
15+
if (path.substr(pos) != SessionId) {
16+
return TempTables.end();
17+
}
18+
19+
return TempTables.find(path.substr(0, pos));
20+
}
21+
522
} // namespace NKikimr::NKqp

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

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
#include <ydb/library/aclib/aclib.h>
44

55
#include <optional>
6+
#include <string_view>
67

78
#include <util/generic/fwd.h>
89
#include <util/generic/hash.h>
@@ -14,14 +15,15 @@ struct TKqpTempTablesState {
1415
struct TTempTableInfo {
1516
TString Name;
1617
TString WorkingDir;
17-
TString Database;
1818
TIntrusiveConstPtr<NACLib::TUserToken> UserToken;
19-
TString Cluster;
2019
};
21-
std::optional<TString> SessionId;
22-
THashMap<std::pair<TString, TString>, TTempTableInfo> TempTables;
20+
TString SessionId;
21+
THashMap<TString, TTempTableInfo> TempTables;
2322

2423
using TConstPtr = std::shared_ptr<const TKqpTempTablesState>;
24+
25+
THashMap<TString, TTempTableInfo>::const_iterator
26+
FindInfo(const std::string_view& path, bool withSessionId = false) const;
2527
};
2628

2729
} // namespace NKikimr::NKqp

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

Lines changed: 21 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -567,7 +567,9 @@ class TKqpCompileService : public TActorBootstrapped<TKqpCompileService> {
567567
Counters->ReportCompileRequestGet(dbCounters);
568568

569569
auto compileResult = QueryCache.FindByUid(*request.Uid, request.KeepInCache);
570-
compileResult = WithCache(std::move(compileResult), request.TempTablesState);
570+
if (HasTempTablesNameClashes(compileResult, request.TempTablesState)) {
571+
compileResult = nullptr;
572+
}
571573
if (compileResult) {
572574
Y_ENSURE(compileResult->Query);
573575
if (compileResult->Query->UserSid == userSid) {
@@ -611,7 +613,9 @@ class TKqpCompileService : public TActorBootstrapped<TKqpCompileService> {
611613
}
612614

613615
auto compileResult = QueryCache.FindByQuery(query, request.KeepInCache);
614-
compileResult = WithCache(std::move(compileResult), request.TempTablesState);
616+
if (HasTempTablesNameClashes(compileResult, request.TempTablesState)) {
617+
compileResult = nullptr;
618+
}
615619

616620
if (compileResult) {
617621
Counters->ReportQueryCacheHit(dbCounters, true);
@@ -676,7 +680,9 @@ class TKqpCompileService : public TActorBootstrapped<TKqpCompileService> {
676680
Counters->ReportRecompileRequestGet(dbCounters);
677681

678682
TKqpCompileResult::TConstPtr compileResult = QueryCache.FindByUid(request.Uid, false);
679-
compileResult = WithCache(std::move(compileResult), request.TempTablesState);
683+
if (HasTempTablesNameClashes(compileResult, request.TempTablesState)) {
684+
compileResult = nullptr;
685+
}
680686

681687
if (compileResult || request.Query) {
682688
Counters->ReportCompileRequestCompile(dbCounters);
@@ -741,11 +747,11 @@ class TKqpCompileService : public TActorBootstrapped<TKqpCompileService> {
741747

742748
bool keepInCache = compileRequest.KeepInCache && compileResult->AllowCache;
743749

744-
bool hasTempTables = WithCache(compileResult, compileRequest.TempTablesState, true) == nullptr;
750+
bool hasTempTablesNameClashes = HasTempTablesNameClashes(compileResult, compileRequest.TempTablesState, true);
745751

746752
try {
747753
if (compileResult->Status == Ydb::StatusIds::SUCCESS) {
748-
if (!hasTempTables) {
754+
if (!hasTempTablesNameClashes) {
749755
UpdateQueryCache(compileResult, keepInCache);
750756
}
751757

@@ -760,7 +766,7 @@ class TKqpCompileService : public TActorBootstrapped<TKqpCompileService> {
760766
request.Cookie, std::move(request.Orbit), std::move(request.CompileServiceSpan), (CollectDiagnostics ? ev->Get()->ReplayMessageUserView : std::nullopt));
761767
}
762768
} else {
763-
if (!hasTempTables) {
769+
if (!hasTempTablesNameClashes) {
764770
if (QueryCache.FindByUid(compileResult->Uid, false)) {
765771
QueryCache.EraseByUid(compileResult->Uid);
766772
}
@@ -814,36 +820,17 @@ class TKqpCompileService : public TActorBootstrapped<TKqpCompileService> {
814820
StartCheckQueriesTtlTimer();
815821
}
816822

817-
TKqpCompileResult::TConstPtr WithCache(
823+
bool HasTempTablesNameClashes(
818824
TKqpCompileResult::TConstPtr compileResult,
819-
TKqpTempTablesState::TConstPtr tempTablesState, bool forInsert = false) {
825+
TKqpTempTablesState::TConstPtr tempTablesState, bool withSessionId = false) {
820826
if (!compileResult) {
821-
return nullptr;
827+
return false;
822828
}
823829
if (!compileResult->PreparedQuery) {
824-
return compileResult;
825-
}
826-
if (forInsert) {
827-
auto hasTempTables = compileResult->PreparedQuery->HasTempTables(tempTablesState);
828-
if (hasTempTables) {
829-
return nullptr;
830-
}
831-
return compileResult;
832-
}
833-
if (!tempTablesState) {
834-
return compileResult;
835-
}
836-
auto tables = compileResult->PreparedQuery->GetQueryTables();
837-
auto tempTables = THashSet<TString>();
838-
for (const auto& [path, info] : tempTablesState->TempTables) {
839-
tempTables.insert(path.second);
840-
}
841-
for (const auto& path: tables) {
842-
if (tempTables.contains(path)) {
843-
return nullptr;
844-
}
830+
return false;
845831
}
846-
return compileResult;
832+
833+
return compileResult->PreparedQuery->HasTempTables(tempTablesState, withSessionId);
847834
}
848835

849836
void UpdateQueryCache(TKqpCompileResult::TConstPtr compileResult, bool keepInCache) {
@@ -867,7 +854,9 @@ class TKqpCompileService : public TActorBootstrapped<TKqpCompileService> {
867854
auto compileRequest = RequestsQueue.FinishActiveRequest(query);
868855
if (parseResult && parseResult->Ast->IsOk()) {
869856
auto compileResult = QueryCache.FindByAst(query, *parseResult->Ast, compileRequest.KeepInCache);
870-
compileResult = WithCache(std::move(compileResult), compileRequest.TempTablesState);
857+
if (HasTempTablesNameClashes(compileResult, compileRequest.TempTablesState)) {
858+
compileResult = nullptr;
859+
}
871860
if (compileResult) {
872861
Counters->ReportQueryCacheHit(compileRequest.DbCounters, true);
873862

‎ydb/core/kqp/gateway/kqp_metadata_loader.cpp

Lines changed: 8 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -30,16 +30,16 @@ struct NavigateEntryResult {
3030
std::optional<TString> QueryName;
3131
};
3232

33-
NavigateEntryResult CreateNavigateEntry(const TString& cluster, const TString& path,
34-
const NYql::IKikimrGateway::TLoadTableMetadataSettings& settings, TKqpTempTablesState::TConstPtr tempTablesState = nullptr) {
33+
NavigateEntryResult CreateNavigateEntry(const TString& path,
34+
const NYql::IKikimrGateway::TLoadTableMetadataSettings& settings, TKqpTempTablesState::TConstPtr tempTablesState = nullptr) {
3535
TNavigate::TEntry entry;
3636
TString currentPath = path;
3737
std::optional<TString> queryName = std::nullopt;
3838
if (tempTablesState) {
39-
auto tempTablesIt = tempTablesState->TempTables.find(std::make_pair(cluster, currentPath));
40-
if (tempTablesState->SessionId && tempTablesIt != tempTablesState->TempTables.end()) {
39+
auto tempTablesInfoIt = tempTablesState->FindInfo(currentPath, false);
40+
if (tempTablesInfoIt != tempTablesState->TempTables.end()) {
4141
queryName = currentPath;
42-
currentPath = currentPath + *tempTablesState->SessionId;
42+
currentPath = currentPath + tempTablesState->SessionId;
4343
}
4444
}
4545
entry.Path = SplitPath(currentPath);
@@ -50,10 +50,8 @@ NavigateEntryResult CreateNavigateEntry(const TString& cluster, const TString& p
5050
return {entry, currentPath, queryName};
5151
}
5252

53-
NavigateEntryResult CreateNavigateEntry(const TString& cluster,
54-
const std::pair<TIndexId, TString>& pair,
53+
NavigateEntryResult CreateNavigateEntry(const std::pair<TIndexId, TString>& pair,
5554
const NYql::IKikimrGateway::TLoadTableMetadataSettings& settings, TKqpTempTablesState::TConstPtr tempTablesState = nullptr) {
56-
Y_UNUSED(cluster);
5755
Y_UNUSED(tempTablesState);
5856

5957
TNavigate::TEntry entry;
@@ -671,8 +669,8 @@ NThreading::TFuture<TTableMetadataResult> TKqpTableMetadataLoader::LoadTableMeta
671669

672670
const auto externalEntryItem = CreateNavigateExternalEntry(id, settings.WithExternalDatasources_);
673671
Y_ABORT_UNLESS(!settings.WithExternalDatasources_ || externalEntryItem, "External data source must be resolved using path only");
674-
auto resNavigate = settings.WithExternalDatasources_ ? *externalEntryItem : CreateNavigateEntry(cluster,
675-
id, settings, TempTablesState);
672+
auto resNavigate = settings.WithExternalDatasources_ ? *externalEntryItem : CreateNavigateEntry(id,
673+
settings, TempTablesState);
676674
const auto entry = resNavigate.Entry;
677675
const auto queryName = resNavigate.QueryName;
678676
const auto externalEntry = settings.WithExternalDatasources_ ? std::optional<NavigateEntryResult>{} : externalEntryItem;

‎ydb/core/kqp/provider/yql_kikimr_provider.cpp

Lines changed: 12 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -121,11 +121,11 @@ struct TKikimrData {
121121
const TKikimrTableDescription* TKikimrTablesData::EnsureTableExists(const TString& cluster,
122122
const TString& table, TPositionHandle pos, TExprContext& ctx) const
123123
{
124-
auto tempTable = TempTables.FindPtr(table);
124+
auto tempTableInfoIt = TempTablesState->FindInfo(table, true);
125125

126126
auto tablePath = table;
127-
if (tempTable) {
128-
tablePath = *tempTable;
127+
if (tempTableInfoIt != TempTablesState->TempTables.end()) {
128+
tablePath = tempTableInfoIt->first;
129129
}
130130

131131
auto desc = Tables.FindPtr(std::make_pair(cluster, tablePath));
@@ -141,11 +141,11 @@ const TKikimrTableDescription* TKikimrTablesData::EnsureTableExists(const TStrin
141141
}
142142

143143
TKikimrTableDescription& TKikimrTablesData::GetOrAddTable(const TString& cluster, const TString& database, const TString& table, ETableType tableType) {
144-
auto tempTable = TempTables.FindPtr(table);
144+
auto tempTableInfoIt = TempTablesState->FindInfo(table, true);
145145

146146
auto tablePath = table;
147-
if (tempTable) {
148-
tablePath = *tempTable;
147+
if (tempTableInfoIt != TempTablesState->TempTables.end()) {
148+
tablePath = tempTableInfoIt->first;
149149
}
150150

151151
if (!Tables.FindPtr(std::make_pair(cluster, tablePath))) {
@@ -165,11 +165,11 @@ TKikimrTableDescription& TKikimrTablesData::GetOrAddTable(const TString& cluster
165165
}
166166

167167
TKikimrTableDescription& TKikimrTablesData::GetTable(const TString& cluster, const TString& table) {
168-
auto tempTable = TempTables.FindPtr(table);
168+
auto tempTableInfoIt = TempTablesState->FindInfo(table, true);
169169

170170
auto tablePath = table;
171-
if (tempTable) {
172-
tablePath = *tempTable;
171+
if (tempTableInfoIt != TempTablesState->TempTables.end()) {
172+
tablePath = tempTableInfoIt->first;
173173
}
174174

175175
auto desc = Tables.FindPtr(std::make_pair(cluster, tablePath));
@@ -181,12 +181,11 @@ TKikimrTableDescription& TKikimrTablesData::GetTable(const TString& cluster, con
181181
const TKikimrTableDescription& TKikimrTablesData::ExistingTable(const TStringBuf& cluster,
182182
const TStringBuf& table) const
183183
{
184-
auto tempTable = TempTables.FindPtr(table);
184+
auto tempTableInfoIt = TempTablesState->FindInfo(table, true);
185185

186186
auto tablePath = table;
187-
188-
if (tempTable) {
189-
tablePath = *tempTable;
187+
if (tempTableInfoIt != TempTablesState->TempTables.end()) {
188+
tablePath = tempTableInfoIt->first;
190189
}
191190

192191
auto desc = Tables.FindPtr(std::make_pair(TString(cluster), TString(tablePath)));

‎ydb/core/kqp/provider/yql_kikimr_provider.h

Lines changed: 9 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -200,16 +200,12 @@ class TKikimrTablesData : public TThrRefBase {
200200
}
201201

202202
void SetTempTables(NKikimr::NKqp::TKqpTempTablesState::TConstPtr tempTablesState) {
203-
if (tempTablesState) {
204-
for (const auto& [path, info] : tempTablesState->TempTables) {
205-
TempTables[path.second + *tempTablesState->SessionId] = path.second;
206-
}
207-
}
203+
TempTablesState = std::move(tempTablesState);
208204
}
209205

210206
private:
211207
THashMap<std::pair<TString, TString>, TKikimrTableDescription> Tables;
212-
THashMap<TString, TString> TempTables;
208+
NKikimr::NKqp::TKqpTempTablesState::TConstPtr TempTablesState;
213209
};
214210

215211
enum class TYdbOperation : ui32 {
@@ -288,11 +284,7 @@ class TKikimrTransactionContextBase : public TThrRefBase {
288284
}
289285

290286
void SetTempTables(NKikimr::NKqp::TKqpTempTablesState::TConstPtr tempTablesState) {
291-
if (tempTablesState) {
292-
for (const auto& [path, info] : tempTablesState->TempTables) {
293-
TempTables[path.second] = path.second + *tempTablesState->SessionId;
294-
}
295-
}
287+
TempTablesState = std::move(tempTablesState);
296288
}
297289

298290
template<class IterableKqpTableOps, class IterableKqpTableInfos>
@@ -327,9 +319,10 @@ class TKikimrTransactionContextBase : public TThrRefBase {
327319
auto newOp = TYdbOperation(op.GetOperation());
328320
TPosition pos(op.GetPosition().GetColumn(), op.GetPosition().GetRow());
329321

330-
auto tempTable = TempTables.FindPtr(table);
331-
if (tempTable) {
332-
table = *tempTable;
322+
auto tempTableInfoIt = TempTablesState->FindInfo(table, false);
323+
324+
if (tempTableInfoIt != TempTablesState->TempTables.end()) {
325+
table = tempTableInfoIt->first + TempTablesState->SessionId;
333326
}
334327

335328
const auto info = tableInfoMap.FindPtr(table);
@@ -433,7 +426,7 @@ class TKikimrTransactionContextBase : public TThrRefBase {
433426
const bool EnableImmediateEffects;
434427
THashMap<TKikimrPathId, TString> TableByIdMap;
435428
TMaybe<NKikimrKqp::EIsolationLevel> EffectiveIsolationLevel;
436-
THashMap<TString, TString> TempTables;
429+
NKikimr::NKqp::TKqpTempTablesState::TConstPtr TempTablesState;
437430
bool Readonly = false;
438431
bool Invalidated = false;
439432
bool Closed = false;
@@ -518,7 +511,7 @@ class TKikimrSessionContext : public TThrRefBase {
518511
if (TxCtx) {
519512
TxCtx->SetTempTables(tempTablesState);
520513
}
521-
TempTablesState = tempTablesState;
514+
TempTablesState = std::move(tempTablesState);
522515
}
523516

524517
const TIntrusiveConstPtr<NACLib::TUserToken>& GetUserToken() const {

‎ydb/core/kqp/query_data/kqp_prepared_query.cpp

Lines changed: 16 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -268,32 +268,32 @@ void TPreparedQueryHolder::FillTables(const google::protobuf::RepeatedPtrField<
268268
}
269269
}
270270

271-
bool TPreparedQueryHolder::HasTempTables(TKqpTempTablesState::TConstPtr tempTablesState) const {
271+
bool TPreparedQueryHolder::HasTempTables(TKqpTempTablesState::TConstPtr tempTablesState, bool withSessionId) const {
272272
if (!tempTablesState) {
273273
return false;
274274
}
275-
YQL_ENSURE(tempTablesState->SessionId);
276-
auto tempTables = THashSet<TString>();
277-
for (const auto& [path, info] : tempTablesState->TempTables) {
278-
tempTables.insert(path.second + *tempTablesState->SessionId);
279-
}
280275
for (const auto& table: QueryTables) {
281-
if (tempTables.contains(table)) {
276+
auto infoIt = tempTablesState->FindInfo(table, withSessionId);
277+
if (infoIt != tempTablesState->TempTables.end()) {
282278
return true;
283279
}
284280
}
285281

286-
for (const auto& tx: Transactions) {
287-
auto optPath = tx->GetSchemeOpTempTablePath();
288-
if (!optPath) {
289-
continue;
290-
} else {
291-
const auto& [isCreate, path] = *optPath;
292-
if (isCreate) {
293-
return true;
282+
283+
if (withSessionId) {
284+
for (const auto& tx: Transactions) {
285+
auto optPath = tx->GetSchemeOpTempTablePath();
286+
if (!optPath) {
287+
continue;
294288
} else {
295-
if (tempTables.contains(JoinPath({path.first, path.second}))) {
289+
const auto& [isCreate, path] = *optPath;
290+
if (isCreate) {
296291
return true;
292+
} else {
293+
auto infoIt = tempTablesState->FindInfo(JoinPath({path.first, path.second}), withSessionId);
294+
if (infoIt != tempTablesState->TempTables.end()) {
295+
return true;
296+
}
297297
}
298298
}
299299
}

‎ydb/core/kqp/query_data/kqp_prepared_query.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -192,7 +192,7 @@ class TPreparedQueryHolder {
192192

193193
void FillTables(const google::protobuf::RepeatedPtrField< ::NKqpProto::TKqpPhyStage>& stages);
194194

195-
bool HasTempTables(TKqpTempTablesState::TConstPtr tempTablesState) const;
195+
bool HasTempTables(TKqpTempTablesState::TConstPtr tempTablesState, bool withSessionId) const;
196196
};
197197

198198

0 commit comments

Comments
 (0)