TML-3287: Raw SQL ending in a line comment renders valid DDL - #30546

Linked issue

Refs TML-3287, a slice of the Linear project SQL expression literals. Depends on no other slice.

Summary

A raw SQL body in a schema can end in a line comment. The planner pastes that body into a larger statement, so everything after it on the same line, such as the closing parenthesis of CHECK (...), was swallowed by the comment and the migration failed. Now every place the planner puts contract SQL inside a statement holds it as an OpaqueSql node and renders it through one function, which ends text containing -- with a line break. Wire names, which hash the SQL, keep line breaks in bodies that contain --, so moving a line break around a comment changes the name. Nothing changes for a body without --.

Skill update

n/a. Schemas do not change. Two upgrade fragments under upgrade-instructions/pending/line-comments-in-raw-sql/: the app fragment says a body with both a -- comment and a line break gets a new wire name once; the extension fragment records the changed node constructors and types.

At a glance

Before this PR, this check constraint produced invalid DDL:

model Order {
  id     Int @id
  amount Int

  @@check(expression: "amount > 0 -- never free", name: "order_amount_positive")
}

ALTER TABLE "Order" ADD CONSTRAINT "order_amount_positive" CHECK (amount > 0 -- never free)
--                                                                            ^ commented out

After this PR the same schema migrates:

ALTER TABLE "Order" ADD CONSTRAINT "order_amount_positive" CHECK (amount > 0 -- never free
)

The rule is one function:

export function renderOpaqueSql(sql: OpaqueSql): string {
  return sql.text.includes('--') ? `${sql.text}\n` : sql.text;
}

Decision

This PR ships three things.

1. OpaqueSql is the DDL node for SQL text Prisma does not parse. A CHECK or policy predicate, an index element list or predicate, a column default expression, or an ALTER COLUMN TYPE conversion. ADR 244 already calls such text opaque; the node gives it a type. It lives in packages/2-sql/4-lanes/relational-core/src/ast/opaque-sql.ts with renderOpaqueSql.
2. Every DDL node that carries such text holds an OpaqueSql, and every site that renders one goes through renderOpaqueSql. FunctionColumnDefault, CheckExpressionConstraint, PostgresCreatePolicy and PostgresCreateIndex hold the node; both adapters' renderers, addCheckConstraint, both targets' buildColumnDefaultSql and the alterColumnType USING clause render through the function. The contract-free factories that migration files call keep taking strings and wrap them, so committed migration files load unchanged.
3. Wire names keep line breaks around --. normalizeSqlBody collapses whitespace to one space as before, except in a body that contains --, where it keeps one line break per line. A line break ends a comment, so it is part of the SQL's meaning there. ADR 234 records the rule.

How it fits together

1. Where contract SQL goes into a statement. The planner builds DDL nodes from the contract and the adapter renders them to SQL. Four nodes carry SQL text the planner does not parse, and four more sites render such text straight from a template string.
2. The node makes the rule visible. A field of type OpaqueSql cannot be interpolated into a template by accident; it has to go through renderOpaqueSql. The four template-string sites wrap their string with opaqueSql(...) before rendering, so they follow the same rule.
3. The line break is the whole fix. A -- comment runs to the end of its line. Ending the text with a line break means whatever the statement writes next starts on a fresh line. For text without -- the function returns the text unchanged, so every existing statement is byte-identical.
4. Names must not collide. An index, check or policy is named by a hash of its normalized SQL. Collapsing all whitespace would make a --c\nb and a --c b the same name although they are different SQL. The normalizer keeps line breaks only when -- is present, so every existing name is unchanged and a body with a comment and a line break gets a name that reflects its meaning.

Reviewer notes

• No committed artefact changes. No committed ops.json, migration.json, contract file or wire name contains --, and fixtures:check shows a clean tree.
• The two buildColumnDefaultSql sites never see a comment, because assertSafeDefaultExpression already refuses -- in defaults. They render through renderOpaqueSql only so the rule holds at every site.
• CreateIndexElements moved from operations/indexes.ts to ddl/nodes.ts, next to DdlIndexElements, and is exported from the Postgres target's ddl entry.
• Column defaults keep refusing --, at authoring and in assertSafeDefaultExpression, because contract infer compares a default as text. Their two render sites go through renderOpaqueSql only so every site follows one rule; the Migration System doc says so.
• Migration files written by the planner print a function default as fn("<sql>") on both targets; the SQLite renderer had passed the node object to the JSON printer, which the first review round caught and a whole-file render test now pins.
• Extension authors who construct these nodes directly pass opaqueSql(text) and read .text; the fragment has the table.
• Three tarball tests cannot run on the author's machine (pnpm install in their scratch project refuses @vercel/detect-agent@1.2.5). CI is the check for them.

Behavior changes & evidence

• A body ending in a line comment migrates. packages/3-targets/6-adapters/postgres/src/core/control-adapter.ts, packages/3-targets/3-targets/postgres/src/core/migrations/operations/constraints.ts, columns.ts. Evidence: packages/3-targets/6-adapters/postgres/test/migrations/opaque-sql-line-comment.integration.test.ts on PGlite: CREATE TABLE with a CHECK and a function default, addCheckConstraint, a policy with USING and WITH CHECK, an index whose element list ends in a comment, a partial index, and an ALTER COLUMN TYPE USING clause; each failed with a syntax error before the change.
• SQLite renders defaults the same way. packages/3-targets/6-adapters/sqlite/src/core/control-adapter.ts. Evidence: packages/3-targets/6-adapters/sqlite/test/lower-to-execute-request.test.ts asserts the whole CREATE TABLE for a default ending in a line comment.
• Planned migration files render the default's text. packages/3-targets/3-targets/sqlite/src/core/migrations/op-factory-call.ts. Evidence: packages/3-targets/3-targets/sqlite/test/migrations/render-typescript.test.ts renders a whole migration.ts and asserts fn("datetime('now')").
• renderOpaqueSql and the node. packages/2-sql/4-lanes/relational-core/src/ast/opaque-sql.ts. Evidence: relational-core/test/ast/opaque-sql.test.ts (text without -- unchanged; -- anywhere, including inside a string constant, appends a line break; the node is frozen).
• Wire names keep line breaks around --. packages/2-sql/1-core/schema-ir/src/naming.ts. Evidence: schema-ir/test/naming.test.ts and target-postgres/test/rls-canonicalize.test.ts (two bodies that differ only by a line break after a comment get different names; bodies without -- and one-line bodies are unchanged; CRLF and lone CR end a line; the function is stable on its own output; the pinned hash table is unchanged).
• Docs. ADR 234 "Normalizer stability"; docs/architecture docs/subsystems/7. Migration System.md, new section "Opaque SQL in DDL".

Compatibility / migration / risk

• Contracts and migrations: no committed artefact changes. Every statement for a body without -- is byte-identical.
• Schema authors: a body that has both a -- comment and a line break gets a new index, check or policy name once; the stored body does not change. The next migration plan drops and recreates the object. The fragment says so.
• Extension authors: the four node constructors take OpaqueSql; code reading .expression, .using, .withCheck or .where reads .text, and a read inside a template string or a call that accepts any value is not caught by the compiler, so search for them. DdlIndexElements changed shape and CreateIndexElements is exported. The contract-free factories (fn, checkExpression, createPolicy, createIndex) still take strings.

Testing performed

On the final HEAD:

• pnpm build, pnpm typecheck, pnpm lint, pnpm lint:deps, pnpm lint:casts (delta 0), pnpm lint:throws (delta 0), pnpm check:error-reference, pnpm lint:framework-vocabulary, pnpm fixtures:check (tree clean), pnpm check:upgrade-coverage --mode pr against the merge base
• pnpm test:packages: all files pass except the 3 tarball files (registry refusal above); one file that timed out under load passes alone
• Targeted integration files: the expression-index migration journey, the RLS journeys and the authoring parity tests. The full test:integration suite runs in CI.
• Two rounds of architect and code review, reports in projects/sql-expression-literals/slice-reviews/1/ on the project branch; every finding is fixed

Follow-ups

• Slice 2b (TML-3288) adds a body ending in a -- comment to its CLI journey once this merges.
• Slice 4 (TML-3290): generated migration files write SQL text as template literals.

Alternatives considered

• Strip trailing line comments from the body before rendering. Rejected: Prisma does not parse SQL, so it cannot tell a comment from -- inside a string constant, and the author's text should reach the database as written.
• Append the line break in each renderer's template string. Rejected: the same rule copied into nine templates, with nothing to stop the tenth from forgetting it. A typed node makes the rule the only way to render the text.
• Keep line breaks in every normalized body. Rejected: it would change every existing multi-line wire name. Keeping them only when -- is present changes no committed name; a user body with a comment and a line break gets a new name once.
• Change the contract-free factories to take OpaqueSql. Rejected: migration files already committed call them with strings and must keep loading.

Checklist

• All commits are signed off (git commit -s) per the DCO.
• I read CONTRIBUTING.md and the change is scoped to one logical concern.
• Tests are updated.
• The PR title is in TML-NNNN: <sentence-case title> form.
• The Skill update section above is filled in.

Notes for the reviewer

See Reviewer notes above.

Agent: charon-96

Summary by CodeRabbit

• Bug Fixes
  • SQL expressions containing line comments now retain line boundaries during rendering and content hashing, preventing comments from consuming subsequent SQL.
  • Improved handling of SQL expressions in defaults, constraints, indexes, and row-level security policies across PostgreSQL and SQLite.
• Migration Notes
  • Previously uncommitted objects with SQL containing both -- and a line break may receive new names when contracts are re-emitted. A migration plan drops and recreates each affected object once.
• Documentation
  • Added guidance on SQL line comments and their rendering and migration impact.