feat: enforce single-statement contract in translator.Parse - #31
Merged
Conversation
With omni's mongo.Parse now strict (any statement parse error fails the whole input), translator.Parse no longer silently ignores content after the first statement: extra statements are rejected with a ParseError instead of being dropped. Comment-only input remains a no-op. Also pins the BYT-9950 statement: createIndex with a constant arithmetic expireAfterSeconds (90 * 24 * 60 * 60) folds to an int32 TTL and creates the index end to end. Requires bumping github.com/bytebase/omni to a version with strict mongo.Parse (folding landed in omni#393); the bump lands in this PR once the omni change merges. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Picks up bytebase/omni#395 (Parse fails on any statement parse error) and bytebase/omni#393 (constant arithmetic folding), which the new single-statement contract and TTL-index tests rely on. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR makes internal/translator.Parse strict about consuming exactly one executable MongoDB shell statement, preventing trailing statements from being silently ignored and aligning gomongo’s behavior with Bytebase’s “one statement per Execute” contract. It also updates the bytebase/omni dependency to pick up stricter parsing behavior and constant-expression folding needed for TTL arithmetic expressions.
Changes:
- Enforced a single-executable-statement contract in
translator.Parse, returning aParseErrorwhen multiple executable statements are present. - Added unit tests to pin the single-statement/no-op behavior and to validate TTL arithmetic folding in translation.
- Added an end-to-end container test asserting
expireAfterSecondsarithmetic is folded to the expected TTL value, and bumpedgithub.com/bytebase/omnito the required version.
Reviewed changes
Copilot reviewed 4 out of 5 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| internal/translator/translator.go | Enforces “exactly one executable statement” semantics and errors on multiple statements. |
| internal/translator/translator_test.go | Adds unit tests covering strict single-statement parsing and TTL arithmetic folding. |
| admin_test.go | Adds container-level integration test verifying TTL arithmetic folding is preserved end-to-end. |
| go.mod | Bumps github.com/bytebase/omni to a newer pseudo-version containing the required parsing/folding changes. |
| go.sum | Updates checksums for the github.com/bytebase/omni dependency bump. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
omni#393 (constant arithmetic folding) was reverted in omni#396: arithmetic expressions in mongosh statements are intentionally unsupported. The BYT-9950 tests now assert that translator.Parse and Execute return a ParseError instead of silently skipping the statement. Bumps omni past the revert. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
h3n4l
enabled auto-merge (squash)
July 30, 2026 03:29
rebelice
approved these changes
Jul 30, 2026
h3n4l
added a commit
to bytebase/bytebase
that referenced
this pull request
Jul 30, 2026
Picks up bytebase/gomongo#31: translator.Parse rejects multi-statement input instead of silently executing only the first statement, and its omni pin aligns with this repo's post-revert version. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
rebelice
pushed a commit
to bytebase/bytebase
that referenced
this pull request
Jul 30, 2026
Picks up bytebase/gomongo#31: translator.Parse rejects multi-statement input instead of silently executing only the first statement, and its omni pin aligns with this repo's post-revert version. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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.
What
translator.Parsenow requires exactly one executable statement: extra statements are rejected with aParseError(previously content after the first statement was silently ignored). Comment-only input remains a no-op (OpNoOp).github.com/bytebase/omnito pick up mongo: make Parse strict, reserve recovery for ParseBestEffort omni#395 (strictmongo.Parse: any statement parse error fails the whole input) and chore: revert "mongo: fold constant arithmetic expressions with JavaScript semantics (#393)" omni#396 (revert of the unintended #393 arithmetic folding).Why
BYT-9950: Bytebase migrations silently skipped MongoDB statements that failed to parse. The fix chain makes every layer strict — omni's
Parsenever drops statements, and gomongo never silently ignores trailing input. Bytebase always passes exactly one split statement perExecutecall, so this contract matches the real caller.Arithmetic expressions such as
expireAfterSeconds: 90 * 24 * 60 * 60remain unsupported by design (folding was reverted in omni#396): they now surface as aParseErrorinstead of being silently skipped.Testing
internal/translatorunit tests pin the single-statement contract (valid single, comment-only no-op, invalid rejected, valid+invalid rejected, two-valid rejected) and that the BYT-9950 arithmetic statement returns aParseError.TestCreateIndexArithmeticTTLcontainer test assertsExecutereturns aParseErrorfor the BYT-9950 statement on mongo 4.4, mongo 8.0, and DocumentDB.go test ./...green locally (containers included);golangci-lintclean.🤖 Generated with Claude Code