Refactor BetweenExpressionConverter

kernel/sql-federation/compiler/src/main/java/org/apache/shardingsphere/sqlfederation/compiler/sql/ast/converter/segment/expression/ExpressionConverter.java

@@ -115,7 +115,7 @@ public static Optional<SqlNode> convert(final ExpressionSegment segment) {
             return InExpressionConverter.convert((InExpression) segment);
         }
         if (segment instanceof BetweenExpression) {
-            return BetweenExpressionConverter.convert((BetweenExpression) segment);
+            return Optional.of(BetweenExpressionConverter.convert((BetweenExpression) segment));
         }
         if (segment instanceof ParameterMarkerExpressionSegment) {
             return ParameterMarkerExpressionConverter.convert((ParameterMarkerExpressionSegment) segment);

kernel/sql-federation/compiler/src/main/java/org/apache/shardingsphere/sqlfederation/compiler/sql/ast/converter/segment/expression/impl/BetweenExpressionConverter.java

@@ -29,7 +29,6 @@
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.LinkedList;
-import java.util.Optional;
 
 /**
  * Between expression converter.
@@ -43,15 +42,11 @@ public final class BetweenExpressionConverter {
      * @param expression between expression
      * @return SQL node
      */
-    public static Optional<SqlNode> convert(final BetweenExpression expression) {
-        if (null == expression) {
-            return Optional.empty();
-        }
+    public static SqlBasicCall convert(final BetweenExpression expression) {
         Collection<SqlNode> sqlNodes = new LinkedList<>();
         ExpressionConverter.convert(expression.getLeft()).ifPresent(sqlNodes::add);
         ExpressionConverter.convert(expression.getBetweenExpr()).ifPresent(sqlNodes::add);
         ExpressionConverter.convert(expression.getAndExpr()).ifPresent(sqlNodes::add);
-        return Optional.of(expression.isNot() ? new SqlBasicCall(SqlStdOperatorTable.NOT_BETWEEN, new ArrayList<>(sqlNodes), SqlParserPos.ZERO)
-                : new SqlBasicCall(SqlStdOperatorTable.BETWEEN, new ArrayList<>(sqlNodes), SqlParserPos.ZERO));
+        return new SqlBasicCall(expression.isNot() ? SqlStdOperatorTable.NOT_BETWEEN : SqlStdOperatorTable.BETWEEN, new ArrayList<>(sqlNodes), SqlParserPos.ZERO);
     }
 }

kernel/sql-federation/compiler/src/test/java/org/apache/shardingsphere/sqlfederation/compiler/compiler/SQLStatementCompilerEngineTest.java

@@ -78,7 +78,7 @@ void assertCompileWithoutCache() {
     void assertCompileWithCache() {
         LoadingCache<ExecutionPlanCacheKey, SQLFederationExecutionPlan> cache = mock(LoadingCache.class);
         SQLFederationExecutionPlan expectedCachedPlan = mock(SQLFederationExecutionPlan.class);
-        when(cache.get(cacheKey)).thenReturn(null, expectedCachedPlan, expectedCachedPlan, expectedCachedPlan);
+        when(cache.get(cacheKey)).thenReturn(expectedCachedPlan);
         when(ExecutionPlanCacheBuilder.build(any(SQLFederationCacheOption.class))).thenReturn(cache);
         SQLStatementCompilerEngine engine = new SQLStatementCompilerEngine(new SQLFederationCacheOption(1, 1L));
         assertThat(engine.compile(cacheKey, true), is(expectedCachedPlan));

kernel/sql-federation/compiler/src/test/java/org/apache/shardingsphere/sqlfederation/compiler/sql/ast/converter/segment/expression/ExpressionConverterTest.java

@@ -17,6 +17,7 @@
 
 package org.apache.shardingsphere.sqlfederation.compiler.sql.ast.converter.segment.expression;
 
+import org.apache.calcite.sql.SqlBasicCall;
 import org.apache.calcite.sql.SqlNode;
 import org.apache.shardingsphere.database.connector.core.type.DatabaseType;
 import org.apache.shardingsphere.infra.exception.generic.UnsupportedSQLOperationException;
@@ -130,9 +131,9 @@ void assertConvertDelegatesToAllSupportedConverters() {
         SqlNode expectedInNode = mock(SqlNode.class);
         InExpression inExpression = new InExpression(0, 0, literalSegment, literalSegment, false);
         when(InExpressionConverter.convert(inExpression)).thenReturn(Optional.of(expectedInNode));
-        SqlNode expectedBetweenNode = mock(SqlNode.class);
+        SqlBasicCall expectedBetweenNode = mock(SqlBasicCall.class);
         BetweenExpression betweenExpression = new BetweenExpression(0, 0, literalSegment, literalSegment, literalSegment, false);
-        when(BetweenExpressionConverter.convert(betweenExpression)).thenReturn(Optional.of(expectedBetweenNode));
+        when(BetweenExpressionConverter.convert(betweenExpression)).thenReturn(expectedBetweenNode);
         SqlNode expectedParameterNode = mock(SqlNode.class);
         ParameterMarkerExpressionSegment parameterSegment = new ParameterMarkerExpressionSegment(0, 0, 0);
         when(ParameterMarkerExpressionConverter.convert(parameterSegment)).thenReturn(Optional.of(expectedParameterNode));

kernel/sql-federation/compiler/src/test/java/org/apache/shardingsphere/sqlfederation/compiler/sql/ast/converter/segment/expression/impl/BetweenExpressionConverterTest.java

@@ -34,20 +34,13 @@
 
 import static org.hamcrest.CoreMatchers.is;
 import static org.hamcrest.MatcherAssert.assertThat;
-import static org.junit.jupiter.api.Assertions.assertFalse;
-import static org.junit.jupiter.api.Assertions.assertTrue;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
 
 @ExtendWith(AutoMockExtension.class)
 @StaticMockSettings(ExpressionConverter.class)
 class BetweenExpressionConverterTest {
 
-    @Test
-    void assertConvertReturnsEmptyForNullExpression() {
-        assertFalse(BetweenExpressionConverter.convert(null).isPresent());
-    }
-
     @Test
     void assertConvertBetweenExpression() {
         ExpressionSegment left = new LiteralExpressionSegment(0, 0, "left");
@@ -59,11 +52,9 @@ void assertConvertBetweenExpression() {
         when(ExpressionConverter.convert(left)).thenReturn(Optional.of(leftNode));
         when(ExpressionConverter.convert(betweenExpr)).thenReturn(Optional.of(betweenNode));
         when(ExpressionConverter.convert(andExpr)).thenReturn(Optional.of(andNode));
-        Optional<SqlNode> actual = BetweenExpressionConverter.convert(new BetweenExpression(0, 0, left, betweenExpr, andExpr, false));
-        assertTrue(actual.isPresent());
-        SqlBasicCall call = (SqlBasicCall) actual.orElse(null);
-        assertThat(call.getOperator(), is(SqlStdOperatorTable.BETWEEN));
-        assertThat(call.getOperandList(), is(Arrays.asList(leftNode, betweenNode, andNode)));
+        SqlBasicCall actual = BetweenExpressionConverter.convert(new BetweenExpression(0, 0, left, betweenExpr, andExpr, false));
+        assertThat(actual.getOperator(), is(SqlStdOperatorTable.BETWEEN));
+        assertThat(actual.getOperandList(), is(Arrays.asList(leftNode, betweenNode, andNode)));
     }
 
     @Test
@@ -77,10 +68,8 @@ void assertConvertNotBetweenExpression() {
         when(ExpressionConverter.convert(left)).thenReturn(Optional.of(leftNode));
         when(ExpressionConverter.convert(betweenExpr)).thenReturn(Optional.of(betweenNode));
         when(ExpressionConverter.convert(andExpr)).thenReturn(Optional.of(andNode));
-        Optional<SqlNode> actual = BetweenExpressionConverter.convert(new BetweenExpression(0, 0, left, betweenExpr, andExpr, true));
-        assertTrue(actual.isPresent());
-        SqlBasicCall call = (SqlBasicCall) actual.orElse(null);
-        assertThat(call.getOperator(), is(SqlStdOperatorTable.NOT_BETWEEN));
-        assertThat(call.getOperandList(), is(Arrays.asList(leftNode, betweenNode, andNode)));
+        SqlBasicCall actual = BetweenExpressionConverter.convert(new BetweenExpression(0, 0, left, betweenExpr, andExpr, true));
+        assertThat(actual.getOperator(), is(SqlStdOperatorTable.NOT_BETWEEN));
+        assertThat(actual.getOperandList(), is(Arrays.asList(leftNode, betweenNode, andNode)));
     }
 }

kernel/sql-federation/compiler/src/test/resources/logback-test.xml

@@ -0,0 +1,36 @@
+<?xml version="1.0"?>
+<!--
+  ~ 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.
+  -->
+
+<configuration>
+    <appender name="console" class="ch.qos.logback.core.ConsoleAppender">
+        <encoder>
+            <pattern>[%-5level] %d{yyyy-MM-dd HH:mm:ss.SSS} [%thread] %logger{36} - %msg%n</pattern>
+        </encoder>
+    </appender>
+
+    <logger name="org.apache.shardingsphere" level="info" additivity="false">
+        <appender-ref ref="console" />
+    </logger>
+
+    <logger name="com.zaxxer.hikari" level="error" />
+
+    <root>
+        <level value="info" />
+        <appender-ref ref="console" />
+    </root>
+</configuration>