This is an automated email from the ASF dual-hosted git repository.
terrymanu pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/shardingsphere.git
The following commit(s) were added to refs/heads/master by this push:
new 11e948a4e96 Fix firebird batch rls cancel without batch (#40098)
11e948a4e96 is described below
commit 11e948a4e9680dfee35442170cacdb5eeab486a4
Author: Maxim Igoshin <[email protected]>
AuthorDate: Sun Oct 18 06:36:42 2026 +0300
Fix firebird batch rls cancel without batch (#40098)
* Reject Firebird op_batch_rls and op_batch_cancel without an open batch
The server checks that the statement has a batch and answers
isc_bad_batch_handle otherwise, keeping the connection.
* Keep Firebird transaction usable after op_batch_rls or op_batch_cancel
without a batch
Return the isc_bad_batch_handle response from the batch release and cancel
executors instead of throwing InvalidBatchHandleException, so the frontend
engine does not mark the transaction as failed and a following COMMIT still
commits the changes made before, as the Firebird server does.
* Add TODO to restore checkNotNull in Firebird batch release and cancel
executors
* Fix CODE_OF_CONDUCT violations in Firebird batch release and cancel tests
* Trigger CI
---
.../batch/FirebirdBatchCancelCommandExecutor.java | 8 ++-
.../batch/FirebirdBatchReleaseCommandExecutor.java | 9 ++-
.../FirebirdBatchCancelCommandExecutorTest.java | 78 +++++++++++++++++++--
.../FirebirdBatchReleaseCommandExecutorTest.java | 80 +++++++++++++++++++++-
4 files changed, 165 insertions(+), 10 deletions(-)
diff --git
a/proxy/frontend/dialect/firebird/src/main/java/org/apache/shardingsphere/proxy/frontend/firebird/command/query/batch/FirebirdBatchCancelCommandExecutor.java
b/proxy/frontend/dialect/firebird/src/main/java/org/apache/shardingsphere/proxy/frontend/firebird/command/query/batch/FirebirdBatchCancelCommandExecutor.java
index 704e69556ea..9ad52c4dfaa 100644
---
a/proxy/frontend/dialect/firebird/src/main/java/org/apache/shardingsphere/proxy/frontend/firebird/command/query/batch/FirebirdBatchCancelCommandExecutor.java
+++
b/proxy/frontend/dialect/firebird/src/main/java/org/apache/shardingsphere/proxy/frontend/firebird/command/query/batch/FirebirdBatchCancelCommandExecutor.java
@@ -18,6 +18,8 @@
package org.apache.shardingsphere.proxy.frontend.firebird.command.query.batch;
import lombok.RequiredArgsConstructor;
+import
org.apache.shardingsphere.database.exception.firebird.exception.protocol.InvalidBatchHandleException;
+import
org.apache.shardingsphere.database.protocol.firebird.err.FirebirdErrorPacketFactory;
import
org.apache.shardingsphere.database.protocol.firebird.packet.command.query.batch.FirebirdBatchCancelCommandPacket;
import
org.apache.shardingsphere.database.protocol.firebird.packet.command.query.batch.FirebirdBatchStatement;
import
org.apache.shardingsphere.database.protocol.firebird.packet.generic.FirebirdGenericResponsePacket;
@@ -43,9 +45,11 @@ public final class FirebirdBatchCancelCommandExecutor
implements CommandExecutor
public Collection<DatabasePacket> execute() throws SQLException {
FirebirdBatchStatementManager batchStatementManager =
FirebirdBatchStatementManager.getInstance();
FirebirdBatchStatement batchStatement =
batchStatementManager.getBatchStatement(connectionSession.getConnectionId(),
packet.getStatementHandle());
- if (null != batchStatement) {
- batchStatementManager.resetBatchStatement(batchStatement);
+ // TODO restore checkNotNull when FirebirdFrontendEngine stops marking
the transaction as failed for InvalidBatchHandleException
+ if (null == batchStatement) {
+ return
Collections.singleton(FirebirdErrorPacketFactory.newInstance(new
InvalidBatchHandleException(packet.getStatementHandle())));
}
+ batchStatementManager.resetBatchStatement(batchStatement);
return Collections.singleton(new FirebirdGenericResponsePacket());
}
}
diff --git
a/proxy/frontend/dialect/firebird/src/main/java/org/apache/shardingsphere/proxy/frontend/firebird/command/query/batch/FirebirdBatchReleaseCommandExecutor.java
b/proxy/frontend/dialect/firebird/src/main/java/org/apache/shardingsphere/proxy/frontend/firebird/command/query/batch/FirebirdBatchReleaseCommandExecutor.java
index d49b6da76fb..35ded6134fc 100644
---
a/proxy/frontend/dialect/firebird/src/main/java/org/apache/shardingsphere/proxy/frontend/firebird/command/query/batch/FirebirdBatchReleaseCommandExecutor.java
+++
b/proxy/frontend/dialect/firebird/src/main/java/org/apache/shardingsphere/proxy/frontend/firebird/command/query/batch/FirebirdBatchReleaseCommandExecutor.java
@@ -18,6 +18,8 @@
package org.apache.shardingsphere.proxy.frontend.firebird.command.query.batch;
import lombok.RequiredArgsConstructor;
+import
org.apache.shardingsphere.database.exception.firebird.exception.protocol.InvalidBatchHandleException;
+import
org.apache.shardingsphere.database.protocol.firebird.err.FirebirdErrorPacketFactory;
import
org.apache.shardingsphere.database.protocol.firebird.packet.command.query.batch.FirebirdBatchReleaseCommandPacket;
import
org.apache.shardingsphere.database.protocol.firebird.packet.generic.FirebirdGenericResponsePacket;
import org.apache.shardingsphere.database.protocol.packet.DatabasePacket;
@@ -40,7 +42,12 @@ public final class FirebirdBatchReleaseCommandExecutor
implements CommandExecuto
@Override
public Collection<DatabasePacket> execute() throws SQLException {
-
FirebirdBatchStatementManager.getInstance().unregisterBatchStatement(connectionSession.getConnectionId(),
packet.getStatementHandle());
+ FirebirdBatchStatementManager batchStatementManager =
FirebirdBatchStatementManager.getInstance();
+ // TODO restore checkNotNull when FirebirdFrontendEngine stops marking
the transaction as failed for InvalidBatchHandleException
+ if (null ==
batchStatementManager.getBatchStatement(connectionSession.getConnectionId(),
packet.getStatementHandle())) {
+ return
Collections.singleton(FirebirdErrorPacketFactory.newInstance(new
InvalidBatchHandleException(packet.getStatementHandle())));
+ }
+
batchStatementManager.unregisterBatchStatement(connectionSession.getConnectionId(),
packet.getStatementHandle());
return Collections.singleton(new FirebirdGenericResponsePacket());
}
}
diff --git
a/proxy/frontend/dialect/firebird/src/test/java/org/apache/shardingsphere/proxy/frontend/firebird/command/query/batch/FirebirdBatchCancelCommandExecutorTest.java
b/proxy/frontend/dialect/firebird/src/test/java/org/apache/shardingsphere/proxy/frontend/firebird/command/query/batch/FirebirdBatchCancelCommandExecutorTest.java
index 0f5d02f249a..2ebaf036a33 100644
---
a/proxy/frontend/dialect/firebird/src/test/java/org/apache/shardingsphere/proxy/frontend/firebird/command/query/batch/FirebirdBatchCancelCommandExecutorTest.java
+++
b/proxy/frontend/dialect/firebird/src/test/java/org/apache/shardingsphere/proxy/frontend/firebird/command/query/batch/FirebirdBatchCancelCommandExecutorTest.java
@@ -17,30 +17,59 @@
package org.apache.shardingsphere.proxy.frontend.firebird.command.query.batch;
+import io.netty.buffer.Unpooled;
+import io.netty.channel.ChannelHandlerContext;
+import io.netty.util.DefaultAttributeMap;
+import
org.apache.shardingsphere.database.connector.core.spi.DatabaseTypedSPILoader;
+import org.apache.shardingsphere.database.connector.core.type.DatabaseType;
+import org.apache.shardingsphere.database.protocol.constant.CommonConstants;
+import
org.apache.shardingsphere.database.protocol.firebird.packet.command.FirebirdCommandPacketType;
import
org.apache.shardingsphere.database.protocol.firebird.packet.command.query.batch.FirebirdBatchCancelCommandPacket;
import
org.apache.shardingsphere.database.protocol.firebird.packet.command.query.batch.FirebirdBatchRegistry;
import
org.apache.shardingsphere.database.protocol.firebird.packet.command.query.batch.FirebirdBatchStatement;
import
org.apache.shardingsphere.database.protocol.firebird.packet.generic.FirebirdGenericResponsePacket;
import org.apache.shardingsphere.database.protocol.packet.DatabasePacket;
+import org.apache.shardingsphere.infra.config.props.ConfigurationProperties;
+import org.apache.shardingsphere.infra.metadata.ShardingSphereMetaData;
+import org.apache.shardingsphere.infra.metadata.database.rule.RuleMetaData;
+import org.apache.shardingsphere.infra.metadata.user.Grantee;
+import org.apache.shardingsphere.infra.spi.type.typed.TypedSPILoader;
+import org.apache.shardingsphere.proxy.backend.context.ProxyContext;
import org.apache.shardingsphere.proxy.backend.session.ConnectionSession;
+import org.apache.shardingsphere.proxy.frontend.command.CommandExecutorTask;
+import
org.apache.shardingsphere.proxy.frontend.firebird.command.query.transaction.FirebirdTransactionIdGenerator;
+import
org.apache.shardingsphere.proxy.frontend.spi.DatabaseProtocolFrontendEngine;
+import
org.apache.shardingsphere.test.infra.framework.extension.mock.AutoMockExtension;
+import
org.apache.shardingsphere.test.infra.framework.extension.mock.StaticMockSettings;
+import org.apache.shardingsphere.transaction.api.TransactionType;
+import org.apache.shardingsphere.transaction.rule.TransactionRule;
+import org.firebirdsql.gds.ISCConstants;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
import org.mockito.Answers;
+import org.mockito.ArgumentCaptor;
import org.mockito.Mock;
-import org.mockito.junit.jupiter.MockitoExtension;
+import java.nio.charset.StandardCharsets;
+import java.sql.Connection;
import java.sql.SQLException;
import java.util.Arrays;
import java.util.Collection;
+import java.util.Collections;
+import java.util.Properties;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.is;
import static org.hamcrest.Matchers.isA;
-import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.never;
+import static org.mockito.Mockito.times;
+import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
-@ExtendWith(MockitoExtension.class)
+@ExtendWith(AutoMockExtension.class)
+@StaticMockSettings(ProxyContext.class)
class FirebirdBatchCancelCommandExecutorTest {
private static final int CONNECTION_ID = 2;
@@ -53,6 +82,12 @@ class FirebirdBatchCancelCommandExecutorTest {
@Mock
private FirebirdBatchCancelCommandPacket packet;
+ @Mock(answer = Answers.RETURNS_DEEP_STUBS)
+ private ChannelHandlerContext channelHandlerContext;
+
+ @Mock
+ private Connection backendConnection;
+
@Test
void assertExecute() throws SQLException {
FirebirdBatchRegistry.getInstance().registerConnection(CONNECTION_ID);
@@ -71,9 +106,42 @@ class FirebirdBatchCancelCommandExecutorTest {
}
@Test
- void assertExecuteWhenBatchNotFound() {
+ void assertExecuteWhenBatchNotFound() throws SQLException {
when(connectionSession.getConnectionId()).thenReturn(CONNECTION_ID);
when(packet.getStatementHandle()).thenReturn(STATEMENT_ID);
- assertDoesNotThrow(() -> new
FirebirdBatchCancelCommandExecutor(packet, connectionSession).execute());
+ Collection<DatabasePacket> actual = new
FirebirdBatchCancelCommandExecutor(packet, connectionSession).execute();
+ assertThat(actual.size(), is(1));
+ assertThat(((FirebirdGenericResponsePacket)
actual.iterator().next()).getErrorCode(),
is(ISCConstants.isc_bad_batch_handle));
+ }
+
+ @Test
+ void assertExecuteWhenBatchNotFoundInTransaction() throws SQLException {
+ ConnectionSession transactionSession =
createConnectionSessionInTransaction();
+ int transactionHandle =
FirebirdTransactionIdGenerator.getInstance().nextTransactionId(CONNECTION_ID);
+ DatabaseProtocolFrontendEngine engine =
DatabaseTypedSPILoader.getService(DatabaseProtocolFrontendEngine.class,
TypedSPILoader.getService(DatabaseType.class, "Firebird"));
+ new CommandExecutorTask(engine, transactionSession,
channelHandlerContext,
Unpooled.buffer().writeInt(FirebirdCommandPacketType.BATCH_CANCEL.getValue()).writeInt(STATEMENT_ID)).run();
+ new CommandExecutorTask(engine, transactionSession,
channelHandlerContext,
Unpooled.buffer().writeInt(FirebirdCommandPacketType.COMMIT.getValue()).writeInt(transactionHandle)).run();
+
FirebirdTransactionIdGenerator.getInstance().unregisterConnection(CONNECTION_ID);
+ ArgumentCaptor<FirebirdGenericResponsePacket> responseCaptor =
ArgumentCaptor.forClass(FirebirdGenericResponsePacket.class);
+ verify(channelHandlerContext,
times(2)).write(responseCaptor.capture());
+ assertThat(responseCaptor.getAllValues().get(0).getErrorCode(),
is(ISCConstants.isc_bad_batch_handle));
+ verify(backendConnection).commit();
+ verify(backendConnection, never()).rollback();
+ }
+
+ private ConnectionSession createConnectionSessionInTransaction() {
+ TransactionRule transactionRule = mock(TransactionRule.class);
+
when(transactionRule.getDefaultType()).thenReturn(TransactionType.LOCAL);
+
when(ProxyContext.getInstance().getContextManager().getMetaDataContexts().getMetaData()).thenReturn(
+ new ShardingSphereMetaData(Collections.emptyList(), mock(),
new RuleMetaData(Collections.singleton(transactionRule)), new
ConfigurationProperties(new Properties())));
+
when(channelHandlerContext.channel().attr(CommonConstants.CHARSET_ATTRIBUTE_KEY).get()).thenReturn(StandardCharsets.UTF_8);
+ ConnectionSession result = new
ConnectionSession(TypedSPILoader.getService(DatabaseType.class, "Firebird"),
new DefaultAttributeMap());
+ result.setConnectionId(CONNECTION_ID);
+ result.setGrantee(new Grantee("foo_user", "%"));
+ result.setAutoCommit(false);
+ result.getDatabaseConnectionManager().handleAutoCommit();
+
result.getDatabaseConnectionManager().getCachedConnections().put("foo_db.foo_ds",
backendConnection);
+
FirebirdTransactionIdGenerator.getInstance().registerConnection(CONNECTION_ID);
+ return result;
}
}
diff --git
a/proxy/frontend/dialect/firebird/src/test/java/org/apache/shardingsphere/proxy/frontend/firebird/command/query/batch/FirebirdBatchReleaseCommandExecutorTest.java
b/proxy/frontend/dialect/firebird/src/test/java/org/apache/shardingsphere/proxy/frontend/firebird/command/query/batch/FirebirdBatchReleaseCommandExecutorTest.java
index 3a3b961fe00..50458f8ce63 100644
---
a/proxy/frontend/dialect/firebird/src/test/java/org/apache/shardingsphere/proxy/frontend/firebird/command/query/batch/FirebirdBatchReleaseCommandExecutorTest.java
+++
b/proxy/frontend/dialect/firebird/src/test/java/org/apache/shardingsphere/proxy/frontend/firebird/command/query/batch/FirebirdBatchReleaseCommandExecutorTest.java
@@ -17,28 +17,58 @@
package org.apache.shardingsphere.proxy.frontend.firebird.command.query.batch;
+import io.netty.buffer.Unpooled;
+import io.netty.channel.ChannelHandlerContext;
+import io.netty.util.DefaultAttributeMap;
+import
org.apache.shardingsphere.database.connector.core.spi.DatabaseTypedSPILoader;
+import org.apache.shardingsphere.database.connector.core.type.DatabaseType;
+import org.apache.shardingsphere.database.protocol.constant.CommonConstants;
+import
org.apache.shardingsphere.database.protocol.firebird.packet.command.FirebirdCommandPacketType;
import
org.apache.shardingsphere.database.protocol.firebird.packet.command.query.batch.FirebirdBatchRegistry;
import
org.apache.shardingsphere.database.protocol.firebird.packet.command.query.batch.FirebirdBatchReleaseCommandPacket;
import
org.apache.shardingsphere.database.protocol.firebird.packet.command.query.batch.FirebirdBatchStatement;
import
org.apache.shardingsphere.database.protocol.firebird.packet.generic.FirebirdGenericResponsePacket;
import org.apache.shardingsphere.database.protocol.packet.DatabasePacket;
+import org.apache.shardingsphere.infra.config.props.ConfigurationProperties;
+import org.apache.shardingsphere.infra.metadata.ShardingSphereMetaData;
+import org.apache.shardingsphere.infra.metadata.database.rule.RuleMetaData;
+import org.apache.shardingsphere.infra.metadata.user.Grantee;
+import org.apache.shardingsphere.infra.spi.type.typed.TypedSPILoader;
+import org.apache.shardingsphere.proxy.backend.context.ProxyContext;
import org.apache.shardingsphere.proxy.backend.session.ConnectionSession;
+import org.apache.shardingsphere.proxy.frontend.command.CommandExecutorTask;
+import
org.apache.shardingsphere.proxy.frontend.firebird.command.query.transaction.FirebirdTransactionIdGenerator;
+import
org.apache.shardingsphere.proxy.frontend.spi.DatabaseProtocolFrontendEngine;
+import
org.apache.shardingsphere.test.infra.framework.extension.mock.AutoMockExtension;
+import
org.apache.shardingsphere.test.infra.framework.extension.mock.StaticMockSettings;
+import org.apache.shardingsphere.transaction.api.TransactionType;
+import org.apache.shardingsphere.transaction.rule.TransactionRule;
+import org.firebirdsql.gds.ISCConstants;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
import org.mockito.Answers;
+import org.mockito.ArgumentCaptor;
import org.mockito.Mock;
-import org.mockito.junit.jupiter.MockitoExtension;
+import java.nio.charset.StandardCharsets;
+import java.sql.Connection;
import java.sql.SQLException;
import java.util.Collection;
+import java.util.Collections;
+import java.util.Properties;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.is;
import static org.hamcrest.Matchers.isA;
import static org.junit.jupiter.api.Assertions.assertNull;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.never;
+import static org.mockito.Mockito.times;
+import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
-@ExtendWith(MockitoExtension.class)
+@ExtendWith(AutoMockExtension.class)
+@StaticMockSettings(ProxyContext.class)
class FirebirdBatchReleaseCommandExecutorTest {
private static final int CONNECTION_ID = 2;
@@ -51,6 +81,12 @@ class FirebirdBatchReleaseCommandExecutorTest {
@Mock
private FirebirdBatchReleaseCommandPacket packet;
+ @Mock(answer = Answers.RETURNS_DEEP_STUBS)
+ private ChannelHandlerContext channelHandlerContext;
+
+ @Mock
+ private Connection backendConnection;
+
@Test
void assertExecute() throws SQLException {
FirebirdBatchRegistry.getInstance().registerConnection(CONNECTION_ID);
@@ -63,4 +99,44 @@ class FirebirdBatchReleaseCommandExecutorTest {
assertNull(FirebirdBatchRegistry.getInstance().getBatchStatement(CONNECTION_ID,
STATEMENT_ID));
FirebirdBatchRegistry.getInstance().unregisterConnection(CONNECTION_ID);
}
+
+ @Test
+ void assertExecuteWhenBatchNotFound() throws SQLException {
+ when(connectionSession.getConnectionId()).thenReturn(CONNECTION_ID);
+ when(packet.getStatementHandle()).thenReturn(STATEMENT_ID);
+ Collection<DatabasePacket> actual = new
FirebirdBatchReleaseCommandExecutor(packet, connectionSession).execute();
+ assertThat(actual.size(), is(1));
+ assertThat(((FirebirdGenericResponsePacket)
actual.iterator().next()).getErrorCode(),
is(ISCConstants.isc_bad_batch_handle));
+ }
+
+ @Test
+ void assertExecuteWhenBatchNotFoundInTransaction() throws SQLException {
+ ConnectionSession transactionSession =
createConnectionSessionInTransaction();
+ int transactionHandle =
FirebirdTransactionIdGenerator.getInstance().nextTransactionId(CONNECTION_ID);
+ DatabaseProtocolFrontendEngine engine =
DatabaseTypedSPILoader.getService(DatabaseProtocolFrontendEngine.class,
TypedSPILoader.getService(DatabaseType.class, "Firebird"));
+ new CommandExecutorTask(engine, transactionSession,
channelHandlerContext,
Unpooled.buffer().writeInt(FirebirdCommandPacketType.BATCH_RLS.getValue()).writeInt(STATEMENT_ID)).run();
+ new CommandExecutorTask(engine, transactionSession,
channelHandlerContext,
Unpooled.buffer().writeInt(FirebirdCommandPacketType.COMMIT.getValue()).writeInt(transactionHandle)).run();
+
FirebirdTransactionIdGenerator.getInstance().unregisterConnection(CONNECTION_ID);
+ ArgumentCaptor<FirebirdGenericResponsePacket> responseCaptor =
ArgumentCaptor.forClass(FirebirdGenericResponsePacket.class);
+ verify(channelHandlerContext,
times(2)).write(responseCaptor.capture());
+ assertThat(responseCaptor.getAllValues().get(0).getErrorCode(),
is(ISCConstants.isc_bad_batch_handle));
+ verify(backendConnection).commit();
+ verify(backendConnection, never()).rollback();
+ }
+
+ private ConnectionSession createConnectionSessionInTransaction() {
+ TransactionRule transactionRule = mock(TransactionRule.class);
+
when(transactionRule.getDefaultType()).thenReturn(TransactionType.LOCAL);
+
when(ProxyContext.getInstance().getContextManager().getMetaDataContexts().getMetaData()).thenReturn(
+ new ShardingSphereMetaData(Collections.emptyList(), mock(),
new RuleMetaData(Collections.singleton(transactionRule)), new
ConfigurationProperties(new Properties())));
+
when(channelHandlerContext.channel().attr(CommonConstants.CHARSET_ATTRIBUTE_KEY).get()).thenReturn(StandardCharsets.UTF_8);
+ ConnectionSession result = new
ConnectionSession(TypedSPILoader.getService(DatabaseType.class, "Firebird"),
new DefaultAttributeMap());
+ result.setConnectionId(CONNECTION_ID);
+ result.setGrantee(new Grantee("foo_user", "%"));
+ result.setAutoCommit(false);
+ result.getDatabaseConnectionManager().handleAutoCommit();
+
result.getDatabaseConnectionManager().getCachedConnections().put("foo_db.foo_ds",
backendConnection);
+
FirebirdTransactionIdGenerator.getInstance().registerConnection(CONNECTION_ID);
+ return result;
+ }
}
