Release/260408#73
Merged
Merged
Conversation
…sive guards - Add ErrorBoundary component to catch render errors at root and tab level, replacing white screen with a visible error message - Fix useEffect dep loop in TableView: remove columnWidths from calculateAutoColumnWidths effect deps and read via ref instead, eliminating extra re-renders on data load - Add null/non-object row guard in TableView render loop to prevent TypeError crashing the render when data contains unexpected values Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Replace the single "Export as SQL" right-click menu item with three separate options — Export DML (INSERT), Export DDL (CREATE), and Export DML & DDL — so users can choose exactly what to export. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
MSSQL DDL is rebuilt from column metadata and lacks foreign keys, unique constraints, check constraints, and indexes. Add a SQL comment block at the top of the output to warn users of this limitation. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Extract do_table_export() to replace ~110 lines duplicated between export_table_data and export_table_data_direct. Remove columns param from ExportWriter::new (CSV header now written via write_csv_header), so get_table_metadata is only called when actually fetching rows — SqlDdl-only mode no longer makes an unnecessary metadata round-trip. Replace dead SqlDdl arm in write_row with unreachable!(). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Freeze bun version to match local development environment, preventing --frozen-lockfile errors due to version differences.
Add a settings switch (default off) to show field comments in small text below the column name in table headers. Comments are truncated to avoid affecting column width. Supports en/zh/ja i18n. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Move the upgradelink-upload job out of release.yml into a dedicated workflow (.github/workflows/upgradelink-upload.yml) triggered manually via workflow_dispatch with a required tag input. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
📝 WalkthroughWalkthroughThis PR adds comprehensive StarRocks database driver support, enhances SQL export with DML/DDL/Full variants, implements MySQL charset/collation APIs, adds PostgreSQL dollar-quote parsing, introduces UI features like column comments and error boundaries, reorganizes the release workflow, pins Bun version, and includes associated test coverage and localizations. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant UI as Connection Dialog
participant API as Frontend API
participant Tauri as Tauri Command
participant DB as MySQL/StarRocks
User->>UI: Select MySQL-family driver
User->>UI: Open "Create Database" dialog
Note over UI: Load charset/collation options
UI->>API: getMysqlCharsets(connectionId)
API->>Tauri: invoke("get_mysql_charsets_by_id", {id})
Tauri->>DB: SHOW CHARACTER SET
DB-->>Tauri: charset list
Tauri-->>API: [charset1, charset2, ...]
API-->>UI: charsets populated
User->>UI: Select charset from dropdown
UI->>API: getMysqlCollations(connectionId, charset)
API->>Tauri: invoke("get_mysql_collations_by_id", {id, charset})
Tauri->>DB: SHOW COLLATION WHERE Charset = ?
DB-->>Tauri: collation list
Tauri-->>API: [collation1, collation2, ...]
API-->>UI: collations populated & filtered
User->>UI: Create database
UI-->>User: Database created with selected charset/collation
sequenceDiagram
actor User
participant TableView
participant ExportAPI
participant Tauri
participant FileSystem
User->>TableView: Right-click table → Export
Note over TableView: New export format options
alt Export SQL DML
User->>TableView: Click "exportSqlDml"
TableView->>ExportAPI: export_table_data(format: "sql_dml")
ExportAPI->>Tauri: Fetch table rows only
else Export SQL DDL
User->>TableView: Click "exportSqlDdl"
TableView->>ExportAPI: export_table_data(format: "sql_ddl")
ExportAPI->>Tauri: Fetch table DDL via get_table_ddl()
Tauri-->>ExportAPI: CREATE TABLE statement
else Export SQL Full
User->>TableView: Click "exportSqlFull"
TableView->>ExportAPI: export_table_data(format: "sql_full")
ExportAPI->>Tauri: Fetch DDL + rows (do_table_export helper)
end
ExportAPI->>FileSystem: Write .sql file
FileSystem-->>User: Export complete
Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly Related PRs
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
CI Feedback 🧐A test triggered by this PR failed. Here is an AI-generated analysis of the failure:
|
Review Summary by QodoRelease 0.3.3 - StarRocks support, SQL export refactoring, and advanced parsing enhancements
WalkthroughsDescription• **StarRocks driver support**: Added comprehensive StarRocks database driver integration with MySQL-compatible connection options, custom port defaults (9030), and SSH tunnel configuration • **SQL export enhancements**: Refactored export logic into reusable do_table_export function and split ExportFormat::Sql into three variants (SqlDml, SqlDdl, SqlFull) for granular control • **Advanced SQL parsing**: Enhanced statement parsing to support MySQL DELIMITER commands, SQLite trigger blocks, Oracle PL/SQL blocks with / terminators, and PostgreSQL dollar-quoted strings • **MySQL charset/collation API**: Added new Tauri commands get_mysql_charsets_by_id and get_mysql_collations_by_id with dynamic loading in UI • **Column comments feature**: Implemented header interaction system with single/double-click distinction for copying column names and sorting, plus persistent column comments display setting • **Developer tools**: Added Developer menu with Reload and Toggle DevTools options for enhanced debugging • **Comprehensive test coverage**: Added integration tests for StarRocks, MySQL delimiter scripts, Oracle PL/SQL procedures, SQLite triggers, PostgreSQL functions, and MariaDB charset operations • **Error handling**: Introduced ErrorBoundary component for graceful error display and added StarRocks error handling in transaction SQL import flow • **UI/UX improvements**: Dynamic MySQL options loading, expanded collation lists for MSSQL and PostgreSQL, improved code formatting and accessibility • **Version bump**: Updated to version 0.3.3 with Bun version pinning to 1.3.10 across CI/CD workflows Diagramflowchart LR
A["StarRocks Driver"] -->|"MySQL-compatible"| B["Connection Pool"]
C["SQL Export"] -->|"Refactored"| D["DML/DDL/Full Variants"]
E["SQL Parsing"] -->|"Enhanced"| F["MySQL DELIMITER<br/>Oracle PL/SQL<br/>PostgreSQL $quotes<br/>SQLite Triggers"]
G["MySQL API"] -->|"New Commands"| H["Charsets & Collations"]
I["UI Features"] -->|"Column Comments<br/>Click Handler"| J["TableView Component"]
K["Error Handling"] -->|"ErrorBoundary"| L["Graceful Errors"]
M["Tests"] -->|"Integration Coverage"| N["StarRocks/MySQL/Oracle<br/>PostgreSQL/SQLite/MariaDB"]
File Changes1. src-tauri/src/commands/transfer.rs
|
Code Review by Qodo
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
Comment on lines
+174
to
+176
| fn build_connect_options(dsn: &str, driver: &str) -> Result<MySqlConnectOptions, String> { | ||
| let mut options = | ||
| MySqlConnectOptions::from_str(dsn).map_err(|e| format!("[CONN_FAILED] {e}"))?; |
There was a problem hiding this comment.
1. build_connect_options() bypasses conn_failed_error() 📘 Rule violation ☼ Reliability
build_connect_options() maps DSN parse failures to a raw [CONN_FAILED] {e} string instead of
using conn_failed_error(), producing inconsistent and less helpful connection-failure messaging.
This breaks the required standardized, hint-augmented error format for connection failures.
Agent Prompt
## Issue description
`src-tauri/src/db/drivers/mysql.rs` returns a generic `[CONN_FAILED] {e}` string when parsing MySQL connection options fails, instead of using the standardized `conn_failed_error()` helper.
## Issue Context
Compliance requires consistent, hint-augmented connection failure messages generated via `conn_failed_error()`.
## Fix Focus Areas
- src-tauri/src/db/drivers/mysql.rs[174-176]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
1373
to
+1376
| let ch = chars[i]; | ||
| let next = chars.get(i + 1).copied(); | ||
| let delimiter_chars: Vec<char> = delimiter.chars().collect(); | ||
|
|
There was a problem hiding this comment.
2. Delimiter alloc per character 🐞 Bug ➹ Performance
parse_sql_statements() allocates a new Vec<char> for the current delimiter on every loop iteration, causing excessive allocations and potentially making large SQL imports extremely slow or appear hung. This is on the hot path for SQL import and is amplified by the 20MB file-size allowance.
Agent Prompt
### Issue description
`parse_sql_statements()` allocates `delimiter_chars: Vec<char>` inside the per-character scan loop, causing massive allocation overhead for large SQL files.
### Issue Context
This function is used by SQL import planning; imports allow up to 20MB, so the current implementation can trigger millions of allocations.
### Fix Focus Areas
- src-tauri/src/commands/transfer.rs[1334-1400]
### Implementation notes
- Move `delimiter_chars` out of the loop (e.g., `let mut delimiter_chars = vec![';'];`).
- When a MySQL `DELIMITER` command is parsed and `delimiter` changes, also refresh `delimiter_chars = delimiter.chars().collect()`.
- Inside the loop, use the cached `delimiter_chars` without reallocating.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 15
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src-tauri/src/connection_input/mod.rs (1)
57-62:⚠️ Potential issue | 🟡 MinorRe-validate port after parsing embedded
host:port.Line 53 validates
form.portbefore parsing, but Line 59 can overwrite it with an embedded value.db:70000currently bypasses range checks.🛠️ Proposed fix
if crate::db::drivers::is_mysql_family_driver(&driver) { if let Some(host) = form.host.clone() { let (normalized_host, normalized_port) = parse_host_embedded_port(&host, form.port); form.host = Some(normalized_host); form.port = normalized_port.or(form.port); + validate_port_range("port", form.port)?; } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src-tauri/src/connection_input/mod.rs` around lines 57 - 62, After parsing an embedded host:port via parse_host_embedded_port in the is_mysql_family_driver branch, re-run the port range validation on the potentially updated form.port (the value assigned to form.port after parse_host_embedded_port) and reject or clear values that fall outside the valid TCP port range (1–65535); update the code that originally validated form.port before parsing to also validate the post-parse value so inputs like "db:70000" do not bypass checks. Ensure you reference the same validation logic (or helper) when validating form.port after parse_host_embedded_port to keep behavior consistent with the initial check on form.port.
🧹 Nitpick comments (11)
src-tauri/src/commands/query.rs (1)
79-110: Extract dollar-quote parser helpers into shared SQL utility.
parse_dollar_quote_tag/skip_dollar_quoteare now duplicated withsrc-tauri/src/db/drivers/postgres.rs; centralizing avoids parser drift between command guards and driver splitting logic.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src-tauri/src/commands/query.rs` around lines 79 - 110, parse_dollar_quote_tag and skip_dollar_quote are duplicated; extract their logic into a shared SQL utility module (e.g., create a new module like sql::dollar_quote) and move the implementations there, then replace the local functions in src-tauri/src/commands/query.rs (parse_dollar_quote_tag, skip_dollar_quote) and the duplicate in src-tauri/src/db/drivers/postgres.rs to call the shared helpers instead; ensure the shared functions keep the same signatures (accepting &[u8] and usize) and update imports/usages accordingly so both command guards and driver parsing use the single centralized implementation.src-tauri/tests/common/shared.rs (1)
8-11: Remove redundant#[allow(dead_code)]on active retry helpers.These items are already used; keeping suppression here weakens dead-code signal for future changes.
🧹 Proposed patch
-#[allow(dead_code)] const CONNECT_RETRY_ATTEMPTS: usize = 20; -#[allow(dead_code)] const CONNECT_RETRY_DELAY_MS: u64 = 500; @@ -#[allow(dead_code)] pub async fn connect_with_retry<T, F, Fut>(mut connect: F) -> TAlso applies to: 93-94
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src-tauri/tests/common/shared.rs` around lines 8 - 11, Remove the redundant #[allow(dead_code)] attributes from the active retry helper constants CONNECT_RETRY_ATTEMPTS and CONNECT_RETRY_DELAY_MS by deleting the attribute lines immediately above those const declarations; also remove the same #[allow(dead_code)] suppression from any other retry-related constants in this module (the other retry helper constants) so that dead-code warnings remain meaningful for future changes.src/components/business/Sidebar/Sidebar.tsx (1)
42-43: Use shared export format type instead of inline union.Defining the format union inline here can drift from
src/services/api.ts. Prefer reusing the sharedTransferFormattype.♻️ Proposed refactor
-import { ConnectionForm, SavedQuery } from "@/services/api"; +import { ConnectionForm, SavedQuery, type TransferFormat } from "@/services/api"; ... - format: "csv" | "json" | "sql_dml" | "sql_ddl" | "sql_full", + format: TransferFormat,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/business/Sidebar/Sidebar.tsx` around lines 42 - 43, Replace the inline union type for the "format" prop with the shared TransferFormat type from src/services/api.ts: import TransferFormat into Sidebar (or the module where the prop/type is declared) and change the property signature from format: "csv" | "json" | "sql_dml" | "sql_ddl" | "sql_full" to format: TransferFormat; ensure any usages of format (e.g., in Sidebar component, prop interfaces or function signatures) compile after updating the import.src/App.tsx (2)
1709-1730: Duplicate schema derivation logic.The schema derivation logic at lines 1717-1725 duplicates the
resolveTableScopefunction defined at lines 185-204. Consider reusingresolveTableScopeto maintain consistency and reduce duplication.♻️ Suggested refactor to reuse resolveTableScope
tableContext={ tab.connectionId && tab.database && tab.tableName && tab.driver ? { connectionId: tab.connectionId, database: tab.database, - schema: - isMysqlFamilyDriver(tab.driver as any) || - tab.driver === "clickhouse" - ? tab.database - : tab.driver === "mssql" - ? "dbo" - : tab.driver === "duckdb" - ? "main" - : "public", + schema: resolveTableScope( + tab.driver, + tab.database, + tab.schema, + ).schema, table: tab.tableName, driver: tab.driver, } : undefined }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/App.tsx` around lines 1709 - 1730, The inline schema derivation duplicates resolveTableScope; update the tableContext construction for the tab (where tableContext is set) to call the existing resolveTableScope function instead of reimplementing the conditional chain: pass the tab.driver and tab.database (and any other expected params) into resolveTableScope to obtain the schema/scope, then use that returned value for the schema field while keeping the other fields (connectionId, database, table, driver) as before; this will eliminate the duplicated logic and keep behavior consistent with resolveTableScope.
190-191: Type castas anybypasses type safety.The cast
isMysqlFamilyDriver(driver as any)bypasses TypeScript's type checking. Sincedriveris astringbutisMysqlFamilyDriverexpectsDriver, consider adding a type guard to maintain type safety.♻️ Optional: Add a safe wrapper in driver-registry
// In src/lib/driver-registry.tsx export const isMysqlFamilyDriverSafe = (driver: string | undefined): boolean => { const config = DRIVER_REGISTRY.find((d) => d.id === driver); return config?.isMysqlFamily ?? false; };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/App.tsx` around lines 190 - 191, The current expression using a type cast bypasses type safety: replace the as any cast by adding a safe string-based guard and using it in the isDatabaseScoped calculation; implement a helper like isMysqlFamilyDriverSafe(driver: string | undefined) in driver-registry that looks up DRIVER_REGISTRY by id and returns config?.isMysqlFamily ?? false, export it, then update the isDatabaseScoped computation to use isMysqlFamilyDriverSafe(driver) || driver === "clickhouse" (referencing the isDatabaseScoped variable and isMysqlFamilyDriver/isMysqlFamilyDriverSafe helpers to locate the code).src/components/ErrorBoundary.tsx (1)
14-46: Consider adding a reset mechanism for error recovery.The ErrorBoundary implementation is correct, but once an error is caught, there's no way to reset the error state. This means if a tab errors, the user must close and reopen it. Consider adding a key-based reset or a "Try Again" button.
♻️ Optional: Add reset capability
interface Props { children: ReactNode; /** Custom fallback rendered instead of the default error UI. */ fallback?: ReactNode; + /** Key to reset error state when changed. */ + resetKey?: string | number; } interface State { error: Error | null; } export class ErrorBoundary extends Component<Props, State> { state: State = { error: null }; static getDerivedStateFromError(error: Error): State { return { error }; } + componentDidUpdate(prevProps: Props) { + if (this.state.error && prevProps.resetKey !== this.props.resetKey) { + this.setState({ error: null }); + } + } + componentDidCatch(error: Error, info: ErrorInfo) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/ErrorBoundary.tsx` around lines 14 - 46, ErrorBoundary currently sets error in getDerivedStateFromError and never clears it, preventing recovery; add a reset mechanism by introducing a resetErrorBoundary method that sets state.error back to null and call it from either (a) a "Try Again" button rendered when this.props.fallback is absent or (b) when a new resetKey prop changes (watch resetKey in componentDidUpdate to clear error). Update render to show a small "Try Again" button alongside the error UI that invokes resetErrorBoundary, and expose resetErrorBoundary as a public method if parent components should trigger resets programmatically; modify Prop types to include optional fallback and optional resetKey as needed.src-tauri/tests/common/starrocks_context.rs (2)
22-30: Consider increasing the WaitFor timeout for StarRocks.StarRocks can take longer than 20 seconds to fully initialize. While
wait_for_porthas a 120-second timeout, the container'sWaitFor::seconds(20)might cause premature port exposure before the service is ready. Consider increasing this or using a more robust readiness check if tests become flaky.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src-tauri/tests/common/starrocks_context.rs` around lines 22 - 30, The current GenericImage construction uses WaitFor::seconds(20) which may be too short for StarRocks; update the readiness handling by increasing the wait timeout (e.g., to 120s or higher) on the with_wait_for call (replace WaitFor::seconds(20) with a larger value) or replace the simple seconds-based wait with a more robust readiness check (e.g., waiting for a specific log line or a successful TCP response) so that GenericImage::with_wait_for/WaitFor usage aligns with the longer shared::wait_for_port(Duration::from_secs(120)) check.
44-56: Redundant environment override call.
starrocks_form_from_local_envalready reads from environment variables (lines 47-51), thenapply_starrocks_env_overridesis called again on line 54. Sinceapply_starrocks_env_overrideschecks the same environment variables, this second call is redundant.♻️ Suggested fix
fn starrocks_form_from_local_env() -> ConnectionForm { let mut form = ConnectionForm { driver: "starrocks".to_string(), host: Some(shared::env_or("STARROCKS_HOST", "localhost")), port: Some(shared::env_i64("STARROCKS_PORT", 9030)), username: Some(shared::env_or("STARROCKS_USER", "root")), password: Some(shared::env_or("STARROCKS_PASSWORD", "")), database: env::var("STARROCKS_DB").ok(), ..Default::default() }; - apply_starrocks_env_overrides(&mut form); form }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src-tauri/tests/common/starrocks_context.rs` around lines 44 - 56, The function starrocks_form_from_local_env currently constructs ConnectionForm by reading environment variables directly and then redundantly calls apply_starrocks_env_overrides(&mut form); remove that redundant call (or alternatively initialize form with Default::default() and let apply_starrocks_env_overrides populate fields) so that only one path reads and applies the STARROCKS_* environment variables; update starrocks_form_from_local_env to either rely solely on the inline env_or/env_i64/env::var assignments or solely on apply_starrocks_env_overrides, and delete the extra apply_starrocks_env_overrides invocation accordingly.src/components/business/DataGrid/tableView/utils.ts (2)
177-188: Consider typing thedriverparameter to avoidas anycast.The
as anycast suggests a type mismatch between the function parameter and whatisMysqlFamilyDriverexpects. Consider updating the parameter type or theisMysqlFamilyDriverfunction signature to accept the same type.♻️ Suggested improvement
-export function quoteIdent(driver: string | undefined, name: string): string { +export function quoteIdent(driver: string | undefined | null, name: string): string { if ( - (driver && isMysqlFamilyDriver(driver as any)) || + (driver && isMysqlFamilyDriver(driver)) || driver === "clickhouse" ) {This would require updating
isMysqlFamilyDriverto acceptstringinstead of a more restricted type.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/business/DataGrid/tableView/utils.ts` around lines 177 - 188, The quoteIdent function uses an unsafe cast `as any` because the driver parameter's type doesn't match isMysqlFamilyDriver's expected type; update the signature of quoteIdent(driver: string | undefined, name: string) to use the exact type isMysqlFamilyDriver accepts (or change isMysqlFamilyDriver to accept string | undefined) so you can call isMysqlFamilyDriver(driver) without casting, then remove the `as any` cast in quoteIdent and adjust any callers if needed; reference the quoteIdent function and isMysqlFamilyDriver to locate and align their types.
280-287: Same type cast issue as inquoteIdent.This is the same
as anypattern. Consider addressing both locations together.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/business/DataGrid/tableView/utils.ts` around lines 280 - 287, The getQualifiedTableName function uses an unsafe "as any" cast when calling isMysqlFamilyDriver (same issue exists in quoteIdent); fix by aligning types instead of casting—either change the driver parameter type on getQualifiedTableName and quoteIdent to the proper DriverType/enum used across the module or update isMysqlFamilyDriver's signature to accept a string (or the actual driver type) so you can call isMysqlFamilyDriver(driver) without casting; remove the "as any" casts and adjust imports/types accordingly so the call sites and helper signatures share a consistent driver type.src-tauri/tests/postgres_stateful_command_integration.rs (1)
819-823: Maketouch_countassertion type-robustAt Line 819, the test assumes
cis always a string. If query decoding returns a numeric JSON value, this panics even when behavior is correct.Suggested patch
- let count = verify.data[0]["c"] - .as_str() - .and_then(|v| v.parse::<i64>().ok()) - .expect("touch_count should parse"); + let count = verify.data[0]["c"] + .as_i64() + .or_else(|| verify.data[0]["c"].as_str().and_then(|v| v.parse::<i64>().ok())) + .expect("touch_count should parse");🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src-tauri/tests/postgres_stateful_command_integration.rs` around lines 819 - 823, The test assumes verify.data[0]["c"] is a string; make the extraction robust to both string and numeric JSON by matching the serde_json::Value: if it's a string, parse to i64, if it's a number use as_i64() (or as_u64() then cast) and then assert_eq!(count, 2). Update the code around verify.data[0]["c"] in the test (the touch_count assertion) to handle Value::String and Value::Number branches and produce a single i64 `count` for the assertion.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/upgradelink-upload.yml:
- Line 18: The workflow currently references the third-party action by a mutable
tag "uses: toolsetlink/upgradelink-action@3.0.2"; replace that tag with the
action's immutable full commit SHA (e.g., "uses:
toolsetlink/upgradelink-action@<full-commit-sha>") to prevent retargeting—locate
the action repo, copy the commit SHA for the desired release, and update the
workflow entry that mentions toolsetlink/upgradelink-action accordingly.
In `@scripts/test-integration.sh`:
- Around line 42-45: The integration matrix omits StarRocks when IT_DB=all;
update the script so the "all" case triggers the StarRocks tests by adding the
same run_integration_test calls used in the starrocks) case. Locate the case or
switch that handles IT_DB="all" in scripts/test-integration.sh and add
run_integration_test "starrocks_integration" and run_integration_test
"starrocks_command_integration" alongside the other DB entries so full CI runs
include the new driver path; use the existing run_integration_test invocation
pattern to ensure consistent behavior.
In `@src-tauri/Cargo.toml`:
- Line 21: Remove the unconditional "devtools" dependency feature from the tauri
dependency entry in Cargo.toml and instead declare a Cargo feature named
"devtools" that enables the tauri/devtools dependency feature; specifically,
stop listing features = ["devtools"] on the tauri dependency and add a
[features] section where devtools = ["tauri/devtools"], so devtools is only
activated when building with --features devtools (and update your dev/local
build commands to pass --features devtools as needed).
In `@src-tauri/src/commands/connection.rs`:
- Around line 422-479: Both get_mysql_charsets_by_id and
get_mysql_collations_by_id must reject non-MySQL drivers up-front like the
create_database_by_id* handlers do: inside the execute_with_retry closure (or
before calling it) check the driver's family and return Err("[UNSUPPORTED] This
operation is only supported for MySQL connections") when it is not
DriverFamily::Mysql. Reference the functions get_mysql_charsets_by_id and
get_mysql_collations_by_id, the existing execute_with_retry helper, and mirror
the driver-family guard pattern used in create_database_by_id* so non-MySQL
connection ids produce the predictable [UNSUPPORTED] error instead of raw SQL
errors.
In `@src-tauri/src/commands/transfer.rs`:
- Around line 297-298: Reject ExportFormat::SqlDdl early in the query-export
entry points so we return a proper error instead of letting ExportWriter later
hit the unreachable! panic: add a guard that checks for ExportFormat::SqlDdl and
returns an Err (or Tauri command error) before constructing ExportWriter or
calling write_csv_header; apply the same check in the other query-export sites
that create ExportWriter (the callsite that uses write_csv_header and the other
locations around the code that pass ExportFormat::SqlDdl at the spots
corresponding to lines 335-336 and 1745 in this diff) so no path ever forwards a
SqlDdl format into ExportWriter for query exports.
- Around line 1203-1221: The Oracle block detector only treats "CREATE OR
REPLACE ..." as a block and only looks for a final token exactly equal to "END",
so update the logic around is_oracle_block and the termination checks to also
treat plain "CREATE" followed directly by a PL/SQL object keyword (e.g., when
tokens.first()==Some("create") and the object keyword appears at index 1 or 3
depending on presence of "or") as a block (cover both "create or replace <obj>"
and "create <obj>" forms referenced in the tokens checks), and change the
end-detection (the code paths around the later checks you flagged) to consider
"END <object_name>;" patterns by inspecting the last two tokens (normalize/trim
punctuation) so that if the penultimate token is "end" and the final token is
the object name (possibly with trailing semicolon or "/") the parser treats the
block as terminated; apply the same adjusted logic in the other occurrences you
noted (the similar checks at the other line ranges).
In `@src-tauri/src/db/drivers/mysql.rs`:
- Around line 63-69: The mysql_family_default_port function incorrectly treats
"tidb" like generic MySQL and returns 3306; update mysql_family_default_port to
check driver.eq_ignore_ascii_case("tidb") and return 4000 for that case
(preserving the existing starrocks => 9030 branch and the fallback => 3306) so
omitted-port connections to tidb use the registry-expected default.
- Around line 174-177: The parse error returned from
MySqlConnectOptions::from_str in build_connect_options currently formats a raw
"[CONN_FAILED]" string; replace that map_err call to invoke the standardized
conn_failed_error helper so the error is enriched with context (dsn and driver)
and TLS/auth/network hints—i.e., call conn_failed_error(...) (passing the
original parse error, dsn, and driver as required by conn_failed_error's
signature) instead of format!("[CONN_FAILED] {e}").
- Around line 233-240: build_dsn_with_ca_path may create a temporary CA file
(ca_cert_path) which is currently leaked if MySqlPoolOptions::connect_with
fails; modify the flow in the function using build_dsn_with_ca_path,
build_connect_options and connect_with so that ca_cert_path is either stored
into the MysqlDriver before awaiting connect_with or is cleaned up on any early
return: e.g., capture the ca_cert_path, attempt connect_with, and on Err call
the existing cleanup routine (or drop/unlink the temp file) before returning the
error; ensure MysqlDriver contains the saved ca_cert_path when connection
succeeds so normal shutdown can clean it up.
In `@src/components/business/DataGrid/TableView.tsx`:
- Around line 1717-1748: The DDL button is incorrectly hidden by the combined
condition checking onCreateQuery; instead render each action independently: keep
the New Query Button rendered when tableContext && onCreateQuery (using
onCreateQuery with tableContext.connectionId/database/driver), and render the
DDL Button only when tableContext && onOpenDDL (leave its onClick as
handleShowDDL or call onOpenDDL if handleShowDDL simply proxies it). Update the
JSX conditions so the two Buttons are in separate conditional blocks keyed to
their respective callbacks (onCreateQuery for the query button, onOpenDDL for
the DDL button).
- Around line 1983-1993: The current header interaction uses
createSingleAndDoubleClickHandler (via headerClickStateRef and
headerInteraction) to make copy the single-click action and sort the
double-click action; change this so sorting (handleSortClick) is the
primary/keyboard-accessible action and copying (handleHeaderCopy) is a secondary
affordance—remove or stop relying on double-click for sort, wire handleSortClick
to the primary click/keyboard handler (and ensure headerActionLabel reflects
that), and move handleHeaderCopy to a separate visible/accessible control or
context menu/button so copy is not the primary click behavior; update uses of
createSingleAndDoubleClickHandler/headerInteraction accordingly.
- Around line 1847-1853: handleExport is being called with format tokens like
"sql_dml" which are concatenated into defaultPath (.${format}) producing
filenames like *.sql_dml; change the export calls or handleExport logic so the
saved filename uses the actual .sql extension. Either map internal format names
to extensions inside handleExport (e.g., formatToExt("sql_dml") -> "sql") when
building defaultPath, or update the DropdownMenuItem calls that pass "sql_dml"
(the onClick arrows that call handleExport("current_page", "sql_dml") and the
similar calls at the other DropdownMenuItem locations) to pass "sql" as the
extension argument while preserving any internal type identifier separately;
reference handleExport and the DropdownMenuItem calls around the
"current_page","sql_dml" usages for locating the fix.
In `@src/components/business/Editor/SqlEditor.tsx`:
- Line 877: The export now generates filenames ending with ".sql_dml" for the
"sql_dml" format; update the export behavior so "sql_dml" uses the standard
".sql" extension by normalizing the format before building the filename. Locate
the handler handleExportResult (and any place it derives filename from the
format string) and map "sql_dml" → "sql" (either by normalizing the format
argument passed from the onClick or inside handleExportResult) so generated
filenames use ".sql" while preserving the original format identifier for export
logic.
In `@src/components/business/Sidebar/ConnectionList.tsx`:
- Around line 459-493: The two useEffect blocks that call
api.connections.getMysqlCharsets and getMysqlCollations must ignore stale
responses: add a cancellation guard inside each effect (e.g., let cancelled =
false; cleanup sets cancelled = true) and only call setMysqlCharsets /
setMysqlCollations / setLoadingMysqlOptions when cancelled is false (and
optionally verify createDbConnectionId and isCreateDbDialogOpen still match),
then return the cleanup function; apply this same cancellation-guard pattern to
both the charset-loading effect and the collation-loading effect so late network
responses cannot overwrite cleared or newer state for createDbConnectionId,
isCreateDbDialogOpen, isMySqlFamilyCreateDb, or createDbForm.charset.
In `@src/main.tsx`:
- Line 8: The import of ErrorBoundary in main.tsx uses a relative path; update
the import to use the project alias by replacing the relative import with the
alias form (importing ErrorBoundary from "@/components/ErrorBoundary") so it
follows the repo convention for TypeScript files; ensure any related
tsconfig/paths are already configured and update only the import statement
referencing ErrorBoundary in main.tsx.
---
Outside diff comments:
In `@src-tauri/src/connection_input/mod.rs`:
- Around line 57-62: After parsing an embedded host:port via
parse_host_embedded_port in the is_mysql_family_driver branch, re-run the port
range validation on the potentially updated form.port (the value assigned to
form.port after parse_host_embedded_port) and reject or clear values that fall
outside the valid TCP port range (1–65535); update the code that originally
validated form.port before parsing to also validate the post-parse value so
inputs like "db:70000" do not bypass checks. Ensure you reference the same
validation logic (or helper) when validating form.port after
parse_host_embedded_port to keep behavior consistent with the initial check on
form.port.
---
Nitpick comments:
In `@src-tauri/src/commands/query.rs`:
- Around line 79-110: parse_dollar_quote_tag and skip_dollar_quote are
duplicated; extract their logic into a shared SQL utility module (e.g., create a
new module like sql::dollar_quote) and move the implementations there, then
replace the local functions in src-tauri/src/commands/query.rs
(parse_dollar_quote_tag, skip_dollar_quote) and the duplicate in
src-tauri/src/db/drivers/postgres.rs to call the shared helpers instead; ensure
the shared functions keep the same signatures (accepting &[u8] and usize) and
update imports/usages accordingly so both command guards and driver parsing use
the single centralized implementation.
In `@src-tauri/tests/common/shared.rs`:
- Around line 8-11: Remove the redundant #[allow(dead_code)] attributes from the
active retry helper constants CONNECT_RETRY_ATTEMPTS and CONNECT_RETRY_DELAY_MS
by deleting the attribute lines immediately above those const declarations; also
remove the same #[allow(dead_code)] suppression from any other retry-related
constants in this module (the other retry helper constants) so that dead-code
warnings remain meaningful for future changes.
In `@src-tauri/tests/common/starrocks_context.rs`:
- Around line 22-30: The current GenericImage construction uses
WaitFor::seconds(20) which may be too short for StarRocks; update the readiness
handling by increasing the wait timeout (e.g., to 120s or higher) on the
with_wait_for call (replace WaitFor::seconds(20) with a larger value) or replace
the simple seconds-based wait with a more robust readiness check (e.g., waiting
for a specific log line or a successful TCP response) so that
GenericImage::with_wait_for/WaitFor usage aligns with the longer
shared::wait_for_port(Duration::from_secs(120)) check.
- Around line 44-56: The function starrocks_form_from_local_env currently
constructs ConnectionForm by reading environment variables directly and then
redundantly calls apply_starrocks_env_overrides(&mut form); remove that
redundant call (or alternatively initialize form with Default::default() and let
apply_starrocks_env_overrides populate fields) so that only one path reads and
applies the STARROCKS_* environment variables; update
starrocks_form_from_local_env to either rely solely on the inline
env_or/env_i64/env::var assignments or solely on apply_starrocks_env_overrides,
and delete the extra apply_starrocks_env_overrides invocation accordingly.
In `@src-tauri/tests/postgres_stateful_command_integration.rs`:
- Around line 819-823: The test assumes verify.data[0]["c"] is a string; make
the extraction robust to both string and numeric JSON by matching the
serde_json::Value: if it's a string, parse to i64, if it's a number use as_i64()
(or as_u64() then cast) and then assert_eq!(count, 2). Update the code around
verify.data[0]["c"] in the test (the touch_count assertion) to handle
Value::String and Value::Number branches and produce a single i64 `count` for
the assertion.
In `@src/App.tsx`:
- Around line 1709-1730: The inline schema derivation duplicates
resolveTableScope; update the tableContext construction for the tab (where
tableContext is set) to call the existing resolveTableScope function instead of
reimplementing the conditional chain: pass the tab.driver and tab.database (and
any other expected params) into resolveTableScope to obtain the schema/scope,
then use that returned value for the schema field while keeping the other fields
(connectionId, database, table, driver) as before; this will eliminate the
duplicated logic and keep behavior consistent with resolveTableScope.
- Around line 190-191: The current expression using a type cast bypasses type
safety: replace the as any cast by adding a safe string-based guard and using it
in the isDatabaseScoped calculation; implement a helper like
isMysqlFamilyDriverSafe(driver: string | undefined) in driver-registry that
looks up DRIVER_REGISTRY by id and returns config?.isMysqlFamily ?? false,
export it, then update the isDatabaseScoped computation to use
isMysqlFamilyDriverSafe(driver) || driver === "clickhouse" (referencing the
isDatabaseScoped variable and isMysqlFamilyDriver/isMysqlFamilyDriverSafe
helpers to locate the code).
In `@src/components/business/DataGrid/tableView/utils.ts`:
- Around line 177-188: The quoteIdent function uses an unsafe cast `as any`
because the driver parameter's type doesn't match isMysqlFamilyDriver's expected
type; update the signature of quoteIdent(driver: string | undefined, name:
string) to use the exact type isMysqlFamilyDriver accepts (or change
isMysqlFamilyDriver to accept string | undefined) so you can call
isMysqlFamilyDriver(driver) without casting, then remove the `as any` cast in
quoteIdent and adjust any callers if needed; reference the quoteIdent function
and isMysqlFamilyDriver to locate and align their types.
- Around line 280-287: The getQualifiedTableName function uses an unsafe "as
any" cast when calling isMysqlFamilyDriver (same issue exists in quoteIdent);
fix by aligning types instead of casting—either change the driver parameter type
on getQualifiedTableName and quoteIdent to the proper DriverType/enum used
across the module or update isMysqlFamilyDriver's signature to accept a string
(or the actual driver type) so you can call isMysqlFamilyDriver(driver) without
casting; remove the "as any" casts and adjust imports/types accordingly so the
call sites and helper signatures share a consistent driver type.
In `@src/components/business/Sidebar/Sidebar.tsx`:
- Around line 42-43: Replace the inline union type for the "format" prop with
the shared TransferFormat type from src/services/api.ts: import TransferFormat
into Sidebar (or the module where the prop/type is declared) and change the
property signature from format: "csv" | "json" | "sql_dml" | "sql_ddl" |
"sql_full" to format: TransferFormat; ensure any usages of format (e.g., in
Sidebar component, prop interfaces or function signatures) compile after
updating the import.
In `@src/components/ErrorBoundary.tsx`:
- Around line 14-46: ErrorBoundary currently sets error in
getDerivedStateFromError and never clears it, preventing recovery; add a reset
mechanism by introducing a resetErrorBoundary method that sets state.error back
to null and call it from either (a) a "Try Again" button rendered when
this.props.fallback is absent or (b) when a new resetKey prop changes (watch
resetKey in componentDidUpdate to clear error). Update render to show a small
"Try Again" button alongside the error UI that invokes resetErrorBoundary, and
expose resetErrorBoundary as a public method if parent components should trigger
resets programmatically; modify Prop types to include optional fallback and
optional resetKey as needed.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 18bc9511-2075-40af-abd9-e01396eaa899
📒 Files selected for processing (54)
.github/workflows/build-multiplatform.yml.github/workflows/ci.yml.github/workflows/release.yml.github/workflows/upgradelink-upload.ymlpackage.jsonscripts/test-integration.shsrc-tauri/Cargo.tomlsrc-tauri/src/commands/connection.rssrc-tauri/src/commands/query.rssrc-tauri/src/commands/transfer.rssrc-tauri/src/connection_input/mod.rssrc-tauri/src/db/drivers/mod.rssrc-tauri/src/db/drivers/mssql.rssrc-tauri/src/db/drivers/mysql.rssrc-tauri/src/db/drivers/oracle.rssrc-tauri/src/db/drivers/postgres.rssrc-tauri/src/lib.rssrc-tauri/src/ssh.rssrc-tauri/tauri.conf.jsonsrc-tauri/tests/common/shared.rssrc-tauri/tests/common/starrocks_context.rssrc-tauri/tests/mariadb_command_integration.rssrc-tauri/tests/mysql_stateful_command_integration.rssrc-tauri/tests/oracle_command_integration.rssrc-tauri/tests/oracle_integration.rssrc-tauri/tests/postgres_integration.rssrc-tauri/tests/postgres_stateful_command_integration.rssrc-tauri/tests/sqlite_command_integration.rssrc-tauri/tests/starrocks_command_integration.rssrc-tauri/tests/starrocks_integration.rssrc/App.tsxsrc/components/ErrorBoundary.tsxsrc/components/business/DataGrid/ComplexValueViewer.tsxsrc/components/business/DataGrid/TableView.tsxsrc/components/business/DataGrid/tableView/utils.tssrc/components/business/DataGrid/tableView/utils.unit.test.tssrc/components/business/Editor/SqlEditor.tsxsrc/components/business/Sidebar/ConnectionList.tsxsrc/components/business/Sidebar/Sidebar.tsxsrc/components/business/Sidebar/connection-list/helpers.tsxsrc/components/business/Sidebar/connection-list/helpers.unit.test.tssrc/components/settings/SettingsDialog.tsxsrc/lib/connection-form/rules.tssrc/lib/connection-form/rules.unit.test.tssrc/lib/driver-registry.tsxsrc/lib/driver-registry.unit.test.tssrc/lib/i18n/locales/en.tssrc/lib/i18n/locales/ja.tssrc/lib/i18n/locales/zh.tssrc/main.tsxsrc/services/api.tssrc/services/api.unit.test.tssrc/services/mocks.service.test.tssrc/services/mocks.ts
| runs-on: ubuntu-22.04 | ||
| steps: | ||
| - name: Upload latest.json to UpgradeLink | ||
| uses: toolsetlink/upgradelink-action@3.0.2 |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Resolve the peeled commit behind tag 3.0.2
git ls-remote https://github.com/toolsetlink/upgradelink-action refs/tags/3.0.2^{} | awk '{print $1}'Repository: codeErrorSleep/dbpaw
Length of output: 104
Pin third-party GitHub Action to an immutable commit SHA.
Using @3.0.2 is mutable and can be retargeted. For release workflows, pin to a full commit SHA instead.
🔒 Suggested change
- uses: toolsetlink/upgradelink-action@3.0.2
+ uses: toolsetlink/upgradelink-action@5d1d2fdafb4372bd0b527909639f3ea786e8856d🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/upgradelink-upload.yml at line 18, The workflow currently
references the third-party action by a mutable tag "uses:
toolsetlink/upgradelink-action@3.0.2"; replace that tag with the action's
immutable full commit SHA (e.g., "uses:
toolsetlink/upgradelink-action@<full-commit-sha>") to prevent retargeting—locate
the action repo, copy the commit SHA for the desired release, and update the
workflow entry that mentions toolsetlink/upgradelink-action accordingly.
Comment on lines
+42
to
+45
| starrocks) | ||
| run_integration_test "starrocks_integration" | ||
| run_integration_test "starrocks_command_integration" | ||
| ;; |
There was a problem hiding this comment.
Add StarRocks to the all integration matrix.
Line 42 adds targeted StarRocks execution, but IT_DB=all still omits StarRocks, so CI full runs won’t cover the new driver path.
✅ Proposed patch
all)
+ run_integration_test "starrocks_integration"
+ run_integration_test "starrocks_command_integration"
run_integration_test "mysql_integration"
run_integration_test "mysql_command_integration"
run_integration_test "mysql_stateful_command_integration"Based on learnings: For new database driver additions, follow the step-by-step checklist in ADD_NEW_DB.md for exact file paths, line numbers, and implementation details; update scripts/test-integration.sh to include the new driver.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@scripts/test-integration.sh` around lines 42 - 45, The integration matrix
omits StarRocks when IT_DB=all; update the script so the "all" case triggers the
StarRocks tests by adding the same run_integration_test calls used in the
starrocks) case. Locate the case or switch that handles IT_DB="all" in
scripts/test-integration.sh and add run_integration_test "starrocks_integration"
and run_integration_test "starrocks_command_integration" alongside the other DB
entries so full CI runs include the new driver path; use the existing
run_integration_test invocation pattern to ensure consistent behavior.
|
|
||
| [dependencies] | ||
| tauri = { version = "2", features = [] } | ||
| tauri = { version = "2", features = ["devtools"] } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Check Rust/Tauri code paths for devtools usage and debug gating ==="
rg -n -C2 --type=rust --type=toml 'devtools|open_devtools|close_devtools|debug_toggle_devtools|cfg\(debug_assertions\)'
echo
echo "=== Check workflows/scripts for release builds enabling custom features ==="
rg -n -C2 'tauri build|cargo tauri build|--features'Repository: codeErrorSleep/dbpaw
Length of output: 3369
🏁 Script executed:
#!/bin/bash
# Check Cargo.toml for build profile overrides
echo "=== Check Cargo.toml for build profile configuration ==="
cat -n src-tauri/Cargo.toml | tail -30
echo
echo "=== Check if menu creation has any conditional guards ==="
rg -n -B5 -A5 'developer_menu' src-tauri/src/lib.rsRepository: codeErrorSleep/dbpaw
Length of output: 2958
🏁 Script executed:
#!/bin/bash
# Check for any release profile configuration in full Cargo.toml
echo "=== Full Cargo.toml to check for [profile.release] section ==="
wc -l src-tauri/Cargo.toml
cat -n src-tauri/Cargo.toml | grep -A10 '\[profile'Repository: codeErrorSleep/dbpaw
Length of output: 150
🏁 Script executed:
#!/bin/bash
# Show entire Cargo.toml
echo "=== Full Cargo.toml content ==="
cat -n src-tauri/Cargo.tomlRepository: codeErrorSleep/dbpaw
Length of output: 2254
Disable Tauri devtools in production by using a feature flag.
The devtools feature is globally enabled and unconditionally added to the Developer menu—enabling end-users to access devtools in production. This weakens security posture.
🔧 Suggested approach
[dependencies]
-tauri = { version = "2", features = ["devtools"] }
+tauri = { version = "2" }
+
+[features]
+default = []
+devtools = ["tauri/devtools"]Enable --features devtools only in local/dev builds.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src-tauri/Cargo.toml` at line 21, Remove the unconditional "devtools"
dependency feature from the tauri dependency entry in Cargo.toml and instead
declare a Cargo feature named "devtools" that enables the tauri/devtools
dependency feature; specifically, stop listing features = ["devtools"] on the
tauri dependency and add a [features] section where devtools =
["tauri/devtools"], so devtools is only activated when building with --features
devtools (and update your dev/local build commands to pass --features devtools
as needed).
Comment on lines
+422
to
+479
| #[tauri::command] | ||
| pub async fn get_mysql_charsets_by_id( | ||
| state: State<'_, AppState>, | ||
| id: i64, | ||
| ) -> Result<Vec<String>, String> { | ||
| super::execute_with_retry(&state, id, None, |driver| async move { | ||
| let result = driver | ||
| .execute_query("SHOW CHARACTER SET".to_string()) | ||
| .await?; | ||
| let mut charsets: Vec<String> = result | ||
| .data | ||
| .iter() | ||
| .filter_map(|row| { | ||
| row.get("Charset") | ||
| .and_then(|v| v.as_str()) | ||
| .map(|s| s.to_string()) | ||
| }) | ||
| .collect(); | ||
| charsets.sort(); | ||
| Ok(charsets) | ||
| }) | ||
| .await | ||
| } | ||
|
|
||
| #[tauri::command] | ||
| pub async fn get_mysql_collations_by_id( | ||
| state: State<'_, AppState>, | ||
| id: i64, | ||
| charset: Option<String>, | ||
| ) -> Result<Vec<String>, String> { | ||
| let sql = match &charset { | ||
| Some(cs) if is_safe_option_token(cs) => { | ||
| format!("SHOW COLLATION WHERE Charset = '{}'", cs) | ||
| } | ||
| Some(cs) => { | ||
| return Err(format!("[VALIDATION_ERROR] Invalid charset: {}", cs)); | ||
| } | ||
| None => "SHOW COLLATION".to_string(), | ||
| }; | ||
| super::execute_with_retry(&state, id, None, |driver| { | ||
| let sql = sql.clone(); | ||
| async move { | ||
| let result = driver.execute_query(sql).await?; | ||
| let mut collations: Vec<String> = result | ||
| .data | ||
| .iter() | ||
| .filter_map(|row| { | ||
| row.get("Collation") | ||
| .and_then(|v| v.as_str()) | ||
| .map(|s| s.to_string()) | ||
| }) | ||
| .collect(); | ||
| collations.sort(); | ||
| Ok(collations) | ||
| } | ||
| }) | ||
| .await | ||
| } |
There was a problem hiding this comment.
Reject non-MySQL connections before running these queries.
These handlers currently execute SHOW CHARACTER SET / SHOW COLLATION for any saved connection id. If a non-MySQL connection reaches them, callers get a raw SQL error instead of a predictable [UNSUPPORTED] response. Mirroring the driver-family guard used in create_database_by_id* would make this API much safer to consume.
Also applies to: 481-536
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src-tauri/src/commands/connection.rs` around lines 422 - 479, Both
get_mysql_charsets_by_id and get_mysql_collations_by_id must reject non-MySQL
drivers up-front like the create_database_by_id* handlers do: inside the
execute_with_retry closure (or before calling it) check the driver's family and
return Err("[UNSUPPORTED] This operation is only supported for MySQL
connections") when it is not DriverFamily::Mysql. Reference the functions
get_mysql_charsets_by_id and get_mysql_collations_by_id, the existing
execute_with_retry helper, and mirror the driver-family guard pattern used in
create_database_by_id* so non-MySQL connection ids produce the predictable
[UNSUPPORTED] error instead of raw SQL errors.
Comment on lines
+297
to
+298
| let mut writer = ExportWriter::new(output_path.clone(), format)?; | ||
| writer.write_csv_header(&columns)?; |
There was a problem hiding this comment.
Reject sql_ddl for query exports before this hits unreachable!.
Both query-export entry points still pass ExportFormat::SqlDdl into the writer, and any non-empty result will panic on the unreachable! branch instead of returning a normal export error.
Suggested guard
+fn validate_query_export_format(format: &ExportFormat) -> Result<(), String> {
+ if matches!(format, ExportFormat::SqlDdl) {
+ return Err("[EXPORT_ERROR] sql_ddl is only supported for table exports".to_string());
+ }
+ Ok(())
+}
+
#[tauri::command]
pub async fn export_query_result(
state: State<'_, AppState>,
@@
) -> Result<ExportResult, String> {
+ validate_query_export_format(&format)?;
let output_path =
resolve_output_path(file_path, "query_result", extension_for_format(&format))?;
@@
pub async fn export_query_result_direct(
@@
) -> Result<ExportResult, String> {
+ validate_query_export_format(&format)?;
let output_path =
resolve_output_path(file_path, "query_result", extension_for_format(&format))?;
@@
- ExportFormat::SqlDdl => unreachable!("SqlDdl rows are never written"),
+ ExportFormat::SqlDdl => {
+ return Err("[EXPORT_ERROR] sql_ddl is only supported for table exports".to_string());
+ }Also applies to: 335-336, 1745-1745
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src-tauri/src/commands/transfer.rs` around lines 297 - 298, Reject
ExportFormat::SqlDdl early in the query-export entry points so we return a
proper error instead of letting ExportWriter later hit the unreachable! panic:
add a guard that checks for ExportFormat::SqlDdl and returns an Err (or Tauri
command error) before constructing ExportWriter or calling write_csv_header;
apply the same check in the other query-export sites that create ExportWriter
(the callsite that uses write_csv_header and the other locations around the code
that pass ExportFormat::SqlDdl at the spots corresponding to lines 335-336 and
1745 in this diff) so no path ever forwards a SqlDdl format into ExportWriter
for query exports.
Comment on lines
1847
to
1853
| <DropdownMenuItem | ||
| onClick={() => void handleExport("current_page", "sql")} | ||
| onClick={() => | ||
| void handleExport("current_page", "sql_dml") | ||
| } | ||
| > | ||
| SQL | ||
| </DropdownMenuItem> |
There was a problem hiding this comment.
Keep the suggested file extension as .sql.
These new format values are passed into handleExport(), which still builds defaultPath with .${format}. That now pre-fills filenames like *.sql_dml even though the actual artifact is still SQL.
Minimal fix near handleExport()
- const defaultPath = `${tableContext.table}_${timestamp}.${format}`;
+ const extension =
+ format === "csv" ? "csv" : format === "json" ? "json" : "sql";
+ const defaultPath = `${tableContext.table}_${timestamp}.${extension}`;Also applies to: 1871-1875, 1893-1899
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/business/DataGrid/TableView.tsx` around lines 1847 - 1853,
handleExport is being called with format tokens like "sql_dml" which are
concatenated into defaultPath (.${format}) producing filenames like *.sql_dml;
change the export calls or handleExport logic so the saved filename uses the
actual .sql extension. Either map internal format names to extensions inside
handleExport (e.g., formatToExt("sql_dml") -> "sql") when building defaultPath,
or update the DropdownMenuItem calls that pass "sql_dml" (the onClick arrows
that call handleExport("current_page", "sql_dml") and the similar calls at the
other DropdownMenuItem locations) to pass "sql" as the extension argument while
preserving any internal type identifier separately; reference handleExport and
the DropdownMenuItem calls around the "current_page","sql_dml" usages for
locating the fix.
Comment on lines
+1983
to
+1993
| const headerActionLabel = t("tableView.header.actionHint", { | ||
| column, | ||
| }); | ||
| const headerClickState = | ||
| headerClickStateRef.current[column] ?? | ||
| (headerClickStateRef.current[column] = { timerId: null }); | ||
| const headerInteraction = createSingleAndDoubleClickHandler( | ||
| headerClickState, | ||
| () => handleHeaderCopy(column), | ||
| () => handleSortClick(column), | ||
| ); |
There was a problem hiding this comment.
Don't make sorting depend solely on double-click.
With copy on click and sort on double-click, header sorting no longer has a clear primary path for keyboard and other non-pointer input. Keep sort reachable through a primary/keyboard action, and move copy to a separate affordance if needed.
Also applies to: 2009-2035
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/business/DataGrid/TableView.tsx` around lines 1983 - 1993, The
current header interaction uses createSingleAndDoubleClickHandler (via
headerClickStateRef and headerInteraction) to make copy the single-click action
and sort the double-click action; change this so sorting (handleSortClick) is
the primary/keyboard-accessible action and copying (handleHeaderCopy) is a
secondary affordance—remove or stop relying on double-click for sort, wire
handleSortClick to the primary click/keyboard handler (and ensure
headerActionLabel reflects that), and move handleHeaderCopy to a separate
visible/accessible control or context menu/button so copy is not the primary
click behavior; update uses of
createSingleAndDoubleClickHandler/headerInteraction accordingly.
| </DropdownMenuItem> | ||
| <DropdownMenuItem | ||
| onClick={() => void handleExportResult("sql")} | ||
| onClick={() => void handleExportResult("sql_dml")} |
There was a problem hiding this comment.
sql_dml now produces a nonstandard default file extension.
After this change, default filename generation uses .sql_dml (from .${format}) instead of .sql, which is a user-facing export regression.
🔧 Proposed fix
- const defaultPath = `query_result_${timestamp}.${format}`;
+ const extension =
+ format === "csv" ? "csv" : format === "json" ? "json" : "sql";
+ const defaultPath = `query_result_${timestamp}.${extension}`;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/business/Editor/SqlEditor.tsx` at line 877, The export now
generates filenames ending with ".sql_dml" for the "sql_dml" format; update the
export behavior so "sql_dml" uses the standard ".sql" extension by normalizing
the format before building the filename. Locate the handler handleExportResult
(and any place it derives filename from the format string) and map "sql_dml" →
"sql" (either by normalizing the format argument passed from the onClick or
inside handleExportResult) so generated filenames use ".sql" while preserving
the original format identifier for export logic.
Comment on lines
+459
to
+493
| useEffect(() => { | ||
| if ( | ||
| !isCreateDbDialogOpen || | ||
| !isMySqlFamilyCreateDb || | ||
| !createDbConnectionId | ||
| ) | ||
| return; | ||
| setLoadingMysqlOptions(true); | ||
| api.connections | ||
| .getMysqlCharsets(Number(createDbConnectionId)) | ||
| .then(setMysqlCharsets) | ||
| .catch(() => setMysqlCharsets(["utf8mb4", "utf8", "latin1"])) | ||
| .finally(() => setLoadingMysqlOptions(false)); | ||
| }, [isCreateDbDialogOpen, isMySqlFamilyCreateDb, createDbConnectionId]); | ||
|
|
||
| useEffect(() => { | ||
| if ( | ||
| !isCreateDbDialogOpen || | ||
| !isMySqlFamilyCreateDb || | ||
| !createDbConnectionId | ||
| ) | ||
| return; | ||
| api.connections | ||
| .getMysqlCollations( | ||
| Number(createDbConnectionId), | ||
| createDbForm.charset || undefined, | ||
| ) | ||
| .then(setMysqlCollations) | ||
| .catch(() => setMysqlCollations([])); | ||
| }, [ | ||
| isCreateDbDialogOpen, | ||
| isMySqlFamilyCreateDb, | ||
| createDbConnectionId, | ||
| createDbForm.charset, | ||
| ]); |
There was a problem hiding this comment.
Ignore stale MySQL option responses.
Both effects update state unconditionally after the request resolves. If the dialog closes, the connection changes, or the user switches charset quickly, an older response can repopulate cleared state or overwrite the newer collation list with stale data.
Use a cancellation guard for the refetch path
useEffect(() => {
- if (
- !isCreateDbDialogOpen ||
- !isMySqlFamilyCreateDb ||
- !createDbConnectionId
- )
- return;
- api.connections
- .getMysqlCollations(
- Number(createDbConnectionId),
- createDbForm.charset || undefined,
- )
- .then(setMysqlCollations)
- .catch(() => setMysqlCollations([]));
+ if (
+ !isCreateDbDialogOpen ||
+ !isMySqlFamilyCreateDb ||
+ !createDbConnectionId
+ ) {
+ setMysqlCollations([]);
+ return;
+ }
+ let cancelled = false;
+ setMysqlCollations([]);
+ api.connections
+ .getMysqlCollations(
+ Number(createDbConnectionId),
+ createDbForm.charset || undefined,
+ )
+ .then((opts) => {
+ if (!cancelled) setMysqlCollations(opts);
+ })
+ .catch(() => {
+ if (!cancelled) setMysqlCollations([]);
+ });
+ return () => {
+ cancelled = true;
+ };
}, [
isCreateDbDialogOpen,
isMySqlFamilyCreateDb,
createDbConnectionId,
createDbForm.charset,
]);Apply the same pattern to the charset-loading effect as well.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| useEffect(() => { | |
| if ( | |
| !isCreateDbDialogOpen || | |
| !isMySqlFamilyCreateDb || | |
| !createDbConnectionId | |
| ) | |
| return; | |
| setLoadingMysqlOptions(true); | |
| api.connections | |
| .getMysqlCharsets(Number(createDbConnectionId)) | |
| .then(setMysqlCharsets) | |
| .catch(() => setMysqlCharsets(["utf8mb4", "utf8", "latin1"])) | |
| .finally(() => setLoadingMysqlOptions(false)); | |
| }, [isCreateDbDialogOpen, isMySqlFamilyCreateDb, createDbConnectionId]); | |
| useEffect(() => { | |
| if ( | |
| !isCreateDbDialogOpen || | |
| !isMySqlFamilyCreateDb || | |
| !createDbConnectionId | |
| ) | |
| return; | |
| api.connections | |
| .getMysqlCollations( | |
| Number(createDbConnectionId), | |
| createDbForm.charset || undefined, | |
| ) | |
| .then(setMysqlCollations) | |
| .catch(() => setMysqlCollations([])); | |
| }, [ | |
| isCreateDbDialogOpen, | |
| isMySqlFamilyCreateDb, | |
| createDbConnectionId, | |
| createDbForm.charset, | |
| ]); | |
| useEffect(() => { | |
| if ( | |
| !isCreateDbDialogOpen || | |
| !isMySqlFamilyCreateDb || | |
| !createDbConnectionId | |
| ) | |
| return; | |
| setLoadingMysqlOptions(true); | |
| api.connections | |
| .getMysqlCharsets(Number(createDbConnectionId)) | |
| .then(setMysqlCharsets) | |
| .catch(() => setMysqlCharsets(["utf8mb4", "utf8", "latin1"])) | |
| .finally(() => setLoadingMysqlOptions(false)); | |
| }, [isCreateDbDialogOpen, isMySqlFamilyCreateDb, createDbConnectionId]); | |
| useEffect(() => { | |
| if ( | |
| !isCreateDbDialogOpen || | |
| !isMySqlFamilyCreateDb || | |
| !createDbConnectionId | |
| ) { | |
| setMysqlCollations([]); | |
| return; | |
| } | |
| let cancelled = false; | |
| setMysqlCollations([]); | |
| api.connections | |
| .getMysqlCollations( | |
| Number(createDbConnectionId), | |
| createDbForm.charset || undefined, | |
| ) | |
| .then((opts) => { | |
| if (!cancelled) setMysqlCollations(opts); | |
| }) | |
| .catch(() => { | |
| if (!cancelled) setMysqlCollations([]); | |
| }); | |
| return () => { | |
| cancelled = true; | |
| }; | |
| }, [ | |
| isCreateDbDialogOpen, | |
| isMySqlFamilyCreateDb, | |
| createDbConnectionId, | |
| createDbForm.charset, | |
| ]); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/business/Sidebar/ConnectionList.tsx` around lines 459 - 493,
The two useEffect blocks that call api.connections.getMysqlCharsets and
getMysqlCollations must ignore stale responses: add a cancellation guard inside
each effect (e.g., let cancelled = false; cleanup sets cancelled = true) and
only call setMysqlCharsets / setMysqlCollations / setLoadingMysqlOptions when
cancelled is false (and optionally verify createDbConnectionId and
isCreateDbDialogOpen still match), then return the cleanup function; apply this
same cancellation-guard pattern to both the charset-loading effect and the
collation-loading effect so late network responses cannot overwrite cleared or
newer state for createDbConnectionId, isCreateDbDialogOpen,
isMySqlFamilyCreateDb, or createDbForm.charset.
| import { Toaster } from "./components/ui/sonner"; | ||
| import "./lib/i18n"; | ||
| import { initI18nFromStore } from "./lib/i18n"; | ||
| import { ErrorBoundary } from "./components/ErrorBoundary"; |
There was a problem hiding this comment.
Use @/ alias for this import.
Please switch this new import to the project alias to match repo conventions.
♻️ Proposed fix
-import { ErrorBoundary } from "./components/ErrorBoundary";
+import { ErrorBoundary } from "@/components/ErrorBoundary";As per coding guidelines, src/**/*.{ts,tsx}: Use path alias @/ mapping to ./src/ for imports in TypeScript files.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| import { ErrorBoundary } from "./components/ErrorBoundary"; | |
| import { ErrorBoundary } from "@/components/ErrorBoundary"; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main.tsx` at line 8, The import of ErrorBoundary in main.tsx uses a
relative path; update the import to use the project alias by replacing the
relative import with the alias form (importing ErrorBoundary from
"@/components/ErrorBoundary") so it follows the repo convention for TypeScript
files; ensure any related tsconfig/paths are already configured and update only
the import statement referencing ErrorBoundary in main.tsx.
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
1 participant
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.
Summary by CodeRabbit
Release Notes v0.3.3
New Features
Improvements
Chores