xiangfu0 commented on code in PR #18759:
URL: https://github.com/apache/pinot/pull/18759#discussion_r3416514489
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/utils/fst/RegexpMatcher.java:
##########
@@ -53,7 +53,17 @@ public RegexpMatcher(String regexQuery, FST<Long> fst) {
/// this class materializing the full result set in memory.
public static void regexMatch(String regexQuery, FST<Long> fst, IntConsumer
dictIdConsumer)
throws IOException {
- new RegexpMatcher(regexQuery, fst).regexMatchOnFST(dictIdConsumer);
+ regexMatch(regexQuery, fst, dictIdConsumer, Integer.MAX_VALUE);
+ }
+
+ /// Same as [#regexMatch(String, FST, IntConsumer)] but stops the walk once
`maxPaths` FST paths have been visited,
+ /// so a broad pattern over a high-cardinality column cannot allocate
without bound. Returns `true` if the walk
+ /// completed (all matches emitted), or `false` if it was stopped at the
limit (the emitted values are partial and
+ /// should be discarded, with the caller falling back to a scan). A
non-positive `maxPaths` disables the cap
+ /// (unbounded walk), as does [Integer#MAX_VALUE].
Review Comment:
Fixed the docs: dropped the `Integer.MAX_VALUE` claim. Only a non-positive
`maxPaths` is documented as disabling the cap (the code normalizes it to run
unbounded); `MAX_VALUE` is just a very large limit, so the docs no longer imply
it is special-cased.
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/utils/fst/RegexpMatcherCaseInsensitive.java:
##########
@@ -55,7 +55,16 @@ public RegexpMatcherCaseInsensitive(String regexQuery,
FST<BytesRef> ifst) {
/// collect them without this class materializing the full result set in
memory.
public static void regexMatch(String regexQuery, FST<BytesRef> ifst,
IntConsumer dictIdConsumer)
throws IOException {
- new RegexpMatcherCaseInsensitive(regexQuery,
ifst).regexMatchOnFST(dictIdConsumer);
+ regexMatch(regexQuery, ifst, dictIdConsumer, Integer.MAX_VALUE);
+ }
+
+ /// Same as [#regexMatch(String, FST, IntConsumer)] but stops the walk once
`maxPaths` FST paths have been visited.
+ /// Returns `true` if the walk completed (all matches emitted), or `false`
if it was stopped at the limit (the
+ /// emitted values are partial and should be discarded, with the caller
falling back to a scan). A non-positive
+ /// `maxPaths` disables the cap (unbounded walk), as does
[Integer#MAX_VALUE].
Review Comment:
Same fix here: removed the `Integer.MAX_VALUE` clause; the docstring now
only states that a non-positive `maxPaths` disables the cap.
--
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]
