Skip to content

Commit

Permalink
Remove useless parameter in ShardingRule to fix sonar issue
Browse files Browse the repository at this point in the history
  • Loading branch information
strongduanmu committed Dec 24, 2024
1 parent 56117f4 commit 572998a
Show file tree
Hide file tree
Showing 14 changed files with 55 additions and 70 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ private ShardingRouteCacheableCheckResult checkSelectCacheable(final SelectState
if (!shardingRule.isAllShardingTables(tableNames)) {
return new ShardingRouteCacheableCheckResult(false, Collections.emptyList());
}
if (1 != tableNames.size() && !shardingRule.isAllBindingTables(tableNames) || containsNonCacheableShardingAlgorithm(tableNames)) {
if (1 != tableNames.size() && !shardingRule.isAllConfigBindingTables(tableNames) || containsNonCacheableShardingAlgorithm(tableNames)) {
return new ShardingRouteCacheableCheckResult(false, Collections.emptyList());
}
List<ShardingCondition> shardingConditions = new WhereClauseShardingConditionEngine(shardingRule, timestampServiceRule).createShardingConditions(statementContext, params);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ public static void checkTableNotExist(final ShardingSphereSchema schema, final C
*/
public static void checkMultipleTable(final ShardingRule shardingRule, final SQLStatementContext sqlStatementContext) {
Collection<String> tableNames = ((TableAvailable) sqlStatementContext).getTablesContext().getTableNames();
boolean isAllShardingTables = shardingRule.isAllShardingTables(tableNames) && (1 == tableNames.size() || shardingRule.isAllBindingTables(tableNames));
boolean isAllShardingTables = shardingRule.isAllShardingTables(tableNames) && (1 == tableNames.size() || shardingRule.isAllConfigBindingTables(tableNames));
boolean isAllSingleTables = !shardingRule.containsShardingTable(tableNames);
ShardingSpherePreconditions.checkState(isAllShardingTables || isAllSingleTables, () -> new DMLWithMultipleShardingTablesException(tableNames));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,6 @@ private void checkAlterViewShardingTables(final ShardingRule shardingRule, final

private void checkBroadcastShardingView(final ShardingRule shardingRule, final String originView, final String targetView) {
ShardingSpherePreconditions.checkState(!shardingRule.isShardingTable(originView) && !shardingRule.isShardingTable(targetView)
|| shardingRule.isAllBindingTables(Arrays.asList(originView, targetView)), () -> new RenamedViewWithoutSameConfigurationException(originView, targetView));
|| shardingRule.isAllConfigBindingTables(Arrays.asList(originView, targetView)), () -> new RenamedViewWithoutSameConfigurationException(originView, targetView));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ public void check(final ShardingRule rule, final ShardingSphereDatabase database
throw new MissingGenerateKeyColumnWithInsertSelectException();
}
TablesContext tablesContext = sqlStatementContext.getTablesContext();
if (rule.containsShardingTable(tablesContext.getTableNames()) && !isAllSameTables(tablesContext.getTableNames()) && !rule.isAllBindingTables(tablesContext.getTableNames())) {
if (rule.containsShardingTable(tablesContext.getTableNames()) && !isAllSameTables(tablesContext.getTableNames()) && !rule.isAllConfigBindingTables(tablesContext.getTableNames())) {
throw new InsertSelectTableViolationException();
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ public static boolean isSchemaContainsIndex(final ShardingSphereSchema schema, f
public static boolean isShardingTablesNotBindingWithView(final Collection<SimpleTableSegment> tableSegments, final ShardingRule shardingRule, final String viewName) {
for (SimpleTableSegment each : tableSegments) {
String logicTable = each.getTableName().getIdentifier().getValue();
if (shardingRule.isShardingTable(logicTable) && !shardingRule.isAllBindingTables(Arrays.asList(viewName, logicTable))) {
if (shardingRule.isShardingTable(logicTable) && !shardingRule.isAllConfigBindingTables(Arrays.asList(viewName, logicTable))) {
return true;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,15 +52,14 @@ public boolean decide(final SelectStatementContext selectStatementContext, final
if (!selectStatementContext.isContainsJoinQuery() || rule.isAllTablesInSameDataSource(tableNames)) {
return false;
}
if (isSelfJoinWithoutShardingColumn(selectStatementContext, rule, database, tableNames)) {
if (isSelfJoinWithoutShardingColumn(selectStatementContext, rule, tableNames)) {
return true;
}
return tableNames.size() > 1 && !rule.isAllBindingTables(database, selectStatementContext, tableNames);
return tableNames.size() > 1 && !rule.isBindingTablesUseShardingColumnsJoin(selectStatementContext, tableNames);
}

private boolean isSelfJoinWithoutShardingColumn(final SelectStatementContext selectStatementContext,
final ShardingRule rule, final ShardingSphereDatabase database, final Collection<String> tableNames) {
return 1 == tableNames.size() && selectStatementContext.isContainsJoinQuery() && !rule.isAllBindingTables(database, selectStatementContext, tableNames);
private boolean isSelfJoinWithoutShardingColumn(final SelectStatementContext selectStatementContext, final ShardingRule rule, final Collection<String> tableNames) {
return 1 == tableNames.size() && selectStatementContext.isContainsJoinQuery() && !rule.isBindingTablesUseShardingColumnsJoin(selectStatementContext, tableNames);
}

private Collection<DataNode> getTableDataNodes(final ShardingRule rule, final ShardingSphereDatabase database, final Collection<String> tableNames) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ private boolean isAllBindingTables(final SQLStatementContext sqlStatementContext
Collection<String> shardingLogicTableNames = sqlStatementContext instanceof TableAvailable
? rule.getShardingLogicTableNames(((TableAvailable) sqlStatementContext).getTablesContext().getTableNames())
: Collections.emptyList();
return shardingLogicTableNames.size() > 1 && rule.isAllBindingTables(shardingLogicTableNames);
return shardingLogicTableNames.size() > 1 && rule.isAllConfigBindingTables(shardingLogicTableNames);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ private static ShardingRouteEngine getDDLRouteEngine(final ShardingSphereDatabas
private static ShardingRouteEngine getCursorRouteEngine(final ShardingRule rule, final ShardingSphereDatabase database, final SQLStatementContext sqlStatementContext,
final HintValueContext hintValueContext, final ShardingConditions shardingConditions, final Collection<String> logicTableNames,
final ConfigurationProperties props) {
boolean allBindingTables = logicTableNames.size() > 1 && rule.isAllBindingTables(database, sqlStatementContext, logicTableNames);
boolean allBindingTables = logicTableNames.size() > 1 && rule.isBindingTablesUseShardingColumnsJoin(sqlStatementContext, logicTableNames);
if (isShardingStandardQuery(rule, logicTableNames, allBindingTables)) {
return new ShardingStandardRouteEngine(getLogicTableName(shardingConditions, logicTableNames), shardingConditions, sqlStatementContext, hintValueContext, props);
}
Expand Down Expand Up @@ -135,13 +135,12 @@ private static ShardingRouteEngine getDQLRouteEngine(final ShardingRule rule, fi
if (sqlStatementContext.getSqlStatement() instanceof DMLStatement && shardingConditions.isAlwaysFalse() || tableNames.isEmpty()) {
return new ShardingUnicastRouteEngine(sqlStatementContext, tableNames, queryContext.getConnectionContext());
}
return getDQLRouteEngineForShardingTable(rule, database, sqlStatementContext, queryContext.getHintValueContext(), shardingConditions, logicTableNames, props);
return getDQLRouteEngineForShardingTable(rule, sqlStatementContext, queryContext.getHintValueContext(), shardingConditions, logicTableNames, props);
}

private static ShardingRouteEngine getDQLRouteEngineForShardingTable(final ShardingRule rule, final ShardingSphereDatabase database,
final SQLStatementContext sqlStatementContext, final HintValueContext hintValueContext,
private static ShardingRouteEngine getDQLRouteEngineForShardingTable(final ShardingRule rule, final SQLStatementContext sqlStatementContext, final HintValueContext hintValueContext,
final ShardingConditions shardingConditions, final Collection<String> logicTableNames, final ConfigurationProperties props) {
boolean allBindingTables = logicTableNames.size() > 1 && rule.isAllBindingTables(database, sqlStatementContext, logicTableNames);
boolean allBindingTables = logicTableNames.size() > 1 && rule.isBindingTablesUseShardingColumnsJoin(sqlStatementContext, logicTableNames);
if (isShardingStandardQuery(rule, logicTableNames, allBindingTables)) {
return new ShardingStandardRouteEngine(getLogicTableName(shardingConditions, logicTableNames), shardingConditions, sqlStatementContext, hintValueContext, props);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ public RouteContext route(final ShardingRule shardingRule) {
result.getRouteUnits().addAll(getBroadcastTableRouteUnits(shardingRule, ""));
return result;
}
if (logicTableNames.size() > 1 && shardingRule.isAllBindingTables(logicTableNames)) {
if (logicTableNames.size() > 1 && shardingRule.isAllConfigBindingTables(logicTableNames)) {
result.getRouteUnits().addAll(getBindingTableRouteUnits(shardingRule, logicTableNames));
} else {
Collection<RouteContext> routeContexts = getRouteContexts(shardingRule, logicTableNames);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
package org.apache.shardingsphere.sharding.rule;

import com.cedarsoftware.util.CaseInsensitiveMap;
import com.cedarsoftware.util.CaseInsensitiveSet;
import com.google.common.base.Splitter;
import com.google.common.base.Strings;
import lombok.Getter;
Expand All @@ -26,16 +27,13 @@
import org.apache.shardingsphere.infra.algorithm.keygen.core.KeyGenerateAlgorithm;
import org.apache.shardingsphere.infra.binder.context.statement.SQLStatementContext;
import org.apache.shardingsphere.infra.binder.context.statement.dml.SelectStatementContext;
import org.apache.shardingsphere.infra.database.core.type.DatabaseTypeRegistry;
import org.apache.shardingsphere.infra.datanode.DataNode;
import org.apache.shardingsphere.infra.exception.core.ShardingSpherePreconditions;
import org.apache.shardingsphere.infra.expr.core.InlineExpressionParserFactory;
import org.apache.shardingsphere.infra.instance.ComputeNodeInstanceContext;
import org.apache.shardingsphere.infra.instance.ComputeNodeInstanceContextAware;
import org.apache.shardingsphere.infra.metadata.database.ShardingSphereDatabase;
import org.apache.shardingsphere.infra.metadata.database.resource.PhysicalDataSourceAggregator;
import org.apache.shardingsphere.infra.metadata.database.rule.RuleMetaData;
import org.apache.shardingsphere.infra.metadata.database.schema.model.ShardingSphereSchema;
import org.apache.shardingsphere.infra.rule.ShardingSphereRule;
import org.apache.shardingsphere.infra.rule.attribute.RuleAttributes;
import org.apache.shardingsphere.infra.rule.attribute.datasource.aggregate.AggregatedDataSourceRuleAttribute;
Expand Down Expand Up @@ -320,12 +318,12 @@ public ShardingTable getShardingTable(final String logicTableName) {
}

/**
* Judge whether logic table is all binding tables or not.
* Judge whether logic table is all config binding tables or not.
*
* @param logicTableNames logic table names
* @return whether logic table is all binding tables or not
* @return whether logic table is all config binding tables or not
*/
public boolean isAllBindingTables(final Collection<String> logicTableNames) {
public boolean isAllConfigBindingTables(final Collection<String> logicTableNames) {
if (logicTableNames.isEmpty()) {
return false;
}
Expand All @@ -339,25 +337,21 @@ public boolean isAllBindingTables(final Collection<String> logicTableNames) {
}

/**
* Judge whether logic table is all binding tables.
* Judge whether logic table is all config binding tables and use sharding columns join.
*
* @param database database
* @param sqlStatementContext sqlStatementContext
* @param logicTableNames logic table names
* @return whether logic table is all binding tables
* @return whether logic table is all config binding tables and use sharding columns join
*/
// TODO rename the method name, add sharding condition judgement in method name @duanzhengqiang
public boolean isAllBindingTables(final ShardingSphereDatabase database, final SQLStatementContext sqlStatementContext, final Collection<String> logicTableNames) {
public boolean isBindingTablesUseShardingColumnsJoin(final SQLStatementContext sqlStatementContext, final Collection<String> logicTableNames) {
if (!(sqlStatementContext instanceof SelectStatementContext && ((SelectStatementContext) sqlStatementContext).isContainsJoinQuery())) {
return isAllBindingTables(logicTableNames);
return isAllConfigBindingTables(logicTableNames);
}
if (!isAllBindingTables(logicTableNames)) {
if (!isAllConfigBindingTables(logicTableNames)) {
return false;
}
String defaultSchemaName = new DatabaseTypeRegistry(sqlStatementContext.getDatabaseType()).getDefaultSchemaName(database.getName());
SelectStatementContext select = (SelectStatementContext) sqlStatementContext;
ShardingSphereSchema schema = select.getTablesContext().getSchemaName().map(database::getSchema).orElseGet(() -> database.getSchema(defaultSchemaName));
return isJoinConditionContainsShardingColumns(schema, select, logicTableNames, select.getWhereSegments());
return isJoinConditionContainsShardingColumns(logicTableNames, select.getWhereSegments());
}

private Optional<BindingTableRule> findBindingTableRule(final Collection<String> logicTableNames) {
Expand Down Expand Up @@ -575,10 +569,9 @@ public boolean isShardingCacheEnabled() {
return null != shardingCache;
}

private boolean isJoinConditionContainsShardingColumns(final ShardingSphereSchema schema, final SelectStatementContext select,
final Collection<String> tableNames, final Collection<WhereSegment> whereSegments) {
Collection<String> databaseJoinConditionTables = new HashSet<>(tableNames.size(), 1F);
Collection<String> tableJoinConditionTables = new HashSet<>(tableNames.size(), 1F);
private boolean isJoinConditionContainsShardingColumns(final Collection<String> tableNames, final Collection<WhereSegment> whereSegments) {
Collection<String> databaseJoinConditionTables = new CaseInsensitiveSet<>(tableNames.size(), 1F);
Collection<String> tableJoinConditionTables = new CaseInsensitiveSet<>(tableNames.size(), 1F);
for (WhereSegment each : whereSegments) {
Collection<AndPredicate> andPredicates = ExpressionExtractor.extractAndPredicates(each.getExpr());
if (andPredicates.size() > 1) {
Expand Down Expand Up @@ -610,14 +603,11 @@ private Collection<String> getJoinConditionTables(final Collection<ExpressionSeg
if (!leftShardingTable.isPresent() || !rightShardingTable.isPresent()) {
continue;
}
ShardingStrategyConfiguration leftConfig = isDatabaseJoinCondition
? getDatabaseShardingStrategyConfiguration(leftShardingTable.get())
: getTableShardingStrategyConfiguration(leftShardingTable.get());
ShardingStrategyConfiguration rightConfig = isDatabaseJoinCondition
? getDatabaseShardingStrategyConfiguration(rightShardingTable.get())
: getTableShardingStrategyConfiguration(rightShardingTable.get());
if (findShardingColumn(leftConfig, leftColumn.getIdentifier().getValue()).isPresent()
&& findShardingColumn(rightConfig, rightColumn.getIdentifier().getValue()).isPresent()) {
ShardingStrategyConfiguration leftConfig =
isDatabaseJoinCondition ? getDatabaseShardingStrategyConfiguration(leftShardingTable.get()) : getTableShardingStrategyConfiguration(leftShardingTable.get());
ShardingStrategyConfiguration rightConfig =
isDatabaseJoinCondition ? getDatabaseShardingStrategyConfiguration(rightShardingTable.get()) : getTableShardingStrategyConfiguration(rightShardingTable.get());
if (findShardingColumn(leftConfig, leftColumn.getIdentifier().getValue()).isPresent() && findShardingColumn(rightConfig, rightColumn.getIdentifier().getValue()).isPresent()) {
result.add(leftColumn.getColumnBoundInfo().getOriginalTable().getValue());
result.add(rightColumn.getColumnBoundInfo().getOriginalTable().getValue());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ void assertCheck() {
@Test
void assertCheckWithException() {
when(rule.isShardingTable(any())).thenReturn(true);
when(rule.isAllBindingTables(any())).thenReturn(false);
when(rule.isAllConfigBindingTables(any())).thenReturn(false);
assertThrows(EngagedViewException.class, () -> new ShardingCreateViewSupportedChecker().check(rule, mock(), mock(), createViewStatementContext));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,7 @@ void assertDecideWithSelfJoinAndShardingColumn() {
ShardingRule rule = createShardingRule();
when(rule.getShardingLogicTableNames(Collections.singleton("foo_tbl"))).thenReturn(Collections.singleton("foo_tbl"));
ShardingSphereDatabase database = createDatabase(rule);
when(rule.isAllBindingTables(database, sqlStatementContext, Collections.singleton("foo_tbl"))).thenReturn(true);
when(rule.isBindingTablesUseShardingColumnsJoin(sqlStatementContext, Collections.singleton("foo_tbl"))).thenReturn(true);
Collection<DataNode> includedDataNodes = new HashSet<>();
assertFalse(decider.decide(sqlStatementContext, Collections.emptyList(), mock(RuleMetaData.class), database, rule, includedDataNodes));
assertThat(includedDataNodes.size(), is(2));
Expand All @@ -173,7 +173,7 @@ void assertDecideWithAllBindingTables() {
when(sqlStatementContext.isContainsJoinQuery()).thenReturn(true);
ShardingRule rule = createShardingRule();
ShardingSphereDatabase database = createDatabase(rule);
when(rule.isAllBindingTables(database, sqlStatementContext, Arrays.asList("foo_tbl", "bar_tbl"))).thenReturn(true);
when(rule.isBindingTablesUseShardingColumnsJoin(sqlStatementContext, Arrays.asList("foo_tbl", "bar_tbl"))).thenReturn(true);
Collection<DataNode> includedDataNodes = new HashSet<>();
assertFalse(decider.decide(sqlStatementContext, Collections.emptyList(), mock(RuleMetaData.class), database, rule, includedDataNodes));
assertThat(includedDataNodes.size(), is(4));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ void assertIsNotGenerateSQLTokenWithNotTableAvailable() {
void assertIsGenerateSQLTokenWithAllBindingTables() {
Collection<String> logicTableNames = Arrays.asList("foo_tbl", "bar_tbl");
when(rule.getShardingLogicTableNames(logicTableNames)).thenReturn(logicTableNames);
when(rule.isAllBindingTables(logicTableNames)).thenReturn(true);
when(rule.isAllConfigBindingTables(logicTableNames)).thenReturn(true);
SelectStatementContext sqlStatementContext = mock(SelectStatementContext.class, RETURNS_DEEP_STUBS);
when(sqlStatementContext.getTablesContext().getTableNames()).thenReturn(logicTableNames);
assertTrue(generator.isGenerateSQLToken(sqlStatementContext));
Expand Down
Loading

0 comments on commit 572998a

Please sign in to comment.