Skip to content

restore: carry struct-level nullable through schema conversion - #1188

Open
huanghaoyuanhhy wants to merge 2 commits into
zilliztech:mainfrom
huanghaoyuanhhy:fix/struct-field-nullable
Open

restore: carry struct-level nullable through schema conversion#1188
huanghaoyuanhhy wants to merge 2 commits into
zilliztech:mainfrom
huanghaoyuanhhy:fix/struct-field-nullable

Conversation

@huanghaoyuanhhy

Copy link
Copy Markdown
Collaborator

Background

Milvus keeps nullable authoritative at the struct level of an array-of-struct field: it is rejected when set per sub-field and instead propagated onto every sub-field by proxy before persisting. Because DescribeCollection returns those propagated child flags verbatim while struct-level handling depends on every consumer keeping StructArrayFieldSchema intact, any conversion that models structs as flat field lists loses the parent bit and re-created collections fail validation (sub-field in non-nullable struct cannot be nullable individually, ...).

The asymmetry itself is tracked upstream at milvus-io/milvus#52924 — this PR is deliberately a downstream compatibility pass so backup behaves like the server does, not a statement about where the UX fix belongs.

Changes

  • core/proto: add bool nullable = 5 to StructArrayFieldSchema, regenerate generated code
  • core/backup: carry the struct-level flag verbatim from DescribeCollection
  • core/restore (primary DDL path + secondary conv path): rebuild it per server rule via shared conv.ApplyStructNullable; derive it from nullable children for backup metas written before this field existed
  • sync swagger definition

Tests

Unit tests cover all three meta shapes (struct flag set → propagated; legacy child-only flags → repaired; both unset → untouched) plus the describe wiring. Full build and golangci-lint clean.

Related to milvus-io/milvus#52924

Milvus validates nullable structs at the struct level: a nullable struct
propagates the flag to every sub-field, and sub-fields must not be
nullable unless their struct is. The backup proto's StructArrayFieldSchema
had no nullable field, so the flag was dropped during backup and restoring
a nullable-struct collection sent sub-fields marked nullable under a
non-nullable struct, which the target cluster rejects.

Store the struct-level flag in the backup meta and rebuild it on restore
with the server's rule, deriving it from nullable sub-fields for backups
written before this field existed.

Signed-off-by: huanghaoyuanhhy <haoyuan.huang@zilliz.com>
@sre-ci-robot

Copy link
Copy Markdown
Collaborator

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: huanghaoyuanhhy

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@mergify

mergify Bot commented Aug 27, 2026

Copy link
Copy Markdown

@huanghaoyuanhhy Please associate the related issue to the body of your Pull Request. (eg. “issue: #”)

@codecov-commenter

codecov-commenter commented Aug 27, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 85.71429% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 44.10%. Comparing base (f81df44) to head (53b75f8).
⚠️ Report is 3 commits behind head on main.

Files with missing lines Patch % Lines
core/backup/coll_ddl_task.go 0.00% 1 Missing ⚠️
core/restore/coll_ddl_task.go 0.00% 1 Missing ⚠️

❌ Your project status has failed because the head coverage (44.10%) is below the target coverage (80.00%). You can increase the head coverage or adjust the target coverage.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1188      +/-   ##
==========================================
+ Coverage   43.14%   44.10%   +0.95%     
==========================================
  Files         134      136       +2     
  Lines       12552    12662     +110     
==========================================
+ Hits         5416     5585     +169     
+ Misses       6731     6664      -67     
- Partials      405      413       +8     

☔ View full report in Codecov by Harness.
📢 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.

@mergify mergify Bot added the ci-passed label Aug 27, 2026
@huanghaoyuanhhy
huanghaoyuanhhy force-pushed the fix/struct-field-nullable branch from 364e6c7 to 2456121 Compare August 27, 2026 06:44
@mergify mergify Bot removed the ci-passed label Aug 27, 2026
Signed-off-by: huanghaoyuanhhy <haoyuan.huang@zilliz.com>
@huanghaoyuanhhy
huanghaoyuanhhy force-pushed the fix/struct-field-nullable branch from 2456121 to 53b75f8 Compare August 27, 2026 06:46
@mergify mergify Bot added the ci-passed label Aug 27, 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.

3 participants