zstan commented on code in PR #13099: URL: https://github.com/apache/ignite/pull/13099#discussion_r3193197453
########## modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/sql/IgniteSqlRollbackToSavepoint.java: ########## @@ -0,0 +1,65 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.ignite.internal.processors.query.calcite.sql; + +import java.util.List; +import com.google.common.collect.ImmutableList; +import org.apache.calcite.sql.SqlDdl; +import org.apache.calcite.sql.SqlIdentifier; +import org.apache.calcite.sql.SqlKind; +import org.apache.calcite.sql.SqlNode; +import org.apache.calcite.sql.SqlOperator; +import org.apache.calcite.sql.SqlSpecialOperator; +import org.apache.calcite.sql.SqlWriter; +import org.apache.calcite.sql.parser.SqlParserPos; + +/** Parse tree for {@code ROLLBACK TO SAVEPOINT name} statement. */ +public class IgniteSqlRollbackToSavepoint extends SqlDdl { + /** */ + protected static final SqlOperator OPERATOR = + new SqlSpecialOperator("ROLLBACK TO SAVEPOINT", SqlKind.OTHER_DDL); + + /** Savepoint name. */ + private final SqlIdentifier name; + + /** + * @param pos Parser position. + * @param name Savepoint name. + */ + public IgniteSqlRollbackToSavepoint(SqlParserPos pos, SqlIdentifier name) { + super(OPERATOR, pos); + + this.name = name; + } + + /** */ + public SqlIdentifier name() { + return name; + } + + /** {@inheritDoc} */ + @Override public List<SqlNode> getOperandList() { + return ImmutableList.of(name); + } + + /** {@inheritDoc} */ + @Override public void unparse(SqlWriter writer, int leftPrec, int rightPrec) { + writer.keyword("ROLLBACK TO SAVEPOINT"); Review Comment: why not ? ```suggestion writer.keyword(getOperator().getName()); ``` ########## modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/exec/ddl/DdlCommandHandler.java: ########## @@ -123,6 +124,34 @@ else if (cmd instanceof NativeCommandWrapper) } } + /** */ + private void handle0(BaseQueryContext qryCtx, TransactionCommand cmd) throws IgniteCheckedException { + if (cmd.type() == TransactionCommand.Type.NOOP) Review Comment: how can we get NOOP here ? need test or remove. ########## modules/calcite/src/test/java/org/apache/ignite/internal/processors/tx/SqlTransactionsSavepointTest.java: ########## @@ -0,0 +1,240 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.ignite.internal.processors.tx; + +import java.util.List; +import org.apache.ignite.cache.query.FieldsQueryCursor; +import org.apache.ignite.cache.query.SqlFieldsQuery; +import org.apache.ignite.calcite.CalciteQueryEngineConfiguration; +import org.apache.ignite.configuration.IgniteConfiguration; +import org.apache.ignite.configuration.SqlConfiguration; +import org.apache.ignite.configuration.TransactionConfiguration; +import org.apache.ignite.internal.processors.query.IgniteSQLException; +import org.apache.ignite.internal.processors.query.QueryEngine; +import org.apache.ignite.internal.processors.query.calcite.QueryChecker; +import org.apache.ignite.internal.processors.query.calcite.integration.AbstractBasicIntegrationTest; +import org.apache.ignite.internal.processors.query.calcite.util.Commons; +import org.apache.ignite.transactions.Transaction; +import org.junit.Test; + +import static java.util.concurrent.TimeUnit.SECONDS; +import static org.apache.ignite.internal.processors.query.calcite.integration.AbstractBasicIntegrationTransactionalTest.SqlTransactionMode.ALL; +import static org.apache.ignite.transactions.TransactionConcurrency.PESSIMISTIC; +import static org.apache.ignite.transactions.TransactionIsolation.READ_COMMITTED; + +/** Tests SQL savepoint commands executed by Calcite. */ +public class SqlTransactionsSavepointTest extends AbstractBasicIntegrationTest { + /** */ + private static final String TBL = "SAVEPOINT_TEST_TABLE"; + + /** {@inheritDoc} */ + @Override protected IgniteConfiguration getConfiguration(String igniteInstanceName) throws Exception { + return super.getConfiguration(igniteInstanceName) + .setTransactionConfiguration(new TransactionConfiguration() + .setTxAwareQueriesEnabled(true)) + .setSqlConfiguration(new SqlConfiguration() + .setQueryEnginesConfiguration(new CalciteQueryEngineConfiguration())); + } + + /** {@inheritDoc} */ + @Override protected void afterTestsStopped() throws Exception { + stopAllGrids(); + + super.afterTestsStopped(); + } + + /** {@inheritDoc} */ + @Override protected void beforeTest() throws Exception { + super.beforeTest(); + + sql("CREATE TABLE " + TBL + "(ID INT PRIMARY KEY, VAL VARCHAR) WITH atomicity=transactional"); + } + + /** */ + @Test + public void testSavepointCommandsInSqlScript() { + try (Transaction tx = client.transactions().txStart(PESSIMISTIC, READ_COMMITTED)) { + sqlScript( + "INSERT INTO " + TBL + " VALUES (1, 'before_sp1');" + + "SAVEPOINT sp1;" + + "INSERT INTO " + TBL + " VALUES (2, 'after_sp1');" + + "SAVEPOINT sp2;" + + "UPDATE " + TBL + " SET VAL = 'after_sp2' WHERE ID = 1;" + + "DELETE FROM " + TBL + " WHERE ID = 2;" + + "ROLLBACK TO SAVEPOINT sp2" + ); + + assertQuery(tx, "SELECT ID, VAL FROM " + TBL + " ORDER BY ID") + .returns(1, "before_sp1") + .returns(2, "after_sp1") + .ordered() Review Comment: redundant, already ordered ########## modules/calcite/src/test/java/org/apache/ignite/internal/processors/tx/SqlTransactionsSavepointTest.java: ########## @@ -0,0 +1,240 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.ignite.internal.processors.tx; + +import java.util.List; +import org.apache.ignite.cache.query.FieldsQueryCursor; +import org.apache.ignite.cache.query.SqlFieldsQuery; +import org.apache.ignite.calcite.CalciteQueryEngineConfiguration; +import org.apache.ignite.configuration.IgniteConfiguration; +import org.apache.ignite.configuration.SqlConfiguration; +import org.apache.ignite.configuration.TransactionConfiguration; +import org.apache.ignite.internal.processors.query.IgniteSQLException; +import org.apache.ignite.internal.processors.query.QueryEngine; +import org.apache.ignite.internal.processors.query.calcite.QueryChecker; +import org.apache.ignite.internal.processors.query.calcite.integration.AbstractBasicIntegrationTest; +import org.apache.ignite.internal.processors.query.calcite.util.Commons; +import org.apache.ignite.transactions.Transaction; +import org.junit.Test; + +import static java.util.concurrent.TimeUnit.SECONDS; +import static org.apache.ignite.internal.processors.query.calcite.integration.AbstractBasicIntegrationTransactionalTest.SqlTransactionMode.ALL; +import static org.apache.ignite.transactions.TransactionConcurrency.PESSIMISTIC; +import static org.apache.ignite.transactions.TransactionIsolation.READ_COMMITTED; + +/** Tests SQL savepoint commands executed by Calcite. */ +public class SqlTransactionsSavepointTest extends AbstractBasicIntegrationTest { + /** */ + private static final String TBL = "SAVEPOINT_TEST_TABLE"; + + /** {@inheritDoc} */ + @Override protected IgniteConfiguration getConfiguration(String igniteInstanceName) throws Exception { + return super.getConfiguration(igniteInstanceName) + .setTransactionConfiguration(new TransactionConfiguration() + .setTxAwareQueriesEnabled(true)) + .setSqlConfiguration(new SqlConfiguration() + .setQueryEnginesConfiguration(new CalciteQueryEngineConfiguration())); + } + + /** {@inheritDoc} */ + @Override protected void afterTestsStopped() throws Exception { + stopAllGrids(); + + super.afterTestsStopped(); + } + + /** {@inheritDoc} */ + @Override protected void beforeTest() throws Exception { + super.beforeTest(); + + sql("CREATE TABLE " + TBL + "(ID INT PRIMARY KEY, VAL VARCHAR) WITH atomicity=transactional"); + } + + /** */ + @Test + public void testSavepointCommandsInSqlScript() { + try (Transaction tx = client.transactions().txStart(PESSIMISTIC, READ_COMMITTED)) { + sqlScript( + "INSERT INTO " + TBL + " VALUES (1, 'before_sp1');" + + "SAVEPOINT sp1;" + Review Comment: i see - it`s possible to create savepoints with equal names, is it expected ? if so - such ambiguity need to be documented too ! ########## docs/_docs/sql-reference/transactions.adoc: ########## @@ -0,0 +1,76 @@ +// Licensed to the Apache Software Foundation (ASF) under one or more +// contributor license agreements. See the NOTICE file distributed with +// this work for additional information regarding copyright ownership. +// The ASF licenses this file to You under the Apache License, Version 2.0 +// (the "License"); you may not use this file except in compliance with +// the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. += Transactions + +This page describes SQL transaction commands supported by the Calcite-based SQL engine. + +== SAVEPOINT + +Creates a named savepoint in the current transaction. + +[source,sql] +---- +SAVEPOINT savepointName +---- + +=== Parameters + +- `savepointName` - the name of the savepoint to create. + +=== Description + +`SAVEPOINT` can be used only inside an explicit `PESSIMISTIC` transaction. + +The command records the current transaction state. Later, you can use `ROLLBACK TO SAVEPOINT` to roll back all transaction changes made after the savepoint was created. + +If a savepoint with the same name already exists, `SAVEPOINT` replaces it. The command does not roll back any transaction changes. It removes the previous savepoint with that name and makes the name refer to the current transaction state. + +== ROLLBACK TO SAVEPOINT + +Rolls back transaction changes to a previously created savepoint. + +[source,sql] +---- +ROLLBACK TO SAVEPOINT savepointName +---- + +=== Parameters + +- `savepointName` - the name of the savepoint to roll back to. + +=== Description + +`ROLLBACK TO SAVEPOINT` can be used only inside an explicit `PESSIMISTIC` transaction. + +The command rolls back all transaction changes made after the specified savepoint was created. The transaction remains active and can be committed or rolled back later. + +When a transaction is rolled back to a savepoint, savepoints created after the target savepoint are released. The target savepoint remains available and can be used again. + +If the specified savepoint does not exist, the command fails. + +== Example + +[source,sql] +---- +INSERT INTO Person(id, name) VALUES (1, 'John'); Review Comment: this is pure sql syntax, do we need additionally append : transactions().txStart(PESSIMISTIC, READ_COMMITTED) and tx.commit(); here ? Just for clarification ? ########## modules/calcite/src/test/java/org/apache/ignite/internal/processors/tx/SqlTransactionsSavepointTest.java: ########## @@ -0,0 +1,240 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.ignite.internal.processors.tx; + +import java.util.List; +import org.apache.ignite.cache.query.FieldsQueryCursor; +import org.apache.ignite.cache.query.SqlFieldsQuery; +import org.apache.ignite.calcite.CalciteQueryEngineConfiguration; +import org.apache.ignite.configuration.IgniteConfiguration; +import org.apache.ignite.configuration.SqlConfiguration; +import org.apache.ignite.configuration.TransactionConfiguration; +import org.apache.ignite.internal.processors.query.IgniteSQLException; +import org.apache.ignite.internal.processors.query.QueryEngine; +import org.apache.ignite.internal.processors.query.calcite.QueryChecker; +import org.apache.ignite.internal.processors.query.calcite.integration.AbstractBasicIntegrationTest; +import org.apache.ignite.internal.processors.query.calcite.util.Commons; +import org.apache.ignite.transactions.Transaction; +import org.junit.Test; + +import static java.util.concurrent.TimeUnit.SECONDS; +import static org.apache.ignite.internal.processors.query.calcite.integration.AbstractBasicIntegrationTransactionalTest.SqlTransactionMode.ALL; +import static org.apache.ignite.transactions.TransactionConcurrency.PESSIMISTIC; +import static org.apache.ignite.transactions.TransactionIsolation.READ_COMMITTED; + +/** Tests SQL savepoint commands executed by Calcite. */ +public class SqlTransactionsSavepointTest extends AbstractBasicIntegrationTest { + /** */ + private static final String TBL = "SAVEPOINT_TEST_TABLE"; + + /** {@inheritDoc} */ + @Override protected IgniteConfiguration getConfiguration(String igniteInstanceName) throws Exception { + return super.getConfiguration(igniteInstanceName) + .setTransactionConfiguration(new TransactionConfiguration() + .setTxAwareQueriesEnabled(true)) + .setSqlConfiguration(new SqlConfiguration() + .setQueryEnginesConfiguration(new CalciteQueryEngineConfiguration())); + } + + /** {@inheritDoc} */ + @Override protected void afterTestsStopped() throws Exception { + stopAllGrids(); + + super.afterTestsStopped(); + } + + /** {@inheritDoc} */ + @Override protected void beforeTest() throws Exception { + super.beforeTest(); + + sql("CREATE TABLE " + TBL + "(ID INT PRIMARY KEY, VAL VARCHAR) WITH atomicity=transactional"); + } + + /** */ + @Test + public void testSavepointCommandsInSqlScript() { + try (Transaction tx = client.transactions().txStart(PESSIMISTIC, READ_COMMITTED)) { + sqlScript( + "INSERT INTO " + TBL + " VALUES (1, 'before_sp1');" + + "SAVEPOINT sp1;" + + "INSERT INTO " + TBL + " VALUES (2, 'after_sp1');" + + "SAVEPOINT sp2;" + + "UPDATE " + TBL + " SET VAL = 'after_sp2' WHERE ID = 1;" + + "DELETE FROM " + TBL + " WHERE ID = 2;" + + "ROLLBACK TO SAVEPOINT sp2" Review Comment: savepoint`s need to be liner as mentioned ? thus such is impossible too ? ``` "ROLLBACK TO SAVEPOINT sp2;" + "ROLLBACK TO SAVEPOINT sp1;" + "ROLLBACK TO SAVEPOINT sp2;" ``` ########## modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/prepare/ddl/TransactionCommand.java: ########## @@ -16,8 +16,47 @@ */ package org.apache.ignite.internal.processors.query.calcite.prepare.ddl; -/** - * No-op command for COMMIT and ROLLBACK - */ +/** Command for transaction control statements. */ public class TransactionCommand implements DdlCommand { + /** Transaction command type. */ + public enum Type { + /** No-op command for COMMIT and ROLLBACK. */ + NOOP, + + /** Create a transaction savepoint. */ + SAVEPOINT, + + /** Roll transaction changes back to a savepoint. */ + ROLLBACK_TO_SAVEPOINT + } + + /** */ + private final Type type; + + /** Savepoint name. */ + private final String savepointName; Review Comment: @Nullable ? ########## modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/exec/ddl/DdlCommandHandler.java: ########## @@ -123,6 +124,34 @@ else if (cmd instanceof NativeCommandWrapper) } } + /** */ + private void handle0(BaseQueryContext qryCtx, TransactionCommand cmd) throws IgniteCheckedException { Review Comment: we don`t need a whole **qryCtx** here only tx, also - tx is autoClosable - who is resbonsible for closing it ? need to document it somehow ########## modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/exec/ddl/DdlCommandHandler.java: ########## @@ -123,6 +124,34 @@ else if (cmd instanceof NativeCommandWrapper) } } + /** */ + private void handle0(BaseQueryContext qryCtx, TransactionCommand cmd) throws IgniteCheckedException { + if (cmd.type() == TransactionCommand.Type.NOOP) + return; + + GridNearTxLocal tx = Commons.queryTransaction(qryCtx, cacheProc.context()); + + if (tx == null) { + throw new IgniteSQLException("Savepoints can be used only inside explicit transactions.", Review Comment: move to final static plz ? <img width="641" height="136" alt="Image" src="https://github.com/user-attachments/assets/f3c62f11-33be-4506-8aef-abcc6e8f8765"; /> -- 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]
