Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix a bug in hidden field handling in std.mergePatch #250

Merged

Conversation

JoshRosen
Copy link
Contributor

This PR fixes a bug related to handling of hidden fields in std.mergePatch:

In both google/jsonnet and google/go-jsonnet, hidden fields are ineligible to participate in field merges:

  • std.mergePatch({a: 1}, {a :: 2}
    • should return {a: 1} (ignoring the hidden patch field)
    • sjsonnet returns {a: 2}
  • std.mergePatch({a: {a: 1}}, {a:: {b: 1}}
    • should return {a: {a: 1}}
    • sjsonnet returns {a: {a: 1, b: 1}}
  • std.mergePatch({a:: {a: 1}}, {a: {b: 1}} (symmetrical to previous case, but swaps hidden between target and patch)
    • should return {a: {b: 1}}
    • sjsonnet returns {a: {a: 1, b: 1}}

The problem is that the current code iterates over the union of visible fields, but never re-checks the specific visibility in target and patch during per-field merges.

This PR fixes that bug and significantly expands unit test coverage for std.mergePatch.

@JoshRosen JoshRosen changed the title Fix a bug related to hidden fields in std.mergePatch Fix a bug related to hidden fields handling in std.mergePatch Dec 31, 2024
@JoshRosen JoshRosen changed the title Fix a bug related to hidden fields handling in std.mergePatch Fix a bug related to hidden field handling in std.mergePatch Dec 31, 2024
@JoshRosen JoshRosen changed the title Fix a bug related to hidden field handling in std.mergePatch Fix a bug in hidden field handling in std.mergePatch Dec 31, 2024
@stephenamar-db stephenamar-db merged commit 9d4dde5 into databricks:master Dec 31, 2024
6 checks passed
@JoshRosen JoshRosen deleted the mergePatch-nested-field-behavior branch January 1, 2025 01:01
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