risdenk commented on code in PR #847:
URL: https://github.com/apache/lucene/pull/847#discussion_r860101008
##########
lucene/queries/src/java/org/apache/lucene/queries/function/valuesource/BytesRefFieldSource.java:
##########
@@ -45,35 +45,33 @@ public FunctionValues getValues(Map<Object, Object>
context, LeafReaderContext r
// To be sorted or not to be sorted, that is the question
// TODO: do it cleaner?
if (fieldInfo != null && fieldInfo.getDocValuesType() ==
DocValuesType.BINARY) {
- final BinaryDocValues binaryValues =
DocValues.getBinary(readerContext.reader(), field);
+ final BinaryDocValues arr = DocValues.getBinary(readerContext.reader(),
field);
return new FunctionValues() {
int lastDocID = -1;
- private BytesRef getValueForDoc(int doc) throws IOException {
+ @Override
+ public boolean exists(int doc) throws IOException {
if (doc < lastDocID) {
throw new IllegalArgumentException(
"docs were sent out-of-order: lastDocID=" + lastDocID + " vs
docID=" + doc);
}
lastDocID = doc;
- int curDocID = binaryValues.docID();
+ int curDocID = arr.docID();
if (doc > curDocID) {
- curDocID = binaryValues.advance(doc);
- }
- if (doc == curDocID) {
- return binaryValues.binaryValue();
- } else {
- return null;
+ curDocID = arr.advance(doc);
}
- }
-
- @Override
- public boolean exists(int doc) throws IOException {
- return getValueForDoc(doc) != null;
+ return doc == curDocID;
}
Review Comment:
`exists` is the same for all of these *FieldSource implementations now, but
I didn't know a good way to remove the duplication. So I left the duplication
for now.
##########
lucene/queries/src/java/org/apache/lucene/queries/function/valuesource/BytesRefFieldSource.java:
##########
@@ -45,35 +45,33 @@ public FunctionValues getValues(Map<Object, Object>
context, LeafReaderContext r
// To be sorted or not to be sorted, that is the question
// TODO: do it cleaner?
if (fieldInfo != null && fieldInfo.getDocValuesType() ==
DocValuesType.BINARY) {
- final BinaryDocValues binaryValues =
DocValues.getBinary(readerContext.reader(), field);
+ final BinaryDocValues arr = DocValues.getBinary(readerContext.reader(),
field);
return new FunctionValues() {
int lastDocID = -1;
- private BytesRef getValueForDoc(int doc) throws IOException {
+ @Override
+ public boolean exists(int doc) throws IOException {
if (doc < lastDocID) {
throw new IllegalArgumentException(
"docs were sent out-of-order: lastDocID=" + lastDocID + " vs
docID=" + doc);
}
lastDocID = doc;
- int curDocID = binaryValues.docID();
+ int curDocID = arr.docID();
if (doc > curDocID) {
- curDocID = binaryValues.advance(doc);
- }
- if (doc == curDocID) {
- return binaryValues.binaryValue();
- } else {
- return null;
+ curDocID = arr.advance(doc);
}
- }
-
- @Override
- public boolean exists(int doc) throws IOException {
- return getValueForDoc(doc) != null;
+ return doc == curDocID;
}
@Override
public boolean bytesVal(int doc, BytesRefBuilder target) throws
IOException {
- BytesRef value = getValueForDoc(doc);
+ BytesRef value;
+ if (exists(doc)) {
+ value = arr.binaryValue();
+ } else {
+ value = null;
+ }
Review Comment:
A nice side effect of removing getValueForDoc and using exists is the logic
is MUCH cleaner now. If value exists then go convert it - otherwise don't. It
avoids the indirection from before. So I think this is using the API better
than before.
##########
lucene/queries/src/java/org/apache/lucene/queries/function/valuesource/LongFieldSource.java:
##########
@@ -147,10 +122,6 @@ protected NumericDocValues getNumericDocValues(
return DocValues.getNumeric(readerContext.reader(), field);
}
- protected MutableValueLong newMutableValueLong() {
- return new MutableValueLong();
- }
-
Review Comment:
This wasn't used anywhere besides getValueFiller and this was the only
difference in getValueFiller compared to the base class.
##########
lucene/queries/src/java/org/apache/lucene/queries/function/valuesource/BytesRefFieldSource.java:
##########
@@ -45,35 +45,33 @@ public FunctionValues getValues(Map<Object, Object>
context, LeafReaderContext r
// To be sorted or not to be sorted, that is the question
// TODO: do it cleaner?
if (fieldInfo != null && fieldInfo.getDocValuesType() ==
DocValuesType.BINARY) {
- final BinaryDocValues binaryValues =
DocValues.getBinary(readerContext.reader(), field);
+ final BinaryDocValues arr = DocValues.getBinary(readerContext.reader(),
field);
Review Comment:
This is only a cosmetic change to make `BytesRefFieldSource` and
`DoubleFieldSource` match the other *FieldSource implementations using arr.
--
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
