Vladsz83 commented on code in PR #12599:
URL: https://github.com/apache/ignite/pull/12599#discussion_r2648018780
##########
modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/exec/AbstractCacheScan.java:
##########
@@ -43,36 +41,52 @@ public abstract class AbstractCacheScan<Row> implements
Iterable<Row>, AutoClose
protected final AffinityTopologyVersion topVer;
/** */
- protected final int[] parts;
+ protected final BitSet parts;
/** */
- protected final boolean explicitParts;
+ private final int[] explicitParts;
/** */
private PartitionReservation reservation;
- /** */
- protected volatile List<GridDhtLocalPartition> reservedParts;
-
/** */
AbstractCacheScan(ExecutionContext<Row> ectx, GridCacheContext<?, ?> cctx,
int[] parts) {
this.ectx = ectx;
this.cctx = cctx;
topVer = ectx.topologyVersion();
- explicitParts = parts != null;
+ explicitParts = parts;
- if (cctx.isReplicated())
- this.parts = IntStream.range(0,
cctx.affinity().partitions()).toArray();
+ int partsCnt = cctx.affinity().partitions();
+
+ if (cctx.isReplicated()) {
+ BitSet partsSet = new BitSet(partsCnt);
Review Comment:
Might be `this.parts`. One line fewer. The same below.
##########
modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/exec/AbstractCacheScan.java:
##########
@@ -43,36 +41,52 @@ public abstract class AbstractCacheScan<Row> implements
Iterable<Row>, AutoClose
protected final AffinityTopologyVersion topVer;
/** */
- protected final int[] parts;
+ protected final BitSet parts;
/** */
- protected final boolean explicitParts;
+ private final int[] explicitParts;
Review Comment:
Do we need to keep it? Can we pass it the reservation manager? WDYT? Another
ticket?
##########
modules/core/src/main/java/org/apache/ignite/spi/indexing/IndexingQueryFilterImpl.java:
##########
@@ -54,6 +54,24 @@ public class IndexingQueryFilterImpl implements
IndexingQueryFilter {
*/
private final boolean treatReplicatedAsPartitioned;
+ /**
+ * Constructor.
+ *
+ * @param ctx Kernal context.
+ * @param topVer Topology version.
+ * @param parts Partitions set.
+ */
+ public IndexingQueryFilterImpl(
Review Comment:
We have also ctor.
```
IndexingQueryFilterImpl(GridKernalContext ctx, @Nullable
AffinityTopologyVersion topVer,
@Nullable int[] partsArr, boolean treatReplicatedAsPartitioned)
```
Can one reuse another?
##########
modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/exec/exp/ExpressionFactoryImpl.java:
##########
@@ -105,13 +105,13 @@ public class ExpressionFactoryImpl<Row> implements
ExpressionFactory<Row> {
private final RexBuilder rexBuilder;
/** */
- private final RelDataType emptyType;
+ private static final RelDataType emptyType = new
RelDataTypeFactory.Builder(Commons.typeFactory()).build();
Review Comment:
Usually we use upper case for `static final` constants. Same below
##########
modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/exec/AbstractCacheScan.java:
##########
@@ -43,36 +41,52 @@ public abstract class AbstractCacheScan<Row> implements
Iterable<Row>, AutoClose
protected final AffinityTopologyVersion topVer;
/** */
- protected final int[] parts;
+ protected final BitSet parts;
/** */
- protected final boolean explicitParts;
+ private final int[] explicitParts;
/** */
private PartitionReservation reservation;
- /** */
- protected volatile List<GridDhtLocalPartition> reservedParts;
-
/** */
AbstractCacheScan(ExecutionContext<Row> ectx, GridCacheContext<?, ?> cctx,
int[] parts) {
this.ectx = ectx;
this.cctx = cctx;
topVer = ectx.topologyVersion();
- explicitParts = parts != null;
+ explicitParts = parts;
Review Comment:
Suggestion: let's mark `parts` as `@Nullable` and rename as `explicitParts`
##########
modules/core/src/main/java/org/apache/ignite/spi/indexing/IndexingQueryFilterImpl.java:
##########
@@ -54,6 +54,24 @@ public class IndexingQueryFilterImpl implements
IndexingQueryFilter {
*/
private final boolean treatReplicatedAsPartitioned;
+ /**
+ * Constructor.
+ *
+ * @param ctx Kernal context.
+ * @param topVer Topology version.
+ * @param parts Partitions set.
+ */
+ public IndexingQueryFilterImpl(
Review Comment:
Looks like thre are no more usages for:
```
IndexingQueryFilterImpl(GridKernalContext ctx, @Nullable
AffinityTopologyVersion topVer,
@Nullable int[] partsArr)
##########
modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/exec/TableScan.java:
##########
@@ -38,6 +41,9 @@
/** */
public class TableScan<Row> extends AbstractCacheColumnsScan<Row> {
+ /** */
+ protected volatile List<GridDhtLocalPartition> reservedParts;
Review Comment:
Is `volatile` because we can read it from another Calcite query thread?
--
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]
