Jackie-Jiang commented on code in PR #10371:
URL: https://github.com/apache/pinot/pull/10371#discussion_r1125183038
##########
pinot-core/src/main/java/org/apache/pinot/core/operator/filter/FilterOperatorUtils.java:
##########
@@ -28,9 +28,207 @@
public class FilterOperatorUtils {
+
+ private static volatile Implementation _instance = new Implementation();
+
private FilterOperatorUtils() {
}
+ public static void setImplementation(Implementation newImplementation) {
+ _instance = newImplementation;
+ }
+
+ public static class Implementation {
Review Comment:
To minimize the change, we can still keep the `FilterOperatorUtils` and all
the static util function, but also make an interface (basically add an
interface for the methods, and make this class the default implementation for
the interface). I'd imagine you want to plug in some other implementation via
the `setImplementation()`, instead of extending this class, you may implement
the interface
##########
pinot-core/src/main/java/org/apache/pinot/core/operator/filter/FilterOperatorUtils.java:
##########
@@ -28,9 +28,207 @@
public class FilterOperatorUtils {
+
+ private static volatile Implementation _instance = new Implementation();
Review Comment:
`volatile` might introduce extra overhead since the util will be accessed by
multiple threads. We will always plug-in the implementation at service start
time, so it should be okay to remove the `volatile`
--
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-unsubscr...@pinot.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
