create/drop pitr internal

[iamlinjunhong]

User description

What type of PR is this?

• API-change
• BUG
• Improvement
• Documentation
• Feature
• Test and CI
• Code Refactoring

Which issue(s) this PR fixes:

issue #21914

What this PR does / why we need it:

create/drop pitr internal

---

PR Type

Enhancement

---

Description

• Add CREATE/DROP PITR internal commands support

• Implement PITR management for cluster, account, database, and table levels

• Add comprehensive test coverage for PITR operations

• Extend SQL parser with INTERNAL keyword support

---

Changes diagram

flowchart LR
  A["SQL Parser"] --> B["Plan Builder"]
  B --> C["Compile Engine"]
  C --> D["PITR Operations"]
  D --> E["mo_catalog.mo_pitr"]
  D --> F["sys_mo_catalog_pitr"]

Loading

---

Changes walkthrough 📝

	Relevant files
Enhancement	14 files ddl.go Add CreatePitr and DropPitr implementation                              +307/-0  build_ddl.go Add PITR plan building functions                                                  +124/-0  compile.go Add PITR operation routing                                                              +14/-0    pitr.go Add Internal flag to PITR structures                                          +8/-0      stmt.go Update PITR statement kind handling                                            +6/-0      types.go Add CreatePitr and DropPitr types                                                +4/-0      build.go Add PITR statement routing                                                              +4/-0      types.go Add GetAccountName interface method                                            +1/-0      keywords.go Add INTERNAL keyword support                                                          +1/-0      compiler_context.go Implement GetAccountName method                                                    +4/-0      sql_executor_context.go Add GetAccountName stub implementation                                      +3/-0      compiler_context.go Add GetAccountName stub implementation                                      +4/-0      mysql_sql.y Add INTERNAL keyword and grammar rules                                      +33/-11  plan.proto Add CreatePitr and DropPitr protobuf messages                        +26/-0
Tests	3 files ddl_test.go Add comprehensive PITR operation tests                                      +158/-0  types_mock.go Add GetAccountName mock method                                                      +4/-0      mock.go Add GetAccountName mock implementation                                      +4/-0
Additional files	2 files plan.pb.go +2314/-1186 mysql_sql.go +7737/-7679

---

Need help?

Type /help how to ... in the comments thread for any questions about Qodo Merge usage.

Check out the documentation for more information.

[qodo-code-review]

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 4 🔵🔵🔵🔵⚪

🧪 PR contains tests

🔒 Security concerns SQL injection: The code constructs SQL queries using fmt.Sprintf with user-provided input (pitr names, database names, table names) without proper sanitization or parameterized queries. This is particularly concerning in functions like CreatePitr and DropPitr where user input is directly interpolated into SQL strings. For example, line 3745-3771 shows direct string interpolation of user-controlled values like pitrName, createPitr.GetAccountName(), createPitr.GetDatabaseName(), etc. into SQL INSERT statements.

⚡ Recommended focus areas for review SQL Injection The CreatePitr and DropPitr functions construct SQL queries using string formatting with user-provided input without proper sanitization. This creates potential SQL injection vulnerabilities. sql := fmt.Sprintf(`insert into mo_catalog.mo_pitr( pitr_id, pitr_name, create_account, create_time, modified_time, level, account_id, account_name, database_name, table_name, obj_id, pitr_length, pitr_unit) values ('%s', '%s', %d, '%s', '%s', '%s', %d, '%s', '%s', '%s', %d, %d, '%s')`, newUUid, pitrName, createPitr.GetCurrentAccountId(), types.CurrentTimestamp().String2(time.UTC, 0), types.CurrentTimestamp().String2(time.UTC, 0), pitrLevel.String(), createPitr.GetCurrentAccountId(), createPitr.GetAccountName(), createPitr.GetDatabaseName(), createPitr.GetTableName(), getPitrObjectId(createPitr), createPitr.GetPitrValue(), createPitr.GetPitrUnit()) Error Handling The error messages in CreatePitr function are inconsistent and potentially misleading. Some error conditions return incorrect error messages that don't match the actual validation being performed. switch pitrLevel { case tree.PITRLEVELCLUSTER: return moerr.NewInternalError(c.proc.Ctx, "cluster level pitr already exists") case tree.PITRLEVELACCOUNT: return moerr.NewInternalErrorf(c.proc.Ctx, "account %s does not exist", createPitr.AccountName) case tree.PITRLEVELDATABASE: return moerr.NewInternalErrorf(c.proc.Ctx, "database `%s` already has a pitr", createPitr.DatabaseName) case tree.PITRLEVELTABLE: return moerr.NewInternalErrorf(c.proc.Ctx, "table %s.%s does not exist", createPitr.DatabaseName, createPitr.TableName) } } Resource Management The result sets from SQL queries are not consistently closed, which could lead to resource leaks. Some defer statements are missing for proper cleanup. sysPitrSql := "select pitr_length, pitr_unit from mo_catalog.mo_pitr where pitr_name = '" + sysMoCatalogPitr + "'" sysRes, err := c.runSqlWithResult(sysPitrSql, sysAccountId) if err != nil { return err } defer sysRes.Close()

[qodo-code-review]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
Security	Prevent SQL injection vulnerability The SQL query is vulnerable to SQL injection attacks because user-controlled values are directly interpolated into the query string. Use parameterized queries or proper escaping to prevent SQL injection vulnerabilities. pkg/sql/compile/ddl.go [3745-3771] -sql := fmt.Sprintf(`insert into mo_catalog.mo_pitr( +sql := `insert into mo_catalog.mo_pitr( pitr_id, pitr_name, create_account, create_time, modified_time, level, account_id, account_name, database_name, table_name, obj_id, pitr_length, - pitr_unit) values ('%s', '%s', %d, '%s', '%s', '%s', %d, '%s', '%s', '%s', %d, %d, '%s')`, - newUUid, - pitrName, - createPitr.GetCurrentAccountId(), - types.CurrentTimestamp().String2(time.UTC, 0), - types.CurrentTimestamp().String2(time.UTC, 0), - pitrLevel.String(), - createPitr.GetCurrentAccountId(), - createPitr.GetAccountName(), - createPitr.GetDatabaseName(), - createPitr.GetTableName(), - getPitrObjectId(createPitr), - createPitr.GetPitrValue(), - createPitr.GetPitrUnit()) + pitr_unit) values (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)` +err = c.runSqlWithParams(sql, newUUid, pitrName, createPitr.GetCurrentAccountId(), + types.CurrentTimestamp().String2(time.UTC, 0), types.CurrentTimestamp().String2(time.UTC, 0), + pitrLevel.String(), createPitr.GetCurrentAccountId(), createPitr.GetAccountName(), + createPitr.GetDatabaseName(), createPitr.GetTableName(), getPitrObjectId(createPitr), + createPitr.GetPitrValue(), createPitr.GetPitrUnit()) + Apply / Chat Suggestion importance[1-10]: 10 __ Why: The suggestion correctly identifies a critical SQL injection vulnerability by using fmt.Sprintf with user-provided data and proposes the standard, secure practice of using parameterized queries.	High
Possible issue	Fix resource leak issue The result set res is not properly closed, which can lead to resource leaks. Always ensure database result sets are closed using defer res.Close() immediately after checking for errors. pkg/sql/compile/ddl.go [3715-3719] checkSql := getSqlForCheckPitrDup(createPitr) res, err := c.runSqlWithResult(checkSql, int32(accountId)) if err != nil { return err } +defer res.Close() Apply / Chat Suggestion importance[1-10]: 8 __ Why: The suggestion correctly identifies a resource leak where the result set res is not closed on all execution paths, which could lead to resource exhaustion.	Medium
General	Fix inconsistent error messages The error messages are inconsistent with the actual condition being checked. When a PITR already exists, the messages should indicate "already exists" rather than "does not exist" for account and table levels. pkg/sql/compile/ddl.go [3727-3732] case tree.PITRLEVELACCOUNT: - return moerr.NewInternalErrorf(c.proc.Ctx, "account %s does not exist", createPitr.AccountName) + return moerr.NewInternalErrorf(c.proc.Ctx, "account level pitr for %s already exists", createPitr.AccountName) case tree.PITRLEVELDATABASE: return moerr.NewInternalErrorf(c.proc.Ctx, "database `%s` already has a pitr", createPitr.DatabaseName) case tree.PITRLEVELTABLE: - return moerr.NewInternalErrorf(c.proc.Ctx, "table %s.%s does not exist", createPitr.DatabaseName, createPitr.TableName) + return moerr.NewInternalErrorf(c.proc.Ctx, "table level pitr for %s.%s already exists", createPitr.DatabaseName, createPitr.TableName) Apply / Chat Suggestion importance[1-10]: 7 __ Why: The suggestion correctly identifies misleading error messages for PITRLEVELACCOUNT and PITRLEVELTABLE, improving clarity and correctness for developers and users.	Medium
Update