Skip to content

AGENTS.md

Latest commit

 

History

History
98 lines (71 loc) · 11.4 KB

AGENTS.md

File metadata and controls

98 lines (71 loc) · 11.4 KB
Raw

Omni

Omni is the SQL toolchain Bytebase runs on: one hand-written recursive descent parser per database engine, and the catalog, completion, analysis, and review packages built on its AST. Bytebase imports these packages. Omni owns every decision about what an engine accepts and how its DDL behaves; Bytebase owns everything that needs its metadata store or a live connection.

Read before working

  • Before adding an engine or a capability layer (catalog, completion, review), read docs/engine-capability-guide.md and docs/PARSER-DEFENSE-MATRIX.md. They say how to build a layer; this file says how to change one.
  • Before changing pg/parser or redshift/parser, read pg/parser/AGENTS.md for the FIRST-set and backtracking rules.
  • Before reading anything under docs/plans, docs/specs, or docs/scenarios, read docs/AGENTS.md: those are historical records.
  • AGENTS.md files are the instruction source of truth; CLAUDE.md files import them.

Repository shape

  • Every engine is an independent Go tree under its own top-level directory. Engines share only metadata (the Bytebase metadata proto) and review (the SQL Review V2 contract). An engine never imports another engine.
  • Fork engines began as mechanical copies and diverge as explicit dialect deltas: redshift from pg; mariadb and tidb from mysql; starrocks from doris. A parser fix in a parent usually applies to its forks. Check, port it in the same PR, and name both in the subject (fix(pg,redshift): ...). Forks carry no copies of the parent's planning docs or guides; a fork's file points at the parent's.
  • harness/ holds nested Go modules that ./... does not reach: external harnesses (conformance, googlesql-spanner, mssql-scriptdom) that tests exec rather than import.
  • <engine>/ast/walk.go is generated by go run ./cmd/genwalker in pg, redshift, mysql, mariadb, tidb, mssql, oracle, snowflake, and googlesql. After changing AST node types, run go generate ./<engine>/ast/... and commit the result. CI verifies only pg, mysql, and mssql stay in sync; for the others, verify it yourself.
  • Non-test code has zero dependencies. pgx, go-sql-driver, go-mssqldb, go-ora, testcontainers, testify, go-cmp, and yaml in go.mod exist for tests and harnesses only.

Engine contract

These rules were paid for by rework in the PostgreSQL engine (docs/engine-capability-guide.md section 4) and bind every engine:

  • Every parse function returns (node, error). Never return nil in place of an error, and never discard one with _.
  • Every AST node carries Loc{Start, End} as byte offsets into the text given to the parser. Start is inclusive; End is exclusive and ends at the node's last token, not at the start of the next one. Unknown is -1, never 0.
  • No input panics the parser: empty, binary, or truncated at any byte. A parse failure is a ParseError carrying a byte offset.
  • Parse is strict. It rejects what the engine rejects and stops at the first error. Recovery, partial trees, and diagnostic lists belong in ParseBestEffort where an engine offers one. Do not add tolerance to Parse.
  • A grammar decision cites the upstream grammar or reference page (gram.y line, documentation URL). When the upstream text does not settle what the engine accepts, ask the engine: that is what the oracle tests are for.

Using the parser

Bytebase's adapters under backend/plugin/parser/<engine> and omni's own packages (catalog, completion, analysis, review) call the parser the same way. Deviate only for a reason written beside the call.

Need Call
Statement boundaries of a script, valid SQL or not <engine>.Split or <engine>/parser.Split. Lexical, never fails. A segment holding no statement (a lone ;, a comment) answers Empty(); pg, redshift, partiql, and cassandra return such segments, the other splitters drop them (see Known deviations)
Every statement of a script with text, AST, and positions <engine>.Parse(script), which returns []Statement (pg, redshift, mssql, oracle, cassandra, cosmosdb, mongo)
One statement the caller already isolated <engine>.Parse or parser.Parse, then assert exactly one non-empty statement
An incomplete fragment, as in completion Complete it first, by wrapping it in a full statement or patching a placeholder at the caret, then parse. A failure there is expected and falls back to lexer-based extraction
Tokens without a tree, as in limit rewriting or caret lookup parser.Tokenize or parser.NewLexer
  • Split first, then parse each segment. Every ParseError and every Loc is then relative to text the caller holds. Whole-script parser.Parse is for single statements and for internal helpers that report no positions.
  • Parse once and pass the AST. A package that receives an AST (analysis, review, a catalog after Exec) does not parse the text again.
  • Positions leave the parser as byte offsets. They become line and column only at the boundary that reports to a user, through review.Index or review.Position: lines 1-based, columns 1-based and counted in code points, which is what Bytebase's Position holds. Do not write the byte-to-rune loop again.
  • A segment parsed on its own has positions relative to the segment. Parse it in place instead, through a ranged entry such as oracle/parser.ParseRange(script, seg.ByteStart, seg.ByteEnd) or a lexer base offset as in doris, so positions come out absolute. Never pad the input with spaces (quadratic on large scripts), and never rewrite Loc fields through reflection.
  • A parse error is returned, or reported as a finding with its position. The one accepted downgrade is feature extraction from a definition the engine already accepted (a synced function signature, an index definition), and the comment on the call says so.
  • ParseError has the same shape in every engine: Message string and Position int, the byte offset into the parsed text, implementing error and reachable with errors.As. An engine may add fields such as End, Line, Column, RelatedText, or Code, but does not rename those two. Parse returns error, not a slice. An engine whose strict parse reports every failure of a multi-statement script (trino, googlesql, doris, starrocks) returns a ParseErrors list that unwraps to each *ParseError; parser.AllErrors(err) and parser.FirstError(err) read it. The elasticsearch REST console parser is the one non-SQL grammar and keeps its SyntaxError{ByteOffset, Message}.
  • Split marks an empty segment with Empty() and the target is to return it, so a consumer that accounts for every ; (Bytebase's execution log) can; the Statement list from Parse holds only statements with an AST.
  • DDL enters a catalog only through catalog.Exec(sql, opts). It splits and parses itself and returns one ExecResult per statement with its line. A synced schema enters through LoadMetadata. Callers do not pre-split for Exec.
  • <engine>/review.Review(ctx, sql, opts, targets) splits and parses once inside, treats a syntax error as a finding at its position, and reports every position through review.Index. The contract is in review/review.go.

Known deviations

The rules above are the target. These places do not meet them yet; do not copy them into new code.

  • Split in mysql, mariadb, tidb, oracle, snowflake, trino, googlesql, doris, and starrocks drops empty segments instead of returning them marked Empty(). Bytebase's adapters for those engines cannot show a lone ; in the execution log until they keep them.
  • On the Bytebase side, ByteOffsetToRunePosition exists five times and adapters disagree on keeping empty segments. Not omni's to fix, but the reason review.Index stays the single implementation.

Tests

  • Engine dialect and DDL fidelity tests live here, beside the package they cover. Bytebase tests its API and workflow against PostgreSQL only and trusts omni's result, so a behavior Bytebase depends on is pinned by an omni test.
  • Test against the engine, not against memory. Parser acceptance, catalog results, and generated DDL are verified against a real engine wherever one can run: testcontainers for pg, mysql, mariadb, tidb, mssql, doris, and starrocks; Trino and the Spanner emulator as CI services; Oracle and Redshift through ORACLE_PARSER_REF_DSN and REDSHIFT_COMPAT_DSN. Gate a test on an environment variable only when it needs a server CI cannot start.
  • Container tests fail when Docker is missing; they do not skip. CI runs every test on every PR with no -short and no nightly. Never check testing.Short(), and never park a known gap behind a skip.
  • Pinned images are part of the recorded expectations: trinodb/trino:482 and the Spanner emulator 1.5.54 in .github/workflows/ci.yml, postgres:17-alpine in pg/parser. Upgrading one means rerunning its oracle tests and reading the drift, not editing expectations to match.
  • CI runs the sweeps at PAREN_FUZZ_SIZE=10000, SPLITTEST_N=1000000, and S3DIFF_N=2000; local defaults are smaller. Trino packages run one at a time because they share the service and run stateful DDL against the same names.
  • A PR runs the packages scripts/affected-packages.sh derives from the diff. A change to go.mod, proto/, scripts/, harness/, docs/, or any root file runs everything.

Verification

During iteration, run the focused test with the regex quoted:

go test ./pg/parser/ -run '^TestName$' -count=1

Before handoff, in order:

  1. gofmt -w on the Go files you modified. Parts of the tree are not gofmt-clean; format only what you touched.
  2. go build ./...
  3. go vet on the changed packages, and fix every finding your change introduced. Some packages carry older findings (unreachable code in oracle/parser); leave those to their own PR.
  4. go generate for any AST package you changed, then confirm git status --porcelain is empty apart from your edits.
  5. make test-<engine> for every engine you touched. To see what CI will run, scripts/affected-packages.sh origin/main.
  6. For proto/: make proto, then make proto-breaking.
  7. For harness/: cd into the module and go test ./...; ./... at the root does not reach it.

Correct failures before rerunning. Report an unavailable prerequisite (no Docker, no DSN) explicitly rather than as a pass.

Documents

  • Scenario checklists go in docs/scenarios/<engine>/, dated plans in docs/plans/, dated designs in docs/specs/, per-engine migration notes in docs/migration/<engine>/, and prompt templates in scripts/prompts/. Nothing of that kind goes in the repo root.
  • SKILL.md, PROGRESS*.json, and SCENARIOS-*.md inside engine directories are artifacts of the batch pipelines that built the engine. They are not guidance for changing it.
  • Write <repo root>, never an absolute path, in prompts and guides.

Commits and pull requests

  • Subject: <type>(<scope>): <what changed>, with type one of fix, feat, test, perf, ci, docs, chore and scope the engine directory, listing every engine the change touches (fix(mysql,mariadb,tidb): ...), or review, catalog, ci for cross-engine work. A Linear key goes in parentheses at the end: fix(oracle): support constraint_state clause on constraints (BYT-10010).
  • One PR per change; a parent and its forks are one change.
  • Bytebase pins omni by commit in its go.mod. A change Bytebase needs is not done until that pin is bumped there and its tests pass.