Skip to content

Conversation

@sandeshkr419
Copy link
Member

@sandeshkr419 sandeshkr419 commented Sep 20, 2025

Description

  • In Lucene.java Resolves: // TODO: improve serialization of BigInteger by changing serialization/de-serialization in this manner:
    • Serialize: case BigInteger i -> out.writeString(field.toString()); -> case BigInteger i -> out.writeByteArray(i.toByteArray());
    • De-serialize: case 10 -> new BigInteger(in.readString()); -> case 10 -> new BigInteger(in.readByteArray());
  • Minor refactoring with if-else to switch cases
  • Minor refactoring around instance pattern initialization

Related Issues

Resolves #[Issue number to be closed when this PR is merged]

Check List

  • Functionality includes testing.
  • API changes companion pull request created, if applicable.
  • Public documentation issue/PR created, if applicable.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

@sandeshkr419 sandeshkr419 requested a review from a team as a code owner September 20, 2025 17:30
@sandeshkr419 sandeshkr419 changed the title Improve serialization of big integer Improve serialization of big integer in Lucene.java Sep 20, 2025
@sandeshkr419 sandeshkr419 self-assigned this Sep 20, 2025
@sandeshkr419 sandeshkr419 added enhancement Enhancement or improvement to existing feature or request high hanging fruit Libraries Lucene Upgrades and Libraries, Any 3rd party library that Core depends on, ex: nebula; team is respo low hanging fruit and removed lucene high hanging fruit labels Sep 20, 2025
@github-actions
Copy link
Contributor

❌ Gradle check result for cfdf651: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

@github-actions
Copy link
Contributor

❌ Gradle check result for cfdf651: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

@asimmahmood1
Copy link
Contributor

asimmahmood1 commented Sep 30, 2025

Since we're changing the read/write logic, we'll need to handle bwc, e.g. use version check, right?

@github-actions
Copy link
Contributor

❌ Gradle check result for 4891b5f: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

@sandeshkr419
Copy link
Member Author

sandeshkr419 commented Sep 30, 2025

@asimmahmood1 You are correct regarding backward compatibility.

I guess the latest gradle checks verify this with mixed cluster tests failing:

java.lang.AssertionError: Failure at [search/90_search_after:294]: hits.hits.0.sort didn't match expected value:
              hits.hits.0.sort: 
                               0: expected BigInteger [15223372036854775800] but was BigInteger [280926859962361748206850451459362722643789230128]

Wondering if whether should we place boolean check on version or to just not do it all. Let me see if any benchmarking can help here to take the decision.

@kkhatua kkhatua added v3.4.0 Issues and PRs related to version 3.4.0 and removed v3.3.0 labels Oct 2, 2025
@opensearch-trigger-bot
Copy link
Contributor

This PR is stalled because it has been open for 30 days with no activity.

@opensearch-trigger-bot opensearch-trigger-bot bot added the stalled Issues that have stalled label Nov 1, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement Enhancement or improvement to existing feature or request Libraries Lucene Upgrades and Libraries, Any 3rd party library that Core depends on, ex: nebula; team is respo low hanging fruit lucene stalled Issues that have stalled v3.4.0 Issues and PRs related to version 3.4.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants