Skip to content

Commit 2d760d8

Browse files
committed
Fix review issues
1 parent 7ccd6f4 commit 2d760d8

File tree

3 files changed

+37
-16
lines changed

3 files changed

+37
-16
lines changed

ydb/core/kqp/session_actor/kqp_query_state.cpp

Lines changed: 32 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -306,20 +306,41 @@ bool TKqpQueryState::HasErrors(const NSchemeCache::TSchemeCacheNavigate& respons
306306
return true;
307307
}
308308

309-
bool TKqpQueryState::HasImpliedAutostartTransactions() const {
310-
if (!HasTxControl()
311-
&& (RequestEv->GetAction() == NKikimrKqp::QUERY_ACTION_EXECUTE
312-
|| RequestEv->GetAction() == NKikimrKqp::QUERY_ACTION_EXECUTE_PREPARED)
313-
&& (RequestEv->GetType() == NKikimrKqp::QUERY_TYPE_SQL_GENERIC_QUERY
314-
|| RequestEv->GetType() == NKikimrKqp::QUERY_TYPE_SQL_GENERIC_SCRIPT
315-
|| RequestEv->GetType() == NKikimrKqp::QUERY_TYPE_SQL_GENERIC_CONCURRENT_QUERY))
309+
bool TKqpQueryState::HasImpliedTx() const {
310+
if (HasTxControl()) {
311+
return false;
312+
}
313+
314+
const NKikimrKqp::EQueryAction action = RequestEv->GetAction();
315+
if (action != NKikimrKqp::QUERY_ACTION_EXECUTE &&
316+
action != NKikimrKqp::QUERY_ACTION_EXECUTE_PREPARED)
316317
{
317-
for (const auto& transactionPtr : PreparedQuery->GetTransactions()) {
318-
if (transactionPtr->GetType() == NKqpProto::TKqpPhyTx::TYPE_GENERIC) { // data transaction
319-
return true;
320-
}
318+
return false;
319+
}
320+
321+
const NKikimrKqp::EQueryType queryType = RequestEv->GetType();
322+
if (queryType != NKikimrKqp::QUERY_TYPE_SQL_GENERIC_QUERY &&
323+
queryType != NKikimrKqp::QUERY_TYPE_SQL_GENERIC_SCRIPT &&
324+
queryType != NKikimrKqp::QUERY_TYPE_SQL_GENERIC_CONCURRENT_QUERY)
325+
{
326+
return false;
327+
}
328+
329+
for (const auto& transactionPtr : PreparedQuery->GetTransactions()) {
330+
switch (transactionPtr->GetType()) {
331+
case NKqpProto::TKqpPhyTx::TYPE_GENERIC: // data transaction
332+
return true;
333+
case NKqpProto::TKqpPhyTx::TYPE_UNSPECIFIED:
334+
case NKqpProto::TKqpPhyTx::TYPE_COMPUTE:
335+
case NKqpProto::TKqpPhyTx::TYPE_DATA: // data transaction, but not in QueryService API
336+
case NKqpProto::TKqpPhyTx::TYPE_SCAN:
337+
case NKqpProto::TKqpPhyTx::TYPE_SCHEME:
338+
case NKqpProto::TKqpPhyTx_EType_TKqpPhyTx_EType_INT_MIN_SENTINEL_DO_NOT_USE_:
339+
case NKqpProto::TKqpPhyTx_EType_TKqpPhyTx_EType_INT_MAX_SENTINEL_DO_NOT_USE_:
340+
break;
321341
}
322342
}
343+
323344
return false;
324345
}
325346

ydb/core/kqp/session_actor/kqp_query_state.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -362,7 +362,7 @@ class TKqpQueryState : public TNonCopyable {
362362
return RequestEv->HasTxControl();
363363
}
364364

365-
bool HasImpliedAutostartTransactions() const;
365+
bool HasImpliedTx() const; // (only for QueryService API) user has not specified TxControl in the request. In this case we behave like Begin/Commit was specified.
366366

367367
const ::Ydb::Table::TransactionControl& GetTxControl() const {
368368
return RequestEv->GetTxControl();

ydb/core/kqp/session_actor/kqp_session_actor.cpp

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -636,7 +636,7 @@ class TKqpSessionActor : public TActorBootstrapped<TKqpSessionActor> {
636636
Counters->ReportBeginTransaction(Settings.DbCounters, Transactions.EvictedTx, Transactions.Size(), Transactions.ToBeAbortedSize());
637637
}
638638

639-
static const Ydb::Table::TransactionControl& GetAutoTransactionControl() {
639+
static const Ydb::Table::TransactionControl& GetImpliedTxControl() {
640640
auto create = []() -> Ydb::Table::TransactionControl {
641641
Ydb::Table::TransactionControl control;
642642
control.mutable_begin_tx()->mutable_serializable_read_write();
@@ -648,9 +648,9 @@ class TKqpSessionActor : public TActorBootstrapped<TKqpSessionActor> {
648648
}
649649

650650
bool PrepareQueryTransaction() {
651-
bool autoTransaction = false;
652-
if (QueryState->HasTxControl() || (autoTransaction = QueryState->HasImpliedAutostartTransactions())) {
653-
const auto& txControl = autoTransaction ? GetAutoTransactionControl() : QueryState->GetTxControl();
651+
const bool hasTxControl = QueryState->HasTxControl();
652+
if (hasTxControl || QueryState->HasImpliedTx()) {
653+
const auto& txControl = hasTxControl ? QueryState->GetTxControl() : GetImpliedTxControl();
654654

655655
QueryState->Commit = txControl.commit_tx();
656656
switch (txControl.tx_selector_case()) {

0 commit comments

Comments
 (0)