fix #594: preserve omitted scalar fields on create or modify external entity

[ako]

Summary

Fixes #594. execCreateExternalEntity unconditionally wrote existingEntity.X = s.X for every property in its or modify branch. When the MDL statement omitted RemoteName, s.RemoteName was the empty string and the pre-existing value was wiped. The resulting Rest$ODataRemoteEntitySource BSON had RemoteName: "", and Studio Pro's Integration Pane visualiser then NREs in ODataRemoteEntitySource.get_RemoteId() with ArgumentNullException("EntityTypeName") (Studio Pro aliases the BSON RemoteName field as the C# property EntityTypeName).

Approach

Convert the scalar fields on CreateExternalEntityStmt (EntitySet, RemoteName, Countable, Creatable, Deletable, Updatable, AllowCreateChangeLocally) to pointers so the executor can distinguish:

• nil → field omitted from the statement → preserve existing value on or modify
• non-nil → explicitly set by the user (including to false or "") → assign

This matches the existing tri-state convention already used for AlterProjectSecurityStmt.DemoUsersEnabled *bool. The visitor allocates pointers only inside each case of the property switch. The executor gates every assignment on non-nil on the modify branch; the initial-create branch still requires EntitySet (no prior value to preserve) and returns a validation error if it's omitted.

Test plan

• New mock-backend test (TestCreateOrModifyExternalEntity_PreservesOmittedFields_Issue594) constructs an existing entity with non-default values for every clobberable field, runs or modify with only EntitySet + AllowCreateChangeLocally set, and asserts all other fields retain their pre-existing values.
• Verified the test catches the bug — temporarily reverting just the RemoteName branch to the old unconditional assignment reports RemoteEntityName = "", want "Account" and the test fails.
• make build — passes
• make test — all packages green
• make lint — Go + TypeScript pass
• ./bin/mxcli check mdl-examples/bug-tests/594-or-modify-external-entity-preserve-omitted.mdl → ✓ Syntax OK (5 statements)
• Manual validation in Studio Pro (user): apply the bug-test MDL to a project, confirm RemoteAccount still has RemoteName: "Account" after the or modify, and the Integration Pane renders without NRE.

Compatibility

The AST change is package-internal — CreateExternalEntityStmt lives in mdl/ast and no external code depends on its fields. Visitor, executor, validate, validate_duplicates, and stmt_summary are all updated. The only existing call sites in cmd_odata_mock_test.go were migrated to local strPtr/boolPtr helpers.

Related

• create or modify external entity wipes omitted RemoteName/EntityTypeName (Studio Pro NRE in ODataRemoteEntitySource.RemoteId) #594 — the underlying executor bug (this PR)
• 10-odata-examples.mdl Level 9.1 crashes Studio Pro project check (NRE in CallExternalAction.SchemaAction) #590, PublishedODataService EntityTypePointer never set — Studio Pro NRE in EntitySet.Check #595 — sibling Studio Pro NREs from the same example file (PRs fix #590: rewrite OData example Level 9.1 to use TripPin schema actions #592, fix #595: PublishedODataService EntityTypePointer wired by qualified entity name #596)
• PR fix #590: rewrite OData example Level 9.1 to use TripPin schema actions #592 — example-file workaround that re-stated RemoteName: 'Account' in Level 8.6c; still defensive but no longer required after this fix lands

🤖 Generated with Claude Code

[github-actions]

AI Code Review

Critical Issues

• None found

Moderate Issues

• None found

Minor Issues

• None found

What Looks Good

1. Correct root cause fix: The PR correctly addresses the issue by distinguishing between omitted fields (nil) and explicitly set zero values using pointers, matching the existing tri-state pattern used elsewhere in the codebase (AlterProjectSecurityStmt.DemoUsersEnabled *bool).
2. Full stack consistency: All necessary layers are updated:
   • AST: CreateExternalEntityStmt fields changed to pointers
   • Visitor: Property parsing now allocates pointers only inside each case
   • Executor: Assignments gated on non-nil checks for modify branch
   • Tests: Comprehensive unit test and MDL bug test added
3. Test quality:
   • New mock backend test directly verifies the fix
   • Bug-test MDL file provides end-to-end validation
   • Test catches the bug when reverted (verified in PR description)
   • All existing tests pass with pointer helper migration
4. Code quality:
   • Clean helper functions (derefString, derefBool) for safe dereferencing
   • Clear validation error for required EntitySet on initial create
   • Maintains existing behavior for initial create while fixing modify branch
   • No unnecessary changes - focused solely on the reported issue
5. Documentation:
   • Bug-test MDL includes clear reproduction steps and verification instructions
   • Commit messages and code comments explain the issue and solution
   • PR description thoroughly covers approach, test plan, and compatibility

Recommendation

Approve - The PR correctly fixes issue #594 with minimal, focused changes that follow existing patterns. All tests pass, the solution is robust, and it maintains backward compatibility while fixing the Studio Pro NRE bug. No changes needed.

---

Automated review via OpenRouter (Nemotron Super 120B) — workflow source