Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix sonar issue on PipelineDDLGenerator #34144

Merged
merged 2 commits into from
Dec 24, 2024
Merged

Fix sonar issue on PipelineDDLGenerator #34144

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f3344a7
Fix sonar issue on WhereClauseShardingConditionEngine
terrymanu Dec 24, 2024
eac2b94
Fix sonar issue on PipelineDDLGenerator
terrymanu Dec 24, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 11 additions & 11 deletions .../java/org/apache/shardingsphere/sharding/cache/checker/ShardingRouteCacheableChecker.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,34 +86,34 @@ private ShardingRouteCacheableCheckResult load(final Key key) {
SQLStatementContext sqlStatementContext = key.getSqlStatementContext();
ShardingRouteCacheableCheckResult result;
if (sqlStatementContext instanceof SelectStatementContext) {
result = checkSelectCacheable((SelectStatementContext) sqlStatementContext, key.getParameters(), key.getDatabase());
result = checkSelectCacheable((SelectStatementContext) sqlStatementContext, key.getParameters());
} else if (sqlStatementContext instanceof UpdateStatementContext) {
result = checkUpdateCacheable((UpdateStatementContext) sqlStatementContext, key.getParameters(), key.getDatabase());
result = checkUpdateCacheable((UpdateStatementContext) sqlStatementContext, key.getParameters());
} else if (sqlStatementContext instanceof InsertStatementContext) {
result = checkInsertCacheable((InsertStatementContext) sqlStatementContext, key.getParameters(), key.getDatabase());
} else if (sqlStatementContext instanceof DeleteStatementContext) {
result = checkDeleteCacheable((DeleteStatementContext) sqlStatementContext, key.getParameters(), key.getDatabase());
result = checkDeleteCacheable((DeleteStatementContext) sqlStatementContext, key.getParameters());
} else {
result = new ShardingRouteCacheableCheckResult(false, Collections.emptyList());
}
key.getParameters().clear();
return result;
}

private ShardingRouteCacheableCheckResult checkSelectCacheable(final SelectStatementContext statementContext, final List<Object> params, final ShardingSphereDatabase database) {
private ShardingRouteCacheableCheckResult checkSelectCacheable(final SelectStatementContext statementContext, final List<Object> params) {
Collection<String> tableNames = new HashSet<>(statementContext.getTablesContext().getTableNames());
if (!shardingRule.isAllShardingTables(tableNames)) {
return new ShardingRouteCacheableCheckResult(false, Collections.emptyList());
}
if (1 != tableNames.size() && !shardingRule.isAllBindingTables(tableNames) || containsNonCacheableShardingAlgorithm(tableNames)) {
return new ShardingRouteCacheableCheckResult(false, Collections.emptyList());
}
List<ShardingCondition> shardingConditions = new WhereClauseShardingConditionEngine(database, shardingRule, timestampServiceRule).createShardingConditions(statementContext, params);
List<ShardingCondition> shardingConditions = new WhereClauseShardingConditionEngine(shardingRule, timestampServiceRule).createShardingConditions(statementContext, params);
return checkShardingConditionsCacheable(shardingConditions);
}

private ShardingRouteCacheableCheckResult checkUpdateCacheable(final UpdateStatementContext statementContext, final List<Object> params, final ShardingSphereDatabase database) {
return checkUpdateOrDeleteCacheable(statementContext, params, database);
private ShardingRouteCacheableCheckResult checkUpdateCacheable(final UpdateStatementContext statementContext, final List<Object> params) {
return checkUpdateOrDeleteCacheable(statementContext, params);
}

private ShardingRouteCacheableCheckResult checkInsertCacheable(final InsertStatementContext statementContext, final List<Object> params, final ShardingSphereDatabase database) {
Expand All @@ -140,11 +140,11 @@ private ShardingRouteCacheableCheckResult checkInsertCacheable(final InsertState
return checkShardingConditionsCacheable(shardingConditions);
}

private ShardingRouteCacheableCheckResult checkDeleteCacheable(final DeleteStatementContext statementContext, final List<Object> params, final ShardingSphereDatabase database) {
return checkUpdateOrDeleteCacheable(statementContext, params, database);
private ShardingRouteCacheableCheckResult checkDeleteCacheable(final DeleteStatementContext statementContext, final List<Object> params) {
return checkUpdateOrDeleteCacheable(statementContext, params);
}

private ShardingRouteCacheableCheckResult checkUpdateOrDeleteCacheable(final SQLStatementContext sqlStatementContext, final List<Object> params, final ShardingSphereDatabase database) {
private ShardingRouteCacheableCheckResult checkUpdateOrDeleteCacheable(final SQLStatementContext sqlStatementContext, final List<Object> params) {
Collection<String> tableNames = ((TableAvailable) sqlStatementContext).getTablesContext().getTableNames();
if (1 != tableNames.size()) {
return new ShardingRouteCacheableCheckResult(false, Collections.emptyList());
Expand All @@ -153,7 +153,7 @@ private ShardingRouteCacheableCheckResult checkUpdateOrDeleteCacheable(final SQL
if (!isShardingTable || containsNonCacheableShardingAlgorithm(tableNames)) {
return new ShardingRouteCacheableCheckResult(false, Collections.emptyList());
}
List<ShardingCondition> shardingConditions = new WhereClauseShardingConditionEngine(database, shardingRule, timestampServiceRule).createShardingConditions(sqlStatementContext, params);
List<ShardingCondition> shardingConditions = new WhereClauseShardingConditionEngine(shardingRule, timestampServiceRule).createShardingConditions(sqlStatementContext, params);
return checkShardingConditionsCacheable(shardingConditions);
}

Expand Down
2 changes: 1 addition & 1 deletion ...ingsphere/sharding/route/engine/condition/engine/InsertClauseShardingConditionEngine.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,7 @@ private Object getShardingValue(final SimpleExpressionSegment expressionSegment,

private List<ShardingCondition> createShardingConditionsWithInsertSelect(final InsertStatementContext sqlStatementContext, final List<Object> params) {
SelectStatementContext selectStatementContext = sqlStatementContext.getInsertSelectContext().getSelectStatementContext();
return new LinkedList<>(new WhereClauseShardingConditionEngine(database, rule, timestampServiceRule).createShardingConditions(selectStatementContext, params));
return new LinkedList<>(new WhereClauseShardingConditionEngine(rule, timestampServiceRule).createShardingConditions(selectStatementContext, params));
}

private void appendGeneratedKeyConditions(final InsertStatementContext sqlStatementContext, final List<ShardingCondition> shardingConditions) {
Expand Down
2 changes: 1 addition & 1 deletion ...apache/shardingsphere/sharding/route/engine/condition/engine/ShardingConditionEngine.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,6 @@ public List<ShardingCondition> createShardingConditions(final SQLStatementContex
TimestampServiceRule timestampServiceRule = globalRuleMetaData.getSingleRule(TimestampServiceRule.class);
return sqlStatementContext instanceof InsertStatementContext
? new InsertClauseShardingConditionEngine(database, shardingRule, timestampServiceRule).createShardingConditions((InsertStatementContext) sqlStatementContext, params)
: new WhereClauseShardingConditionEngine(database, shardingRule, timestampServiceRule).createShardingConditions(sqlStatementContext, params);
: new WhereClauseShardingConditionEngine(shardingRule, timestampServiceRule).createShardingConditions(sqlStatementContext, params);
}
}
17 changes: 2 additions & 15 deletions ...dingsphere/sharding/route/engine/condition/engine/WhereClauseShardingConditionEngine.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,7 @@
import lombok.RequiredArgsConstructor;
import org.apache.shardingsphere.infra.binder.context.extractor.SQLStatementContextExtractor;
import org.apache.shardingsphere.infra.binder.context.statement.SQLStatementContext;
import org.apache.shardingsphere.infra.binder.context.type.TableAvailable;
import org.apache.shardingsphere.infra.binder.context.type.WhereAvailable;
import org.apache.shardingsphere.infra.database.core.type.DatabaseTypeRegistry;
import org.apache.shardingsphere.infra.metadata.database.ShardingSphereDatabase;
import org.apache.shardingsphere.infra.metadata.database.schema.model.ShardingSphereSchema;
import org.apache.shardingsphere.sharding.exception.data.ShardingValueDataTypeException;
import org.apache.shardingsphere.sharding.route.engine.condition.AlwaysFalseShardingCondition;
import org.apache.shardingsphere.sharding.route.engine.condition.Column;
Expand Down Expand Up @@ -63,9 +59,7 @@
@RequiredArgsConstructor
public final class WhereClauseShardingConditionEngine {

private final ShardingSphereDatabase database;

private final ShardingRule shardingRule;
private final ShardingRule rule;

private final TimestampServiceRule timestampServiceRule;

Expand Down Expand Up @@ -103,19 +97,12 @@ private Collection<ShardingCondition> createShardingConditions(final ExpressionS
return result;
}

private ShardingSphereSchema getSchema(final SQLStatementContext sqlStatementContext, final ShardingSphereDatabase database) {
String defaultSchemaName = new DatabaseTypeRegistry(sqlStatementContext.getDatabaseType()).getDefaultSchemaName(database.getName());
return sqlStatementContext instanceof TableAvailable
? ((TableAvailable) sqlStatementContext).getTablesContext().getSchemaName().map(database::getSchema).orElseGet(() -> database.getSchema(defaultSchemaName))
: database.getSchema(defaultSchemaName);
}

private Map<Column, Collection<ShardingConditionValue>> createShardingConditionValueMap(final Collection<ExpressionSegment> predicates, final List<Object> params) {
Map<Column, Collection<ShardingConditionValue>> result = new HashMap<>(predicates.size(), 1F);
for (ExpressionSegment each : predicates) {
for (ColumnSegment columnSegment : ColumnExtractor.extract(each)) {
String tableName = columnSegment.getColumnBoundInfo().getOriginalTable().getValue();
Optional<String> shardingColumn = shardingRule.findShardingColumn(columnSegment.getColumnBoundInfo().getOriginalColumn().getValue(), tableName);
Optional<String> shardingColumn = rule.findShardingColumn(columnSegment.getColumnBoundInfo().getOriginalColumn().getValue(), tableName);
if (!shardingColumn.isPresent()) {
continue;
}
Expand Down
24 changes: 7 additions & 17 deletions ...sphere/sharding/route/engine/condition/engine/WhereClauseShardingConditionEngineTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,7 @@

package org.apache.shardingsphere.sharding.route.engine.condition.engine;

import org.apache.shardingsphere.infra.binder.context.segment.table.TablesContext;
import org.apache.shardingsphere.infra.binder.context.statement.dml.SelectStatementContext;
import org.apache.shardingsphere.infra.config.props.ConfigurationProperties;
import org.apache.shardingsphere.infra.database.core.type.DatabaseType;
import org.apache.shardingsphere.infra.metadata.database.ShardingSphereDatabase;
import org.apache.shardingsphere.infra.spi.type.typed.TypedSPILoader;
import org.apache.shardingsphere.sharding.route.engine.condition.ShardingCondition;
import org.apache.shardingsphere.sharding.route.engine.condition.value.ListShardingConditionValue;
import org.apache.shardingsphere.sharding.route.engine.condition.value.RangeShardingConditionValue;
Expand All @@ -45,11 +40,10 @@
import java.util.Collections;
import java.util.List;
import java.util.Optional;
import java.util.Properties;

import static org.hamcrest.CoreMatchers.instanceOf;
import static org.hamcrest.CoreMatchers.is;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;
Expand All @@ -60,21 +54,17 @@ class WhereClauseShardingConditionEngineTest {
private WhereClauseShardingConditionEngine shardingConditionEngine;

@Mock
private ShardingRule shardingRule;
private ShardingRule rule;

@Mock
private SelectStatementContext sqlStatementContext;

@Mock
private WhereSegment whereSegment;

@Mock
private TablesContext tablesContext;

@BeforeEach
void setUp() {
shardingConditionEngine = new WhereClauseShardingConditionEngine(ShardingSphereDatabase.create("test_db",
TypedSPILoader.getService(DatabaseType.class, "FIXTURE"), new ConfigurationProperties(new Properties())), shardingRule, mock(TimestampServiceRule.class));
shardingConditionEngine = new WhereClauseShardingConditionEngine(rule, mock(TimestampServiceRule.class));
when(sqlStatementContext.getWhereSegments()).thenReturn(Collections.singleton(whereSegment));
}

Expand All @@ -87,10 +77,10 @@ void assertCreateShardingConditionsForSelectRangeStatement() {
ExpressionSegment andSegment = new LiteralExpressionSegment(0, 0, and);
BetweenExpression betweenExpression = new BetweenExpression(0, 0, left, betweenSegment, andSegment, false);
when(whereSegment.getExpr()).thenReturn(betweenExpression);
when(shardingRule.findShardingColumn(any(), any())).thenReturn(Optional.of("foo_sharding_col"));
when(rule.findShardingColumn(any(), any())).thenReturn(Optional.of("foo_sharding_col"));
List<ShardingCondition> actual = shardingConditionEngine.createShardingConditions(sqlStatementContext, Collections.emptyList());
assertThat(actual.get(0).getStartIndex(), is(0));
assertTrue(actual.get(0).getValues().get(0) instanceof RangeShardingConditionValue);
assertThat(actual.get(0).getValues().get(0), instanceOf(RangeShardingConditionValue.class));
}

@Test
Expand All @@ -101,9 +91,9 @@ void assertCreateShardingConditionsForSelectInStatement() {
right.getItems().add(literalExpressionSegment);
InExpression inExpression = new InExpression(0, 0, left, right, false);
when(whereSegment.getExpr()).thenReturn(inExpression);
when(shardingRule.findShardingColumn(any(), any())).thenReturn(Optional.of("foo_sharding_col"));
when(rule.findShardingColumn(any(), any())).thenReturn(Optional.of("foo_sharding_col"));
List<ShardingCondition> actual = shardingConditionEngine.createShardingConditions(sqlStatementContext, Collections.emptyList());
assertThat(actual.get(0).getStartIndex(), is(0));
assertTrue(actual.get(0).getValues().get(0) instanceof ListShardingConditionValue);
assertThat(actual.get(0).getValues().get(0), instanceOf(ListShardingConditionValue.class));
}
}
5 changes: 3 additions & 2 deletions ...org/apache/shardingsphere/data/pipeline/core/metadata/generator/PipelineDDLGenerator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,8 @@

package org.apache.shardingsphere.data.pipeline.core.metadata.generator;

import lombok.RequiredArgsConstructor;
import lombok.AccessLevel;
import lombok.NoArgsConstructor;
import lombok.extern.slf4j.Slf4j;
import org.apache.shardingsphere.data.pipeline.core.sqlbuilder.dialect.DialectPipelineSQLBuilder;
import org.apache.shardingsphere.infra.database.core.spi.DatabaseTypedSPILoader;
Expand All @@ -31,7 +32,7 @@
/**
* Pipeline DDL generator.
*/
@RequiredArgsConstructor
@NoArgsConstructor(access = AccessLevel.PRIVATE)
@Slf4j
public final class PipelineDDLGenerator {

Expand Down
Loading