Re: [PR] IGNITE-28629 Support savepoint in SQL [ignite]
vldpyatkov merged PR #13099: URL: https://github.com/apache/ignite/pull/13099 -- 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-28629 Support savepoint in SQL [ignite]
vldpyatkov commented on code in PR #13099:
URL: https://github.com/apache/ignite/pull/13099#discussion_r3195358706
##
modules/core/src/main/java/org/apache/ignite/internal/processors/cache/distributed/near/GridNearTxLocal.java:
##
@@ -3035,9 +3035,10 @@ private void resume(long threadId) throws
IgniteCheckedException {
/**
* Creates savepoint for a pessimistic transaction.
*
- * @param name Savepoint name.
+ * @param name Savepoint name.
Review Comment:
It is not a tab, but a series of spaces. I do not know, but I just pressed a
hot key, and the text was aligned in comparison with our stylesheet.
Anyway, I have to get a visa before merging it. We will look. Is it an issue
or our old good style...
--
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-28629 Support savepoint in SQL [ignite]
vldpyatkov commented on code in PR #13099:
URL: https://github.com/apache/ignite/pull/13099#discussion_r3195358706
##
modules/core/src/main/java/org/apache/ignite/internal/processors/cache/distributed/near/GridNearTxLocal.java:
##
@@ -3035,9 +3035,10 @@ private void resume(long threadId) throws
IgniteCheckedException {
/**
* Creates savepoint for a pessimistic transaction.
*
- * @param name Savepoint name.
+ * @param name Savepoint name.
Review Comment:
It is not a tab, but a series of spaces. I do not know, but I just pressed a
hot key and the test was aligned in comparison with our stylesheet.
Anyway, I have to get a visa before merging it. We will look. Is it an issue
or our old good style...
--
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-28629 Support savepoint in SQL [ignite]
vldpyatkov commented on code in PR #13099:
URL: https://github.com/apache/ignite/pull/13099#discussion_r3195332582
##
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:
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-28629 Support savepoint in SQL [ignite]
zstan commented on code in PR #13099:
URL: https://github.com/apache/ignite/pull/13099#discussion_r3195213047
##
modules/core/src/main/java/org/apache/ignite/internal/processors/cache/distributed/near/GridNearTxLocal.java:
##
@@ -3035,9 +3035,10 @@ private void resume(long threadId) throws
IgniteCheckedException {
/**
* Creates savepoint for a pessimistic transaction.
*
- * @param name Savepoint name.
+ * @param name Savepoint name.
Review Comment:
tabulation ?
--
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-28629 Support savepoint in SQL [ignite]
sonarqubecloud[bot] commented on PR #13099: URL: https://github.com/apache/ignite/pull/13099#issuecomment-4387624788 ## [](https://sonarcloud.io/dashboard?id=apache_ignite&pullRequest=13099) **Quality Gate failed** Failed conditions  [7.9% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_ignite&pullRequest=13099&metric=new_duplicated_lines_density&view=list) (required ≤ 5%) [See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_ignite&pullRequest=13099) -- 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-28629 Support savepoint in SQL [ignite]
zstan commented on code in PR #13099:
URL: https://github.com/apache/ignite/pull/13099#discussion_r3195025155
##
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:
you can push not **qryCtx** but lambda or simple tx =
Commons.queryTransaction(qryCtx, cacheProc.context()); I believe we need to
reduce function parameters scope if it possible, wdyt ?
--
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-28629 Support savepoint in SQL [ignite]
zstan commented on code in PR #13099:
URL: https://github.com/apache/ignite/pull/13099#discussion_r3194954163
##
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);
Review Comment:
now - it`s ok
--
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-28629 Support savepoint in SQL [ignite]
sonarqubecloud[bot] commented on PR #13099: URL: https://github.com/apache/ignite/pull/13099#issuecomment-4387129607 ## [](https://sonarcloud.io/dashboard?id=apache_ignite&pullRequest=13099) **Quality Gate failed** Failed conditions  [8.1% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_ignite&pullRequest=13099&metric=new_duplicated_lines_density&view=list) (required ≤ 5%) [See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_ignite&pullRequest=13099) -- 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-28629 Support savepoint in SQL [ignite]
vldpyatkov commented on code in PR #13099:
URL: https://github.com/apache/ignite/pull/13099#discussion_r3194656142
##
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:
The behavior is also documented:
`When a transaction is rolled back to a savepoint, savepoints created after
the target savepoint are released.`
--
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-28629 Support savepoint in SQL [ignite]
vldpyatkov commented on code in PR #13099: URL: https://github.com/apache/ignite/pull/13099#discussion_r3194645044 ## 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: We do not have a syntax for a SQL script to start/stop transactions. I do not think that a part of Java syntax is appropriate here. But you're right; we would like to have BEGIN, COMMIT, and ROLLBACK functions in SQL. -- 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-28629 Support savepoint in SQL [ignite]
vldpyatkov commented on code in PR #13099:
URL: https://github.com/apache/ignite/pull/13099#discussion_r3194610588
##
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:
I do not see a way to avoid `qryCtx` because it is used to get a transaction.
As for the question, "Who is responsible for closing?" It is a transaction
from the client layer (suspended and resumed to use in SQL flow), so the client
is responsible for this one.
--
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-28629 Support savepoint in SQL [ignite]
vldpyatkov commented on code in PR #13099:
URL: https://github.com/apache/ignite/pull/13099#discussion_r3194581500
##
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:
Extracted it in a const SAVEPOINTS_EXPLICIT_TX_ONLY
--
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-28629 Support savepoint in SQL [ignite]
vldpyatkov commented on code in PR #13099:
URL: https://github.com/apache/ignite/pull/13099#discussion_r3194517515
##
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:
It is present behavior, a reaction to COMMIT/ROLLBAK SQL command.
--
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-28629 Support savepoint in SQL [ignite]
vldpyatkov commented on code in PR #13099:
URL: https://github.com/apache/ignite/pull/13099#discussion_r3194125161
##
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:
Your example looks wrong because you cannot use sp2 after you already rolled
back to sp1.
--
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-28629 Support savepoint in SQL [ignite]
vldpyatkov commented on code in PR #13099:
URL: https://github.com/apache/ignite/pull/13099#discussion_r3194112893
##
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:
It is documented in transactions.adoc
`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.`
--
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-28629 Support savepoint in SQL [ignite]
vldpyatkov commented on code in PR #13099:
URL: https://github.com/apache/ignite/pull/13099#discussion_r3194105054
##
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 getOperandList() {
+return ImmutableList.of(name);
+}
+
+/** {@inheritDoc} */
+@Override public void unparse(SqlWriter writer, int leftPrec, int
rightPrec) {
+writer.keyword("ROLLBACK TO SAVEPOINT");
Review Comment:
Ok.
--
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-28629 Support savepoint in SQL [ignite]
vldpyatkov commented on code in PR #13099:
URL: https://github.com/apache/ignite/pull/13099#discussion_r3194101013
##
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);
Review Comment:
I am not sure that is right, but it is a style in our code. For example,
IgniteSqlCommit, IgniteSqlRollback.
--
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-28629 Support savepoint in SQL [ignite]
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 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
Re: [PR] IGNITE-28629 Support savepoint in SQL [ignite]
zstan commented on code in PR #13099:
URL: https://github.com/apache/ignite/pull/13099#discussion_r3193155366
##
modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/sql/IgniteSqlSavepoint.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 SAVEPOINT name} statement. */
+public class IgniteSqlSavepoint extends SqlDdl {
+/** */
+protected static final SqlOperator OPERATOR =
+new SqlSpecialOperator("SAVEPOINT", SqlKind.OTHER_DDL);
+
+/** Savepoint name. */
+private final SqlIdentifier name;
+
+/**
+ * @param pos Parser position.
+ * @param name Savepoint name.
+ */
+public IgniteSqlSavepoint(SqlParserPos pos, SqlIdentifier name) {
+super(OPERATOR, pos);
+
+this.name = name;
+}
+
+/** */
+public SqlIdentifier name() {
+return name;
+}
+
+/** {@inheritDoc} */
+@Override public List getOperandList() {
+return ImmutableList.of(name);
Review Comment:
+1 here too, let`s reduce guava usage
##
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 getOperandList() {
+return ImmutableList.of(name);
Review Comment:
+1 with sonar here
##
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 i
Re: [PR] IGNITE-28629 Support savepoint in SQL [ignite]
sonarqubecloud[bot] commented on PR #13099: URL: https://github.com/apache/ignite/pull/13099#issuecomment-4377639987 ## [](https://sonarcloud.io/dashboard?id=apache_ignite&pullRequest=13099) **Quality Gate failed** Failed conditions  [2 New Code Smells](https://sonarcloud.io/dashboard?id=apache_ignite&pullRequest=13099) (required ≤ 1) [See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_ignite&pullRequest=13099) ##  Catch issues before they fail your Quality Gate with our IDE extension  [SonarQube for IDE](https://www.sonarsource.com/products/sonarlint/features/connected-mode/?referrer=pull-request) -- 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-28629 Support savepoint in SQL [ignite]
sonarqubecloud[bot] commented on PR #13099: URL: https://github.com/apache/ignite/pull/13099#issuecomment-4373308006 ## [](https://sonarcloud.io/dashboard?id=apache_ignite&pullRequest=13099) **Quality Gate failed** Failed conditions  [2 New Code Smells](https://sonarcloud.io/dashboard?id=apache_ignite&pullRequest=13099) (required ≤ 1) [See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_ignite&pullRequest=13099) ##  Catch issues before they fail your Quality Gate with our IDE extension  [SonarQube for IDE](https://www.sonarsource.com/products/sonarlint/features/connected-mode/?referrer=pull-request) -- 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]
