Reject unsupported modifiers after $each in $addToSet

[richardsimmonds]

Summary

Fixes documentdb#505 — $addToSet incorrectly accepted $position, $slice, and $sort modifiers (and any other field after $each), silently applying the update as if only $each was given. MongoDB rejects these specs.

Changes

• pg_documentdb/src/update/bson_update_operators.c: in ValidateAddToSetWithDollarEach, after detecting $each as the first field, iterate the update value and error if any additional field is present:
  Found unexpected fields after $each in $addToSet: { "$each" : [...], "$position" : 1 }
  using ERRCODE_DOCUMENTDB_BADVALUE, matching MongoDB's error text.
• Regression tests added to bson_update_document_tests (both pg_documentdb and internal/pg_documentdb_distributed copies) covering $position, $slice, $sort, and an arbitrary trailing field after $each.

Behavior

Before: {$addToSet: {key: {$each: [...], $position: 1}}} succeeded and added elements.
After: errors with Found unexpected fields after $each in $addToSet: ... — same as MongoDB.

Note: $addToSet with a document value whose first field is NOT $each (e.g. {a: 1, b: 2}) is still treated as a literal element to add, unchanged — this matches MongoDB, which only engages modifier parsing when $each is the first field.

[richardsimmonds]

Code Review Summary

Verdict: LGTM — recommend merge. No blocking issues. One minor, non-blocking divergence from MongoDB noted inline.

Correctness

• Validation is correctly scoped: it only runs when TryGetBsonValueToPgbsonElement confirms $each is the first field of the update value, so literal-document adds like {a: 1, b: 2} are untouched — matches MongoDB, and the PR description calls this out explicitly.
• The call site (HandleUpdateDollarAddToSet) guarantees updateValue is a non-empty BSON_TYPE_DOCUMENT before ValidateAddToSetWithDollarEach runs, so BsonValueInitIterator cannot hit its non-document error path. No null-deref or iterator-init risk.
• The double bson_iter_next correctly detects any second field, including a duplicate $each.
• Error text Found unexpected fields after $each in $addToSet: {...} and ERRCODE_DOCUMENTDB_BADVALUE match mongod's message and BadValue code.

Safety

• No leaks: BsonValueToJsonForLogging allocates only in the error path, cleaned up by PG error handling. Iterator is stack-local over borrowed data.

Tests

• SQL + expected output updated in both pg_documentdb and internal/pg_documentdb_distributed copies, covering $position, $slice, $sort, and an arbitrary trailing field (x). Matches the existing $addToSet test patterns in the file, including the NULL-arg signature differences between the two copies — correctly handled.

Scope / DCO / CI

• Diff is minimal: 18 lines of C + 28 lines of tests, nothing unrelated.
• Commit c7931f6 has a valid Signed-off-by line.
• CI: no status checks reported on the fork for this SHA — nothing to verify.

---

Reviewed by Hermes reviewer profile

[richardsimmonds]

Minor (non-blocking): mongod's AddToSetNode::init validates that $each is an array before checking for unexpected trailing fields. Here the order is inverted, so a spec like {$addToSet: {letters: {$each: "c", $position: 1}}} raises BadValue (Found unexpected fields...) in DocumentDB but TypeMismatch (The argument to $each... must be an array) in MongoDB. Both reject the spec, so this is cosmetic — but for exact parity, move this block below the existing $each-must-be-array check, and consider a regression test for the combined case to pin whichever precedence you choose.