Skip to content

assertDocValuesEquals should support sparse sorted doc_values #14839

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

parkertimmins
Copy link

In LuceneTestCase, assertDocValuesEquals compares the doc_values in two indices. For SortedSetDocValues, as well as other doc value types, it does not assume that every document has a value. But for SortedDocValues, it requires that every doc have a value. This restricts what indices assertDocValuesEquals can be used to test. So this PR removes this restriction and does not assume that every doc has a value.

In LuceneTestCase, assertDocValuesEquals compares the doc_values
in two indices. For SortedSetDocValues, as well as other doc value
types, it does not assume that every document has a value. But for
SortedDocValues, it requires that every doc have a value. Remove this
restriction.
Copy link

This PR does not have an entry in lucene/CHANGES.txt. Consider adding one. If the PR doesn't need a changelog entry, then add the skip-changelog label to it and you will stop receiving this reminder on future updates to the PR.

document.removeFields("sparseset");
if (random.nextBoolean()) {
document.add(new SortedDocValuesField("sparseset", new BytesRef(title)));
}
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This function is also used in TestDuelingCodecsAtNight. This comment:

@SuppressCodecs({"Direct"}) // it can be too much for these codecs
made me a bit concerned that adding an additional field will slow down the nightlies. But I ran several tests with the same seed both with and without the above block, and saw no appreciable difference. (In fact, the tests ran slightly faster on average with new field, though this must be noise.)

@jpountz
Copy link
Contributor

jpountz commented Jun 25, 2025

Can you add a CHANGES entry under 10.3 before I merge?

@github-actions github-actions bot added this to the 10.3.0 milestone Jun 25, 2025
@parkertimmins
Copy link
Author

@jpountz Whoops, I missed that it needed a change log entry. Thanks for the heads up. Add an entry to the 10.3 section. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants