xtern commented on code in PR #2101:
URL: https://github.com/apache/ignite-3/pull/2101#discussion_r1205722535
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/schema/IgniteTableImpl.java:
##########
@@ -617,21 +645,27 @@ private static <RowT> CompletableFuture<List<RowT>>
handleInsertResults(
RowHandler<RowT> handler,
CompletableFuture<List<RowT>>[] futs
) {
+ updateStat.set(true);
+
return CompletableFuture.allOf(futs)
.thenApply(response -> {
- List<String> conflictRows = new ArrayList<>();
+ List<String> conflictRows = null;
for (CompletableFuture<List<RowT>> future : futs) {
List<RowT> values = future.join();
+ if (conflictRows == null && values != null &&
!values.isEmpty()) {
+ conflictRows = new ArrayList<>(values.size());
+ }
+
if (values != null) {
for (RowT row : values) {
conflictRows.add(handler.toString(row));
}
}
}
- if (!conflictRows.isEmpty()) {
+ if (conflictRows != null && !conflictRows.isEmpty()) {
Review Comment:
As I see it, if `conflictRows` is not `null`, it cannot be empty.
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/schema/IgniteTableImpl.java:
##########
@@ -531,16 +536,17 @@ private ColocationGroup partitionedGroup() {
}
private class StatisticsImpl implements Statistic {
- private static final int STATS_CLI_UPDATE_THRESHOLD = 200;
+ private static final int STATS_UPDATE_THRESHOLD =
DistributionZoneManager.DEFAULT_PARTITION_COUNT;
- AtomicInteger statReqCnt = new AtomicInteger();
+ private final AtomicLong lastUpd = new AtomicLong();
- private volatile long localRowCnt;
+ private volatile long localRowCnt = 0L;
/** {@inheritDoc} */
@Override
+ // TODO: need to be refactored
https://issues.apache.org/jira/browse/IGNITE-19558
public Double getRowCount() {
- if (statReqCnt.getAndIncrement() % STATS_CLI_UPDATE_THRESHOLD ==
0) {
+ if (updateStat.get()) {
Review Comment:
Will this work correctly if node is started with persisted data?
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/schema/IgniteTableImpl.java:
##########
@@ -617,21 +645,27 @@ private static <RowT> CompletableFuture<List<RowT>>
handleInsertResults(
RowHandler<RowT> handler,
CompletableFuture<List<RowT>>[] futs
) {
+ updateStat.set(true);
+
return CompletableFuture.allOf(futs)
.thenApply(response -> {
- List<String> conflictRows = new ArrayList<>();
+ List<String> conflictRows = null;
for (CompletableFuture<List<RowT>> future : futs) {
List<RowT> values = future.join();
+ if (conflictRows == null && values != null &&
!values.isEmpty()) {
+ conflictRows = new ArrayList<>(values.size());
+ }
+
if (values != null) {
for (RowT row : values) {
conflictRows.add(handler.toString(row));
}
}
Review Comment:
I suggest not repeating the condition `values != null`.
```suggestion
if (values == null || values.isEmpty()) {
continue;
}
if (conflictRows == null) {
conflictRows = new ArrayList<>(values.size());
}
for (RowT row : values) {
conflictRows.add(handler.toString(row));
}
```
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/exec/rel/IndexScanNode.java:
##########
@@ -173,7 +173,7 @@ private Publisher<RowT>
partitionPublisher(PartitionWithTerm partWithTerm, @Null
}
} else {
assert schemaIndex.type() == Type.HASH;
- assert cond != null && cond.lower() != null : "Invalid hash index
condition.";
+ assert cond != null && (cond.lower() != null || cond.upper() !=
null) : "Invalid hash index condition.";
Review Comment:
Why we need such change?
We don't use upper bound here and seems this will be equal to
```
assert cond != null : "Invalid hash index condition.";
```
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
