xtern commented on code in PR #6835:
URL: https://github.com/apache/ignite-3/pull/6835#discussion_r2468113781
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/exec/fsm/MultiStatementHandler.java:
##########
@@ -177,6 +177,13 @@ CompletableFuture<AsyncSqlCursor<InternalSqlRow>>
processNext() {
break;
}
+ if (!DdlBatchingHelper.isCompatible(
Review Comment:
We also have `DdlBatchingBenchmark`... which apparently needs to be updated
in accordance with the new batching rules (now it simply shows a multiple
decrease in performance comparing to the main branch).
If you don't want to deal with this in the current PR, let's create a
separate ticket for this.
##########
modules/sql-engine/src/test/java/org/apache/ignite/internal/sql/engine/exec/DdlBatchingTest.java:
##########
@@ -196,13 +198,134 @@ void batchWithConditionalDdl() {
assertIndexExists("t1_ind_2");
}
+ @Test
+ void batchIsSplitByAlter() {
+ AsyncSqlCursor<InternalSqlRow> cursor = gatewayNode.executeQuery(
+ "CREATE TABLE t1 (id INT PRIMARY KEY, val_1 INT, val_2 INT);"
+ + "ALTER TABLE t1 ADD COLUMN val_3 INT;"
+ + "ALTER TABLE t1 DROP COLUMN val_2;"
+ + "CREATE TABLE t2 (id INT PRIMARY KEY, val_1 INT,
val_2 INT);"
+ );
+
+ // CREATE TABLE t1 (id INT PRIMARY KEY, val_1 INT, val_2 INT)
+ assertDdlResult(cursor, true);
+ assertThat(cursor.hasNextResult(), is(true));
+ assertThat(cursor.nextResult(), willSucceedFast());
+
+ // ALTER TABLE t ADD COLUMN val_3 INT;
+ cursor = cursor.nextResult().join();
+ assertDdlResult(cursor, true);
+ assertThat(cursor.hasNextResult(), is(true));
+ assertThat(cursor.nextResult(), willSucceedFast());
+
+ // ALTER TABLE t DROP COLUMN val_2 INT;
+ cursor = cursor.nextResult().join();
+ assertDdlResult(cursor, true);
+ assertThat(cursor.hasNextResult(), is(true));
+ assertThat(cursor.nextResult(), willSucceedFast());
+
+ // CREATE TABLE t2 (id INT PRIMARY KEY, val_1 INT, val_2 INT)
+ cursor = cursor.nextResult().join();
+ assertDdlResult(cursor, true);
+ assertThat(cursor.hasNextResult(), is(false));
+
+ // ALTER splits the batch
+ assertEquals(4, executeCallCounter.get());
+
+ assertTableExists("t1");
+ assertTableExists("t2");
+ }
+
+ @Test
+ void batchIsSplitByDrop() {
+ System.out.println("XXX: " + executeCallCounter.get());
Review Comment:
Looks like some debug output
##########
modules/sql-engine/src/test/java/org/apache/ignite/internal/sql/engine/exec/DdlBatchingTest.java:
##########
@@ -482,10 +603,11 @@ private void assertIndexNotExists(String name) {
private CatalogManager catalogManagerDecorator(CatalogManager original) {
return (CatalogManager) Proxy.newProxyInstance(
QueryTimeoutTest.class.getClassLoader(),
- new Class<?>[] {CatalogManager.class},
+ new Class<?>[]{CatalogManager.class},
(proxy, method, args) -> {
if ("execute".equals(method.getName())) {
- executeCallCounter.incrementAndGet();
+ int i = executeCallCounter.incrementAndGet();
+ System.out.println(IgniteStringFormatter.format("CALL:
try={}, args={}", i, Arrays.toString(args)));
Review Comment:
looks like some debug output that should be removed before merge?
--
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]
