Copilot commented on code in PR #17419:
URL: https://github.com/apache/pinot/pull/17419#discussion_r2643480537
##########
pinot-broker/src/main/java/org/apache/pinot/broker/requesthandler/MultiStageBrokerRequestHandler.java:
##########
@@ -581,10 +587,19 @@ private BrokerResponse
query(QueryEnvironment.CompiledQuery query, long requestI
return new BrokerResponseNative(QueryErrorCode.EXECUTION_TIMEOUT);
}
+ int stageCount = dispatchableSubPlan.getQueryStageMap().size();
+ int opChainCount = dispatchableSubPlan.getQueryStageMap().values().stream()
Review Comment:
The dispatchableSubPlan.getQueryStageMap() is called twice. Consider storing
the result in a variable to avoid redundant calls.
```suggestion
Map<Integer, DispatchablePlanFragment> queryStageMap =
dispatchableSubPlan.getQueryStageMap();
int stageCount = queryStageMap.size();
int opChainCount = queryStageMap.values().stream()
```
##########
pinot-common/src/main/java/org/apache/pinot/common/datatable/StatMap.java:
##########
@@ -166,6 +169,25 @@ public Object getAny(K key) {
}
}
+ /**
+ * Returns the value associated with the key name.
+ *
+ * In general, it is better to use the type-specific getters with the enum
key directly, but sometimes it is
+ * impossible or requires complex to read code (like complex unsafe casts).
+ *
+ * @param keyName The name of the key.
+ * @param defaultValue The default value to return if the key is not found.
+ * @throws ClassCastException if the value cannot be cast to the same static
type as the default value.
+ */
+ public <E> E getUnsafe(String keyName, E defaultValue)
+ throws ClassCastException {
+ K key = getKey(keyName);
+ if (key == null) {
+ return defaultValue;
+ }
+ return (E) getAny(key);
Review Comment:
The getUnsafe() method should document the behavior when the key exists but
has a null value, as getAny() can return null. Currently, it's unclear whether
this would return null or defaultValue.
##########
pinot-common/src/main/java/org/apache/pinot/common/datatable/StatMap.java:
##########
@@ -166,6 +169,25 @@ public Object getAny(K key) {
}
}
+ /**
+ * Returns the value associated with the key name.
+ *
+ * In general, it is better to use the type-specific getters with the enum
key directly, but sometimes it is
+ * impossible or requires complex to read code (like complex unsafe casts).
Review Comment:
The phrase 'requires complex to read code' should be 'requires
complex-to-read code' or 'requires code that is complex to read'.
```suggestion
* impossible or requires complex-to-read code (like complex unsafe casts).
```
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
