dweiss commented on a change in pull request #1732:
URL: https://github.com/apache/lucene-solr/pull/1732#discussion_r468106643
##########
File path:
lucene/core/src/java/org/apache/lucene/codecs/blocktree/BlockTreeTermsWriter.java
##########
@@ -709,7 +709,7 @@ private PendingBlock writeBlock(int prefixLength, boolean
isFloor, int floorLead
PendingTerm term = (PendingTerm) ent;
- assert StringHelper.startsWith(term.termBytes, prefix): "term.term="
+ term.termBytes + " prefix=" + prefix;
+ assert StringHelper.startsWith(term.termBytes, prefix): "term.term="
+ new String(term.termBytes) + " prefix=" + prefix;
Review comment:
This is wrong, uses default locale.
##########
File path: lucene/core/src/java/org/apache/lucene/analysis/Analyzer.java
##########
@@ -94,7 +94,7 @@
* Create a new Analyzer, reusing the same set of components per-thread
* across calls to {@link #tokenStream(String, Reader)}.
*/
- public Analyzer() {
Review comment:
Can you not change those scopes in public API classes? This applies here
and in other places -- protected changed to package-scope for source is not
really an API-compatible change.
##########
File path:
lucene/core/src/java/org/apache/lucene/index/DocumentsWriterFlushControl.java
##########
@@ -324,12 +324,12 @@ synchronized void doOnAbort(DocumentsWriterPerThread
perThread) {
}
}
- private void checkoutAndBlock(DocumentsWriterPerThread perThread) {
+ private synchronized void checkoutAndBlock(DocumentsWriterPerThread
perThread) {
Review comment:
These are serious changes... you're adding synchronization on core
classes. I don't think they should be piggybacked on top of trivial ones - I'm
sure @s1monw would chip in whether this synchronization here makes sense but
he'll probably overlook if it's a bulk of trivial changes on top.
##########
File path: lucene/core/src/java/org/apache/lucene/index/DocValuesUpdate.java
##########
@@ -152,12 +152,12 @@ static BytesRef readFrom(DataInput in, BytesRef scratch)
throws IOException {
}
NumericDocValuesUpdate(Term term, String field, Long value) {
- this(term, field, value != null ? value.longValue() : -1,
BufferedUpdates.MAX_INT, value != null);
+ this(term, field, value != null ? value : -1, BufferedUpdates.MAX_INT,
value != null);
}
- private NumericDocValuesUpdate(Term term, String field, long value, int
docIDUpTo, boolean hasValue) {
Review comment:
previous version was correct camel case (upTo).
----------------------------------------------------------------
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.
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
