This is an automated email from the ASF dual-hosted git repository.
sunnianjun 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 9508435109c Refactor OpenGaussErrorPacketFactory and
PostgreSQLErrorPacketFactory (#28133)
9508435109c is described below
commit 9508435109cbba6688ce2cebbf5c2897e80e1905
Author: Liang Zhang <[email protected]>
AuthorDate: Thu Aug 17 13:15:30 2023 +0800
Refactor OpenGaussErrorPacketFactory and PostgreSQLErrorPacketFactory
(#28133)
* Refactor OpenGaussErrorPacketFactory and PostgreSQLErrorPacketFactory
* Refactor OpenGaussErrorPacketFactory and PostgreSQLErrorPacketFactory
---
.../opengauss/err/OpenGaussErrorPacketFactory.java | 21 ++++++---------------
.../err/OpenGaussErrorPacketFactoryTest.java | 7 ++-----
.../err/PostgreSQLErrorPacketFactory.java | 22 +++++++---------------
.../err/PostgreSQLErrorPacketFactoryTest.java | 4 ++--
4 files changed, 17 insertions(+), 37 deletions(-)
diff --git
a/proxy/frontend/type/opengauss/src/main/java/org/apache/shardingsphere/proxy/frontend/opengauss/err/OpenGaussErrorPacketFactory.java
b/proxy/frontend/type/opengauss/src/main/java/org/apache/shardingsphere/proxy/frontend/opengauss/err/OpenGaussErrorPacketFactory.java
index c15a80c7515..400ed58d9f2 100644
---
a/proxy/frontend/type/opengauss/src/main/java/org/apache/shardingsphere/proxy/frontend/opengauss/err/OpenGaussErrorPacketFactory.java
+++
b/proxy/frontend/type/opengauss/src/main/java/org/apache/shardingsphere/proxy/frontend/opengauss/err/OpenGaussErrorPacketFactory.java
@@ -23,9 +23,8 @@ import lombok.NoArgsConstructor;
import
org.apache.shardingsphere.db.protocol.opengauss.packet.command.generic.OpenGaussErrorResponsePacket;
import
org.apache.shardingsphere.db.protocol.postgresql.constant.PostgreSQLMessageSeverityLevel;
import org.apache.shardingsphere.infra.database.core.type.DatabaseType;
-import
org.apache.shardingsphere.infra.exception.core.external.sql.ShardingSphereSQLException;
+import
org.apache.shardingsphere.infra.exception.core.external.sql.sqlstate.XOpenSQLState;
import
org.apache.shardingsphere.infra.exception.dialect.SQLExceptionTransformEngine;
-import
org.apache.shardingsphere.infra.exception.dialect.exception.SQLDialectException;
import
org.apache.shardingsphere.infra.exception.postgresql.vendor.PostgreSQLVendorError;
import org.apache.shardingsphere.infra.spi.type.typed.TypedSPILoader;
import org.opengauss.util.PSQLException;
@@ -51,11 +50,8 @@ public final class OpenGaussErrorPacketFactory {
if (serverErrorMessage.isPresent()) {
return new OpenGaussErrorResponsePacket(serverErrorMessage.get());
}
- if (cause instanceof SQLException || cause instanceof
ShardingSphereSQLException || cause instanceof SQLDialectException) {
- return
createErrorResponsePacket(SQLExceptionTransformEngine.toSQLException(cause,
TypedSPILoader.getService(DatabaseType.class, "PostgreSQL")));
- }
- // TODO OpenGauss need consider FrontendConnectionLimitException
- return createErrorResponsePacketForUnknownException(cause);
+ SQLException sqlException =
SQLExceptionTransformEngine.toSQLException(cause,
TypedSPILoader.getService(DatabaseType.class, "PostgreSQL"));
+ return createErrorResponsePacket(sqlException);
}
private static Optional<ServerErrorMessage> findServerErrorMessage(final
Exception cause) {
@@ -63,15 +59,10 @@ public final class OpenGaussErrorPacketFactory {
}
private static OpenGaussErrorResponsePacket
createErrorResponsePacket(final SQLException cause) {
- // TODO consider what severity to use
- String sqlState = Strings.isNullOrEmpty(cause.getSQLState()) ?
PostgreSQLVendorError.SYSTEM_ERROR.getSqlState().getValue() :
cause.getSQLState();
+ String sqlState = Strings.isNullOrEmpty(cause.getSQLState()) ||
XOpenSQLState.GENERAL_ERROR.getValue().equals(cause.getSQLState())
+ ? PostgreSQLVendorError.SYSTEM_ERROR.getSqlState().getValue()
+ : cause.getSQLState();
String message = Strings.isNullOrEmpty(cause.getMessage()) ?
cause.toString() : cause.getMessage();
return new
OpenGaussErrorResponsePacket(PostgreSQLMessageSeverityLevel.ERROR, sqlState,
message);
}
-
- private static OpenGaussErrorResponsePacket
createErrorResponsePacketForUnknownException(final Exception cause) {
- // TODO add FIELD_TYPE_CODE for common error and consider what
severity to use
- String message = Strings.isNullOrEmpty(cause.getLocalizedMessage()) ?
cause.toString() : cause.getLocalizedMessage();
- return new
OpenGaussErrorResponsePacket(PostgreSQLMessageSeverityLevel.ERROR,
PostgreSQLVendorError.SYSTEM_ERROR.getSqlState().getValue(), message);
- }
}
diff --git
a/proxy/frontend/type/opengauss/src/test/java/org/apache/shardingsphere/proxy/frontend/opengauss/err/OpenGaussErrorPacketFactoryTest.java
b/proxy/frontend/type/opengauss/src/test/java/org/apache/shardingsphere/proxy/frontend/opengauss/err/OpenGaussErrorPacketFactoryTest.java
index f38c50e9d8a..689be82bc5e 100644
---
a/proxy/frontend/type/opengauss/src/test/java/org/apache/shardingsphere/proxy/frontend/opengauss/err/OpenGaussErrorPacketFactoryTest.java
+++
b/proxy/frontend/type/opengauss/src/test/java/org/apache/shardingsphere/proxy/frontend/opengauss/err/OpenGaussErrorPacketFactoryTest.java
@@ -29,8 +29,6 @@ import java.util.Map;
import static org.hamcrest.CoreMatchers.is;
import static org.hamcrest.MatcherAssert.assertThat;
-import static org.mockito.Mockito.mock;
-import static org.mockito.Mockito.when;
class OpenGaussErrorPacketFactoryTest {
@@ -70,14 +68,13 @@ class OpenGaussErrorPacketFactoryTest {
@Test
void assertNewInstanceWithUnknownException() {
- Exception cause = mock(Exception.class);
- when(cause.getLocalizedMessage()).thenReturn("LocalizedMessage");
+ Exception cause = new RuntimeException("No reason");
OpenGaussErrorResponsePacket actual =
OpenGaussErrorPacketFactory.newInstance(cause);
Map<Character, String> actualFields = getFieldsInPacket(actual);
assertThat(actualFields.size(), is(4));
assertThat(actualFields.get(OpenGaussErrorResponsePacket.FIELD_TYPE_SEVERITY),
is("ERROR"));
assertThat(actualFields.get(OpenGaussErrorResponsePacket.FIELD_TYPE_CODE),
is("58000"));
-
assertThat(actualFields.get(OpenGaussErrorResponsePacket.FIELD_TYPE_MESSAGE),
is("LocalizedMessage"));
+
assertThat(actualFields.get(OpenGaussErrorResponsePacket.FIELD_TYPE_MESSAGE),
is("Unknown exception: No reason"));
assertThat(actualFields.get(OpenGaussErrorResponsePacket.FIELD_TYPE_ERRORCODE),
is("0"));
}
diff --git
a/proxy/frontend/type/postgresql/src/main/java/org/apache/shardingsphere/proxy/frontend/postgresql/err/PostgreSQLErrorPacketFactory.java
b/proxy/frontend/type/postgresql/src/main/java/org/apache/shardingsphere/proxy/frontend/postgresql/err/PostgreSQLErrorPacketFactory.java
index bc914519562..26605e10b3d 100644
---
a/proxy/frontend/type/postgresql/src/main/java/org/apache/shardingsphere/proxy/frontend/postgresql/err/PostgreSQLErrorPacketFactory.java
+++
b/proxy/frontend/type/postgresql/src/main/java/org/apache/shardingsphere/proxy/frontend/postgresql/err/PostgreSQLErrorPacketFactory.java
@@ -22,12 +22,11 @@ import lombok.AccessLevel;
import lombok.NoArgsConstructor;
import
org.apache.shardingsphere.db.protocol.postgresql.constant.PostgreSQLMessageSeverityLevel;
import
org.apache.shardingsphere.db.protocol.postgresql.packet.generic.PostgreSQLErrorResponsePacket;
+import org.apache.shardingsphere.infra.database.core.type.DatabaseType;
+import
org.apache.shardingsphere.infra.exception.core.external.sql.sqlstate.XOpenSQLState;
import
org.apache.shardingsphere.infra.exception.dialect.SQLExceptionTransformEngine;
-import
org.apache.shardingsphere.infra.exception.dialect.exception.SQLDialectException;
import
org.apache.shardingsphere.infra.exception.postgresql.exception.PostgreSQLException;
import
org.apache.shardingsphere.infra.exception.postgresql.vendor.PostgreSQLVendorError;
-import org.apache.shardingsphere.infra.database.core.type.DatabaseType;
-import
org.apache.shardingsphere.infra.exception.core.external.sql.ShardingSphereSQLException;
import org.apache.shardingsphere.infra.spi.type.typed.TypedSPILoader;
import org.postgresql.util.PSQLException;
import org.postgresql.util.ServerErrorMessage;
@@ -52,11 +51,8 @@ public final class PostgreSQLErrorPacketFactory {
if (serverErrorMessage.isPresent()) {
return createErrorResponsePacket(serverErrorMessage.get());
}
- if (cause instanceof SQLException || cause instanceof
ShardingSphereSQLException || cause instanceof SQLDialectException) {
- return
createErrorResponsePacket(SQLExceptionTransformEngine.toSQLException(cause,
TypedSPILoader.getService(DatabaseType.class, "PostgreSQL")));
- }
- // TODO PostgreSQL need consider FrontendConnectionLimitException
- return createErrorResponsePacketForUnknownException(cause);
+ SQLException sqlException =
SQLExceptionTransformEngine.toSQLException(cause,
TypedSPILoader.getService(DatabaseType.class, "PostgreSQL"));
+ return createErrorResponsePacket(sqlException);
}
private static Optional<ServerErrorMessage> findServerErrorMessage(final
Exception cause) {
@@ -78,7 +74,9 @@ public final class PostgreSQLErrorPacketFactory {
if (cause instanceof PSQLException && null != ((PSQLException)
cause).getServerErrorMessage()) {
return createErrorResponsePacket(((PSQLException)
cause).getServerErrorMessage());
}
- String sqlState = Strings.isNullOrEmpty(cause.getSQLState()) ?
PostgreSQLVendorError.SYSTEM_ERROR.getSqlState().getValue() :
cause.getSQLState();
+ String sqlState = Strings.isNullOrEmpty(cause.getSQLState()) ||
XOpenSQLState.GENERAL_ERROR.getValue().equals(cause.getSQLState())
+ ? PostgreSQLVendorError.SYSTEM_ERROR.getSqlState().getValue()
+ : cause.getSQLState();
String message = Strings.isNullOrEmpty(cause.getMessage()) ?
cause.toString() : cause.getMessage();
return
PostgreSQLErrorResponsePacket.newBuilder(PostgreSQLMessageSeverityLevel.ERROR,
sqlState, message).build();
}
@@ -86,10 +84,4 @@ public final class PostgreSQLErrorPacketFactory {
private static PostgreSQLErrorResponsePacket
createErrorResponsePacket(final PostgreSQLException.ServerErrorMessage
serverErrorMessage) {
return
PostgreSQLErrorResponsePacket.newBuilder(serverErrorMessage.getSeverity(),
serverErrorMessage.getSqlState(), serverErrorMessage.getMessage()).build();
}
-
- private static PostgreSQLErrorResponsePacket
createErrorResponsePacketForUnknownException(final Exception cause) {
- // TODO add FIELD_TYPE_CODE for common error and consider what
severity to use
- String message = Strings.isNullOrEmpty(cause.getLocalizedMessage()) ?
cause.toString() : cause.getLocalizedMessage();
- return
PostgreSQLErrorResponsePacket.newBuilder(PostgreSQLMessageSeverityLevel.ERROR,
PostgreSQLVendorError.SYSTEM_ERROR, message).build();
- }
}
diff --git
a/proxy/frontend/type/postgresql/src/test/java/org/apache/shardingsphere/proxy/frontend/postgresql/err/PostgreSQLErrorPacketFactoryTest.java
b/proxy/frontend/type/postgresql/src/test/java/org/apache/shardingsphere/proxy/frontend/postgresql/err/PostgreSQLErrorPacketFactoryTest.java
index b6d06f22888..b6cc9aba9a1 100644
---
a/proxy/frontend/type/postgresql/src/test/java/org/apache/shardingsphere/proxy/frontend/postgresql/err/PostgreSQLErrorPacketFactoryTest.java
+++
b/proxy/frontend/type/postgresql/src/test/java/org/apache/shardingsphere/proxy/frontend/postgresql/err/PostgreSQLErrorPacketFactoryTest.java
@@ -60,8 +60,8 @@ class PostgreSQLErrorPacketFactoryTest {
@Test
void assertRuntimeException() throws ReflectiveOperationException {
- PostgreSQLErrorResponsePacket actual =
PostgreSQLErrorPacketFactory.newInstance(new RuntimeException("test"));
+ PostgreSQLErrorResponsePacket actual =
PostgreSQLErrorPacketFactory.newInstance(new RuntimeException("No reason"));
Map<Character, String> fields = (Map<Character, String>)
Plugins.getMemberAccessor().get(PostgreSQLErrorResponsePacket.class.getDeclaredField("fields"),
actual);
-
assertThat(fields.get(PostgreSQLErrorResponsePacket.FIELD_TYPE_MESSAGE),
is("test"));
+
assertThat(fields.get(PostgreSQLErrorResponsePacket.FIELD_TYPE_MESSAGE),
is("Unknown exception: No reason"));
}
}
