Re: [PR] IGNITE-25583 Sql. Implement direct mapping for explicit transactions [ignite-3]

2025-07-16 Thread via GitHub


korlov42 merged PR #6235:
URL: https://github.com/apache/ignite-3/pull/6235


-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]



Re: [PR] IGNITE-25583 Sql. Implement direct mapping for explicit transactions [ignite-3]

2025-07-16 Thread via GitHub


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


##
modules/runner/src/integrationTest/java/org/apache/ignite/internal/runner/app/client/ItThinClientSqlTest.java:
##
@@ -376,6 +392,109 @@ void testTransactionRollbackRevertsSqlUpdate() {
 assertEquals(1, res.currentPage().iterator().next().intValue(0));
 }
 
+@Test
+void testExplicitTransactionKvCase() {
+IgniteClient client = client();
+IgniteSql sql = client.sql();
+
+sql.execute(null, "CREATE TABLE my_table (id INT PRIMARY KEY, val 
INT)");
+
+// First, let's fill the table and check every row in implicit tx.
+// This should also prepare partition awareness metadata.
+int count = 10;
+for (int i = 0; i < count; i++) {
+try (ResultSet ignored = sql.execute(null, "INSERT INTO 
my_table VALUES (?, ?)", i, i)) {
+// No-op.
+}
+}
+
+for (int i = 0; i < count; i++) {
+try (ResultSet rs = sql.execute(null, "SELECT * FROM 
my_table WHERE id = ?", i)) {
+assertEquals(i, rs.next().intValue(1));
+}
+}
+
+// Now let's clean the table and do the same steps but within explicit 
tx.
+
+try (ResultSet ignored = sql.execute(null, "DELETE FROM my_table")) 
{
+// No-op.
+}
+
+Transaction tx = client.transactions().begin();
+for (int i = 0; i < count; i++) {
+try (ResultSet ignored = sql.execute(tx, "INSERT INTO my_table 
VALUES (?, ?)", i, i)) {
+// No-op.
+}
+}
+
+for (int i = 0; i < count; i++) {
+try (ResultSet rs = sql.execute(tx, "SELECT * FROM 
my_table WHERE id = ?", i)) {
+assertEquals(i, rs.next().intValue(1));
+}
+}
+
+// All just inserted rows should not be visible yet
+for (int i = 0; i < count; i++) {
+try (ResultSet rs = sql.execute(null, "SELECT * FROM 
my_table WHERE id = ?", i)) {

Review Comment:
   Good catch! This looks like general problem with remote transaction since KV 
api fails as well. I filed a separate ticket for this case 
https://issues.apache.org/jira/browse/IGNITE-25921



##
modules/sql-engine/src/integrationTest/java/org/apache/ignite/internal/sql/api/ItSqlApiBaseTest.java:
##
@@ -269,10 +270,12 @@ public void checkTransactionsWithDml() {
 
 // Outdated tx.
 Transaction outerTx0 = outerTx;
-assertThrowsSqlException(
+assertThrowsWithCode(
+IgniteException.class,

Review Comment:
   doesn't look good indeed. Revered this change and introduced exception 
remapping instead



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]



Re: [PR] IGNITE-25583 Sql. Implement direct mapping for explicit transactions [ignite-3]

2025-07-15 Thread via GitHub


xtern commented on code in PR #6235:
URL: https://github.com/apache/ignite-3/pull/6235#discussion_r2209338104


##
modules/runner/src/integrationTest/java/org/apache/ignite/internal/runner/app/client/ItThinClientSqlTest.java:
##
@@ -376,6 +392,109 @@ void testTransactionRollbackRevertsSqlUpdate() {
 assertEquals(1, res.currentPage().iterator().next().intValue(0));
 }
 
+@Test
+void testExplicitTransactionKvCase() {
+IgniteClient client = client();
+IgniteSql sql = client.sql();
+
+sql.execute(null, "CREATE TABLE my_table (id INT PRIMARY KEY, val 
INT)");
+
+// First, let's fill the table and check every row in implicit tx.
+// This should also prepare partition awareness metadata.
+int count = 10;
+for (int i = 0; i < count; i++) {
+try (ResultSet ignored = sql.execute(null, "INSERT INTO 
my_table VALUES (?, ?)", i, i)) {
+// No-op.
+}
+}
+
+for (int i = 0; i < count; i++) {
+try (ResultSet rs = sql.execute(null, "SELECT * FROM 
my_table WHERE id = ?", i)) {
+assertEquals(i, rs.next().intValue(1));
+}
+}
+
+// Now let's clean the table and do the same steps but within explicit 
tx.
+
+try (ResultSet ignored = sql.execute(null, "DELETE FROM my_table")) 
{
+// No-op.
+}
+
+Transaction tx = client.transactions().begin();
+for (int i = 0; i < count; i++) {
+try (ResultSet ignored = sql.execute(tx, "INSERT INTO my_table 
VALUES (?, ?)", i, i)) {
+// No-op.
+}
+}
+
+for (int i = 0; i < count; i++) {
+try (ResultSet rs = sql.execute(tx, "SELECT * FROM 
my_table WHERE id = ?", i)) {
+assertEquals(i, rs.next().intValue(1));
+}
+}
+
+// All just inserted rows should not be visible yet
+for (int i = 0; i < count; i++) {
+try (ResultSet rs = sql.execute(null, "SELECT * FROM 
my_table WHERE id = ?", i)) {

Review Comment:
   I changed a bit this test to use separate external RO-tx here. And test 
fails at this line with
   ```
   Caused by: org.apache.ignite.internal.tx.LockException: IGN-TX-4 Failed to 
acquire a lock due to a possible deadlock 
[locker=019811d2-7bc3--2722-84170001, 
holder=019811d2-7b6f--2722-84170001] TraceId:789c7be0
   ```
   
   On the main branch such case passes.
   
   Is it expected?
   
   Full test case
   ```java
   @Test
   void testExplicitTransactionKvCase() {
   IgniteClient client = client();
   IgniteSql sql = client.sql();
   
   sql.execute(null, "CREATE TABLE my_table (id INT PRIMARY KEY, val 
INT)");
   
   // First, let's fill the table and check every row in implicit tx.
   // This should also prepare partition awareness metadata.
   int count = 10;
   for (int i = 0; i < count; i++) {
   try (ResultSet ignored = sql.execute(null, "INSERT INTO 
my_table VALUES (?, ?)", i, i)) {
   // No-op.
   }
   }
   
   for (int i = 0; i < count; i++) {
   try (ResultSet rs = sql.execute(null, "SELECT * FROM 
my_table WHERE id = ?", i)) {
   assertEquals(i, rs.next().intValue(1));
   }
   }
   
   // Now let's clean the table and do the same steps but within 
explicit tx.
   
   try (ResultSet ignored = sql.execute(null, "DELETE FROM 
my_table")) {
   // No-op.
   }
   
   Transaction tx = client.transactions().begin();
   Transaction tx1 = client.transactions().begin(new 
TransactionOptions().readOnly(true));
   for (int i = 0; i < count; i++) {
   try (ResultSet ignored = sql.execute(tx, "INSERT INTO 
my_table VALUES (?, ?)", i, i)) {
   // No-op.
   }
   }
   
   for (int i = 0; i < count; i++) {
   try (ResultSet rs = sql.execute(tx, "SELECT * FROM 
my_table WHERE id = ?", i)) {
   assertEquals(i, rs.next().intValue(1));
   }
   }
   
   // All just inserted rows should not be visible yet
   for (int i = 0; i < count; i++) {
   try (ResultSet rs = sql.execute(tx1, "SELECT * FROM 
my_table WHERE id = ?", i)) {
   assertFalse(rs.hasNext());
   }
   }
   
   tx.commit();
   tx1.rollback();
   
   // And now changes are published.
   for (int i = 0; i < count; i++) {
   try (ResultSet rs = sql.execute(null, "SELECT * FROM 
my_table WHERE id = ?", i)) {
   assertEquals(i, rs.next().intValue(1));
   }
   }
   }
   ```



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub

Re: [PR] IGNITE-25583 Sql. Implement direct mapping for explicit transactions [ignite-3]

2025-07-15 Thread via GitHub


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


##
modules/client-handler/src/main/java/org/apache/ignite/client/handler/requests/sql/ClientSqlExecuteRequest.java:
##
@@ -79,12 +83,21 @@ public static CompletableFuture process(
 ClientResourceRegistry resources,
 ClientHandlerMetricSource metrics,
 HybridTimestampTracker timestampTracker,
-boolean sqlPartitionAwarenessSupported
+boolean sqlPartitionAwarenessSupported,

Review Comment:
   javadoc updated



##
modules/sql-engine/src/test/java/org/apache/ignite/internal/sql/engine/prepare/partitionawareness/PartitionAwarenessMetadataTest.java:
##
@@ -165,9 +165,9 @@ private static Stream shortKeyMetadata() {
 Arguments.of("SELECT * FROM t WHERE c1=? and c2=? and c3=3", 
null),
 
 // KV PUT
-Arguments.of("INSERT INTO t VALUES (?, ?, ?)",  
dynamicParams(2)),
-Arguments.of("INSERT INTO t (c1, c2, c3) VALUES (?, ?, ?)", 
dynamicParams(2)),
-Arguments.of("INSERT INTO t (c3, c1, c2) VALUES (?, ?, ?)", 
dynamicParams(0)),
+Arguments.of("INSERT INTO t VALUES (?, ?, ?)",  
dynamicParamsTrackingRequired(2)),

Review Comment:
   done



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]



Re: [PR] IGNITE-25583 Sql. Implement direct mapping for explicit transactions [ignite-3]

2025-07-15 Thread via GitHub


xtern commented on code in PR #6235:
URL: https://github.com/apache/ignite-3/pull/6235#discussion_r2209338104


##
modules/runner/src/integrationTest/java/org/apache/ignite/internal/runner/app/client/ItThinClientSqlTest.java:
##
@@ -376,6 +392,109 @@ void testTransactionRollbackRevertsSqlUpdate() {
 assertEquals(1, res.currentPage().iterator().next().intValue(0));
 }
 
+@Test
+void testExplicitTransactionKvCase() {
+IgniteClient client = client();
+IgniteSql sql = client.sql();
+
+sql.execute(null, "CREATE TABLE my_table (id INT PRIMARY KEY, val 
INT)");
+
+// First, let's fill the table and check every row in implicit tx.
+// This should also prepare partition awareness metadata.
+int count = 10;
+for (int i = 0; i < count; i++) {
+try (ResultSet ignored = sql.execute(null, "INSERT INTO 
my_table VALUES (?, ?)", i, i)) {
+// No-op.
+}
+}
+
+for (int i = 0; i < count; i++) {
+try (ResultSet rs = sql.execute(null, "SELECT * FROM 
my_table WHERE id = ?", i)) {
+assertEquals(i, rs.next().intValue(1));
+}
+}
+
+// Now let's clean the table and do the same steps but within explicit 
tx.
+
+try (ResultSet ignored = sql.execute(null, "DELETE FROM my_table")) 
{
+// No-op.
+}
+
+Transaction tx = client.transactions().begin();
+for (int i = 0; i < count; i++) {
+try (ResultSet ignored = sql.execute(tx, "INSERT INTO my_table 
VALUES (?, ?)", i, i)) {
+// No-op.
+}
+}
+
+for (int i = 0; i < count; i++) {
+try (ResultSet rs = sql.execute(tx, "SELECT * FROM 
my_table WHERE id = ?", i)) {
+assertEquals(i, rs.next().intValue(1));
+}
+}
+
+// All just inserted rows should not be visible yet
+for (int i = 0; i < count; i++) {
+try (ResultSet rs = sql.execute(null, "SELECT * FROM 
my_table WHERE id = ?", i)) {

Review Comment:
   I changed a bit this test to use separate external RO-tx here. And test 
fails here with
   ```
   Caused by: org.apache.ignite.internal.tx.LockException: IGN-TX-4 Failed to 
acquire a lock due to a possible deadlock 
[locker=019811d2-7bc3--2722-84170001, 
holder=019811d2-7b6f--2722-84170001] TraceId:789c7be0
   ```
   
   On the main branch such case passes.
   
   Is it expected?
   
   Full test case
   ```java
   @Test
   void testExplicitTransactionKvCase() {
   IgniteClient client = client();
   IgniteSql sql = client.sql();
   
   sql.execute(null, "CREATE TABLE my_table (id INT PRIMARY KEY, val 
INT)");
   
   // First, let's fill the table and check every row in implicit tx.
   // This should also prepare partition awareness metadata.
   int count = 10;
   for (int i = 0; i < count; i++) {
   try (ResultSet ignored = sql.execute(null, "INSERT INTO 
my_table VALUES (?, ?)", i, i)) {
   // No-op.
   }
   }
   
   for (int i = 0; i < count; i++) {
   try (ResultSet rs = sql.execute(null, "SELECT * FROM 
my_table WHERE id = ?", i)) {
   assertEquals(i, rs.next().intValue(1));
   }
   }
   
   // Now let's clean the table and do the same steps but within 
explicit tx.
   
   try (ResultSet ignored = sql.execute(null, "DELETE FROM 
my_table")) {
   // No-op.
   }
   
   Transaction tx = client.transactions().begin();
   Transaction tx1 = client.transactions().begin(new 
TransactionOptions().readOnly(true));
   for (int i = 0; i < count; i++) {
   try (ResultSet ignored = sql.execute(tx, "INSERT INTO 
my_table VALUES (?, ?)", i, i)) {
   // No-op.
   }
   }
   
   for (int i = 0; i < count; i++) {
   try (ResultSet rs = sql.execute(tx, "SELECT * FROM 
my_table WHERE id = ?", i)) {
   assertEquals(i, rs.next().intValue(1));
   }
   }
   
   // All just inserted rows should not be visible yet
   for (int i = 0; i < count; i++) {
   try (ResultSet rs = sql.execute(tx1, "SELECT * FROM 
my_table WHERE id = ?", i)) {
   assertFalse(rs.hasNext());
   }
   }
   
   tx.commit();
   tx1.rollback();
   
   // And now changes are published.
   for (int i = 0; i < count; i++) {
   try (ResultSet rs = sql.execute(null, "SELECT * FROM 
my_table WHERE id = ?", i)) {
   assertEquals(i, rs.next().intValue(1));
   }
   }
   }
   ```



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

Re: [PR] IGNITE-25583 Sql. Implement direct mapping for explicit transactions [ignite-3]

2025-07-15 Thread via GitHub


xtern commented on code in PR #6235:
URL: https://github.com/apache/ignite-3/pull/6235#discussion_r2209338104


##
modules/runner/src/integrationTest/java/org/apache/ignite/internal/runner/app/client/ItThinClientSqlTest.java:
##
@@ -376,6 +392,109 @@ void testTransactionRollbackRevertsSqlUpdate() {
 assertEquals(1, res.currentPage().iterator().next().intValue(0));
 }
 
+@Test
+void testExplicitTransactionKvCase() {
+IgniteClient client = client();
+IgniteSql sql = client.sql();
+
+sql.execute(null, "CREATE TABLE my_table (id INT PRIMARY KEY, val 
INT)");
+
+// First, let's fill the table and check every row in implicit tx.
+// This should also prepare partition awareness metadata.
+int count = 10;
+for (int i = 0; i < count; i++) {
+try (ResultSet ignored = sql.execute(null, "INSERT INTO 
my_table VALUES (?, ?)", i, i)) {
+// No-op.
+}
+}
+
+for (int i = 0; i < count; i++) {
+try (ResultSet rs = sql.execute(null, "SELECT * FROM 
my_table WHERE id = ?", i)) {
+assertEquals(i, rs.next().intValue(1));
+}
+}
+
+// Now let's clean the table and do the same steps but within explicit 
tx.
+
+try (ResultSet ignored = sql.execute(null, "DELETE FROM my_table")) 
{
+// No-op.
+}
+
+Transaction tx = client.transactions().begin();
+for (int i = 0; i < count; i++) {
+try (ResultSet ignored = sql.execute(tx, "INSERT INTO my_table 
VALUES (?, ?)", i, i)) {
+// No-op.
+}
+}
+
+for (int i = 0; i < count; i++) {
+try (ResultSet rs = sql.execute(tx, "SELECT * FROM 
my_table WHERE id = ?", i)) {
+assertEquals(i, rs.next().intValue(1));
+}
+}
+
+// All just inserted rows should not be visible yet
+for (int i = 0; i < count; i++) {
+try (ResultSet rs = sql.execute(null, "SELECT * FROM 
my_table WHERE id = ?", i)) {

Review Comment:
   I changed a bit this test to use separate external RO-tx here. And test 
fails here with
   ```
   Caused by: org.apache.ignite.internal.tx.LockException: IGN-TX-4 Failed to 
acquire a lock due to a possible deadlock 
[locker=019811d2-7bc3--2722-84170001, 
holder=019811d2-7b6f--2722-84170001] TraceId:789c7be0
   ```
   
   On the main branch such case passes.
   
   Is it ok?
   
   Full test case
   ```java
   @Test
   void testExplicitTransactionKvCase() {
   IgniteClient client = client();
   IgniteSql sql = client.sql();
   
   sql.execute(null, "CREATE TABLE my_table (id INT PRIMARY KEY, val 
INT)");
   
   // First, let's fill the table and check every row in implicit tx.
   // This should also prepare partition awareness metadata.
   int count = 10;
   for (int i = 0; i < count; i++) {
   try (ResultSet ignored = sql.execute(null, "INSERT INTO 
my_table VALUES (?, ?)", i, i)) {
   // No-op.
   }
   }
   
   for (int i = 0; i < count; i++) {
   try (ResultSet rs = sql.execute(null, "SELECT * FROM 
my_table WHERE id = ?", i)) {
   assertEquals(i, rs.next().intValue(1));
   }
   }
   
   // Now let's clean the table and do the same steps but within 
explicit tx.
   
   try (ResultSet ignored = sql.execute(null, "DELETE FROM 
my_table")) {
   // No-op.
   }
   
   Transaction tx = client.transactions().begin();
   Transaction tx1 = client.transactions().begin(new 
TransactionOptions().readOnly(true));
   for (int i = 0; i < count; i++) {
   try (ResultSet ignored = sql.execute(tx, "INSERT INTO 
my_table VALUES (?, ?)", i, i)) {
   // No-op.
   }
   }
   
   for (int i = 0; i < count; i++) {
   try (ResultSet rs = sql.execute(tx, "SELECT * FROM 
my_table WHERE id = ?", i)) {
   assertEquals(i, rs.next().intValue(1));
   }
   }
   
   // All just inserted rows should not be visible yet
   for (int i = 0; i < count; i++) {
   try (ResultSet rs = sql.execute(tx1, "SELECT * FROM 
my_table WHERE id = ?", i)) {
   assertFalse(rs.hasNext());
   }
   }
   
   tx.commit();
   tx1.rollback();
   
   // And now changes are published.
   for (int i = 0; i < count; i++) {
   try (ResultSet rs = sql.execute(null, "SELECT * FROM 
my_table WHERE id = ?", i)) {
   assertEquals(i, rs.next().intValue(1));
   }
   }
   }
   ```



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
U

Re: [PR] IGNITE-25583 Sql. Implement direct mapping for explicit transactions [ignite-3]

2025-07-15 Thread via GitHub


xtern commented on code in PR #6235:
URL: https://github.com/apache/ignite-3/pull/6235#discussion_r2209239698


##
modules/sql-engine/src/integrationTest/java/org/apache/ignite/internal/sql/api/ItSqlApiBaseTest.java:
##
@@ -269,10 +270,12 @@ public void checkTransactionsWithDml() {
 
 // Outdated tx.
 Transaction outerTx0 = outerTx;
-assertThrowsSqlException(
+assertThrowsWithCode(
+IgniteException.class,

Review Comment:
   is it ok that thin client and embedded client produce different exceptions 
for the same statement?
   I mean the thin client doesn't wrap it in a SqlException, but the embedded 
does?



##
modules/runner/src/integrationTest/java/org/apache/ignite/internal/runner/app/client/ItThinClientSqlTest.java:
##
@@ -376,6 +392,109 @@ void testTransactionRollbackRevertsSqlUpdate() {
 assertEquals(1, res.currentPage().iterator().next().intValue(0));
 }
 
+@Test
+void testExplicitTransactionKvCase() {
+IgniteClient client = client();
+IgniteSql sql = client.sql();
+
+sql.execute(null, "CREATE TABLE my_table (id INT PRIMARY KEY, val 
INT)");
+
+// First, let's fill the table and check every row in implicit tx.
+// This should also prepare partition awareness metadata.
+int count = 10;
+for (int i = 0; i < count; i++) {
+try (ResultSet ignored = sql.execute(null, "INSERT INTO 
my_table VALUES (?, ?)", i, i)) {
+// No-op.
+}
+}
+
+for (int i = 0; i < count; i++) {
+try (ResultSet rs = sql.execute(null, "SELECT * FROM 
my_table WHERE id = ?", i)) {
+assertEquals(i, rs.next().intValue(1));
+}
+}
+
+// Now let's clean the table and do the same steps but within explicit 
tx.
+
+try (ResultSet ignored = sql.execute(null, "DELETE FROM my_table")) 
{
+// No-op.
+}
+
+Transaction tx = client.transactions().begin();
+for (int i = 0; i < count; i++) {
+try (ResultSet ignored = sql.execute(tx, "INSERT INTO my_table 
VALUES (?, ?)", i, i)) {
+// No-op.
+}
+}
+
+for (int i = 0; i < count; i++) {
+try (ResultSet rs = sql.execute(tx, "SELECT * FROM 
my_table WHERE id = ?", i)) {
+assertEquals(i, rs.next().intValue(1));
+}
+}
+
+// All just inserted rows should not be visible yet
+for (int i = 0; i < count; i++) {
+try (ResultSet rs = sql.execute(null, "SELECT * FROM 
my_table WHERE id = ?", i)) {

Review Comment:
   I changed a bit this test to use separate external RO-tx here. And test 
fails with
   ```
   Caused by: org.apache.ignite.internal.tx.LockException: IGN-TX-4 Failed to 
acquire a lock due to a possible deadlock 
[locker=019811d2-7bc3--2722-84170001, 
holder=019811d2-7b6f--2722-84170001] TraceId:789c7be0
   ```
   
   On the main branch such case passes.
   
   Is it ok?
   
   Full test case
   ```java
   @Test
   void testExplicitTransactionKvCase() {
   IgniteClient client = client();
   IgniteSql sql = client.sql();
   
   sql.execute(null, "CREATE TABLE my_table (id INT PRIMARY KEY, val 
INT)");
   
   // First, let's fill the table and check every row in implicit tx.
   // This should also prepare partition awareness metadata.
   int count = 10;
   for (int i = 0; i < count; i++) {
   try (ResultSet ignored = sql.execute(null, "INSERT INTO 
my_table VALUES (?, ?)", i, i)) {
   // No-op.
   }
   }
   
   for (int i = 0; i < count; i++) {
   try (ResultSet rs = sql.execute(null, "SELECT * FROM 
my_table WHERE id = ?", i)) {
   assertEquals(i, rs.next().intValue(1));
   }
   }
   
   // Now let's clean the table and do the same steps but within 
explicit tx.
   
   try (ResultSet ignored = sql.execute(null, "DELETE FROM 
my_table")) {
   // No-op.
   }
   
   Transaction tx = client.transactions().begin();
   Transaction tx1 = client.transactions().begin(new 
TransactionOptions().readOnly(true));
   for (int i = 0; i < count; i++) {
   try (ResultSet ignored = sql.execute(tx, "INSERT INTO 
my_table VALUES (?, ?)", i, i)) {
   // No-op.
   }
   }
   
   for (int i = 0; i < count; i++) {
   try (ResultSet rs = sql.execute(tx, "SELECT * FROM 
my_table WHERE id = ?", i)) {
   assertEquals(i, rs.next().intValue(1));
   }
   }
   
   // All just inserted rows should not be visible yet
   for (int i = 0; i < count; i++) {
   try (ResultSet rs = sql.execute(tx1, "SELECT * FROM 
my_table WHERE id = ?", 

Re: [PR] IGNITE-25583 Sql. Implement direct mapping for explicit transactions [ignite-3]

2025-07-15 Thread via GitHub


zstan commented on code in PR #6235:
URL: https://github.com/apache/ignite-3/pull/6235#discussion_r2206647966


##
modules/sql-engine/src/test/java/org/apache/ignite/internal/sql/engine/prepare/partitionawareness/PartitionAwarenessMetadataTest.java:
##
@@ -165,9 +165,9 @@ private static Stream shortKeyMetadata() {
 Arguments.of("SELECT * FROM t WHERE c1=? and c2=? and c3=3", 
null),
 
 // KV PUT
-Arguments.of("INSERT INTO t VALUES (?, ?, ?)",  
dynamicParams(2)),
-Arguments.of("INSERT INTO t (c1, c2, c3) VALUES (?, ?, ?)", 
dynamicParams(2)),
-Arguments.of("INSERT INTO t (c3, c1, c2) VALUES (?, ?, ?)", 
dynamicParams(0)),
+Arguments.of("INSERT INTO t VALUES (?, ?, ?)",  
dynamicParamsTrackingRequired(2)),

Review Comment:
   lets also add :
   ```
   Arguments.of("SELECT * FROM t WHERE c3=? and c2=1", dynamicParams(0)),
   Arguments.of("SELECT * FROM t WHERE c1=1 and c2=? and c3=?", 
dynamicParams(1)),
   ```



##
modules/client-handler/src/main/java/org/apache/ignite/client/handler/requests/sql/ClientSqlExecuteRequest.java:
##
@@ -79,12 +83,21 @@ public static CompletableFuture process(
 ClientResourceRegistry resources,
 ClientHandlerMetricSource metrics,
 HybridTimestampTracker timestampTracker,
-boolean sqlPartitionAwarenessSupported
+boolean sqlPartitionAwarenessSupported,

Review Comment:
   append params to java doc plz



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]