msokolov commented on code in PR #12560:
URL: https://github.com/apache/lucene/pull/12560#discussion_r1327797753
##########
lucene/expressions/src/java/org/apache/lucene/expressions/ExpressionFunctionValues.java:
##########
@@ -39,21 +39,21 @@ class ExpressionFunctionValues extends DoubleValues {
}
@Override
- public boolean advanceExact(int doc) {
+ public boolean advanceExact(int doc) throws IOException {
if (currentDoc == doc) {
return true;
}
+ for (DoubleValues v : functionValues) {
+ v.advanceExact(doc);
Review Comment:
I'm still a bit confused - it seems like we're pushing things around, but I
know you have some compelling-looking results so it must be OK? But I think it
would be great if you could distill whatever knowledge you've garnered into a
comment here. The missing link for me is -- what is ExpressionFunctionValues
representing? Is that used for functions only (like `max()`) or generally for
any expression? I guess if it's only for functions then it makes sense that we
are now getting lazy advance for other expressions that might not need all
their subs. But then have we lost some laziness for functions? Does this work
for short-circuited booleans to? Like `(1 == 1) || expensive()`?
##########
lucene/expressions/src/java/org/apache/lucene/expressions/ExpressionValueSource.java:
##########
@@ -90,16 +90,24 @@ public DoubleValues getValues(LeafReaderContext
readerContext, DoubleValues scor
static DoubleValues zeroWhenUnpositioned(DoubleValues in) {
return new DoubleValues() {
- boolean positioned = false;
+ int currentDoc = -1;
+ double value;
+ boolean computed = false;
@Override
public double doubleValue() throws IOException {
- return positioned ? in.doubleValue() : 0;
+ if (computed == false) {
+ value = in.advanceExact(currentDoc) ? in.doubleValue() : 0;
+ computed = true;
+ }
+ return value;
}
@Override
- public boolean advanceExact(int doc) throws IOException {
- return positioned = in.advanceExact(doc);
+ public boolean advanceExact(int doc) {
+ currentDoc = doc;
Review Comment:
is there any sense in checking if we are advancing to the same doc?
--
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: issues-unsubscr...@lucene.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For additional commands, e-mail: issues-h...@lucene.apache.org
