You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@shardingsphere.apache.org by GitBox <gi...@apache.org> on 2022/02/15 08:22:15 UTC
[GitHub] [shardingsphere] lanchengx opened a new pull request #15415: [DistSQL] Refactor RAL's AlterStatementExecutor
lanchengx opened a new pull request #15415:
URL: https://github.com/apache/shardingsphere/pull/15415
For https://github.com/apache/shardingsphere/issues/15414
Changes proposed in this pull request:
- Refactor RALBackendHandlerFactory
- Refactor RALBackendHandler
- Refactor AlterStatementExecutor and its implementation class
- Refactor CreateTrafficRuleHandler
--
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: notifications-unsubscribe@shardingsphere.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shardingsphere] RaigorJiang merged pull request #15415: [DistSQL] Refactor RAL's AlterStatementExecutor
Posted by GitBox <gi...@apache.org>.
RaigorJiang merged pull request #15415:
URL: https://github.com/apache/shardingsphere/pull/15415
--
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: notifications-unsubscribe@shardingsphere.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shardingsphere] RaigorJiang commented on a change in pull request #15415: [DistSQL] Refactor RAL's AlterStatementExecutor
Posted by GitBox <gi...@apache.org>.
RaigorJiang commented on a change in pull request #15415:
URL: https://github.com/apache/shardingsphere/pull/15415#discussion_r806706365
##########
File path: shardingsphere-proxy/shardingsphere-proxy-backend/src/main/java/org/apache/shardingsphere/proxy/backend/text/distsql/ral/RALBackendHandlerFactory.java
##########
@@ -50,18 +70,39 @@
* @throws SQLException SQL exception
*/
public static TextProtocolBackendHandler newInstance(final DatabaseType databaseType, final RALStatement sqlStatement, final ConnectionSession connectionSession) throws SQLException {
+ TextProtocolBackendHandler result = null;
if (sqlStatement instanceof QueryableRALStatement) {
- return QueryableRALBackendHandlerFactory.newInstance((QueryableRALStatement) sqlStatement, connectionSession);
+ result = ScalingQueryableRALBackendHandlerFactory.newInstance((QueryableRALStatement) sqlStatement, connectionSession);
}
if (sqlStatement instanceof UpdatableRALStatement) {
- return UpdatableRALBackendHandlerFactory.newInstance((UpdatableRALStatement) sqlStatement);
+ result = ScalingUpdatableRALBackendHandlerFactory.newInstance((UpdatableRALStatement) sqlStatement);
}
if (sqlStatement instanceof CommonDistSQLStatement) {
- return CommonDistSQLBackendHandlerFactory.newInstance((CommonDistSQLStatement) sqlStatement, connectionSession);
+ result = CommonDistSQLBackendHandlerFactory.newInstance((CommonDistSQLStatement) sqlStatement, connectionSession);
}
if (sqlStatement instanceof AdvancedDistSQLStatement) {
- return AdvancedDistSQLBackendHandlerFactory.newInstance(databaseType, (AdvancedDistSQLStatement) sqlStatement, connectionSession);
+ result = AdvancedDistSQLBackendHandlerFactory.newInstance(databaseType, (AdvancedDistSQLStatement) sqlStatement, connectionSession);
+ }
+ if (result == null) {
+ HandlerParameter parameter = new HandlerParameter().setStatement(sqlStatement).setConnectionSession(connectionSession).setDatabaseType(databaseType);
Review comment:
Could it be more concise if using the constructor?
--
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: notifications-unsubscribe@shardingsphere.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shardingsphere] RaigorJiang commented on a change in pull request #15415: [DistSQL] Refactor RAL's AlterStatementExecutor
Posted by GitBox <gi...@apache.org>.
RaigorJiang commented on a change in pull request #15415:
URL: https://github.com/apache/shardingsphere/pull/15415#discussion_r806709463
##########
File path: shardingsphere-proxy/shardingsphere-proxy-backend/src/main/java/org/apache/shardingsphere/proxy/backend/text/distsql/ral/query/ScalingQueryableRALBackendHandler.java
##########
@@ -32,13 +32,13 @@
import java.util.List;
/**
- * Queryable RAL backend handler.
+ * Scaling queryable RAL backend handler.
Review comment:
If `Queryable Scaling RAL backend handler` will be better?
--
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: notifications-unsubscribe@shardingsphere.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shardingsphere] RaigorJiang commented on a change in pull request #15415: [DistSQL] Refactor RAL's AlterStatementExecutor
Posted by GitBox <gi...@apache.org>.
RaigorJiang commented on a change in pull request #15415:
URL: https://github.com/apache/shardingsphere/pull/15415#discussion_r806707736
##########
File path: shardingsphere-proxy/shardingsphere-proxy-backend/src/main/java/org/apache/shardingsphere/proxy/backend/text/distsql/ral/RALBackendHandlerFactory.java
##########
@@ -50,18 +70,39 @@
* @throws SQLException SQL exception
*/
public static TextProtocolBackendHandler newInstance(final DatabaseType databaseType, final RALStatement sqlStatement, final ConnectionSession connectionSession) throws SQLException {
+ TextProtocolBackendHandler result = null;
if (sqlStatement instanceof QueryableRALStatement) {
- return QueryableRALBackendHandlerFactory.newInstance((QueryableRALStatement) sqlStatement, connectionSession);
+ result = ScalingQueryableRALBackendHandlerFactory.newInstance((QueryableRALStatement) sqlStatement, connectionSession);
}
if (sqlStatement instanceof UpdatableRALStatement) {
- return UpdatableRALBackendHandlerFactory.newInstance((UpdatableRALStatement) sqlStatement);
+ result = ScalingUpdatableRALBackendHandlerFactory.newInstance((UpdatableRALStatement) sqlStatement);
}
if (sqlStatement instanceof CommonDistSQLStatement) {
- return CommonDistSQLBackendHandlerFactory.newInstance((CommonDistSQLStatement) sqlStatement, connectionSession);
+ result = CommonDistSQLBackendHandlerFactory.newInstance((CommonDistSQLStatement) sqlStatement, connectionSession);
}
if (sqlStatement instanceof AdvancedDistSQLStatement) {
- return AdvancedDistSQLBackendHandlerFactory.newInstance(databaseType, (AdvancedDistSQLStatement) sqlStatement, connectionSession);
+ result = AdvancedDistSQLBackendHandlerFactory.newInstance(databaseType, (AdvancedDistSQLStatement) sqlStatement, connectionSession);
+ }
+ if (result == null) {
+ HandlerParameter parameter = new HandlerParameter().setStatement(sqlStatement).setConnectionSession(connectionSession).setDatabaseType(databaseType);
+ result = getHandler(sqlStatement, parameter);
+ }
+ return result;
+ }
+
+ private static RALBackendHandler newInstance(final Class<? extends RALBackendHandler> clazz) {
+ try {
+ return clazz.newInstance();
+ } catch (final InstantiationException | IllegalAccessException ex) {
+ throw new UnsupportedOperationException(String.format("Can not find public constructor for class `%s`", clazz.getName()));
+ }
+ }
+
+ private static RALBackendHandler getHandler(final RALStatement sqlStatement, final HandlerParameter<RALStatement> parameter) {
+ Class<? extends RALBackendHandler> clz = handlerClz.get(sqlStatement.getClass().getName());
+ if (null == clz) {
Review comment:
`clazz` may be a more common name
--
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: notifications-unsubscribe@shardingsphere.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shardingsphere] RaigorJiang merged pull request #15415: [DistSQL] Refactor RAL's AlterStatementExecutor
Posted by GitBox <gi...@apache.org>.
RaigorJiang merged pull request #15415:
URL: https://github.com/apache/shardingsphere/pull/15415
--
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: notifications-unsubscribe@shardingsphere.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shardingsphere] codecov-commenter commented on pull request #15415: [DistSQL] Refactor RAL's AlterStatementExecutor
Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on pull request #15415:
URL: https://github.com/apache/shardingsphere/pull/15415#issuecomment-1040372848
# [Codecov](https://codecov.io/gh/apache/shardingsphere/pull/15415?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#15415](https://codecov.io/gh/apache/shardingsphere/pull/15415?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (54c7b86) into [master](https://codecov.io/gh/apache/shardingsphere/commit/1f8b03a186d76baaf8b5bfb3596ba87ab99466a3?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (1f8b03a) will **increase** coverage by `0.09%`.
> The diff coverage is `46.42%`.
[](https://codecov.io/gh/apache/shardingsphere/pull/15415?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #15415 +/- ##
============================================
+ Coverage 60.39% 60.49% +0.09%
- Complexity 1952 1954 +2
============================================
Files 3219 3220 +1
Lines 48123 48250 +127
Branches 8168 8233 +65
============================================
+ Hits 29066 29188 +122
+ Misses 16715 16692 -23
- Partials 2342 2370 +28
```
| [Impacted Files](https://codecov.io/gh/apache/shardingsphere/pull/15415?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...d/text/distsql/ral/QueryableRALBackendHandler.java](https://codecov.io/gh/apache/shardingsphere/pull/15415/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2hhcmRpbmdzcGhlcmUtcHJveHkvc2hhcmRpbmdzcGhlcmUtcHJveHktYmFja2VuZC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2hhcmRpbmdzcGhlcmUvcHJveHkvYmFja2VuZC90ZXh0L2Rpc3RzcWwvcmFsL1F1ZXJ5YWJsZVJBTEJhY2tlbmRIYW5kbGVyLmphdmE=) | `0.00% <0.00%> (ø)` | |
| [...ral/common/CommonDistSQLBackendHandlerFactory.java](https://codecov.io/gh/apache/shardingsphere/pull/15415/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2hhcmRpbmdzcGhlcmUtcHJveHkvc2hhcmRpbmdzcGhlcmUtcHJveHktYmFja2VuZC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2hhcmRpbmdzcGhlcmUvcHJveHkvYmFja2VuZC90ZXh0L2Rpc3RzcWwvcmFsL2NvbW1vbi9Db21tb25EaXN0U1FMQmFja2VuZEhhbmRsZXJGYWN0b3J5LmphdmE=) | `45.45% <0.00%> (+12.12%)` | :arrow_up: |
| [.../ral/common/updatable/AlterTrafficRuleHandler.java](https://codecov.io/gh/apache/shardingsphere/pull/15415/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2hhcmRpbmdzcGhlcmUtcHJveHkvc2hhcmRpbmdzcGhlcmUtcHJveHktYmFja2VuZC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2hhcmRpbmdzcGhlcmUvcHJveHkvYmFja2VuZC90ZXh0L2Rpc3RzcWwvcmFsL2NvbW1vbi91cGRhdGFibGUvQWx0ZXJUcmFmZmljUnVsZUhhbmRsZXIuamF2YQ==) | `100.00% <ø> (ø)` | |
| [.../common/updatable/AlterTransactionRuleHandler.java](https://codecov.io/gh/apache/shardingsphere/pull/15415/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2hhcmRpbmdzcGhlcmUtcHJveHkvc2hhcmRpbmdzcGhlcmUtcHJveHktYmFja2VuZC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2hhcmRpbmdzcGhlcmUvcHJveHkvYmFja2VuZC90ZXh0L2Rpc3RzcWwvcmFsL2NvbW1vbi91cGRhdGFibGUvQWx0ZXJUcmFuc2FjdGlvblJ1bGVIYW5kbGVyLmphdmE=) | `0.00% <0.00%> (ø)` | |
| [...ral/common/updatable/CreateTrafficRuleHandler.java](https://codecov.io/gh/apache/shardingsphere/pull/15415/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2hhcmRpbmdzcGhlcmUtcHJveHkvc2hhcmRpbmdzcGhlcmUtcHJveHktYmFja2VuZC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2hhcmRpbmdzcGhlcmUvcHJveHkvYmFja2VuZC90ZXh0L2Rpc3RzcWwvcmFsL2NvbW1vbi91cGRhdGFibGUvQ3JlYXRlVHJhZmZpY1J1bGVIYW5kbGVyLmphdmE=) | `93.93% <ø> (ø)` | |
| [...end/text/distsql/ral/RALBackendHandlerFactory.java](https://codecov.io/gh/apache/shardingsphere/pull/15415/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2hhcmRpbmdzcGhlcmUtcHJveHkvc2hhcmRpbmdzcGhlcmUtcHJveHktYmFja2VuZC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2hhcmRpbmdzcGhlcmUvcHJveHkvYmFja2VuZC90ZXh0L2Rpc3RzcWwvcmFsL1JBTEJhY2tlbmRIYW5kbGVyRmFjdG9yeS5qYXZh) | `53.84% <50.00%> (-1.71%)` | :arrow_down: |
| [...xy/backend/text/distsql/ral/RALBackendHandler.java](https://codecov.io/gh/apache/shardingsphere/pull/15415/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2hhcmRpbmdzcGhlcmUtcHJveHkvc2hhcmRpbmdzcGhlcmUtcHJveHktYmFja2VuZC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2hhcmRpbmdzcGhlcmUvcHJveHkvYmFja2VuZC90ZXh0L2Rpc3RzcWwvcmFsL1JBTEJhY2tlbmRIYW5kbGVyLmphdmE=) | `62.50% <62.50%> (ø)` | |
| [...d/text/distsql/ral/UpdatableRALBackendHandler.java](https://codecov.io/gh/apache/shardingsphere/pull/15415/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2hhcmRpbmdzcGhlcmUtcHJveHkvc2hhcmRpbmdzcGhlcmUtcHJveHktYmFja2VuZC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2hhcmRpbmdzcGhlcmUvcHJveHkvYmFja2VuZC90ZXh0L2Rpc3RzcWwvcmFsL1VwZGF0YWJsZVJBTEJhY2tlbmRIYW5kbGVyLmphdmE=) | `100.00% <100.00%> (ø)` | |
| [...al/common/updatable/AlterSQLParserRuleHandler.java](https://codecov.io/gh/apache/shardingsphere/pull/15415/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2hhcmRpbmdzcGhlcmUtcHJveHkvc2hhcmRpbmdzcGhlcmUtcHJveHktYmFja2VuZC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2hhcmRpbmdzcGhlcmUvcHJveHkvYmFja2VuZC90ZXh0L2Rpc3RzcWwvcmFsL2NvbW1vbi91cGRhdGFibGUvQWx0ZXJTUUxQYXJzZXJSdWxlSGFuZGxlci5qYXZh) | `72.41% <100.00%> (ø)` | |
| [...l/ral/query/QueryableScalingRALBackendHandler.java](https://codecov.io/gh/apache/shardingsphere/pull/15415/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2hhcmRpbmdzcGhlcmUtcHJveHkvc2hhcmRpbmdzcGhlcmUtcHJveHktYmFja2VuZC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2hhcmRpbmdzcGhlcmUvcHJveHkvYmFja2VuZC90ZXh0L2Rpc3RzcWwvcmFsL3F1ZXJ5L1F1ZXJ5YWJsZVNjYWxpbmdSQUxCYWNrZW5kSGFuZGxlci5qYXZh) | `83.33% <100.00%> (ø)` | |
| ... and [18 more](https://codecov.io/gh/apache/shardingsphere/pull/15415/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/shardingsphere/pull/15415?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/shardingsphere/pull/15415?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [1f8b03a...54c7b86](https://codecov.io/gh/apache/shardingsphere/pull/15415?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
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: notifications-unsubscribe@shardingsphere.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shardingsphere] RaigorJiang commented on a change in pull request #15415: [DistSQL] Refactor RAL's AlterStatementExecutor
Posted by GitBox <gi...@apache.org>.
RaigorJiang commented on a change in pull request #15415:
URL: https://github.com/apache/shardingsphere/pull/15415#discussion_r806702912
##########
File path: shardingsphere-proxy/shardingsphere-proxy-backend/src/main/java/org/apache/shardingsphere/proxy/backend/text/distsql/ral/RALBackendHandlerFactory.java
##########
@@ -24,22 +24,42 @@
import org.apache.shardingsphere.distsql.parser.statement.ral.QueryableRALStatement;
import org.apache.shardingsphere.distsql.parser.statement.ral.RALStatement;
import org.apache.shardingsphere.distsql.parser.statement.ral.UpdatableRALStatement;
+import org.apache.shardingsphere.distsql.parser.statement.ral.common.alter.AlterSQLParserRuleStatement;
+import org.apache.shardingsphere.distsql.parser.statement.ral.common.alter.AlterTransactionRuleStatement;
+import org.apache.shardingsphere.distsql.parser.statement.rdl.create.AlterTrafficRuleStatement;
+import org.apache.shardingsphere.distsql.parser.statement.rdl.create.CreateTrafficRuleStatement;
import org.apache.shardingsphere.infra.database.type.DatabaseType;
import org.apache.shardingsphere.proxy.backend.session.ConnectionSession;
import org.apache.shardingsphere.proxy.backend.text.TextProtocolBackendHandler;
+import org.apache.shardingsphere.proxy.backend.text.distsql.ral.RALBackendHandler.HandlerParameter;
import org.apache.shardingsphere.proxy.backend.text.distsql.ral.advanced.AdvancedDistSQLBackendHandlerFactory;
-import org.apache.shardingsphere.proxy.backend.text.distsql.ral.query.QueryableRALBackendHandlerFactory;
import org.apache.shardingsphere.proxy.backend.text.distsql.ral.common.CommonDistSQLBackendHandlerFactory;
-import org.apache.shardingsphere.proxy.backend.text.distsql.ral.update.UpdatableRALBackendHandlerFactory;
+import org.apache.shardingsphere.proxy.backend.text.distsql.ral.common.updatable.AlterSQLParserRuleHandler;
+import org.apache.shardingsphere.proxy.backend.text.distsql.ral.common.updatable.AlterTrafficRuleHandler;
+import org.apache.shardingsphere.proxy.backend.text.distsql.ral.common.updatable.AlterTransactionRuleHandler;
+import org.apache.shardingsphere.proxy.backend.text.distsql.ral.common.updatable.CreateTrafficRuleHandler;
+import org.apache.shardingsphere.proxy.backend.text.distsql.ral.query.ScalingQueryableRALBackendHandlerFactory;
+import org.apache.shardingsphere.proxy.backend.text.distsql.ral.update.ScalingUpdatableRALBackendHandlerFactory;
import java.sql.SQLException;
+import java.util.LinkedHashMap;
+import java.util.Map;
/**
* RAL backend handler factory.
*/
@NoArgsConstructor(access = AccessLevel.PRIVATE)
public final class RALBackendHandlerFactory {
+ private static Map<String, Class<? extends RALBackendHandler>> handlerClz = new LinkedHashMap<>();
Review comment:
Is handlerMap better? (than handlerClz)
--
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: notifications-unsubscribe@shardingsphere.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shardingsphere] codecov-commenter commented on pull request #15415: [DistSQL] Refactor RAL's AlterStatementExecutor
Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on pull request #15415:
URL: https://github.com/apache/shardingsphere/pull/15415#issuecomment-1040372848
# [Codecov](https://codecov.io/gh/apache/shardingsphere/pull/15415?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#15415](https://codecov.io/gh/apache/shardingsphere/pull/15415?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (54c7b86) into [master](https://codecov.io/gh/apache/shardingsphere/commit/1f8b03a186d76baaf8b5bfb3596ba87ab99466a3?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (1f8b03a) will **increase** coverage by `0.09%`.
> The diff coverage is `46.42%`.
[](https://codecov.io/gh/apache/shardingsphere/pull/15415?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #15415 +/- ##
============================================
+ Coverage 60.39% 60.49% +0.09%
- Complexity 1952 1954 +2
============================================
Files 3219 3220 +1
Lines 48123 48250 +127
Branches 8168 8233 +65
============================================
+ Hits 29066 29188 +122
+ Misses 16715 16692 -23
- Partials 2342 2370 +28
```
| [Impacted Files](https://codecov.io/gh/apache/shardingsphere/pull/15415?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...d/text/distsql/ral/QueryableRALBackendHandler.java](https://codecov.io/gh/apache/shardingsphere/pull/15415/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2hhcmRpbmdzcGhlcmUtcHJveHkvc2hhcmRpbmdzcGhlcmUtcHJveHktYmFja2VuZC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2hhcmRpbmdzcGhlcmUvcHJveHkvYmFja2VuZC90ZXh0L2Rpc3RzcWwvcmFsL1F1ZXJ5YWJsZVJBTEJhY2tlbmRIYW5kbGVyLmphdmE=) | `0.00% <0.00%> (ø)` | |
| [...ral/common/CommonDistSQLBackendHandlerFactory.java](https://codecov.io/gh/apache/shardingsphere/pull/15415/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2hhcmRpbmdzcGhlcmUtcHJveHkvc2hhcmRpbmdzcGhlcmUtcHJveHktYmFja2VuZC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2hhcmRpbmdzcGhlcmUvcHJveHkvYmFja2VuZC90ZXh0L2Rpc3RzcWwvcmFsL2NvbW1vbi9Db21tb25EaXN0U1FMQmFja2VuZEhhbmRsZXJGYWN0b3J5LmphdmE=) | `45.45% <0.00%> (+12.12%)` | :arrow_up: |
| [.../ral/common/updatable/AlterTrafficRuleHandler.java](https://codecov.io/gh/apache/shardingsphere/pull/15415/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2hhcmRpbmdzcGhlcmUtcHJveHkvc2hhcmRpbmdzcGhlcmUtcHJveHktYmFja2VuZC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2hhcmRpbmdzcGhlcmUvcHJveHkvYmFja2VuZC90ZXh0L2Rpc3RzcWwvcmFsL2NvbW1vbi91cGRhdGFibGUvQWx0ZXJUcmFmZmljUnVsZUhhbmRsZXIuamF2YQ==) | `100.00% <ø> (ø)` | |
| [.../common/updatable/AlterTransactionRuleHandler.java](https://codecov.io/gh/apache/shardingsphere/pull/15415/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2hhcmRpbmdzcGhlcmUtcHJveHkvc2hhcmRpbmdzcGhlcmUtcHJveHktYmFja2VuZC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2hhcmRpbmdzcGhlcmUvcHJveHkvYmFja2VuZC90ZXh0L2Rpc3RzcWwvcmFsL2NvbW1vbi91cGRhdGFibGUvQWx0ZXJUcmFuc2FjdGlvblJ1bGVIYW5kbGVyLmphdmE=) | `0.00% <0.00%> (ø)` | |
| [...ral/common/updatable/CreateTrafficRuleHandler.java](https://codecov.io/gh/apache/shardingsphere/pull/15415/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2hhcmRpbmdzcGhlcmUtcHJveHkvc2hhcmRpbmdzcGhlcmUtcHJveHktYmFja2VuZC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2hhcmRpbmdzcGhlcmUvcHJveHkvYmFja2VuZC90ZXh0L2Rpc3RzcWwvcmFsL2NvbW1vbi91cGRhdGFibGUvQ3JlYXRlVHJhZmZpY1J1bGVIYW5kbGVyLmphdmE=) | `93.93% <ø> (ø)` | |
| [...end/text/distsql/ral/RALBackendHandlerFactory.java](https://codecov.io/gh/apache/shardingsphere/pull/15415/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2hhcmRpbmdzcGhlcmUtcHJveHkvc2hhcmRpbmdzcGhlcmUtcHJveHktYmFja2VuZC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2hhcmRpbmdzcGhlcmUvcHJveHkvYmFja2VuZC90ZXh0L2Rpc3RzcWwvcmFsL1JBTEJhY2tlbmRIYW5kbGVyRmFjdG9yeS5qYXZh) | `53.84% <50.00%> (-1.71%)` | :arrow_down: |
| [...xy/backend/text/distsql/ral/RALBackendHandler.java](https://codecov.io/gh/apache/shardingsphere/pull/15415/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2hhcmRpbmdzcGhlcmUtcHJveHkvc2hhcmRpbmdzcGhlcmUtcHJveHktYmFja2VuZC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2hhcmRpbmdzcGhlcmUvcHJveHkvYmFja2VuZC90ZXh0L2Rpc3RzcWwvcmFsL1JBTEJhY2tlbmRIYW5kbGVyLmphdmE=) | `62.50% <62.50%> (ø)` | |
| [...d/text/distsql/ral/UpdatableRALBackendHandler.java](https://codecov.io/gh/apache/shardingsphere/pull/15415/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2hhcmRpbmdzcGhlcmUtcHJveHkvc2hhcmRpbmdzcGhlcmUtcHJveHktYmFja2VuZC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2hhcmRpbmdzcGhlcmUvcHJveHkvYmFja2VuZC90ZXh0L2Rpc3RzcWwvcmFsL1VwZGF0YWJsZVJBTEJhY2tlbmRIYW5kbGVyLmphdmE=) | `100.00% <100.00%> (ø)` | |
| [...al/common/updatable/AlterSQLParserRuleHandler.java](https://codecov.io/gh/apache/shardingsphere/pull/15415/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2hhcmRpbmdzcGhlcmUtcHJveHkvc2hhcmRpbmdzcGhlcmUtcHJveHktYmFja2VuZC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2hhcmRpbmdzcGhlcmUvcHJveHkvYmFja2VuZC90ZXh0L2Rpc3RzcWwvcmFsL2NvbW1vbi91cGRhdGFibGUvQWx0ZXJTUUxQYXJzZXJSdWxlSGFuZGxlci5qYXZh) | `72.41% <100.00%> (ø)` | |
| [...l/ral/query/QueryableScalingRALBackendHandler.java](https://codecov.io/gh/apache/shardingsphere/pull/15415/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2hhcmRpbmdzcGhlcmUtcHJveHkvc2hhcmRpbmdzcGhlcmUtcHJveHktYmFja2VuZC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2hhcmRpbmdzcGhlcmUvcHJveHkvYmFja2VuZC90ZXh0L2Rpc3RzcWwvcmFsL3F1ZXJ5L1F1ZXJ5YWJsZVNjYWxpbmdSQUxCYWNrZW5kSGFuZGxlci5qYXZh) | `83.33% <100.00%> (ø)` | |
| ... and [18 more](https://codecov.io/gh/apache/shardingsphere/pull/15415/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/shardingsphere/pull/15415?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/shardingsphere/pull/15415?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [1f8b03a...54c7b86](https://codecov.io/gh/apache/shardingsphere/pull/15415?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
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: notifications-unsubscribe@shardingsphere.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shardingsphere] RaigorJiang commented on a change in pull request #15415: [DistSQL] Refactor RAL's AlterStatementExecutor
Posted by GitBox <gi...@apache.org>.
RaigorJiang commented on a change in pull request #15415:
URL: https://github.com/apache/shardingsphere/pull/15415#discussion_r806702912
##########
File path: shardingsphere-proxy/shardingsphere-proxy-backend/src/main/java/org/apache/shardingsphere/proxy/backend/text/distsql/ral/RALBackendHandlerFactory.java
##########
@@ -24,22 +24,42 @@
import org.apache.shardingsphere.distsql.parser.statement.ral.QueryableRALStatement;
import org.apache.shardingsphere.distsql.parser.statement.ral.RALStatement;
import org.apache.shardingsphere.distsql.parser.statement.ral.UpdatableRALStatement;
+import org.apache.shardingsphere.distsql.parser.statement.ral.common.alter.AlterSQLParserRuleStatement;
+import org.apache.shardingsphere.distsql.parser.statement.ral.common.alter.AlterTransactionRuleStatement;
+import org.apache.shardingsphere.distsql.parser.statement.rdl.create.AlterTrafficRuleStatement;
+import org.apache.shardingsphere.distsql.parser.statement.rdl.create.CreateTrafficRuleStatement;
import org.apache.shardingsphere.infra.database.type.DatabaseType;
import org.apache.shardingsphere.proxy.backend.session.ConnectionSession;
import org.apache.shardingsphere.proxy.backend.text.TextProtocolBackendHandler;
+import org.apache.shardingsphere.proxy.backend.text.distsql.ral.RALBackendHandler.HandlerParameter;
import org.apache.shardingsphere.proxy.backend.text.distsql.ral.advanced.AdvancedDistSQLBackendHandlerFactory;
-import org.apache.shardingsphere.proxy.backend.text.distsql.ral.query.QueryableRALBackendHandlerFactory;
import org.apache.shardingsphere.proxy.backend.text.distsql.ral.common.CommonDistSQLBackendHandlerFactory;
-import org.apache.shardingsphere.proxy.backend.text.distsql.ral.update.UpdatableRALBackendHandlerFactory;
+import org.apache.shardingsphere.proxy.backend.text.distsql.ral.common.updatable.AlterSQLParserRuleHandler;
+import org.apache.shardingsphere.proxy.backend.text.distsql.ral.common.updatable.AlterTrafficRuleHandler;
+import org.apache.shardingsphere.proxy.backend.text.distsql.ral.common.updatable.AlterTransactionRuleHandler;
+import org.apache.shardingsphere.proxy.backend.text.distsql.ral.common.updatable.CreateTrafficRuleHandler;
+import org.apache.shardingsphere.proxy.backend.text.distsql.ral.query.ScalingQueryableRALBackendHandlerFactory;
+import org.apache.shardingsphere.proxy.backend.text.distsql.ral.update.ScalingUpdatableRALBackendHandlerFactory;
import java.sql.SQLException;
+import java.util.LinkedHashMap;
+import java.util.Map;
/**
* RAL backend handler factory.
*/
@NoArgsConstructor(access = AccessLevel.PRIVATE)
public final class RALBackendHandlerFactory {
+ private static Map<String, Class<? extends RALBackendHandler>> handlerClz = new LinkedHashMap<>();
Review comment:
Is handlerMap better? (than handlerClz)
##########
File path: shardingsphere-proxy/shardingsphere-proxy-backend/src/main/java/org/apache/shardingsphere/proxy/backend/text/distsql/ral/RALBackendHandlerFactory.java
##########
@@ -50,18 +70,39 @@
* @throws SQLException SQL exception
*/
public static TextProtocolBackendHandler newInstance(final DatabaseType databaseType, final RALStatement sqlStatement, final ConnectionSession connectionSession) throws SQLException {
+ TextProtocolBackendHandler result = null;
if (sqlStatement instanceof QueryableRALStatement) {
- return QueryableRALBackendHandlerFactory.newInstance((QueryableRALStatement) sqlStatement, connectionSession);
+ result = ScalingQueryableRALBackendHandlerFactory.newInstance((QueryableRALStatement) sqlStatement, connectionSession);
}
if (sqlStatement instanceof UpdatableRALStatement) {
- return UpdatableRALBackendHandlerFactory.newInstance((UpdatableRALStatement) sqlStatement);
+ result = ScalingUpdatableRALBackendHandlerFactory.newInstance((UpdatableRALStatement) sqlStatement);
}
if (sqlStatement instanceof CommonDistSQLStatement) {
- return CommonDistSQLBackendHandlerFactory.newInstance((CommonDistSQLStatement) sqlStatement, connectionSession);
+ result = CommonDistSQLBackendHandlerFactory.newInstance((CommonDistSQLStatement) sqlStatement, connectionSession);
}
if (sqlStatement instanceof AdvancedDistSQLStatement) {
- return AdvancedDistSQLBackendHandlerFactory.newInstance(databaseType, (AdvancedDistSQLStatement) sqlStatement, connectionSession);
+ result = AdvancedDistSQLBackendHandlerFactory.newInstance(databaseType, (AdvancedDistSQLStatement) sqlStatement, connectionSession);
+ }
+ if (result == null) {
+ HandlerParameter parameter = new HandlerParameter().setStatement(sqlStatement).setConnectionSession(connectionSession).setDatabaseType(databaseType);
Review comment:
Could it be more concise if using the constructor?
##########
File path: shardingsphere-proxy/shardingsphere-proxy-backend/src/main/java/org/apache/shardingsphere/proxy/backend/text/distsql/ral/RALBackendHandlerFactory.java
##########
@@ -50,18 +70,39 @@
* @throws SQLException SQL exception
*/
public static TextProtocolBackendHandler newInstance(final DatabaseType databaseType, final RALStatement sqlStatement, final ConnectionSession connectionSession) throws SQLException {
+ TextProtocolBackendHandler result = null;
if (sqlStatement instanceof QueryableRALStatement) {
- return QueryableRALBackendHandlerFactory.newInstance((QueryableRALStatement) sqlStatement, connectionSession);
+ result = ScalingQueryableRALBackendHandlerFactory.newInstance((QueryableRALStatement) sqlStatement, connectionSession);
}
if (sqlStatement instanceof UpdatableRALStatement) {
- return UpdatableRALBackendHandlerFactory.newInstance((UpdatableRALStatement) sqlStatement);
+ result = ScalingUpdatableRALBackendHandlerFactory.newInstance((UpdatableRALStatement) sqlStatement);
}
if (sqlStatement instanceof CommonDistSQLStatement) {
- return CommonDistSQLBackendHandlerFactory.newInstance((CommonDistSQLStatement) sqlStatement, connectionSession);
+ result = CommonDistSQLBackendHandlerFactory.newInstance((CommonDistSQLStatement) sqlStatement, connectionSession);
}
if (sqlStatement instanceof AdvancedDistSQLStatement) {
- return AdvancedDistSQLBackendHandlerFactory.newInstance(databaseType, (AdvancedDistSQLStatement) sqlStatement, connectionSession);
+ result = AdvancedDistSQLBackendHandlerFactory.newInstance(databaseType, (AdvancedDistSQLStatement) sqlStatement, connectionSession);
+ }
+ if (result == null) {
+ HandlerParameter parameter = new HandlerParameter().setStatement(sqlStatement).setConnectionSession(connectionSession).setDatabaseType(databaseType);
+ result = getHandler(sqlStatement, parameter);
+ }
+ return result;
+ }
+
+ private static RALBackendHandler newInstance(final Class<? extends RALBackendHandler> clazz) {
+ try {
+ return clazz.newInstance();
+ } catch (final InstantiationException | IllegalAccessException ex) {
+ throw new UnsupportedOperationException(String.format("Can not find public constructor for class `%s`", clazz.getName()));
+ }
+ }
+
+ private static RALBackendHandler getHandler(final RALStatement sqlStatement, final HandlerParameter<RALStatement> parameter) {
+ Class<? extends RALBackendHandler> clz = handlerClz.get(sqlStatement.getClass().getName());
+ if (null == clz) {
Review comment:
`clazz` may be a more common name
##########
File path: shardingsphere-proxy/shardingsphere-proxy-backend/src/main/java/org/apache/shardingsphere/proxy/backend/text/distsql/ral/query/ScalingQueryableRALBackendHandler.java
##########
@@ -32,13 +32,13 @@
import java.util.List;
/**
- * Queryable RAL backend handler.
+ * Scaling queryable RAL backend handler.
Review comment:
If `Queryable Scaling RAL backend handler` will be better?
--
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: notifications-unsubscribe@shardingsphere.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org