hidingmyname opened a new issue, #16922:
URL: https://github.com/apache/lucene/issues/16922
### Summary
The javadoc for
`UpgradeIndexMergePolicy.shouldUpgradeSegment(SegmentCommitInfo si)`
says the default implementation returns
`!Version.LATEST.equals(si.getVersion())`. `SegmentCommitInfo` has no
`getVersion()` method. The version is on the wrapped `SegmentInfo`, and the
method body just below it calls `si.info.getVersion()`.
### Where
`lucene/core/src/java/org/apache/lucene/index/UpgradeIndexMergePolicy.java:63-70`:
```java
/**
* Returns if the given segment should be upgraded. The default
implementation will return {@code
* !Version.LATEST.equals(si.getVersion())}, so all segments created with
a different version
* number than this Lucene version will get upgraded.
*/
protected boolean shouldUpgradeSegment(SegmentCommitInfo si) {
return !Version.LATEST.equals(si.info.getVersion());
}
```
`getVersion()` is defined on `SegmentInfo` (`SegmentInfo.java:289`), and
`SegmentCommitInfo` exposes that object as a field
(`SegmentCommitInfo.java:38`):
```java
public final SegmentInfo info;
```
### Why it matters
`shouldUpgradeSegment` is `protected` and exists to be overridden. The
javadoc
is basically telling subclass authors "here's the default, change it to fit
your needs". So the snippet is likely to be copied as the starting point for
an override, and as written it won't compile against the `si` parameter.
It's a small fix, but the code example is the main content of this javadoc.
### Suggested fix
```diff
/**
* Returns if the given segment should be upgraded. The default
implementation will return {@code
- * !Version.LATEST.equals(si.getVersion())}, so all segments created with
a different version
+ * !Version.LATEST.equals(si.info.getVersion())}, so all segments created
with a different version
* number than this Lucene version will get upgraded.
*/
```
(The formatter may want to re-wrap that line.)
### Upstream reference
Lucene.NET ported the same javadoc (`si.Version`) and corrected it to
`si.Info.Version` in apache/lucenenet@`c300581a4` ("SWEEP: Added
StringComparison.Ordinal to all string.Equals() calls, ..."). It wasn't a doc
commit as such: the comment was fixed while that sweep changed the
`Equals` call it describes.
```diff
--- a/src/Lucene.Net/Index/UpgradeIndexMergePolicy.cs
+++ b/src/Lucene.Net/Index/UpgradeIndexMergePolicy.cs
@@ -65,7 +65,7 @@ public UpgradeIndexMergePolicy(MergePolicy @base)
/// <summary>
/// Returns <c>true</c> if the given segment should be upgraded.
The default implementation
- /// will return
<c>!Constants.LUCENE_MAIN_VERSION.Equals(si.Version)</c>,
+ /// will return
<c>!Constants.LUCENE_MAIN_VERSION.Equals(si.Info.Version,
StringComparison.Ordinal)</c>,
/// so all segments created with a different version number than
this Lucene version will
/// get upgraded.
/// </summary>
```
Current Lucene.NET:
```csharp
/// will return
<c>!Constants.LUCENE_MAIN_VERSION.Equals(si.Info.Version,
StringComparison.Ordinal)</c>,
/// so all segments created with a different version number than
this Lucene version will
/// get upgraded.
/// </summary>
protected virtual bool ShouldUpgradeSegment(SegmentCommitInfo si)
{
return !Constants.LUCENE_MAIN_VERSION.Equals(si.Info.Version,
StringComparison.Ordinal);
}
```
The relevant part for Java is just `si.` → `si.info.`. The rest is specific
to
the 4.8 string-based version check and C# string comparison.
I can submit a PR for this.
--
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]
