Copilot commented on code in PR #2396:
URL: https://github.com/apache/fluss/pull/2396#discussion_r2703849120
##########
fluss-server/src/main/java/org/apache/fluss/server/replica/ReplicaManager.java:
##########
@@ -153,7 +153,6 @@ public class ReplicaManager {
private final Map<TableBucket, HostedReplica> allReplicas =
MapUtils.newConcurrentHashMap();
private final TabletServerMetadataCache metadataCache;
- private final ExecutorService ioExecutor;
private final ProjectionPushdownCache projectionsCache = new
ProjectionPushdownCache();
Review Comment:
The field declaration for `ioExecutor` has been removed, but it is still
being used at line 286 in the constructor (`this.kvSnapshotResource =
KvSnapshotResource.create(serverId, conf, ioExecutor);`). This will cause a
compilation error. The field should not have been removed.
##########
fluss-server/src/main/java/org/apache/fluss/server/utils/ServerRpcMessageUtils.java:
##########
@@ -822,6 +822,13 @@ public static Map<TableBucket, FetchReqInfo>
getFetchLogData(FetchLogRequest req
projectionFields = null;
}
+ final boolean isSelectedByIds;
+ if (fetchLogReqForTable.isProjectByIds()) {
+ isSelectedByIds = fetchLogReqForTable.isProjectByIds();
+ } else {
+ isSelectedByIds = false;
+ }
Review Comment:
The conditional logic here is redundant. The condition `if
(fetchLogReqForTable.isProjectByIds())` always evaluates the same as the
assignment `isSelectedByIds = fetchLogReqForTable.isProjectByIds()`. This can
be simplified to just `boolean isSelectedByIds =
fetchLogReqForTable.isProjectByIds();` without the conditional check, since the
default value for a missing optional boolean field in protobuf is false.
```suggestion
final boolean isSelectedByIds =
fetchLogReqForTable.isProjectByIds();
```
##########
fluss-rpc/src/main/proto/FlussApi.proto:
##########
@@ -702,6 +702,7 @@ message PbFetchLogReqForTable {
required bool projection_pushdown_enabled = 2;
repeated int32 projected_fields = 3 [packed = true];
repeated PbFetchLogReqForBucket buckets_req = 4;
Review Comment:
The new `project_by_ids` field lacks a documentation comment. Consider
adding a comment explaining what this field means, for example: "// if true,
projected_fields are interpreted as field IDs; if false, they are field
positions".
```suggestion
repeated PbFetchLogReqForBucket buckets_req = 4;
// If true, projected_fields are interpreted as field IDs; if false, they
are field positions.
```
--
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]
