[GitHub] [ignite-3] korlov42 commented on a diff in pull request #1232: IGNITE-17330 Support read only transactions from sql side

[GitBox <gi...@apache.org>]

korlov42 commented on code in PR #1232:
URL: https://github.com/apache/ignite-3/pull/1232#discussion_r1001714375


##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/metadata/FragmentDescription.java:
##########
@@ -34,6 +35,8 @@ public class FragmentDescription implements Serializable {
 
     private Long2ObjectMap<List<String>> remoteSources;
 
+    private HybridTimestamp txTs;

Review Comment:
   Fragment description is definitely not the place to add transaction to



##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/SqlQueryProcessor.java:
##########
@@ -620,4 +629,10 @@ public CompletableFuture<Boolean> notify(@NotNull IndexEventParameters parameter
                     .thenApply(v -> false);
         }
     }
+
+    /** Returns {@code true} if this is data modification operation. */
+    private static boolean dataModificationOp(SqlNode sqlNode) {
+        return sqlNode instanceof SqlInsert || sqlNode instanceof SqlMerge || sqlNode instanceof SqlDelete

Review Comment:
   probably it would be better to replace this with `SqlKind.DML.contains(sqlNode.getKind())`



##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/util/BaseQueryContext.java:
##########
@@ -159,7 +158,8 @@ public List<MetadataHandler<?>> handlers(Class<? extends MetadataHandler<?>> hnd
 
     private final Object[] parameters;
 
-    private final InternalTransaction tx;
+    /** Transaction representation: it can be implementation or clock representation for RO transactions. */
+    private final Object tx;

Review Comment:
   I don't think It's good idea to keep different objects in a single field



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org