Skip to content

feat: support pretty() no-op and Int32/NumberInt string args#25

Merged
d-bytebase merged 5 commits intobytebase:mainfrom
h3n4l:vk/0465-byt-9080-mongodb
Mar 24, 2026
Merged

feat: support pretty() no-op and Int32/NumberInt string args#25
d-bytebase merged 5 commits intobytebase:mainfrom
h3n4l:vk/0465-byt-9080-mongodb

Conversation

@h3n4l
Copy link
Member

@h3n4l h3n4l commented Mar 24, 2026

Summary

Context

Telemetry (BYT-9080) showed these as the most frequent gomongo compatibility failures causing unnecessary mongosh fallbacks. Deprecated methods (insert, count, update) are intentionally left unsupported — they fall back to mongosh.

Test plan

  • TestPrettyNoOp in collection_test.go — verifies find().pretty(), find().sort().pretty(), aggregate().pretty()
  • TestInt32StringArg in bson_helpers_test.go — verifies Int32("123"), NumberInt("456"), NumberLong("1774250313")
  • Full test suite passes across MongoDB 4, 8, and DocumentDB
  • golangci-lint clean

🤖 Generated with Claude Code

h3n4l and others added 4 commits March 24, 2026 11:39
…080)

Fix 5 mongosh compatibility issues that caused unnecessary fallbacks:
- pretty(): treat as no-op cursor method (output already formatted)
- insert(): translate to insertOne/insertMany based on argument type
- count(): translate to countDocuments
- update(): translate to updateOne/updateMany based on multi option
- Int32("string")/NumberInt("string"): accept string arguments in parser

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Fix 2 mongosh compatibility issues that caused unnecessary fallbacks:
- pretty(): treat as no-op cursor method (output already formatted)
- Int32("string")/NumberInt("string"): accept string arguments in parser

Deprecated methods (insert, count, update) intentionally left as
unsupported to keep gomongo simple — they fall back to mongosh.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Move TestPrettyNoOp to collection_test.go (cursor method behavior)
- Move TestInt32StringArg to bson_helpers_test.go (BSON helper behavior)
- Delete compat_test.go
- Add TODO comment on replace directive (remove after parser published)

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Update github.com/bytebase/parser to include Int32/NumberInt string
argument support (parser#64). Remove temporary local replace directive.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings March 24, 2026 05:38
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR improves mongosh compatibility in the gomongo translator by (1) treating pretty() as an accepted cursor method without changing behavior and (2) allowing Int32() / NumberInt() to accept numeric strings, aligned with existing Long / NumberLong behavior. It also bumps github.com/bytebase/parser to a version that includes the required grammar updates.

Changes:

  • Add pretty() handling as a no-op cursor method to avoid unsupported-method failures.
  • Extend Int32() / NumberInt() helper conversion to accept string arguments.
  • Upgrade github.com/bytebase/parser dependency to pick up grammar support.

Reviewed changes

Copilot reviewed 6 out of 7 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
internal/translator/visitor.go Allows pretty() in cursor method chains as a no-op to prevent unsupported-method errors.
internal/translator/collection.go Trailing whitespace/newline adjustment at EOF (no functional change).
internal/translator/bson_helpers.go Adds string-literal support to convertInt32Helper for Int32("123") / NumberInt("123").
internal/translator/bson_helpers_test.go Adds an end-to-end test for string args (currently missing assertions for some inserted fields).
collection_test.go Adds coverage for find().pretty(), find().sort().pretty(), and aggregate().pretty() chaining.
go.mod Bumps github.com/bytebase/parser to a newer pseudo-version.
go.sum Updates checksums for the upgraded parser dependency.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

- Assert NumberInt("456") stores as int32(456), not just no-error
- Assert NumberLong("1774250313") stores as int64(1774250313)
- Insert in reverse order in pretty() sort test so assertion is meaningful

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@d-bytebase d-bytebase enabled auto-merge (squash) March 24, 2026 06:15
@d-bytebase d-bytebase merged commit f15b9a5 into bytebase:main Mar 24, 2026
2 checks passed
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.

4 participants