Repository navigation
Conversation
There was a problem hiding this comment.
🟡 Changes recommended
Type-only updates can persist invalid records, and packed-field concurrent updates can still overwrite each other.
3 open findings
What changed in this PR
Updates zone-record PUT behavior to merge RFC fields correctly across MySQL and file stores.
Changes:
- Packs merged records before MySQL updates.
- Removes obsolete RDATA during type changes.
- Adds broad PUT regression coverage.
| File | Description |
|---|---|
CHANGELOG.md |
Documents corrected PUT behavior. |
lib/zone_record/store/base.js |
Adds shared RDATA field discovery. |
lib/zone_record/store/file.js |
Clears old RDATA during type changes. |
lib/zone_record/store/mysql.js |
Merges and maps updates to database columns. |
lib/zone_record/test/index.js |
Tests partial updates and type changes. |
🧠 Review effort: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Copilot spotted that a type-only PUT, such as A to MX, saved an MX with no preference or exchange. A PUT that changes the type, or a field packed into one column such as an SOA serial, now checks the merged record with the RR class and answers 400, as a bad create now does too. A type change also clears legacy weight, priority and other. On mysql the UPDATE writes only real columns, every rdata column on a type change, applies only to the type it was merged against, and answers 409 when the row changed in between. A NULL column no longer reads back as a null RR field.
aberoham
force-pushed
the
zone-record-partial-put
branch
from
October 14, 2026 22:37
1b8a637 to
9ec3e0a
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
On mysql,
ZoneRecord.put()passed RFC field names straight to theUPDATE. Changingexchangeon an MX record, sending the whole record back, or changing the type failed with an unknown column. Only fields that are also column names, such asttl, could be edited.put()now merges the change onto the stored record, packs it with the same map ascreate(), and writes only the columns whose value changed. Packing the merged record matters where fields share a column. NAPTR keepsflags,serviceandregexpinaddress, so changingregexpalone keeps the other two. Concurrent edits of different columns don't undo each other. Concurrent edits of fields packed into one column still can.A type change drops the old type's rdata on both stores, using each type's
rdataFields. On mysql that clears every column the old type used, includingdescriptionfor NAPTR, IPSECKEY, NSEC and NXT. The file stores used to keep the old fields beside the new ones.A type change, or a change to a field packed into one column such as an SOA serial, must leave a valid record, checked with the RR class as
create()does.{ type: 'MX' }on an A record now answers 400, and so does a create with bad rdata. On mysql the change only applies to the type it was merged against, so a type change in between answers 409.