Skip to content

fix: keep default values of proto3 fields without presence in protobuf ingestion - #20485

Merged
amaechler merged 4 commits into
apache:masterfrom
amaechler:protobuf-proto3-default-values
Oct 6, 2026
Merged

amaechler merged 4 commits into
apache:masterfrom
amaechler:protobuf-proto3-default-values

Conversation

@amaechler

Copy link
Copy Markdown
Contributor

Fixes #18792.

Description

Protobuf ingestion turned proto3 fields that are not marked optional into null whenever they used their default value (0, "", false, or the first enum value). Users had to wrap such fields in nvl() in transforms to get the actual value back.

ProtobufConverter.convertMessage built each row from Message.getAllFields(). That method only returns fields that are "set", and for proto3 fields without explicit presence a default value is indistinguishable from unset when serialized, so those fields were dropped from the row.

convertMessage now also walks the message descriptor and adds every singular field where FieldDescriptor.hasPresence() is false and which getAllFields() did not already return, using msg.getField() to read the default. This mirrors JsonFormat.printer().alwaysPrintFieldsWithNoPresence(), with one deliberate difference: empty repeated and map fields are still omitted, as before, so they keep ingesting as null rather than an empty array.

Release note

Protobuf ingestion now keeps the default value of proto3 fields that are not marked optional. Previously an int32 field holding 0 (or a string holding "", a bool holding false, an enum holding its first value) was ingested as null. Such fields now ingest as their actual value.


This PR has:

  • been self-reviewed.
  • a release note entry in the PR description.
  • added unit tests or modified existing tests to cover new code paths, ensuring the threshold for code coverage is met.

…estion

ProtobufConverter built rows from Message.getAllFields(), which omits singular
fields without presence tracking (proto3 fields not marked optional) when they
hold their default value. Those columns were ingested as null instead of 0, "",
false or the first enum value. Add such fields explicitly.

@FrankChen021 FrankChen021 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

🟢 Approval recommended

The converter now backfills only singular fields without presence from the message descriptor, while preserving existing handling for present, optional, oneof, message, repeated, and map fields. The added tests cover implicit proto3 scalar/enum defaults and defaults inside a present nested message. No actionable correctness issue was found in the merge-base diff or the relevant surrounding protobuf ingestion code.

Reviewed 3 of 3 changed files.

Validation: git diff --check 1265b47c9a51252295a4aed42d95c459b00bf2a3 1893e6228d585f8e1ce6018c0f5a0d6b9dfd74fe passed.


This is an automated review by Codex GPT-5.6-Luna(max)

Ingests proto3 records through Kafka and Kinesis supervisors and queries the
default-valued fields. Also fixes the ProbufData typo in MoreResources and adds
the missing timeout on test_protobufDataFormat.
@amaechler
amaechler merged commit a758f52 into apache:master Oct 6, 2026
27 checks passed
@amaechler
amaechler deleted the protobuf-proto3-default-values branch October 6, 2026 20:37
@github-actions github-actions Bot added this to the 39.0.0 milestone Oct 6, 2026
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.

Protobuf transform handling with default values

3 participants