You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@shardingsphere.apache.org by zh...@apache.org on 2022/01/16 15:20:38 UTC

[shardingsphere] branch master updated: Fix alter SQL parser rule faild when use default SQL parser rule configuration. (#14793)

This is an automated email from the ASF dual-hosted git repository.

zhangliang 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 6e716b7  Fix alter SQL parser rule faild when use default SQL parser rule configuration. (#14793)
6e716b7 is described below

commit 6e716b7d3a4eb285d87b3c59e096b0915cf20db5
Author: Raigor <ra...@gmail.com>
AuthorDate: Sun Jan 16 23:19:50 2022 +0800

    Fix alter SQL parser rule faild when use default SQL parser rule configuration. (#14793)
    
    * Fix alter SQl parser rule faild when use default SQL parser rule configuration.
    
    * Optimize methods.
    
    * add test for AlterSQLParserRuleExecutor.
---
 .../src/main/antlr4/imports/RALStatement.g4        |  2 +-
 .../alter/excutor/AlterSQLParserRuleExecutor.java  | 28 ++++---
 .../alter/AlterSQLParserRuleExecutorTest.java      | 97 ++++++++++++++++++++++
 3 files changed, 115 insertions(+), 12 deletions(-)

diff --git a/shardingsphere-distsql/shardingsphere-distsql-parser/src/main/antlr4/imports/RALStatement.g4 b/shardingsphere-distsql/shardingsphere-distsql-parser/src/main/antlr4/imports/RALStatement.g4
index c3b04d4..786599b 100644
--- a/shardingsphere-distsql/shardingsphere-distsql-parser/src/main/antlr4/imports/RALStatement.g4
+++ b/shardingsphere-distsql/shardingsphere-distsql-parser/src/main/antlr4/imports/RALStatement.g4
@@ -144,7 +144,7 @@ sqlStatementCache
     ;
 
 cacheOption
-    : (INITIAL_CAPACITY EQ initialCapacity)? (COMMA MAXIMUM_SIZE EQ maximumSize)? (COMMA CONCURRENCY_LEVEL EQ concurrencyLevel)? 
+    : (INITIAL_CAPACITY EQ initialCapacity)? (COMMA? MAXIMUM_SIZE EQ maximumSize)? (COMMA? CONCURRENCY_LEVEL EQ concurrencyLevel)? 
     ;
 
 initialCapacity
diff --git a/shardingsphere-proxy/shardingsphere-proxy-backend/src/main/java/org/apache/shardingsphere/proxy/backend/text/distsql/ral/common/alter/excutor/AlterSQLParserRuleExecutor.java b/shardingsphere-proxy/shardingsphere-proxy-backend/src/main/java/org/apache/shardingsphere/proxy/backend/text/distsql/ral/common/alter/excutor/AlterSQLParserRuleExecutor.java
index 247de35..c6c6d7d 100644
--- a/shardingsphere-proxy/shardingsphere-proxy-backend/src/main/java/org/apache/shardingsphere/proxy/backend/text/distsql/ral/common/alter/excutor/AlterSQLParserRuleExecutor.java
+++ b/shardingsphere-proxy/shardingsphere-proxy-backend/src/main/java/org/apache/shardingsphere/proxy/backend/text/distsql/ral/common/alter/excutor/AlterSQLParserRuleExecutor.java
@@ -24,6 +24,7 @@ import org.apache.shardingsphere.infra.config.RuleConfiguration;
 import org.apache.shardingsphere.mode.metadata.MetaDataContexts;
 import org.apache.shardingsphere.mode.metadata.persist.MetaDataPersistService;
 import org.apache.shardingsphere.parser.config.SQLParserRuleConfiguration;
+import org.apache.shardingsphere.parser.rule.builder.DefaultSQLParserRuleConfigurationBuilder;
 import org.apache.shardingsphere.proxy.backend.context.ProxyContext;
 import org.apache.shardingsphere.proxy.backend.response.header.ResponseHeader;
 import org.apache.shardingsphere.proxy.backend.response.header.update.UpdateResponseHeader;
@@ -50,19 +51,24 @@ public final class AlterSQLParserRuleExecutor implements AlterStatementExecutor
     private void updateSQLParserRule() {
         MetaDataContexts metaDataContexts = ProxyContext.getInstance().getContextManager().getMetaDataContexts();
         Collection<RuleConfiguration> globalRuleConfigurations = metaDataContexts.getGlobalRuleMetaData().getConfigurations();
-        RuleConfiguration ruleConfiguration = globalRuleConfigurations.stream().filter(configuration -> configuration instanceof SQLParserRuleConfiguration).findFirst().orElse(null);
-        if (ruleConfiguration instanceof SQLParserRuleConfiguration) {
-            SQLParserRuleConfiguration toBeAdd = buildSQLParserRuleConfiguration((SQLParserRuleConfiguration) ruleConfiguration);
-            globalRuleConfigurations.removeIf(configuration -> configuration instanceof SQLParserRuleConfiguration);
-            globalRuleConfigurations.add(toBeAdd);
-            Optional<MetaDataPersistService> metaDataPersistService = metaDataContexts.getMetaDataPersistService();
-            if (metaDataPersistService.isPresent() && null != metaDataPersistService.get().getPropsService()) {
-                metaDataPersistService.get().getGlobalRuleService().persist(globalRuleConfigurations, true);
-            }
+        RuleConfiguration currentRuleConfiguration = globalRuleConfigurations.stream().filter(configuration -> configuration instanceof SQLParserRuleConfiguration).findFirst().orElse(null);
+        SQLParserRuleConfiguration toBeAlteredRuleConfiguration = getToBeAlteredRuleConfig(currentRuleConfiguration);
+        globalRuleConfigurations.removeIf(configuration -> configuration instanceof SQLParserRuleConfiguration);
+        globalRuleConfigurations.add(toBeAlteredRuleConfiguration);
+        Optional<MetaDataPersistService> metaDataPersistService = metaDataContexts.getMetaDataPersistService();
+        if (metaDataPersistService.isPresent() && null != metaDataPersistService.get().getGlobalRuleService()) {
+            metaDataPersistService.get().getGlobalRuleService().persist(globalRuleConfigurations, true);
         }
     }
-
-    private SQLParserRuleConfiguration buildSQLParserRuleConfiguration(final SQLParserRuleConfiguration ruleConfiguration) {
+    
+    private SQLParserRuleConfiguration getToBeAlteredRuleConfig(final RuleConfiguration currentRuleConfiguration) {
+        if (null == currentRuleConfiguration) {
+            return buildWithCurrentRuleConfiguration(new DefaultSQLParserRuleConfigurationBuilder().build());
+        }
+        return buildWithCurrentRuleConfiguration((SQLParserRuleConfiguration) currentRuleConfiguration);
+    }
+    
+    private SQLParserRuleConfiguration buildWithCurrentRuleConfiguration(final SQLParserRuleConfiguration ruleConfiguration) {
         SQLParserRuleConfiguration result = new SQLParserRuleConfiguration();
         result.setSqlCommentParseEnabled(null == sqlStatement.getSqlCommentParseEnable() ? ruleConfiguration.isSqlCommentParseEnabled() 
                 : sqlStatement.getSqlCommentParseEnable());
diff --git a/shardingsphere-proxy/shardingsphere-proxy-backend/src/test/java/org/apache/shardingsphere/proxy/backend/text/distsql/ral/common/alter/AlterSQLParserRuleExecutorTest.java b/shardingsphere-proxy/shardingsphere-proxy-backend/src/test/java/org/apache/shardingsphere/proxy/backend/text/distsql/ral/common/alter/AlterSQLParserRuleExecutorTest.java
new file mode 100644
index 0000000..cb86e65
--- /dev/null
+++ b/shardingsphere-proxy/shardingsphere-proxy-backend/src/test/java/org/apache/shardingsphere/proxy/backend/text/distsql/ral/common/alter/AlterSQLParserRuleExecutorTest.java
@@ -0,0 +1,97 @@
+/*
+ * 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.shardingsphere.proxy.backend.text.distsql.ral.common.alter;
+
+import org.apache.shardingsphere.distsql.parser.segment.CacheOptionSegment;
+import org.apache.shardingsphere.distsql.parser.statement.ral.common.alter.AlterSQLParserRuleStatement;
+import org.apache.shardingsphere.infra.config.RuleConfiguration;
+import org.apache.shardingsphere.mode.manager.ContextManager;
+import org.apache.shardingsphere.parser.config.SQLParserRuleConfiguration;
+import org.apache.shardingsphere.parser.rule.builder.DefaultSQLParserRuleConfigurationBuilder;
+import org.apache.shardingsphere.proxy.backend.context.ProxyContext;
+import org.apache.shardingsphere.proxy.backend.text.distsql.ral.common.alter.excutor.AlterSQLParserRuleExecutor;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.junit.MockitoJUnitRunner;
+
+import java.util.Collection;
+import java.util.LinkedList;
+
+import static org.hamcrest.CoreMatchers.is;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertThat;
+import static org.junit.Assert.assertTrue;
+import static org.mockito.Mockito.RETURNS_DEEP_STUBS;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+@RunWith(MockitoJUnitRunner.class)
+public final class AlterSQLParserRuleExecutorTest {
+    
+    @Test
+    public void assertExecuteWithoutCurrentRuleConfiguration() {
+        ContextManager contextManager = mock(ContextManager.class, RETURNS_DEEP_STUBS);
+        when(contextManager.getMetaDataContexts().getGlobalRuleMetaData().getConfigurations()).thenReturn(new LinkedList<>());
+        ProxyContext.getInstance().init(contextManager);
+        new AlterSQLParserRuleExecutor(getSQLStatement()).execute();
+        Collection<RuleConfiguration> globalRuleConfigurations = contextManager.getMetaDataContexts().getGlobalRuleMetaData().getConfigurations();
+        RuleConfiguration ruleConfiguration = globalRuleConfigurations.stream().filter(configuration -> configuration instanceof SQLParserRuleConfiguration).findAny().orElse(null);
+        assertNotNull(ruleConfiguration);
+        SQLParserRuleConfiguration sqlParserRuleConfiguration = (SQLParserRuleConfiguration) ruleConfiguration;
+        assertTrue(sqlParserRuleConfiguration.isSqlCommentParseEnabled());
+        assertThat(sqlParserRuleConfiguration.getSqlStatementCache().getInitialCapacity(), is(1000));
+        assertThat(sqlParserRuleConfiguration.getSqlStatementCache().getMaximumSize(), is(1000L));
+        assertThat(sqlParserRuleConfiguration.getSqlStatementCache().getConcurrencyLevel(), is(3));
+        assertThat(sqlParserRuleConfiguration.getParseTreeCache().getInitialCapacity(), is(64));
+        assertThat(sqlParserRuleConfiguration.getParseTreeCache().getMaximumSize(), is(512L));
+        assertThat(sqlParserRuleConfiguration.getParseTreeCache().getConcurrencyLevel(), is(3));
+    }
+    
+    @Test
+    public void assertExecuteWithDefaultRuleConfiguration() {
+        Collection<RuleConfiguration> globalRuleConfiguration = new LinkedList<>();
+        globalRuleConfiguration.add(new DefaultSQLParserRuleConfigurationBuilder().build());
+        ContextManager contextManager = mock(ContextManager.class, RETURNS_DEEP_STUBS);
+        when(contextManager.getMetaDataContexts().getGlobalRuleMetaData().getConfigurations()).thenReturn(globalRuleConfiguration);
+        ProxyContext.getInstance().init(contextManager);
+        new AlterSQLParserRuleExecutor(getSQLStatement()).execute();
+        Collection<RuleConfiguration> globalRuleConfigurations = contextManager.getMetaDataContexts().getGlobalRuleMetaData().getConfigurations();
+        RuleConfiguration ruleConfiguration = globalRuleConfigurations.stream().filter(configuration -> configuration instanceof SQLParserRuleConfiguration).findAny().orElse(null);
+        assertNotNull(ruleConfiguration);
+        SQLParserRuleConfiguration sqlParserRuleConfiguration = (SQLParserRuleConfiguration) ruleConfiguration;
+        assertTrue(sqlParserRuleConfiguration.isSqlCommentParseEnabled());
+        assertThat(sqlParserRuleConfiguration.getSqlStatementCache().getInitialCapacity(), is(1000));
+        assertThat(sqlParserRuleConfiguration.getSqlStatementCache().getMaximumSize(), is(1000L));
+        assertThat(sqlParserRuleConfiguration.getSqlStatementCache().getConcurrencyLevel(), is(3));
+        assertThat(sqlParserRuleConfiguration.getParseTreeCache().getInitialCapacity(), is(64));
+        assertThat(sqlParserRuleConfiguration.getParseTreeCache().getMaximumSize(), is(512L));
+        assertThat(sqlParserRuleConfiguration.getParseTreeCache().getConcurrencyLevel(), is(3));
+    }
+    
+    private AlterSQLParserRuleStatement getSQLStatement() {
+        AlterSQLParserRuleStatement result = new AlterSQLParserRuleStatement();
+        result.setSqlCommentParseEnable(Boolean.TRUE);
+        result.setSqlStatementCache(getCacheOption(1000, 1000L, 3));
+        result.setParseTreeCache(getCacheOption(64, 512L, 3));
+        return result;
+    }
+    
+    private CacheOptionSegment getCacheOption(final Integer initialCapacity, final Long maximumSize, final Integer concurrencyLevel) {
+        return new CacheOptionSegment(initialCapacity, maximumSize, concurrencyLevel);
+    }
+}