Skip to content

[GRPC] Add BigInteger support for unsigned_long fields in gRPC transport#20346

Merged
karenyrx merged 5 commits intoopensearch-project:mainfrom
karenyrx:unsigned_long
Jan 5, 2026
Merged

[GRPC] Add BigInteger support for unsigned_long fields in gRPC transport#20346
karenyrx merged 5 commits intoopensearch-project:mainfrom
karenyrx:unsigned_long

Conversation

@karenyrx
Copy link
Copy Markdown
Contributor

@karenyrx karenyrx commented Jan 1, 2026

Description

When performing gRPC search requests with sort criteria on unsigned_long index mapping fields would fail with an IllegalArgumentException.:

code: INVALID_ARGUMENT
message: java.lang.IllegalArgumentException: Cannot convert 2147395412 to FieldValue
at org.opensearch.transport.grpc.proto.response.common.FieldValueProtoUtils.toProto(FieldValueProtoUtils.java:78)
at org.opensearch.transport.grpc.proto.response.search.SearchSortValuesProtoUtils.toProto(SearchSortValuesProtoUtils.java:37)

The root cause was:

  • OpenSearch uses BigInteger in Java to represent unsigned_long field values (which can exceed signed long's range)
  • The FieldValueProtoUtils.toProto() method had no handler for BigInteger type
  • Sort values containing BigInteger objects would fall through to the default case and throw an exception

Fix

This PR fixes this issue by adding BigInteger case in the toProto() switch statement, and mapping BigInteger to protobuf's uint64_value.

Test Plan

  1. Create index mapping (with unsigned_long)
curl -X PUT "localhost:9200/vehicles?pretty" -H 'Content-Type: application/json' -d'
{
  "mappings": {
    "properties": {
      "make_id": {
        "type": "unsigned_long"
      },
      "model_id": {
        "type": "unsigned_long"
      },
      "variant_name": {
        "type": "text",
        "fields": {
          "keyword": {
            "type": "keyword"
          }
        }
      },
      "year": {
        "type": "integer"
      },
      "price": {
        "type": "long"
      }
    }
  }
}
'
  1. Index sample docs
curl -X POST "localhost:9200/_bulk?pretty" -H 'Content-Type: application/json' -d'
{ "index": { "_index": "vehicles", "_id": "1" } }
{ "make_id": 2147395412, "model_id": 9223372036854775807, "variant_name": "Default", "year": 2024, "price": 50000 }
{ "index": { "_index": "vehicles", "_id": "2" } }
{ "make_id": 18446744073709551615, "model_id": 18446744073709551000, "variant_name": "Premium", "year": 2024, "price": 75000 }
{ "index": { "_index": "vehicles", "_id": "3" } }
{ "make_id": 12345678901234567890, "model_id": 5000000000, "variant_name": "Default", "year": 2023, "price": 45000 }
{ "index": { "_index": "vehicles", "_id": "4" } }
{ "make_id": 1000000000, "model_id": 2000000000, "variant_name": "Default", "year": 2025, "price": 60000 }
'
  1. Test gRPC search request
grpcurl -plaintext \
  -import-path ~/opensearch/ \
  -proto ~/opensearch/protos/services/search_service.proto \
  -d @ localhost:9400 \
  org.opensearch.protobufs.services.SearchService/Search <<'EOM'
{
  "index": "vehicles",
  "search_request_body": {
    "size": 250,
    "from": 0,
    "sort": [
      {
        "field_with_direction": {
          "sort_order_map": {
            "make_id": "SORT_ORDER_DESC"
          }
        }
      },
      {
        "field_with_direction": {
          "sort_order_map": {
            "model_id": "SORT_ORDER_DESC"
          }
        }
      }
    ],
    "query": {
      "bool": {
        "filter": [],
        "minimum_should_match": {},
        "must": [
          {
            "match": {
              "field": "variant_name",
              "query": {
                "string": "Default"
              }
            }
          }
        ],
        "must_not": [],
        "should": []
      }
    }
  }
}
EOM

Search Response:

  • Before the fix
ERROR:
  Code: InvalidArgument
  Message: java.lang.IllegalArgumentException: Cannot convert 12345678901234567890 to FieldValue
        at org.opensearch.transport.grpc.proto.response.common.FieldValueProtoUtils.toProto(FieldValueProtoUtils.java:79)
        at org.opensearch.transport.grpc.proto.response.common.FieldValueProtoUtils.toProto(FieldValueProtoUtils.java:37)
        at org.opensearch.transport.grpc.proto.response.search.SearchSortValuesProtoUtils.toProto(SearchSortValuesProtoUtils.java:37)
        at org.opensearch.transport.grpc.proto.response.search.SearchHitProtoUtils.toProto(SearchHitProtoUtils.java:88)
  • After fix
{
  "took": "11",
  "xShards": {
    "successful": 1,
    "total": 1,
    "skipped": 0
  },
  "hits": {
    "total": {
      "totalHits": {
        "relation": "TOTAL_HITS_RELATION_EQ",
        "value": "3"
      }
    },
    "hits": [
      {
        "xIndex": "vehicles",
        "xId": "3",
        "xScore": {
          "nullValue": "NULL_VALUE_NULL"
        },
        "xSource": "eyAibWFrZV9pZCI6IDEyMzQ1Njc4OTAxMjM0NTY3ODkwLCAibW9kZWxfaWQiOiA1MDAwMDAwMDAwLCAidmFyaWFudF9uYW1lIjogIkRlZmF1bHQiLCAieWVhciI6IDIwMjMsICJwcmljZSI6IDQ1MDAwIH0=",
        "sort": [
          {
            "generalNumber": {
              "uint64Value": "12345678901234567890"
            }
          },
          {
            "generalNumber": {
              "uint64Value": "5000000000"
            }
          }
        ]
      },
      {
        "xIndex": "vehicles",
        "xId": "1",
        "xScore": {
          "nullValue": "NULL_VALUE_NULL"
        },
        "xSource": "eyAibWFrZV9pZCI6IDIxNDczOTU0MTIsICJtb2RlbF9pZCI6IDkyMjMzNzIwMzY4NTQ3NzU4MDcsICJ2YXJpYW50X25hbWUiOiAiRGVmYXVsdCIsICJ5ZWFyIjogMjAyNCwgInByaWNlIjogNTAwMDAgfQ==",
        "sort": [
          {
            "generalNumber": {
              "uint64Value": "2147395412"
            }
          },
          {
            "generalNumber": {
              "uint64Value": "9223372036854775807"
            }
          }
        ]
      },
      {
        "xIndex": "vehicles",
        "xId": "4",
        "xScore": {
          "nullValue": "NULL_VALUE_NULL"
        },
        "xSource": "eyAibWFrZV9pZCI6IDEwMDAwMDAwMDAsICJtb2RlbF9pZCI6IDIwMDAwMDAwMDAsICJ2YXJpYW50X25hbWUiOiAiRGVmYXVsdCIsICJ5ZWFyIjogMjAyNSwgInByaWNlIjogNjAwMDAgfQ==",
        "sort": [
          {
            "generalNumber": {
              "uint64Value": "1000000000"
            }
          },
          {
            "generalNumber": {
              "uint64Value": "2000000000"
            }
          }
        ]
      }
    ],
    "maxScore": {
      "nullValue": "NULL_VALUE_NULL"
    }
  }
}

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.

Summary by CodeRabbit

  • New Features

    • BigInteger support added for unsigned 64-bit fields in gRPC transport, preserving full unsigned values beyond signed long range.
  • Tests

    • Comprehensive tests covering conversions and round-trips for unsigned 64-bit values, including edge cases, overflow and negative-value validations.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Jan 1, 2026

📝 Walkthrough

Walkthrough

Adds BigInteger support for unsigned 64-bit values in gRPC transport FieldValue utilities, including conversion/validation in serialization/deserialization and accompanying tests covering in-range, out-of-range, and round-trip cases.

Changes

Cohort / File(s) Summary
Changelog
CHANGELOG.md
Added entries documenting BigInteger support for unsigned_long fields under "Unreleased 3.x > Added".
Core implementation
modules/transport-grpc/src/main/java/.../FieldValueProtoUtils.java
Added BigInteger handling in toProto(Object) (validate range, convert to uint64 GeneralNumber) and in fromProto(FieldValue, boolean) (return BigInteger when uint64 exceeds signed Long.MAX_VALUE, otherwise Long); updated imports and javadoc. Review focus: correctness of range checks, uint64 <-> BigInteger conversions, and maintained existing behavior for signed values.
Tests
modules/transport-grpc/src/test/java/.../FieldValueProtoUtilsTests.java
Added tests covering: fromProto for uint64 > Long.MAX_VALUE returning BigInteger; toProto for max unsigned long and in-range BigInteger; round-trip conversions; and error cases for out-of-range or negative BigInteger inputs. Review focus: coverage of edge cases and assertions/messages for IllegalArgumentException.

Sequence Diagram(s)

(omitted — changes are internal utility and tests without multi-component sequential interactions)

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Suggested reviewers

  • msfroh
  • lucy66hw

Poem

🐰 I nibble bits both small and grand,
From signed lands to unsigned sand.
Sixty-four hops, I guard the range,
Converting safe, no value strange. ✨

Pre-merge checks

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 25.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and specifically describes the main change: adding BigInteger support for unsigned_long fields in gRPC transport, which directly addresses the core issue fixed by this PR.
Description check ✅ Passed The description is comprehensive and follows the template structure, including problem statement with error details, root cause analysis, fix explanation, detailed test plan with commands and expected outputs (before/after), and a completed checklist.

📜 Recent review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 40c1e3f and 14cf2ec.

📒 Files selected for processing (1)
  • CHANGELOG.md
✅ Files skipped from review due to trivial changes (1)
  • CHANGELOG.md
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (20)
  • GitHub Check: gradle-check
  • GitHub Check: assemble (21, ubuntu-latest)
  • GitHub Check: assemble (25, ubuntu-24.04-arm)
  • GitHub Check: assemble (25, windows-latest)
  • GitHub Check: assemble (21, ubuntu-24.04-arm)
  • GitHub Check: assemble (21, windows-latest)
  • GitHub Check: assemble (25, ubuntu-latest)
  • GitHub Check: precommit (21, macos-15-intel)
  • GitHub Check: precommit (25, windows-latest)
  • GitHub Check: precommit (25, macos-15)
  • GitHub Check: precommit (21, ubuntu-latest)
  • GitHub Check: precommit (21, windows-latest)
  • GitHub Check: precommit (21, windows-2025, true)
  • GitHub Check: precommit (25, ubuntu-24.04-arm)
  • GitHub Check: precommit (25, macos-15-intel)
  • GitHub Check: precommit (25, ubuntu-latest)
  • GitHub Check: precommit (21, macos-15)
  • GitHub Check: precommit (21, ubuntu-24.04-arm)
  • GitHub Check: Analyze (java)
  • GitHub Check: detect-breaking-change

Comment @coderabbitai help to get the list of available commands and usage tips.

@karenyrx karenyrx changed the title [GRPC] Add BigInteger Support for unsigned_long fields in gRPC transport [GRPC] Add BigInteger support for unsigned_long fields in gRPC transport Jan 1, 2026
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Jan 1, 2026

❌ Gradle check result for 5a4b572: 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
Copy Markdown
Contributor

github-actions bot commented Jan 2, 2026

❌ Gradle check result for 6b2eb9d: 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?

Signed-off-by: Karen X <karenxyr@gmail.com>
Signed-off-by: Karen X <karenxyr@gmail.com>
Signed-off-by: Karen X <karenxyr@gmail.com>
Signed-off-by: Karen X <karenxyr@gmail.com>
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Jan 4, 2026

❌ Gradle check result for 40c1e3f: 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
Copy Markdown
Contributor

github-actions bot commented Jan 4, 2026

❌ Gradle check result for 40c1e3f: 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?

@karenyrx karenyrx marked this pull request as ready for review January 4, 2026 14:30
@karenyrx karenyrx requested a review from a team as a code owner January 4, 2026 14:30
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

♻️ Duplicate comments (1)
CHANGELOG.md (1)

21-22: Remove duplicate changelog entry.

Line 21 contains duplicated text within itself and both lines reference the same PR. Keep only line 22 which has the correct format.

🔎 Proposed fix
 - Add `cluster.initial_cluster_manager_nodes` to testClusters OVERRIDABLE_SETTINGS ([#20348](https://github.com/opensearch-project/OpenSearch/pull/20348))
-- Add BigInteger support for unsigned_long fields in gRPC Add BigInteger support for unsigned_long fields in gRPC transport ([#20346](https://github.com/opensearch-project/OpenSearch/pull/20346))
 - Add BigInteger support for unsigned_long fields in gRPC transport ([#20346](https://github.com/opensearch-project/OpenSearch/pull/20346))
🧹 Nitpick comments (1)
modules/transport-grpc/src/test/java/org/opensearch/transport/grpc/proto/response/common/FieldValueProtoUtilsTests.java (1)

230-246: Consider adding round-trip test for values exceeding Long.MAX_VALUE.

The current round-trip tests only cover values within the signed long range. Consider adding a test case for values like 2^63 or 2^64-1 that should round-trip as BigInteger to ensure the full unsigned long range is covered.

🔎 Suggested addition
public void testRoundTripBigIntegerExceedingLongMax() {
    // Test round-trip for values exceeding Long.MAX_VALUE
    BigInteger[] testValues = {
        new BigInteger("9223372036854775808"), // Long.MAX_VALUE + 1
        new BigInteger("18446744073709551615") // 2^64 - 1
    };

    for (BigInteger original : testValues) {
        FieldValue fieldValue = FieldValueProtoUtils.toProto(original);
        Object result = FieldValueProtoUtils.fromProto(fieldValue);

        assertNotNull("Result should not be null for " + original, result);
        assertTrue("Result should be BigInteger for " + original, result instanceof BigInteger);
        assertEquals("Value should match for " + original, original, result);
    }
}
📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between d866be8 and 40c1e3f.

📒 Files selected for processing (3)
  • CHANGELOG.md
  • modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/response/common/FieldValueProtoUtils.java
  • modules/transport-grpc/src/test/java/org/opensearch/transport/grpc/proto/response/common/FieldValueProtoUtilsTests.java
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2026-01-02T19:23:29.698Z
Learnt from: karenyrx
Repo: opensearch-project/OpenSearch PR: 20335
File: modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/request/search/SearchSourceBuilderProtoUtils.java:155-172
Timestamp: 2026-01-02T19:23:29.698Z
Learning: In the transport-grpc module, suggest and aggregations protos were removed from SearchRequestBody in protobufs 1.0.0 because they haven't been vetted for accuracy in the API specification. The URL parameter suggest support (suggest_field, suggest_mode, suggest_size, suggest_text) is a minimized subset and not intended as a replacement for full Suggester functionality.

Applied to files:

  • modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/response/common/FieldValueProtoUtils.java
📚 Learning: 2026-01-02T19:23:16.689Z
Learnt from: karenyrx
Repo: opensearch-project/OpenSearch PR: 20335
File: modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/request/search/SearchSourceBuilderProtoUtils.java:155-172
Timestamp: 2026-01-02T19:23:16.689Z
Learning: In OpenSearch's gRPC transport module, treat removals of previously-unsupported fields (those throwing UnsupportedOperationException) from experimental proto schemas as non-breaking changes. When reviewing changes to proto-related Java code in this module, document that such removals do not count as breaking API changes, and ensure tests reflect compatibility expectations accordingly.

Applied to files:

  • modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/response/common/FieldValueProtoUtils.java
  • modules/transport-grpc/src/test/java/org/opensearch/transport/grpc/proto/response/common/FieldValueProtoUtilsTests.java
📚 Learning: 2026-01-02T19:23:29.698Z
Learnt from: karenyrx
Repo: opensearch-project/OpenSearch PR: 20335
File: modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/request/search/SearchSourceBuilderProtoUtils.java:155-172
Timestamp: 2026-01-02T19:23:29.698Z
Learning: The gRPC search API in OpenSearch is marked as "experimental" in official documentation, so changes to proto schemas that remove previously unsupported fields (those throwing UnsupportedOperationException) are not considered breaking changes.

Applied to files:

  • CHANGELOG.md
🧬 Code graph analysis (1)
modules/transport-grpc/src/test/java/org/opensearch/transport/grpc/proto/response/common/FieldValueProtoUtilsTests.java (1)
modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/response/common/FieldValueProtoUtils.java (1)
  • FieldValueProtoUtils (23-160)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: gradle-check
🔇 Additional comments (8)
modules/transport-grpc/src/test/java/org/opensearch/transport/grpc/proto/response/common/FieldValueProtoUtilsTests.java (5)

14-14: LGTM!

Import added correctly to support BigInteger testing.


187-206: LGTM!

Test correctly validates that fromProto returns BigInteger when the uint64 value exceeds Long.MAX_VALUE. The two's complement representation is well-documented in comments.


208-217: LGTM!

Test correctly validates that BigInteger representing max unsigned long is encoded as uint64 with the expected bit pattern (-1L).


219-228: LGTM!

Test validates that BigInteger values within signed long range are correctly encoded as uint64.


248-265: LGTM!

Test correctly validates that BigInteger values outside the unsigned long range (both too large and negative) throw IllegalArgumentException with appropriate error messages.

modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/response/common/FieldValueProtoUtils.java (3)

10-14: LGTM!

Imports correctly added for Numbers utility and BigInteger to support unsigned long handling.


44-52: LGTM!

Javadoc correctly updated to document BigInteger as a supported type.


136-146: LGTM!

The logic correctly handles unsigned 64-bit values by checking if the long representation is negative (indicating the unsigned value exceeds Long.MAX_VALUE). Numbers.toUnsignedBigInteger is the appropriate utility for this conversion.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Jan 4, 2026

❕ Gradle check result for 40c1e3f: UNSTABLE

Please review all flaky tests that succeeded after retry and create an issue if one does not already exist to track the flaky failure.

@codecov
Copy link
Copy Markdown

codecov bot commented Jan 4, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 73.31%. Comparing base (d866be8) to head (14cf2ec).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files
@@             Coverage Diff              @@
##               main   #20346      +/-   ##
============================================
+ Coverage     73.30%   73.31%   +0.01%     
+ Complexity    71777    71761      -16     
============================================
  Files          5784     5784              
  Lines        328141   328150       +9     
  Branches      47269    47270       +1     
============================================
+ Hits         240531   240573      +42     
+ Misses        68329    68263      -66     
- Partials      19281    19314      +33     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Signed-off-by: Varun Bharadwaj <varunbharadwaj1995@gmail.com>
Copy link
Copy Markdown
Contributor

@varunbharadwaj varunbharadwaj left a comment

Choose a reason for hiding this comment

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

Looks good to me. Thanks.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Jan 5, 2026

❌ Gradle check result for 14cf2ec: 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
Copy Markdown
Contributor

github-actions bot commented Jan 5, 2026

❌ Gradle check result for 14cf2ec: 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
Copy Markdown
Contributor

github-actions bot commented Jan 5, 2026

✅ Gradle check result for 14cf2ec: SUCCESS

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants