…tecture (#18529)

* docs: add design for advisor statement text architecture fix

Document the bug where extractStatementText uses relative line numbers
from ANTLR but indexes into the full SQL text, and the solution to pass
ParsedStatements (with per-statement Text) instead of just ASTs.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <[email protected]>

* docs: add implementation plan for advisor statement text fix

Detailed task-by-task plan for migrating advisors to use per-statement
ParsedStatements.Text instead of extractStatementText() with line numbers.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <[email protected]>

* feat(advisor): add ParsedStatements field to Context

Adds ParsedStatements []base.ParsedStatement to advisor.Context to provide
per-statement text alongside AST, fixing the statement text extraction bug.

Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <[email protected]>

* feat(advisor): populate ParsedStatements in SQLReviewCheck

Pass the full ParsedStatement slice to advisors, making per-statement
text available without line-number-based extraction.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <[email protected]>

* feat(advisor/pg): add getParsedStatements helper

Adds ParsedStatementInfo struct and getParsedStatements() function that
provides per-statement text directly, eliminating need for line-based
text extraction.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <[email protected]>

* refactor(advisor/pg): migrate where-required-select to use statement text

Use getParsedStatements() and per-statement Text field instead of
extractStatementText() with line numbers.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <[email protected]>

* refactor(advisor/pg): migrate where-required-update-delete to use statement text

Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <[email protected]>

* refactor(advisor/pg): migrate no-select-all to use statement text

Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <[email protected]>

* refactor(advisor/pg): migrate no-leading-wildcard-like to use statement text

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <[email protected]>

* refactor(advisor/pg): migrate non-transactional to use statement text

Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <[email protected]>

* refactor(advisor/pg): migrate disallow-commit to use statement text

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <[email protected]>

* refactor(advisor/pg): migrate disallow-on-del-cascade to use statement text

Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <[email protected]>

* refactor(advisor/pg): migrate affected-row-limit to use statement text

Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <[email protected]>

* refactor(advisor/pg): migrate dml-dry-run to use statement text

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <[email protected]>

* refactor(advisor/pg): migrate table-require-pk to use statement text

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <[email protected]>

* refactor(advisor/pg): migrate table-no-fk to use statement text

Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <[email protected]>

* refactor(advisor/pg): migrate disallow-partition to use statement text

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <[email protected]>

* refactor(advisor/pg): migrate table-comment-convention to use statement text

Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <[email protected]>

* refactor(advisor/pg): migrate naming-fully-qualified to use statement text

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <[email protected]>

* refactor(advisor/pg): migrate naming-primary-key-convention to use statement text

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <[email protected]>

* refactor(advisor/pg): migrate migration-compatibility to use statement text

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <[email protected]>

* refactor(advisor/pg): migrate insert-row-limit to use statement text

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <[email protected]>

* refactor(advisor/pg): migrate insert-must-specify-column to use statement text

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <[email protected]>

* refactor(advisor/pg): migrate insert-disallow-order-by-rand to use statement text

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <[email protected]>

* refactor(advisor/pg): migrate builtin-prior-backup-check to use statement text

Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <[email protected]>

* refactor(advisor/pg): remove extractStatementText function

No longer needed now that all advisors use per-statement Text directly.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <[email protected]>

* docs(advisor): add deprecation comments to AST and Statements fields

Mark AST and Statements fields as deprecated in favor of ParsedStatements.
These fields are still populated for backward compatibility with non-PG
advisors, but new code should use ParsedStatements for accessing
per-statement text.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <[email protected]>

* fix(advisor/pg): skip empty statements in getParsedStatements

Empty statements (e.g., trailing semicolons) have nil AST. Skip them
instead of returning an error, matching the behavior of ExtractASTs.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <[email protected]>

* refactor(advisor/pg): add getTextFromTokens helper using GetTextFromRuleContext

Add helper function that uses GetTextFromRuleContext to extract text
including hidden channel tokens (whitespace, comments). This provides
clean text extraction from token stream.

For statement text display, use strings.TrimSpace on statementText to
handle leading/trailing newlines while preserving full statement context.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <[email protected]>

* refactor(advisor/pg): use getTextFromTokens for clean statement text

Update advisor_statement_where_required_select to store tokens and use
getTextFromTokens(tokens, ctx) to extract clean text from the token
stream. This provides more precise error messages showing the specific
context that violated the rule.

Update test expectation to reflect new behavior: subquery violations
now show the subquery text instead of the full statement.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <[email protected]>

* refactor(advisor/pg): use getTextFromTokens for clean statement text in error messages

Updated PostgreSQL advisors to use tokens + getTextFromTokens() pattern
for generating error messages with clean statement text.

Changes:
- Add tokens *antlr.CommonTokenStream field to rule structs
- Use getTextFromTokens(r.tokens, ctx) for error message content
- Remove unused statementText fields where applicable
- Update test expectations (no trailing semicolons in extracted text)

Files updated:
- advisor_table_no_fk.go
- advisor_table_disallow_partition.go
- advisor_migration_compatibility.go
- advisor_insert_must_specify_column.go
- advisor_insert_disallow_order_by_rand.go
- advisor_builtin_prior_backup_check.go
- advisor_naming_primary_key_convention.go (removed unused field)
- advisor_table_require_pk.go (fixed handleCreatestmt)

Files that keep statementText (for functional reasons):
- advisor_naming_fully_qualified.go (re-parses statement)
- advisor_insert_row_limit.go (EXPLAIN queries)
- advisor_statement_dml_dry_run.go (EXPLAIN queries)
- advisor_statement_affected_row_limit.go (EXPLAIN queries)
- advisor_statement_disallow_on_del_cascade.go (position conversion)
- advisor_statement_non_transactional.go (IsNonTransactionStatement)

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <[email protected]>

* refactor(advisor): add getParsedStatements helper for MySQL, TiDB, Oracle, MSSQL, Snowflake

Add ParsedStatementInfo struct and getParsedStatements() helper function
to all database engines (MySQL, TiDB, Oracle, MSSQL, Snowflake) following
the PostgreSQL pattern.

This provides a unified way to access statement information including:
- Tree/Node: the parsed AST
- Tokens: for ANTLR-based engines (MySQL, Oracle, MSSQL, Snowflake)
- BaseLine: the line offset for error reporting
- Text: the original statement text

Each engine also gets getTextFromTokens() (ANTLR-based) or uses node.Text()
(TiDB) to extract clean text from specific AST contexts.

All new functions are marked with nolint:unused as they will be used
in Phase 2 when advisors are updated to use these helpers.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <[email protected]>

* refactor(advisor): remove deprecated AST field from Context

- Remove AST field from advisor.Context struct
- Update getANTLRTree() in all engines (MySQL, PostgreSQL, Oracle,
  MSSQL, Snowflake, OceanBase, Redshift) to use ParsedStatements
- Update getTiDBNodes() to use ParsedStatements
- Remove checkContext.AST assignment in sql_review.go
- Remove fallback code and getParsedStatementsFromAST() functions
- Fix Oracle SplitSQL to properly set BaseLine field

This completes the migration from the deprecated AST field to
ParsedStatements, which provides per-statement text directly
without needing line-number-based extraction.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <[email protected]>

* refactor(advisor): remove unused Charset and Collation fields from Context

These fields were set but never read by any advisor. Advisors that
deal with charset/collation get this information from AST nodes or
column metadata instead.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <[email protected]>

* refactor(advisor): remove FullStatement field from Context

Replace FullStatement with:
- StatementsTotalSize for size limit checks
- Direct use of ParsedStatements for prepareTransformation functions
- Simplified line-only position conversion

Changes:
- Add StatementsTotalSize field to Context for size checks
- Refactor prepareTransformation in PG, MSSQL, Oracle to accept
  []*base.ParseResult instead of re-parsing the statement string
- Simplify MySQL rule_online_migration to use ConvertANTLRLineToPosition
- Remove FullStatement field and its assignment

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <[email protected]>

* refactor(advisor): remove unused helper functions from engine utils

Remove unused getParsedStatements, getTextFromTokens, and
ParsedStatementInfo from MSSQL, MySQL, Oracle, Snowflake, and TiDB.
These were scaffolding for a future migration that was superseded
by the current approach. PostgreSQL retains these as they are
actively used.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <[email protected]>

* fix(plsql): update split test to expect BaseLine values

Update TestOracleSplitMultiSQL to include BaseLine expectations
after the previous commit that added BaseLine calculation to
Oracle SplitSQL.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <[email protected]>

* fix(tests): update test expectations for preserved statement formatting

Update test data to expect newlines preserved in statement text instead
of spaces. This reflects the correct behavior where original SQL
formatting is maintained in error messages.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <[email protected]>

* refactor(advisor): add unified GetANTLRParseResults function

Add a unified GetANTLRParseResults function in the advisor package
that extracts ANTLR parse results from the advisor context. All
ANTLR-based engines (MySQL, MSSQL, Oracle, Snowflake, PostgreSQL,
OceanBase) now use this shared implementation instead of duplicating
the same logic in each engine's utils.go.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <[email protected]>

* refactor(advisor): remove wrapper getANTLRTree functions

All ANTLR-based engines now directly use advisor.GetANTLRParseResults()
instead of individual wrapper functions. This eliminates code duplication.

- Remove utils.go files from MSSQL, Oracle, Snowflake, OceanBase (only had wrapper)
- Remove getANTLRTree wrapper from MySQL and PostgreSQL utils.go
- Update all advisor files to use advisor.GetANTLRParseResults() directly
- Redshift keeps its own getANTLRTree (different return type - single tree)

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <[email protected]>

* refactor(advisor/pg): remove ParsedStatementInfo, use unified GetANTLRParseResults

- Replace getParsedStatements() with advisor.GetANTLRParseResults()
- Remove ParsedStatementInfo struct (use base.ParseResult directly)
- Extract statement text on-demand using getTextFromTokens()
- Clean up utils.go imports

This makes PostgreSQL advisors consistent with other engines by:
1. Using the unified GetANTLRParseResults() from advisor package
2. Not storing statement text upfront, extracting it when needed

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <[email protected]>

* fix(advisor): correct line position in generateFinalAdvice

When generateFinalAdvice/validateFinalState is called after walking all
statements, r.baseLine is set to the last statement's baseline. However,
AddAdvice adds r.baseLine to the position, which is wrong for tables
defined in earlier statements.

Fix by:
1. Store absolute line numbers (line + baseLine) at time of encounter
2. Directly append to adviceList in generateFinalAdvice, bypassing
   AddAdvice's automatic baseLine offset

Affected files:
- mssql/rule_table_require_pk.go
- mssql/rule_table_no_foreign_key.go
- pg/advisor_table_require_pk.go

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <[email protected]>

* fix(advisor): correct line position in Snowflake and PG advisors

Fix the same line position bug in two more advisors:

1. snowflake/rule_table_require_pk.go
2. pg/advisor_index_total_number_limit.go

The bug: When generateFinalAdvice/GetAdviceList is called after walking
all statements, r.baseLine is set to the last statement's baseline.
However, the stored line numbers were relative, causing incorrect
positions for tables defined in earlier statements.

Fix by:
1. Store absolute line numbers (line + baseLine) at time of encounter
2. Directly append to adviceList in post-walk functions, bypassing
   AddAdvice's automatic baseLine offset

Also added test cases to verify correct multi-statement line positions.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <[email protected]>

---------

Co-authored-by: Claude Opus 4.5 <[email protected]>

… settings (#18531)

Add Chinese, Japanese, Spanish, and Vietnamese translations for "Require issue approval" and "Require plan check no error" settings to ensure all locales have proper translations instead of displaying English fallback text.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-authored-by: Claude Sonnet 4.5 <[email protected]>

* fix: instance environment filter

* fix: reset selected value

(cherry picked from commit 90b54dd)

Co-authored-by: ecmadao <[email protected]>

(cherry picked from commit eeff481)

Co-authored-by: ecmadao <[email protected]>

The rollout list date filter was failing because the WHERE clause
tried to reference `updated_at` which is a computed column alias
in the SELECT clause. SQL doesn't allow referencing column aliases
in the WHERE clause of the same query.

Fix by using the same COALESCE subquery expression in the filter
instead of the column alias.

Also changed frontend to use toISOString() for more reliable
RFC3339 date formatting.

Added unit tests for GetListRolloutFilter.

🤖 Generated with [Claude Code](https://claude.com/claude-code)


(cherry picked from commit bc60fd7)

Co-authored-by: boojack <[email protected]>
Co-authored-by: Claude Opus 4.5 <[email protected]>

(cherry picked from commit be2b3db)

Co-authored-by: ecmadao <[email protected]>

Issue labels are properties of Issue, not Plan. Previously they were
shown on the Plan page which was confusing. Now labels are selected
in a popover when clicking "Ready for Review", making the two-step
Plan → Issue creation flow clearer.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-authored-by: Claude <[email protected]>