Skip to content

Fix some unexpected behavior for thrift field mask.#362

Open
shenyj3 wants to merge 4 commits intomainfrom
bugfix/field-mask-align-to-kitex
Open

Fix some unexpected behavior for thrift field mask.#362
shenyj3 wants to merge 4 commits intomainfrom
bugfix/field-mask-align-to-kitex

Conversation

@shenyj3
Copy link
Copy Markdown
Contributor

@shenyj3 shenyj3 commented Mar 30, 2026

Motivation

Fix some unexpected behavior for thrift field mask.

Required fields gated by field mask

Pilota-generated code checks _field_mask.field(id) for ALL fields, including required
ones. If the field mask selects $.some_struct.optional_field but not $.some_struct.required_field, the required
required_field field is omitted from the output.

Scalar not treated as all fields while map encoding

set_field_mask(fm) propagates a scalar (leaf) FieldMask to each map value struct's
_field_mask field. A scalar FM has no field-level children, so all inner fields are gated and NOT serialized.

Solution

  1. Do not check existence of required field mask while encoding.
  2. Treat Scalar as all fields.

@shenyj3 shenyj3 requested review from a team as code owners March 30, 2026 17:07
@shenyj3 shenyj3 force-pushed the bugfix/field-mask-align-to-kitex branch 3 times, most recently from 678b7c8 to da2b3ce Compare March 30, 2026 17:38
@codecov
Copy link
Copy Markdown

codecov bot commented Mar 30, 2026

Codecov Report

❌ Patch coverage is 80.20566% with 77 lines in your changes missing coverage. Please review.
✅ Project coverage is 67.89%. Comparing base (7c30b6e) to head (d4cacf2).

Files with missing lines Patch % Lines
pilota-build/src/codegen/thrift/ty.rs 49.61% 65 Missing ⚠️
pilota-build/src/codegen/thrift/mod.rs 20.00% 8 Missing ⚠️
pilota-thrift-fieldmask/src/fieldmask.rs 98.40% 4 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #362      +/-   ##
==========================================
+ Coverage   67.50%   67.89%   +0.39%     
==========================================
  Files          88       88              
  Lines       25044    25404     +360     
==========================================
+ Hits        16905    17249     +344     
- Misses       8139     8155      +16     

☔ 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.

@shenyj3 shenyj3 force-pushed the bugfix/field-mask-align-to-kitex branch from da2b3ce to 6508ffc Compare March 31, 2026 04:08
@shenyj3 shenyj3 force-pushed the bugfix/field-mask-align-to-kitex branch 6 times, most recently from 961d69f to 1cfc4b4 Compare April 2, 2026 13:09
@shenyj3 shenyj3 force-pushed the bugfix/field-mask-align-to-kitex branch from 1cfc4b4 to d4cacf2 Compare April 2, 2026 13:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

1 participant