From 48fdb49f5f638ebd936b9dca514468f402145799 Mon Sep 17 00:00:00 2001 From: zhangliang Date: Fri, 12 Dec 2025 16:15:37 +0800 Subject: [PATCH] Refactor BetweenExpressionConverter --- .../expression/ExpressionConverter.java | 2 +- .../impl/BetweenExpressionConverter.java | 9 ++--- .../SQLStatementCompilerEngineTest.java | 2 +- .../expression/ExpressionConverterTest.java | 5 +-- .../impl/BetweenExpressionConverterTest.java | 23 ++++-------- .../src/test/resources/logback-test.xml | 36 +++++++++++++++++++ 6 files changed, 49 insertions(+), 28 deletions(-) create mode 100644 kernel/sql-federation/compiler/src/test/resources/logback-test.xml diff --git a/kernel/sql-federation/compiler/src/main/java/org/apache/shardingsphere/sqlfederation/compiler/sql/ast/converter/segment/expression/ExpressionConverter.java b/kernel/sql-federation/compiler/src/main/java/org/apache/shardingsphere/sqlfederation/compiler/sql/ast/converter/segment/expression/ExpressionConverter.java index d7ee6d0412181..d983ef3b676ea 100644 --- a/kernel/sql-federation/compiler/src/main/java/org/apache/shardingsphere/sqlfederation/compiler/sql/ast/converter/segment/expression/ExpressionConverter.java +++ b/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 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); diff --git a/kernel/sql-federation/compiler/src/main/java/org/apache/shardingsphere/sqlfederation/compiler/sql/ast/converter/segment/expression/impl/BetweenExpressionConverter.java b/kernel/sql-federation/compiler/src/main/java/org/apache/shardingsphere/sqlfederation/compiler/sql/ast/converter/segment/expression/impl/BetweenExpressionConverter.java index 816df4d1f24a2..8ce0c4661e836 100644 --- a/kernel/sql-federation/compiler/src/main/java/org/apache/shardingsphere/sqlfederation/compiler/sql/ast/converter/segment/expression/impl/BetweenExpressionConverter.java +++ b/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 convert(final BetweenExpression expression) { - if (null == expression) { - return Optional.empty(); - } + public static SqlBasicCall convert(final BetweenExpression expression) { Collection 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); } } diff --git a/kernel/sql-federation/compiler/src/test/java/org/apache/shardingsphere/sqlfederation/compiler/compiler/SQLStatementCompilerEngineTest.java b/kernel/sql-federation/compiler/src/test/java/org/apache/shardingsphere/sqlfederation/compiler/compiler/SQLStatementCompilerEngineTest.java index 839f6c2a1899c..f3b3b2dcd6657 100644 --- a/kernel/sql-federation/compiler/src/test/java/org/apache/shardingsphere/sqlfederation/compiler/compiler/SQLStatementCompilerEngineTest.java +++ b/kernel/sql-federation/compiler/src/test/java/org/apache/shardingsphere/sqlfederation/compiler/compiler/SQLStatementCompilerEngineTest.java @@ -78,7 +78,7 @@ void assertCompileWithoutCache() { void assertCompileWithCache() { LoadingCache 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)); diff --git a/kernel/sql-federation/compiler/src/test/java/org/apache/shardingsphere/sqlfederation/compiler/sql/ast/converter/segment/expression/ExpressionConverterTest.java b/kernel/sql-federation/compiler/src/test/java/org/apache/shardingsphere/sqlfederation/compiler/sql/ast/converter/segment/expression/ExpressionConverterTest.java index 1c2cc027552d7..b4e9b4e7437fb 100644 --- a/kernel/sql-federation/compiler/src/test/java/org/apache/shardingsphere/sqlfederation/compiler/sql/ast/converter/segment/expression/ExpressionConverterTest.java +++ b/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)); diff --git a/kernel/sql-federation/compiler/src/test/java/org/apache/shardingsphere/sqlfederation/compiler/sql/ast/converter/segment/expression/impl/BetweenExpressionConverterTest.java b/kernel/sql-federation/compiler/src/test/java/org/apache/shardingsphere/sqlfederation/compiler/sql/ast/converter/segment/expression/impl/BetweenExpressionConverterTest.java index 91b1525ab7e77..4fa55c890cb41 100644 --- a/kernel/sql-federation/compiler/src/test/java/org/apache/shardingsphere/sqlfederation/compiler/sql/ast/converter/segment/expression/impl/BetweenExpressionConverterTest.java +++ b/kernel/sql-federation/compiler/src/test/java/org/apache/shardingsphere/sqlfederation/compiler/sql/ast/converter/segment/expression/impl/BetweenExpressionConverterTest.java @@ -34,8 +34,6 @@ 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; @@ -43,11 +41,6 @@ @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 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 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))); } } diff --git a/kernel/sql-federation/compiler/src/test/resources/logback-test.xml b/kernel/sql-federation/compiler/src/test/resources/logback-test.xml new file mode 100644 index 0000000000000..03e8b9152bce4 --- /dev/null +++ b/kernel/sql-federation/compiler/src/test/resources/logback-test.xml @@ -0,0 +1,36 @@ + + + + + + + [%-5level] %d{yyyy-MM-dd HH:mm:ss.SSS} [%thread] %logger{36} - %msg%n + + + + + + + + + + + + + +