You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by "jackjlli (via GitHub)" <gi...@apache.org> on 2024/04/11 20:17:15 UTC
[PR] Unify the usage of broker empty results [pinot]
jackjlli opened a new pull request, #12907:
URL: https://github.com/apache/pinot/pull/12907
This PR tries to unify the usage of broker empty results in the code, so that empty result only refers to the same object with the same memory address.
This could be useful for validations like `if (response == BrokerResponseNative.EMPTY_RESULT) {...}`.
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
Re: [PR] Unify the usage of broker empty results [pinot]
Posted by "jackjlli (via GitHub)" <gi...@apache.org>.
jackjlli commented on code in PR #12907:
URL: https://github.com/apache/pinot/pull/12907#discussion_r1561751433
##########
pinot-common/src/main/java/org/apache/pinot/common/response/broker/BrokerResponseNative.java:
##########
@@ -127,7 +127,7 @@ private static BrokerResponseNative getBrokerResponseExplainPlanOutput() {
/**
* Get a new empty {@link BrokerResponseNative}.
*/
- public static BrokerResponseNative empty() {
+ private static BrokerResponseNative empty() {
return new BrokerResponseNative();
Review Comment:
I see your point. It seems there are quite a few fields that need to be updated. That doesn't make sense to just return an instance here. Let me think of some other ways to do it. Thanks!
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
Re: [PR] Unify the usage of broker empty results [pinot]
Posted by "Jackie-Jiang (via GitHub)" <gi...@apache.org>.
Jackie-Jiang commented on code in PR #12907:
URL: https://github.com/apache/pinot/pull/12907#discussion_r1561651910
##########
pinot-common/src/main/java/org/apache/pinot/common/response/broker/BrokerResponseNative.java:
##########
@@ -127,7 +127,7 @@ private static BrokerResponseNative getBrokerResponseExplainPlanOutput() {
/**
* Get a new empty {@link BrokerResponseNative}.
*/
- public static BrokerResponseNative empty() {
+ private static BrokerResponseNative empty() {
return new BrokerResponseNative();
Review Comment:
Does it make sense to just return the singleton instance here?
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
Re: [PR] Unify the usage of broker empty results [pinot]
Posted by "jackjlli (via GitHub)" <gi...@apache.org>.
jackjlli closed pull request #12907: Unify the usage of broker empty results
URL: https://github.com/apache/pinot/pull/12907
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org