Let Lucene remove floats and lone-surrogate strings from LIST and SET keys by porunov · Pull Request #5110 · JanusGraph/janusgraph

porunov · 2026-10-16T01:26:48Z

On a Lucene mixed index, most floats removed from a key of LIST or SET cardinality stayed in the element's document, so eq and the ranges kept finding the element, also once it held no value of the key: after g.V(v).properties("weights").drop(), g.V().has("weights", 0.1f) still returned v. The same happened to the float which such a key of an edge held before it was set to another one, and to strings with an unpaired surrogate, whatever their mapping. Lucene stores a float as the double which it widens to, and a string in UTF-8, with U+FFFD in place of an unpaired surrogate, but it compared a removed value with the stored ones by the float's own decimal string, 0.1 against 0.10000000149011612, and by the string with its surrogate. This PR compares a value in the form in which Lucene stores it and reads it back. What Lucene writes doesn't change. A string longer than 16,000 UTF-16 units with an unpaired surrogate is removed only with #5098, as master's storage gives it back with a question mark in place of the surrogate.

Changes

LuceneIndex.convertToStringValue, whose string removeFieldIfNeeded compares with the stringValue() of each stored field of the key, for a removal and for the addition of a value which a SET key may hold already:
- A number is given as buildStoreField stores it: a whole number as a long, and a decimal, Float or Double, as a double. So a float's string is that of the double which it widens to.
- A string, lower-cased or not as its field stores it, is given as it comes back from UTF-8, new BytesRef(converted).utf8ToString(), which is how Lucene's stored fields encode it: with U+FFFD in place of each unpaired surrogate. Every other string stays as it is.
- Long strings: a removal compares the value which the transaction reads from storage. On master, StringSerializer compresses a string longer than 16,000 UTF-16 units from getBytes(UTF_8), which writes a question mark in place of an unpaired surrogate, so such a value reads back with question marks and still doesn't match the stored U+FFFD. Keep the unpaired surrogates of strings longer than 16,000 characters #5098 keeps the surrogates in storage, and with it such a string is removed too.
Unchanged: what Lucene writes, its queries, and the other index backends, which remove these values already.
Docs: a changelog entry.

Tests

JanusGraphIndexTest.testRemovedFloatsOfAKeyOfSeveralValuesAreNotFound (new), for LIST and SET on every index backend, in the indexes mixed (vertices) and edges:
- Two vertices hold 0.1f, -0.3f, 3.14f, Float.MIN_VALUE, Float.MAX_VALUE and 1.5f, and one of them -2.5f too; all but -2.5f are removed. An edge's 0.1f is set to 3.14f, and another edge's 0.1f is removed while the edge keeps its document by a value of kind.
- With the index forced, eq of a removed float finds no vertex, and its count() is 0. -2.5f finds its vertex, and a range over every finite float, gte(-Float.MAX_VALUE), finds and counts that vertex alone. 0.1f finds and counts no edge; 3.14f and the range find the first edge, and kind the second.
JanusGraphIndexTest.testRemovedStringsWithUnpairedSurrogatesOfAKeyOfSeveralValuesAreNotFound (new), for LIST and SET on every index backend: keys mapped as DEFAULT, TEXT, STRING and TEXTSTRING, as far as the backend supports them, in the index mixed. Two vertices hold "Ab\uD800c Removed", and one of them "Kept" too. Before the removal, textContains("removed") on the keys of words and textPrefix("Ab") on the keys of whole strings find and count both vertices; after it, neither, while "kept" and "Kept" find the other vertex. The whole strings are checked by a prefix as eq of a string with an unpaired surrogate finds nothing on Elasticsearch and Solr.
LuceneIndexValueRemovalTest (new), on LuceneIndex itself, for LIST and SET, reading the stored values of the document back from the store's directory:
- testRemovedFloatsLeaveTheDocument: twelve floats, among them Float.MIN_NORMAL, the zeros, NaN and the infinities, are removed from two documents, of which one keeps 2.5f. That one holds 2.5 alone and the other nothing, and eq of a removed float finds and counts no document.
- testAFloatAddedAgainIsHeldOnceByASetAndTwiceByAList: 0.1f added twice is held once by a SET and twice by a LIST, and a removal leaves none and one.
- testRemovedStringsWithUnpairedSurrogatesLeaveTheDocument, for each mapping: "Ab\uD800c", "\uDC00" and "x\uD83D\uDE00y", whose surrogates are a pair, are added beside "Kept". Before the removal, eq of each of them, on the keys of whole strings, and textContains("ab"), on the keys of words, find the document; after it, they find nothing, and "Kept" alone stays, in the field of the words and in the field of the whole strings of a TEXTSTRING key.
- testRemovedValuesOfEachDataTypeLeaveTheDocument: a key of each data type, Byte, Short, Integer, Long, Float, Double, Date, Instant, Boolean, UUID and strings of each mapping, holds a value which it keeps, then one which is removed, and keeps the first as Lucene reads it back. Pairs such as -0.0 and 0.0, and Long.MAX_VALUE and Long.MIN_VALUE, check that the comparison takes neither value for the other.

Without the fix, with master's LuceneIndex, 18 of the 20 runs of these tests fail on Lucene (the other two are those of testRemovedValueOfADualMappedKeyOfSeveralValuesIsNotFound, which -Dtest=...#testRemoved* picks too):

Test	Failure
`testRemovedFloatsOfAKeyOfSeveralValuesAreNotFound`, `LIST` and `SET`	`0.1 ==> expected: <false> but was: <true>`: the removed `0.1f` finds the vertices, and both edges
`testRemovedStringsWithUnpairedSurrogatesOfAKeyOfSeveralValuesAreNotFound`, `LIST` and `SET`	`defaultString ==> expected: <[]>`, but `textContains("removed")` finds both vertices
`testRemovedFloatsLeaveTheDocument`, `LIST` and `SET`	the first document holds `0.10000000149011612`, `-0.30000001192092896`, `3.140000104904175`, `1.401298464324817E-45`, `1.1754943508222875E-38` and `3.4028234663852886E38` besides `2.5`
`testAFloatAddedAgainIsHeldOnceByASetAndTwiceByAList`	`SET`: `0.10000000149011612` twice after the second addition; `LIST`: still twice after the removal
`testRemovedStringsWithUnpairedSurrogatesLeaveTheDocument`, all 8	the strings are found before the removal, and afterwards the document holds `ab\uFFFDc` and `\uFFFD` besides `kept`
`testRemovedValuesOfEachDataTypeLeaveTheDocument`, `LIST` and `SET`	the `float` key holds `0.10000000149011612` besides `-0.0`; the keys before it pass

The two halves fail their own tests alone, and the tests tell other ways of comparing apart:

Without the widening, the 8 runs of the tests of floats fail, and those of strings pass; without the round trip through UTF-8, the 10 runs of the tests of strings fail, and those of floats pass.
Widening every number to a double, whole numbers too, fails testRemovedValuesOfEachDataTypeLeaveTheDocument: byte keeps -128.
Comparing numbers by their values as doubles fails testRemovedFloatsLeaveTheDocument, which keeps NaN, and testRemovedValuesOfEachDataTypeLeaveTheDocument, whose double key loses -0.0 to the removal of 0.0.

Verification

Locally, Java 11, on master dd4f0f845:

With checkstyle: janusgraph-driver (130), janusgraph-core (228), janusgraph-inmemory (652, 85 skipped), janusgraph-berkeleyje (350, 9 skipped) and janusgraph-lucene (411, 148 skipped): BerkeleyLuceneTest (108), LuceneIndexTest (263, 148 skipped), LuceneIndexValueRemovalTest (14), MixedIndexUpdateApplierTest (14), CdcSkipMutationTest (9), LuceneIndexKeyInformationTest (2) and BerkeleyLuceneIndexMetricTest (1).
Elasticsearch 9.5.4, in a container: BerkeleyElasticsearchTest's runs of the two new tests (4), with testRemovedValueOfADualMappedKeyOfSeveralValuesIsNotFound (2).
Solr 9.10.1, in the JVM (MiniSolrCloudCluster): the same 6 runs.
With Fix text predicates and SET keys on Lucene and Solr #5077 and Fix has(key) on Lucene for LIST and SET keys of numbers, dates and booleans #5104: the three merge without conflicts. On the merge, BerkeleyLuceneTest (114), LuceneIndexTest (270, 148 skipped), LuceneIndexExistsQueryTest (6) and LuceneIndexValueRemovalTest (14) pass, also with has(key) checks added to the two new tests: has("weights") finds and counts the vertex which keeps -2.5f alone, and the edge set to 3.14f, and has(key) of each string key the vertex which keeps "Kept". With master's LuceneIndex on the merge, has("weights") finds both vertices and both edges.
With Keep the unpaired surrogates of strings longer than 16,000 characters #5098: a string of 16,103 UTF-16 units with an unpaired surrogate, on a LIST key mapped as TEXT and on one mapped as STRING, is found by textContains and textPrefix before its removal and by neither after it. Without Keep the unpaired surrogates of strings longer than 16,000 characters #5098 it reads back from storage with a question mark in place of the surrogate, and is still found after the removal.

Notes

Values left by earlier versions stay in the index: the removed values, and a second copy of a value which a SET key held when the value was set again, of which a removal now takes out one. A reindex writes the document of each element which holds a value of the index anew, from storage, without them. IndexRepairJob writes nothing for an element which holds no value of the index, so its document keeps them: on master, after 0.1f was removed from three vertices, of which one kept -2.5f, one kept a value of another key of the index and one kept nothing, REINDEX left 0.1f in the third one's document alone. The changelog says so, and how to clear those too: delete the directories of the mixed indexes concerned while no instance runs, and reindex them, during which their queries miss what isn't written yet.
Not affected: restore, which writes a document whole without comparing values, and so REINDEX, the transaction recovery and the CDC applier (MixedIndexUpdateApplier), which write through it. Keys of SINGLE cardinality, whose value is replaced without a comparison. Geoshapes, which Lucene supports only on keys of SINGLE cardinality.
Values stored alike, a trade-off: Lucene stores some distinct values alike: two instants of one millisecond, and two strings which differ only in an unpaired surrogate, or in one and U+FFFD. Where a SET key holds two such values, adding the second replaces the entry of the first, so that once either is removed the other isn't found, though the element still holds it: "Ab\uD800c", then "Ab\uDC00c", then removing the first, leaves the vertex unfound by textPrefix("Ab") and textContainsPrefix("ab"), which master found.
- That was so for instants before, and for such strings where the second one held U+FFFD. It is so now also where the second one holds the surrogate, where master kept two entries. Removing such strings at all, which master never did, is worth that.
- With Fix text predicates and SET keys on Lucene and Solr #5077, which replaces the entries of a lower-cased text value only on a re-addition, keys mapped as DEFAULT and TEXT keep the two apart; keys mapped as STRING, and the field of the whole strings of a TEXTSTRING key, still merge them. The changelog describes 1.2.0 with Fix text predicates and SET keys on Lucene and Solr #5077.
- A follow-up once Fix text predicates and SET keys on Lucene and Solr #5077 is in: treat a value whose stored form loses information like a lower-cased text value, and replace its entries only on a re-addition, so that no mapping merges two such values. That is a string whose new BytesRef(s).utf8ToString() differs from it, and maybe an instant with a part of a millisecond.
Long strings: the changelog says that strings longer than 16,000 characters are removed as well from 1.2.0 on, which assumes Keep the unpaired surrogates of strings longer than 16,000 characters #5098 is in 1.2.0. One which an earlier version stored keeps its question marks in storage, and stays in the index when it's removed.
has(key) of a LIST or SET key of numbers fails on Lucene until Fix has(key) on Lucene for LIST and SET keys of numbers, dates and booleans #5104, so the test of floats checks for a vertex without values by a range over every finite float rather than by has(key).
convertToStringValue still mirrors buildStoreField by hand, which is how the two drifted apart. Deriving it from buildStoreField(...).stringValue() gives the same strings, but rewrites the lines which Fix text predicates and SET keys on Lucene and Solr #5077 changes and conflicts with it, so it's left for after Fix text predicates and SET keys on Lucene and Solr #5077.
Cost: a BytesRef and a string more for each string compared, for keys of several values only: once for each removal of a string, and for each string added to a SET key. Numbers take the same work as before.
Elasticsearch and Solr: eq of a string with an unpaired surrogate on a key mapped as STRING found nothing on Elasticsearch 9.5.4 and Solr 9.10.1, also before the removal. Left out.

🤖 Generated with Claude Code

Copilot

Warning

Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.

Copilot review overview

4 open findings

The early return causes the test to be reported as passing (not skipped) when the backend doesn’t… · New
This comment is internally inconsistent with the data: 1.5f is included in removed, although… · New
Using an Object[][] for heterogeneous test parameters is brittle (index-based access with… · New
@ValueSource(strings = ...) relies on implicit String→enum conversion for the Cardinality… · New

What changed in this PR

Fixes Lucene mixed-index value removals for LIST/SET keys by aligning removal-time comparisons with Lucene’s stored representations (notably float widening to double and UTF-8 replacement for lone surrogates).

Changes:

Update LuceneIndex.convertToStringValue to compare values in the stored/read-back form (double-widened numbers, UTF-8-roundtripped strings).
Add Lucene-level and cross-backend tests covering removed floats and strings with unpaired surrogates for LIST/SET cardinalities.
Add a changelog entry describing the behavior change and operational notes about previously-indexed residual values.

File	Description
janusgraph-lucene/src/main/java/org/janusgraph/diskstorage/lucene/LuceneIndex.java	Adjusts value-to-string conversion to match Lucene stored forms for accurate removals/deduplication.
janusgraph-lucene/src/test/java/org/janusgraph/diskstorage/lucene/LuceneIndexValueRemovalTest.java	Adds focused LuceneIndex tests validating stored-field contents and query behavior after removals.
janusgraph-backend-testutils/src/main/java/org/janusgraph/graphdb/JanusGraphIndexTest.java	Adds integration tests ensuring removals are not queryable across backends for `LIST`/`SET`.
docs/changelog.md	Documents the behavior change and trade-offs for Lucene mixed indexes.

🧠 Review effort: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Copilot

Warning

Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.

Copilot review overview

4 open findings

The comment says floats are stored as the widened double, but the implementation uses… · New
The section heading only mentions floats, but the entry also describes (and this PR implements)… · New
Fix grammar in the comment: plural subject should use “have” instead of “has”. · New
Using Object[][] for strongly-typed test metadata forces pervasive casting and makes it easy to… · New

4 resolved since last review

🧠 Review effort: Lite

… keys Fixes JanusGraph#5109 LuceneIndex removes a value of a key of several values from a document by comparing the string which convertToStringValue gives for the value with the stored values of the field, as Lucene reads them back. For two kinds of values that string wasn't the stored one: - A float is stored as the double which it widens to, whose decimal string differs from the float's for most floats: "0.1" against "0.10000000149011612" for 0.1f. The zeros, NaN, the infinities and floats such as 1.5f have the same string either way. - A string is stored in UTF-8, which has no encoding for an unpaired surrogate, so Lucene writes U+FFFD in its place and reads that back. Such a value stayed in the document when it was removed, and eq, the ranges and the text predicates kept finding the element by it, also once the element held no value of the key. So did the value which an edge held before it was set to another one. And a value given again to a SET key whose document held it was held twice, as the addition didn't find the entry which it replaces. convertToStringValue now gives a number as buildStoreField stores it, a whole number as a long and a decimal as a double, and a string as it reads back from UTF-8. What Lucene writes doesn't change. A string longer than 16,000 UTF-16 units is removed only where storage gives it back with its unpaired surrogates, which JanusGraph#5098 makes it do: master reads it back with question marks in their place. Values which Lucene stores alike, such as strings which differ only in an unpaired surrogate, or in one and U+FFFD, now always share one entry where a SET key holds both, as two instants of one millisecond did: the addition of the second replaces the entry of the first, as it compares equal now, so removing either leaves the other unfound. Values which an earlier version left in a document stay there until the document is written whole again: a reindex writes the document of each element which holds a value of the index, and leaves the others. Signed-off-by: Oleksandr Porunov <alexandr.porunov@gmail.com> Co-authored-by: Oleksandr Porunov <alexandr.porunov@gmail.com> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>

Copilot

Warning

Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.

Copilot review overview

2 open findings

This logic contradicts the stated intent ('Float or Double as a double'):… · New
The comment is inconsistent with the test data: removed includes 1.5f even though the comment… · New

4 resolved since last review

🧠 Review effort: Lite

Copilot

Warning

Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.

Copilot review overview

3 open issues

The test reads directly from a fixed on-disk location based on… · New
This performs an UTF-8 round-trip allocation for every string compared/added-to-SET, even when the… · New
JUnit parameterization for enums is clearer and less converter-dependent with… · New

2 resolved since last review

_{Review effort: Lite}

+    private static List<String> storedValues(String document, String field) throws IOException {
+        try (FSDirectory directory = FSDirectory.open(new File(StorageSetup.getHomeDir("lucene"), STORE).toPath());
+             DirectoryReader reader = DirectoryReader.open(directory)) {
+            final IndexSearcher searcher = new IndexSearcher(reader);
+            final TopDocs hits = searcher.search(new TermQuery(new Term(LuceneIndex.DOCID, document)), 2);
+            assertEquals(1, hits.scoreDocs.length, document);
+            return Arrays.asList(searcher.storedFields().document(hits.scoreDocs[0].doc).getValues(field));
+        }
+    }


        }
-        return converted;
+        //Lucene stores a string in UTF-8, with U+FFFD in place of each unpaired surrogate
+        return AttributeUtils.isString(value) ? new BytesRef(converted).utf8ToString() : converted;


+    @ParameterizedTest
+    @ValueSource(strings = {"LIST", "SET"})
+    public void testRemovedFloatsLeaveTheDocument(Cardinality cardinality) throws Exception {


porunov requested a balanced review from Copilot October 16, 2026 01:26

Copilot AI reviewed Oct 16, 2026

View reviewed changes

porunov mentioned this pull request Oct 16, 2026

Work around Lucene miswriting long strings with unpaired surrogates #5112

Open

Copilot stopped reviewing on behalf of porunov due to an error October 16, 2026 03:41
Workflow completed but conclusion is not available. This may indicate rate limiting or API issues.

porunov requested a balanced review from Copilot October 16, 2026 11:20

Copilot AI reviewed Oct 16, 2026

View reviewed changes

porunov force-pushed the lucene-removes-float-values-of-several-values branch from d6d7b15 to 5222731 Compare October 16, 2026 13:25

porunov requested a balanced review from Copilot October 16, 2026 13:27

Copilot AI reviewed Oct 16, 2026

View reviewed changes

Comment thread janusgraph-lucene/src/main/java/org/janusgraph/diskstorage/lucene/LuceneIndex.java

Comment thread janusgraph-backend-testutils/src/main/java/org/janusgraph/graphdb/JanusGraphIndexTest.java

Copilot stopped reviewing on behalf of porunov due to an error October 16, 2026 19:27
The job was not acquired by a runner even after multiple attempts.

porunov requested a balanced review from Copilot October 17, 2026 15:20

Copilot AI reviewed Oct 17, 2026

View reviewed changes

Copilot stopped reviewing on behalf of porunov due to an error October 17, 2026 19:10
The job was not acquired by a runner even after multiple attempts.

Navigation Menu

Repository navigation

Let Lucene remove floats and lone-surrogate strings from LIST and SET keys - #5110

Let Lucene remove floats and lone-surrogate strings from LIST and SET keys#5110
porunov wants to merge 1 commit into
JanusGraph:masterfrom
porunov:lucene-removes-float-values-of-several-values

porunov commented Oct 16, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

porunov commented Oct 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changes

Tests

Verification

Notes

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

porunov commented Oct 16, 2026 •

edited

Loading