Move NeedSnapshot out from kqp_session_actor.cpp

3 files changed, 61 insertions, 59 deletions

diff --git a/ydb/core/kqp/session_actor/kqp_session_actor.cpp b/ydb/core/kqp/session_actor/kqp_session_actor.cpp
index 7864c67e648..bae9630412a 100644
--- a/ydb/core/kqp/session_actor/kqp_session_actor.cpp
+++ b/ydb/core/kqp/session_actor/kqp_session_actor.cpp
@@ -55,64 +55,6 @@ namespace {
 #define LOG_D(msg) LOG_DEBUG_S(*TlsActivationContext, NKikimrServices::KQP_SESSION, LogPrefix() << msg)
 #define LOG_T(msg) LOG_TRACE_S(*TlsActivationContext, NKikimrServices::KQP_SESSION, LogPrefix() << msg)
 
-inline bool NeedSnapshot(const TKqpTransactionContext& txCtx, const NYql::TKikimrConfiguration& config, bool rollbackTx,
-    bool commitTx, const NKqpProto::TKqpPhyQuery& physicalQuery)
-{
-    if (*txCtx.EffectiveIsolationLevel != NKikimrKqp::ISOLATION_LEVEL_SERIALIZABLE)
-        return false;
-
-    if (!config.FeatureFlags.GetEnableMvccSnapshotReads())
-        return false;
-
-    if (txCtx.GetSnapshot().IsValid())
-        return false;
-
-    if (rollbackTx)
-        return false;
-    if (!commitTx)
-        return true;
-
-    size_t readPhases = 0;
-    bool hasEffects = false;
-
-    for (const auto &tx : physicalQuery.GetTransactions()) {
-        switch (tx.GetType()) {
-            case NKqpProto::TKqpPhyTx::TYPE_COMPUTE:
-                // ignore pure computations
-                break;
-
-            default:
-                ++readPhases;
-                break;
-        }
-
-        if (tx.GetHasEffects()) {
-            hasEffects = true;
-        }
-    }
-
-    if (config.FeatureFlags.GetEnableKqpImmediateEffects() && physicalQuery.GetHasUncommittedChangesRead()) {
-        return true;
-    }
-
-    // We don't want snapshot when there are effects at the moment,
-    // because it hurts performance when there are multiple single-shard
-    // reads and a single distributed commit. Taking snapshot costs
-    // similar to an additional distributed transaction, and it's very
-    // hard to predict when that happens, causing performance
-    // degradation.
-    if (hasEffects) {
-        return false;
-    }
-
-    // We need snapshot when there are multiple table read phases, most
-    // likely it involves multiple tables and we would have to use a
-    // distributed commit otherwise. Taking snapshot helps as avoid TLI
-    // for read-only transactions, and costs less than a final distributed
-    // commit.
-    return readPhases > 1;
-}
-
 class TRequestFail : public yexception {
 public:
     Ydb::StatusIds::StatusCode Status;
@@ -125,7 +67,6 @@ public:
     {}
 };
 
-
 struct TKqpQueryState {
     TActorId Sender;
     ui64 ProxyRequestId = 0;
diff --git a/ydb/core/kqp/session_actor/kqp_tx.cpp b/ydb/core/kqp/session_actor/kqp_tx.cpp
index c253e526ae2..201e051cd84 100644
--- a/ydb/core/kqp/session_actor/kqp_tx.cpp
+++ b/ydb/core/kqp/session_actor/kqp_tx.cpp
@@ -135,5 +135,63 @@ TKqpTransactionInfo TKqpTransactionContext::GetInfo() const {
     return txInfo;
 }
 
+bool NeedSnapshot(const TKqpTransactionContext& txCtx, const NYql::TKikimrConfiguration& config, bool rollbackTx,
+    bool commitTx, const NKqpProto::TKqpPhyQuery& physicalQuery)
+{
+    if (*txCtx.EffectiveIsolationLevel != NKikimrKqp::ISOLATION_LEVEL_SERIALIZABLE)
+        return false;
+
+    if (!config.FeatureFlags.GetEnableMvccSnapshotReads())
+        return false;
+
+    if (txCtx.GetSnapshot().IsValid())
+        return false;
+
+    if (rollbackTx)
+        return false;
+    if (!commitTx)
+        return true;
+
+    size_t readPhases = 0;
+    bool hasEffects = false;
+
+    for (const auto &tx : physicalQuery.GetTransactions()) {
+        switch (tx.GetType()) {
+            case NKqpProto::TKqpPhyTx::TYPE_COMPUTE:
+                // ignore pure computations
+                break;
+
+            default:
+                ++readPhases;
+                break;
+        }
+
+        if (tx.GetHasEffects()) {
+            hasEffects = true;
+        }
+    }
+
+    if (config.FeatureFlags.GetEnableKqpImmediateEffects() && physicalQuery.GetHasUncommittedChangesRead()) {
+        return true;
+    }
+
+    // We don't want snapshot when there are effects at the moment,
+    // because it hurts performance when there are multiple single-shard
+    // reads and a single distributed commit. Taking snapshot costs
+    // similar to an additional distributed transaction, and it's very
+    // hard to predict when that happens, causing performance
+    // degradation.
+    if (hasEffects) {
+        return false;
+    }
+
+    // We need snapshot when there are multiple table read phases, most
+    // likely it involves multiple tables and we would have to use a
+    // distributed commit otherwise. Taking snapshot helps as avoid TLI
+    // for read-only transactions, and costs less than a final distributed
+    // commit.
+    return readPhases > 1;
+}
+
 } // namespace NKqp
 } // namespace NKikimr
diff --git a/ydb/core/kqp/session_actor/kqp_tx.h b/ydb/core/kqp/session_actor/kqp_tx.h
index 7629dc08f7d..28e02aadbeb 100644
--- a/ydb/core/kqp/session_actor/kqp_tx.h
+++ b/ydb/core/kqp/session_actor/kqp_tx.h
@@ -339,4 +339,7 @@ bool MergeLocks(const NKikimrMiniKQL::TType& type, const NKikimrMiniKQL::TValue&
 std::pair<bool, std::vector<NYql::TIssue>> MergeLocks(const NKikimrMiniKQL::TType& type,
     const NKikimrMiniKQL::TValue& value, TKqpTransactionContext& txCtx);
 
+bool NeedSnapshot(const TKqpTransactionContext& txCtx, const NYql::TKikimrConfiguration& config, bool rollbackTx,
+    bool commitTx, const NKqpProto::TKqpPhyQuery& physicalQuery);
+
 }  // namespace NKikimr::NKqp