JAkutenshi commented on code in PR #6546:
URL: https://github.com/apache/ignite-3/pull/6546#discussion_r2330063487
##########
modules/distribution-zones/src/main/java/org/apache/ignite/internal/distributionzones/rebalance/RebalanceUtil.java:
##########
@@ -425,7 +425,7 @@ private static CompletableFuture<Void>
tablePartitionAssignment(
for (int partId = 0; partId < zoneDescriptor.partitions(); partId++) {
TablePartitionId replicaGrpId = new
TablePartitionId(tableDescriptor.id(), partId);
- // TODO https://issues.apache.org/jira/browse/IGNITE-19763 We
should distinguish empty stable assignments on
+ // TODO https://issues.apache.org/jira/browse/IGNITE-26395 We
should distinguish empty stable assignments on
Review Comment:
Ticket is empty, don't forget to fill it
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/prepare/ddl/ClusterWideNodeFilterValidator.java:
##########
@@ -0,0 +1,87 @@
+/*
+ * 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.ignite.internal.sql.engine.prepare.ddl;
+
+import static java.util.stream.Collectors.toSet;
+import static
org.apache.ignite.internal.distributionzones.DistributionZonesUtil.filterNodeAttributes;
+import static org.apache.ignite.internal.lang.IgniteStringFormatter.format;
+import static
org.apache.ignite.internal.util.CompletableFutures.nullCompletedFuture;
+import static org.apache.ignite.lang.ErrorGroups.Common.INTERNAL_ERR;
+import static org.apache.ignite.lang.ErrorGroups.Sql.STMT_VALIDATION_ERR;
+
+import java.util.Set;
+import java.util.concurrent.CompletableFuture;
+import javax.annotation.Nullable;
+import
org.apache.ignite.internal.cluster.management.topology.api.LogicalTopologyService;
+import
org.apache.ignite.internal.cluster.management.topology.api.LogicalTopologySnapshot;
+import org.apache.ignite.internal.distributionzones.NodeWithAttributes;
+import org.apache.ignite.sql.SqlException;
+
+/**
+ * Node filter validator that checks presence of nodes matching given node
filter across cluster.
+ */
+public class ClusterWideNodeFilterValidator implements NodeFilterValidator {
+ private final LogicalTopologyService logicalTopologyService;
+
+ public ClusterWideNodeFilterValidator(LogicalTopologyService
logicalTopologyService) {
+ this.logicalTopologyService = logicalTopologyService;
+ }
+
+ @Override
+ public CompletableFuture<Void> validate(@Nullable String nodeFilter) {
+ if (nodeFilter == null) {
+ return nullCompletedFuture();
+ }
+
+ Set<NodeWithAttributes> filtered =
filteredTopologyNodes(logicalTopologyService.localLogicalTopology(),
nodeFilter);
+
+ if (!filtered.isEmpty()) {
+ return nullCompletedFuture();
+ } else {
+ return logicalTopologyService.logicalTopologyOnLeader()
+ .handle((snapshot, e) -> {
+ if (e == null) {
+ if (filteredTopologyNodes(snapshot,
nodeFilter).isEmpty()) {
+ throw new SqlException(
+ STMT_VALIDATION_ERR,
+ format("Node filter does not match any
node in the cluster [filter='{}'].", nodeFilter)
+ );
+ } else {
+ return null;
+ }
+ } else {
+ throw new SqlException(
+ INTERNAL_ERR,
+ "Distributed validation of the node filter
for distribution zone failed [filter='{}'].",
+ e
+ );
+ }
+ });
+ }
+ }
+
+ private static Set<NodeWithAttributes>
filteredTopologyNodes(LogicalTopologySnapshot snapshot, String nodeFilter) {
+ Set<NodeWithAttributes> nodes = snapshot.nodes().stream()
Review Comment:
Validation looks like a hot path so streams isn't a good idea ([AI3 Code
Guide App. B
p.1](https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=199534707#JavaCodeStyleGuide-1UsingStreamAPI)).
But I see that the converter uses streams generously, so the change is up to
you too.
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/prepare/ddl/ClusterWideNodeFilterValidator.java:
##########
@@ -0,0 +1,87 @@
+/*
+ * 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.ignite.internal.sql.engine.prepare.ddl;
+
+import static java.util.stream.Collectors.toSet;
+import static
org.apache.ignite.internal.distributionzones.DistributionZonesUtil.filterNodeAttributes;
+import static org.apache.ignite.internal.lang.IgniteStringFormatter.format;
+import static
org.apache.ignite.internal.util.CompletableFutures.nullCompletedFuture;
+import static org.apache.ignite.lang.ErrorGroups.Common.INTERNAL_ERR;
+import static org.apache.ignite.lang.ErrorGroups.Sql.STMT_VALIDATION_ERR;
+
+import java.util.Set;
+import java.util.concurrent.CompletableFuture;
+import javax.annotation.Nullable;
+import
org.apache.ignite.internal.cluster.management.topology.api.LogicalTopologyService;
+import
org.apache.ignite.internal.cluster.management.topology.api.LogicalTopologySnapshot;
+import org.apache.ignite.internal.distributionzones.NodeWithAttributes;
+import org.apache.ignite.sql.SqlException;
+
+/**
+ * Node filter validator that checks presence of nodes matching given node
filter across cluster.
+ */
+public class ClusterWideNodeFilterValidator implements NodeFilterValidator {
+ private final LogicalTopologyService logicalTopologyService;
+
+ public ClusterWideNodeFilterValidator(LogicalTopologyService
logicalTopologyService) {
+ this.logicalTopologyService = logicalTopologyService;
+ }
+
+ @Override
+ public CompletableFuture<Void> validate(@Nullable String nodeFilter) {
+ if (nodeFilter == null) {
+ return nullCompletedFuture();
+ }
+
+ Set<NodeWithAttributes> filtered =
filteredTopologyNodes(logicalTopologyService.localLogicalTopology(),
nodeFilter);
+
+ if (!filtered.isEmpty()) {
+ return nullCompletedFuture();
+ } else {
Review Comment:
Mostly there and below I would propose do not use trivial `else` branch, but
just follow code after, it will allow do not use additional 4space indentation.
But it's up to you.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscr...@ignite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
