hossman commented on PR #15784:
URL: https://github.com/apache/lucene/pull/15784#issuecomment-5517863955
> ... it is indeed unfortunate that the API is the same :(
"unfortunate" seems like a massive understatement.
Fundamentally there are two things that happened in this issue that are
"very, very bad" for end users:
1. A public facing feature ("Traverse all/most nodes in the HNSW graph above
an explicit `traversalSimilarity` and return them if they score above a
`returnSimilarity`") was removed.
* This remove occurred even though the feature / public classes were not
deprecated
* This removal happened in *MINOR* release
I understand that -- in your opinion -- this feature was "replaced" by a
superior feature, that may be subjectively easier to tune (via an adaptive
approach) -- but it does not change the fact that existing functionality was
removed, w/o first being deprecated, in a minor release.
To draw an analogy, this is roughly equivalent to people upgrading and then
discovering that Lucene removed the ability to do a numeric range query from
`low` to `high` values (ie: `10 < x < 20`, because some developers decided that
it made more sense to do query numeric around a `targetValue` using an
(adaptive) relative `delta` (ie: `x =~ 15 +/- 5`)
2. Public `Query` classes with public constructors were modified so that
their constructor args have the same type but radically different meaning
* This results in a _functional_ backcompat break that does not cause any
compilation failures or linkage failures.
* Users who upgrade will just **_silently_** get radically different
results relative to what they expect given the original intent of those
constructor arguments
To continue the analogy above, this is equivalent to changing the method
signature of `LongPoint.newRangeQuery(String field,long min,long max)` to
`LongPoint.newRangeQuery(String field,long targetValue,long delta)` ... so that
instead of `LongPoint.newRangeQuery("foo",10,20)` matching values between 10
and 20 like it did in Lucene 10.4, now it matches values between -10 and +10.
----
> I guess in the case of a named function, one can make a new function with
a new name, and I guess deprecate the old one. In this case the function was a
constructor, so doing something like that would have required a new class,
which seems kind of heavy.
That's *EXACTLY* what I think should have been done -- because then the old
classes could have been deprecated, but still usable and functional until
Lucene 11, and neither of the above problems would exist. (The amount of
[duplication needed](https://issues.apache.org/jira/browse/SOLR-18416) isn't
even very high, and it wouldn't have added any additional maintenance cost to
the index side of things)
As things stand now, you can't even "re-add" the old classes back in to a
10.5.x or a 10.6 release, to "fix the backcompat break" for existing users,
because that would break anyone who has just _started_ using these classes in
10.5 and _expects_ the params to be `..., float resultSimilarity, float decay,
...`
Ugh.
--
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]
