Release/260402#63
Merged
Merged
Conversation
Add unit tests for previously uncovered logic in the frontend layer: - api.ts: isTauri(), normalizeImportDriver(), getImportDriverCapability(), invoke routing (Tauri path, mock mode, error fallback), and command name mapping for all API namespaces - DataGrid/tableView/utils.ts: sortRows, collectSearchMatches, escapeSQL, quoteIdent, calculateAutoColumnWidths, and additional formatSQLValue cases - connection-form/rules.ts: isMysqlFamilyDriver, isFileBasedDriver, allowsHostWithPort, requiresPasswordOnCreate, normalizeTextValue - connection-form/validate.ts: happy paths, required field validation, password rules per driver and mode, host format checks - sqlEditorDatabase.ts: normalizeDatabaseOptions and resolvePreferredDatabase edge cases (empty list, no fallback, all-undefined params) - keyboard.ts: isModKey boolean combinations and null target guard - Sidebar/connection-list/helpers.ts: sanitizeConnectionErrorMessage, getExportFilter, and getConnectionStatusLabel Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
PolarDB-X returns a vendor-specific error message containing "Preparedoes not support" rather than the standard error code 1295, causing queries to fail instead of falling back to simple protocol. Extend is_prepared_protocol_unsupported_error() to match this variant and add a regression test with the exact PolarDB-X error string. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Create src/lib/driver-registry.tsx as the single source of truth for all driver attributes (ports, flags, icons, import capability). Update api.ts, rules.ts, helpers.tsx, ConnectionList.tsx, and SavedQueriesList.tsx to derive data from the registry instead of inline arrays/switch statements. Adding a new driver now requires editing only one file. Add driver-registry.unit.test.ts with 28 test cases covering all helper functions and registry invariants. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Add .claude/commands/new-db.md: /new-db slash command that scaffolds all boilerplate for a new database driver (Rust driver file, mod.rs registration, ssh.rs port, connection_input, Cargo.toml, frontend driver-registry, i18n, integration test skeleton) - Add ADD_NEW_DB.md: precise reference checklist with exact file paths, line numbers, and common pitfalls for adding a new database driver - Update CLAUDE.md: link to ADD_NEW_DB.md from the driver development section so Claude loads it automatically on each session Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Implement Oracle driver via the `oracle` crate (OCI-based), register it in the driver enum and connection dispatcher, add SSH tunnel default port (1521), wire up import transaction SQL (SELECT 1 FROM DUAL / COMMIT / ROLLBACK), and register the driver config in the frontend registry. Unit tests updated to reflect 9 supported drivers and Oracle-specific capabilities (schema browsing, import support). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Add driver integration tests (oracle_integration.rs), command integration tests (oracle_command_integration.rs), and shared test context helper (oracle_context.rs). Update test-integration.sh to support IT_DB=oracle and include Oracle tests in the `all` target. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
PostgreSQL array columns (_INT2/4/8, _FLOAT4/8, _BOOL, _TEXT, _VARCHAR,
_BPCHAR, _NAME, _UUID, _NUMERIC, _JSON, _JSONB) were previously falling
through to the String fallback and rendering as "{1,2,3}" in the grid.
Now decoded as proper serde_json::Value::Array with correct element types
and NULL element support.
Also adds ComplexValueViewer dialog (JSON / Tree / Table tabs) for
inspecting complex cell values, fixes cell editing to show full JSON
instead of [object Object], and centralises all cell→string conversions
into a single cellValueToString() utility to eliminate duplicate logic.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The cliff.zh.toml config file doesn't exist, so the Chinese changelog step was not producing actual Chinese output. Simplify the workflow to only generate English changelog.
- SQL editor Run button shows spinner and disables during query execution, preventing duplicate submissions (derives state from existing activeQueryId) - Clicking a table in the sidebar now creates a placeholder tab immediately with a skeleton screen, replacing blank wait time with instant feedback - Sidebar database/table tree nodes show a spinner while lazily loading children (tables or columns), tracked per-node via Set<string> state Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Sidebar tabs were capped at max-w-[200px] from the base TabsTrigger component, leaving gaps when sidebar was wider than 400px. JSON viewer tabs lacked flex-1, so they only took content width and left the right side empty. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
📝 WalkthroughWalkthroughThis PR introduces Oracle database driver support to DbPaw, including a complete Rust driver implementation with SSH tunneling, new TypeScript-based driver registry system, UI enhancements for complex value viewing and table interactions, extensive documentation for adding new database drivers, and integration test coverage. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant App as App.tsx
participant Sidebar as ConnectionList
participant Driver as OracleDriver
participant DB as Oracle DB
participant Grid as TableView
User->>App: Click table in sidebar
App->>App: Create placeholder tab (isLoading=true)
App->>Sidebar: Trigger revealSidebarForTab()
Sidebar->>Sidebar: Update selectedTableNode
App->>Driver: execute_query(SELECT * FROM table OFFSET 0 FETCH NEXT 50)
Driver->>DB: Connect via OCI
DB-->>Driver: ResultSet
Driver->>Driver: Convert rows to JSON
Driver-->>App: QueryResult with data
App->>App: setTabs(isLoading=false, data=result)
App->>Grid: Pass table data + isLoading=false
Grid-->>User: Render table with complex value hover buttons
User->>Grid: Hover on nested JSON cell
Grid->>Grid: Open ComplexValueViewer modal
Grid-->>User: Show JSON/Tree/Table tabs
sequenceDiagram
participant Client as Frontend
participant Registry as driver-registry
participant FormRules as connection-form/rules
participant API as api.ts
Client->>Registry: Import DRIVER_REGISTRY, getDefaultPort()
Registry-->>Client: [postgres, mysql, oracle, ...]
Client->>FormRules: Call isMysqlFamilyDriver(driver)
FormRules->>Registry: Import helper
Registry-->>FormRules: Boolean
FormRules-->>Client: Validation result
Client->>API: normalizeImportDriver(driver)
API->>Registry: getDriverConfig(normalized)
Registry-->>API: DriverConfig
API-->>Client: importCapability (supported/read_only/unsupported)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Review Summary by QodoRelease 0.3.2 - Oracle driver, PostgreSQL arrays, centralized driver registry, and enhanced data handling
WalkthroughsDescription• **Oracle database driver implementation**: Complete Rust driver with connection management, schema browsing, DDL retrieval, paginated data fetching, filtering, sorting, and query execution with proper type conversion • **PostgreSQL array type support**: Added support for 10 array types (_BOOL, _INT2, _INT4, _INT8, _FLOAT4, _FLOAT8, _NUMERIC, _TEXT, _JSON, _JSONB) with proper null handling and JSON serialization • **Centralized driver registry**: New driver-registry module consolidating driver configurations, capabilities, and utility functions to eliminate code duplication across frontend components • **Complex data type handling**: New utilities and components for formatting, displaying, and editing complex values (JSON, arrays) including ComplexValueViewer modal with multiple view modes (JSON, Tree, Table) • **Column autocomplete for SQL filters**: New ColumnAutocompleteInput component providing intelligent column name suggestions for WHERE and ORDER BY clauses with keyboard navigation • **Loading state indicators**: Added loading spinners for table data fetching, query execution, and database/table expansion operations with improved UX • **MySQL error handling improvement**: Enhanced prepared statement protocol error detection with PolarDB-X support • **Comprehensive test coverage**: Added 15+ new unit and integration test files covering Oracle driver, PostgreSQL arrays, API service, connection validation, and utility functions • **Documentation**: Added database driver scaffolding guides in English and Chinese, plus table selection optimization proposal • **Version bump**: Updated to version 0.3.2 with simplified release workflow (removed Chinese changelog generation) Diagramflowchart LR
A["Oracle Driver<br/>Implementation"] -->|"Registers in"| B["Driver Registry"]
C["PostgreSQL Array<br/>Support"] -->|"Uses"| B
B -->|"Powers"| D["Connection List<br/>& Forms"]
E["Complex Value<br/>Utilities"] -->|"Enables"| F["ComplexValueViewer<br/>Modal"]
G["Column Autocomplete<br/>Input"] -->|"Enhances"| H["Table View<br/>Filters"]
I["Loading States"] -->|"Improves"| H
J["Comprehensive<br/>Tests"] -->|"Validates"| B
J -->|"Validates"| E
J -->|"Validates"| G
File Changes1. src-tauri/src/db/drivers/oracle.rs
|
Code Review by Qodo
|
CI Feedback 🧐A test triggered by this PR failed. Here is an AI-generated analysis of the failure:
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
Comment on lines
+205
to
+212
| async fn test_connection(&self) -> Result<(), String> { | ||
| self.run_blocking(|conn| { | ||
| conn.query("SELECT 1 FROM DUAL", &[] as &[&dyn oracle::sql_type::ToSql]) | ||
| .map_err(|e| format!("[CONN_FAILED] {e}"))? | ||
| .next() | ||
| .ok_or("[CONN_FAILED] Empty response from DUAL")? | ||
| .map_err(|e| format!("[CONN_FAILED] {e}"))?; | ||
| Ok(()) |
There was a problem hiding this comment.
1. test_connection() bypasses conn_failed_error() 📘 Rule violation ≡ Correctness
OracleDriver::test_connection() converts Oracle connection/query errors to generic `[CONN_FAILED]
{e} strings, bypassing conn_failed_error()` and losing the standardized, user-friendly hints
required for connection failures. This can lead to inconsistent and less actionable error messages
in the UI.
Agent Prompt
## Issue description
`OracleDriver::test_connection()` returns generic `[CONN_FAILED] {e}` strings for Oracle errors, bypassing `conn_failed_error()` which standardizes and enriches connection failure messages.
## Issue Context
Compliance requires connection failure paths to use `conn_failed_error()` so the UI can show consistent, user-friendly hints (e.g., Oracle Instant Client not installed, TLS/auth/network hints).
## Fix Focus Areas
- src-tauri/src/db/drivers/oracle.rs[205-212]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+142
to
+154
| let service_name = effective_form | ||
| .database | ||
| .clone() | ||
| .map(|v| v.trim().to_string()) | ||
| .filter(|v| !v.is_empty()) | ||
| .unwrap_or_else(|| "ORCL".to_string()); | ||
| let username = effective_form | ||
| .username | ||
| .clone() | ||
| .map(|v| v.trim().to_string()) | ||
| .filter(|v| !v.is_empty()) | ||
| .ok_or("[VALIDATION_ERROR] username cannot be empty")?; | ||
| let password = effective_form.password.clone().unwrap_or_default(); |
There was a problem hiding this comment.
2. Oracle service name clobbered 🐞 Bug ≡ Correctness
OracleDriver.connect reads ConnectionForm.database as the Oracle service name, but the command-layer connection creation overwrites form.database with the request database parameter, which (for Oracle) is coming from ALL_USERS schema names. This causes Oracle connections to attempt //host:port/<SCHEMA> and fail in table browsing/export/metadata calls.
Agent Prompt
## Issue description
Oracle uses `ConnectionForm.database` as the Easy Connect service name, but the backend currently overwrites `form.database` with the per-request `database` param. In the Oracle flow the UI uses the `database` param to represent a schema (from `ALL_USERS`), so this overwrite turns the service name into a schema name and breaks connections.
## Issue Context
- OracleDriver builds `//host:port/<service_name>` from `form.database`.
- `ensure_connection_with_db` unconditionally replaces `form.database` when a request provides a `database` value.
- For Oracle, `list_databases()` returns schemas/users, and the UI passes those values as the `database` request param.
## Fix Focus Areas
- src-tauri/src/commands/mod.rs[58-65]
- src-tauri/src/commands/mod.rs[97-105]
- src-tauri/src/db/drivers/oracle.rs[121-162]
## Suggested fix
1. In `ensure_connection_with_db` / `ensure_connection_with_db_from_app_state`, after loading the `form`, gate the `form.database = Some(db_name)` override by driver type.
- For example: do **not** override `form.database` when `form.driver == "oracle"`.
- (Optional) if you need the per-request value for Oracle, consider mapping it into `form.schema` instead (only when `form.schema` is empty), since schema is the Oracle-scoped concept.
2. (Optional but recommended) Add a regression test around `ensure_connection_with_db` to ensure an Oracle form’s `database` (service name) is preserved when a `database` request param is provided.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+653
to
+707
| let first_kw = first_sql_keyword(&sql_clean); | ||
| let is_read = matches!( | ||
| first_kw.as_deref(), | ||
| Some("select") | Some("with") | Some("show") | ||
| ); | ||
|
|
||
| self.run_blocking(move |conn| { | ||
| if is_read { | ||
| let rows = conn | ||
| .query(&sql_clean, &[] as &[&dyn oracle::sql_type::ToSql]) | ||
| .map_err(|e| format!("[QUERY_ERROR] {e}"))?; | ||
|
|
||
| // Collect column metadata before consuming rows | ||
| let col_info: Vec<(String, String)> = rows | ||
| .column_info() | ||
| .iter() | ||
| .map(|c| (c.name().to_string(), format!("{}", c.oracle_type()))) | ||
| .collect(); | ||
| let columns: Vec<QueryColumn> = col_info | ||
| .iter() | ||
| .map(|(name, ty)| QueryColumn { | ||
| name: name.clone(), | ||
| r#type: ty.clone(), | ||
| }) | ||
| .collect(); | ||
|
|
||
| let mut data = Vec::new(); | ||
| for row_result in rows { | ||
| let row = row_result.map_err(|e| format!("[QUERY_ERROR] {e}"))?; | ||
| let mut map = serde_json::Map::new(); | ||
| for (i, (name, _)) in col_info.iter().enumerate() { | ||
| map.insert(name.clone(), oracle_value_to_json(&row, i)); | ||
| } | ||
| data.push(serde_json::Value::Object(map)); | ||
| } | ||
|
|
||
| Ok(QueryResult { | ||
| row_count: data.len() as i64, | ||
| data, | ||
| columns, | ||
| time_taken_ms: start.elapsed().as_millis() as i64, | ||
| success: true, | ||
| error: None, | ||
| }) | ||
| } else { | ||
| // DML or DDL — use Statement API to get affected-row count | ||
| let mut stmt = conn | ||
| .statement(&sql_clean) | ||
| .build() | ||
| .map_err(|e| format!("[QUERY_ERROR] {e}"))?; | ||
| stmt.execute(&[] as &[&dyn oracle::sql_type::ToSql]) | ||
| .map_err(|e| format!("[QUERY_ERROR] {e}"))?; | ||
| let row_count = stmt.row_count().unwrap_or(0) as i64; | ||
| // Commit so the change is visible after the connection closes. | ||
| conn.commit().map_err(|e| format!("[QUERY_ERROR] commit failed: {e}"))?; |
There was a problem hiding this comment.
3. With dml misrouted 🐞 Bug ≡ Correctness
OracleDriver.execute_query treats any SQL whose first keyword is WITH as a read query and routes it through conn.query() without committing. Oracle supports WITH-prefixed INSERT/UPDATE/DELETE/MERGE, so those statements will either error as “not a query” or execute without a commit.
Agent Prompt
## Issue description
`OracleDriver.execute_query` currently classifies all `WITH ...` statements as read-only and runs them via `conn.query()` without committing. Oracle allows `WITH` to prefix DML (INSERT/UPDATE/DELETE/MERGE), so this misclassification causes failures or uncommitted changes.
Concrete example that will be misclassified today:
```sql
WITH cte AS (SELECT 1 AS id, 'x' AS name FROM DUAL)
INSERT INTO MY_TABLE(id, name)
SELECT id, name FROM cte
```
## Issue Context
The current logic:
- `is_read` is true for `select | with | show`
- only the `else` branch calls `conn.commit()`
## Fix Focus Areas
- src-tauri/src/db/drivers/oracle.rs[650-707]
## Suggested fix approaches
Pick one:
1. **Safer fallback execution**
- If `first_kw == Some("with")`, attempt to execute using the Statement API in a way that can handle both query and non-query:
- Try building a statement and calling a query-style execution.
- If that fails with a known “not a query / invalid statement type” Oracle error, retry with execute+commit.
- Ensure you only fallback on specific, expected error codes/messages so you don’t mask real SQL errors.
2. **Parse second keyword for WITH**
- When the first keyword is `WITH`, scan forward (token-based) for the first *non-CTE* keyword that determines the statement kind (`SELECT`, `INSERT`, `UPDATE`, `DELETE`, `MERGE`).
- Treat `WITH ... SELECT` as read; treat `WITH ... INSERT/UPDATE/DELETE/MERGE` as write and commit.
Also add a unit/integration test covering a `WITH ... INSERT` statement to prevent regression.
ⓘ 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: 19
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/components/business/Sidebar/ConnectionList.tsx (2)
883-890:⚠️ Potential issue | 🟠 MajorDon't bail before hydrating the target connection tree.
If
activeTableTargetpoints at a connection whose databases have not been loaded yet,targetDatabaseisundefinedand this exits without ever callingfetchAndSetDatabases()/connectConnection(). After a cold load or reconnect, the reveal never completes until the user manually expands that connection.🤖 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 883 - 890, In ensureDatabaseTablesLoaded: don't return early when targetDatabase is undefined; instead if targetConnection exists but its databases are not loaded (targetDatabase === undefined and targetConnection is present), call the existing fetchAndSetDatabases() and/or connectConnection() for that connectionId to hydrate the connection tree, then re-resolve targetDatabase and proceed to reveal the table; reference ensureDatabaseTablesLoaded, activeTableTarget, connections, connectionId, databaseName, fetchAndSetDatabases, and connectConnection when making the change.
872-882:⚠️ Potential issue | 🟠 MajorExpand the database group before trying to reveal a table.
When
showSavedQueriesInTreeis enabled, the target node lives under${connectionId}::databases. This effect only opens the connection and database keys, so the table row never mounts and the later autoscroll quietly gives up.🤖 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 872 - 882, The effect that expands nodes currently calls setExpandedConnections and setExpandedDatabases but misses expanding the intermediate "databases" group when showSavedQueriesInTree is true; update the logic (around setExpandedConnections / setExpandedDatabases in ConnectionList.tsx) to also add the composite parent key `${connectionId}::databases` to the expanded connections set when showSavedQueriesInTree is enabled so the database group mounts before attempting to reveal the table (use the same Set update pattern as setExpandedConnections to add `${connectionId}::databases` alongside connectionId and ensure dbKey is added to setExpandedDatabases).
🧹 Nitpick comments (8)
docs/table-selection-optimization.md (5)
238-240: Performance: Avoid inline arrow functions in JSX for frequently rendered cells.The event handlers are defined as inline arrow functions, which creates new function instances on every render. In a large table with many cells, this can cause performance issues and unnecessary re-renders.
Consider using a memoized callback factory or event delegation pattern. For example:
// Outside the cell render, create a memoized handler factory const createCellMouseDownHandler = useCallback( (rowIndex: number, colIndex: number, column: string) => (e: React.MouseEvent) => handleCellMouseDown(e, rowIndex, colIndex, column), [handleCellMouseDown] ); // Or use event delegation on the parent table/tbodyAlternatively, attach handlers to the parent
<table>or<tbody>and useevent.targetto determine which cell was clicked.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/table-selection-optimization.md` around lines 238 - 240, Replace the inline arrow handlers on each cell with memoized handler factories or an event-delegation approach to avoid recreating functions on every render; specifically, create useCallback factories like createCellMouseDownHandler, createCellMouseEnterHandler and createCellClickHandler that accept rowIndex/colIndex/column and return the actual event handler which calls the existing handleCellMouseDown, handleCellMouseEnter and handleCellClick, or alternatively move handlers to the table/tbody and implement event delegation using event.target to resolve the cell indices—then update the JSX to reference the stable handlers instead of inline arrows.
358-365: Consider documenting accessibility and performance implications.The "Optional Enhancements" section mentions keyboard shortcuts, but the document would benefit from explicitly addressing:
Accessibility: How will screen readers announce range selections? Should ARIA attributes like
aria-selectedbe added to cells in the range?Performance: During drag selection,
setSelectedRangewill be called on every mouse move across cells. For large tables, consider throttling the state updates or using CSS-only visual feedback during drag with state update only on mouseup.Touch devices: The current proposal is mouse-only. Touch devices would need different interaction patterns (e.g., long-press to start selection).
Testing: Consider adding a section about how to test the range selection functionality (unit tests for selection logic, E2E tests for user interactions).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/table-selection-optimization.md` around lines 358 - 365, The "Optional Enhancements" section needs explicit guidance on accessibility, performance, touch support, and testing: document adding ARIA attributes (e.g., use aria-selected on table cells and describe how screen readers should announce range selections), recommend performance strategies for setSelectedRange (throttle/debounce updates or use CSS-only visual feedback during drag and update state on mouseup), describe touch-specific patterns (long-press to begin selection and touch-move handling), and add testing guidance (unit tests for selection logic and E2E tests for drag/touch/keyboard interactions like Shift+Click and Ctrl/Cmd+Click). Ensure the doc references the selection update entry point setSelectedRange and the cell attribute aria-selected so implementers can find where to apply these changes.
1-365: Suggestion: Use relative references instead of absolute line numbers.Throughout the document, specific line numbers are referenced (e.g., "第 543-561 行", "第 2004-2083 行"). These will become stale as the codebase evolves, making the documentation less maintainable.
Consider using function names, code patterns, or section markers instead:
- ✗ "位置:
TableView.tsx第 543-561 行"- ✓ "位置:
TableView.tsx中的handleCellClick函数"- ✓ "位置:
TableView.tsx中的单元格渲染逻辑 (<td>元素)"This makes the document more resilient to code changes and easier to maintain.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/table-selection-optimization.md` around lines 1 - 365, The document uses fragile absolute line-number references (e.g., "第 543-561 行"); update all such occurrences to refer to stable code anchors instead — use function/handler names and markup identifiers like handleCellClick, handleIndexMouseDown, handleIndexMouseEnter, the cell rendering <td> block, the selected-range state declarations (selectedRange, selectedRangeRef), the mouseup useEffect, getSelectedRangeCopyText and the ContextMenuItem copy handler in TableView.tsx — replace each "第 X-Y 行" note with a descriptive reference to the function/section or JSX element so readers can locate the code even if lines shift.
285-309: Add bounds checking for array access.Line 296 accesses
columns[c]without verifying thatcis within bounds. While the loop conditionc <= endColIndexshould ensure this, defensive programming would add a check to prevent potential runtime errors ifselectedRangestate becomes inconsistent.Consider adding:
const col = columns[c]; if (!col) continue; // Skip if column index is out of boundsAlso, consider memoizing this function only when
selectedRangechanges significantly, not on everycurrentDatachange, to avoid unnecessary recomputations.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/table-selection-optimization.md` around lines 285 - 309, In getSelectedRangeCopyText, add defensive bounds checking before using columns[c] (e.g., const col = columns[c]; if (!col) continue) to avoid runtime errors if selectedRange is inconsistent, and reduce unnecessary recomputation by removing currentData from the useCallback dependency array—keep [selectedRange, columns, getCellDisplayValue] so the memo only updates when the selection, column set, or cell-display logic changes rather than on every currentData mutation.
163-169: Empty function body may cause confusion.The
handleCellClickfunction is defined with an empty body and a comment explaining that "actual selection logic is in handleCellMouseDown." However, this function is still attached toonClickin the JSX (line 240). Consider either:
- Removing the
onClickhandler entirely if it's no longer needed, or- Adding a clear comment explaining why the handler must remain (e.g., for accessibility or to prevent event bubbling)
Additionally, the function doesn't use its parameters
rowIndexandcol, which suggests they can be removed from the signature.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/table-selection-optimization.md` around lines 163 - 169, handleCellClick currently has an empty body but remains bound to the onClick handler in the JSX; either remove the onClick binding where handleCellClick is used, or keep the handler and make its intent explicit: if it's required to consume the click (e.g., to stop propagation/accessibility), replace the empty body with a clear comment and minimal behavior (e.g., stopPropagation or blur) and remove unused parameters (rowIndex, col) from the signature; update any references to handleCellClick in the JSX accordingly (either delete the onClick prop or keep it and ensure the handler documents/implements the necessary no-op behavior).ADD_NEW_DB.md (2)
26-32: Line number references may become stale.The documentation references specific line numbers (e.g.,
mod.rs:64-121for the trait, lines 133-163 forconnect()), but the actual locations differ in the currentmod.rs(trait is around lines 79-128,connect()is around 140-175). Consider:
- Using function/type names instead of line numbers where possible
- Adding a note that line numbers are approximate and may shift
💡 Suggested change for Step 1
-必须实现 `DatabaseDriver` trait 的全部方法（定义见 `src-tauri/src/db/drivers/mod.rs:64-121`）： +必须实现 `DatabaseDriver` trait 的全部方法（定义见 `src-tauri/src/db/drivers/mod.rs` 中的 `pub trait DatabaseDriver`）：🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ADD_NEW_DB.md` around lines 26 - 32, The docs currently point to exact line numbers for the DatabaseDriver trait and connect(), which will become stale; update the text to reference the actual symbol names (e.g., the DatabaseDriver trait and the connect() function/impl) instead of file:line ranges and add a short note that line numbers are approximate and may shift as the codebase changes; ensure you mention the source file (mod.rs) by name and list the required trait methods (test_connection, get_databases, get_table_names, get_table_structure, get_table_info, get_table_data, execute_query, cancel_query, get_schema_names, get_table_ddl, get_schema_overview, close) so readers can locate them by name.
28-32: Add language specifiers to fenced code blocks.Several code blocks are missing language specifiers, which affects syntax highlighting and lint compliance:
- Lines 28-32: list of trait methods
- Lines 262-266: file paths
- Lines 278-280: file path
💡 Suggested fixes
-``` +```text test_connection, get_databases, get_table_names, get_table_structure,-``` +```text src-tauri/tests/common/{driver}_context.rs ← testcontainers 容器配置-``` +```text src-tauri/tests/{driver}_stateful_command_integration.rsAlso applies to: 262-266, 278-280
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ADD_NEW_DB.md` around lines 28 - 32, Several fenced code blocks lack language specifiers (e.g., the block containing "test_connection, get_databases, get_table_names, get_table_structure, get_table_info, get_table_data, execute_query, cancel_query, get_schema_names, get_table_ddl, get_schema_overview, close" and the blocks listing file paths like "src-tauri/tests/common/{driver}_context.rs ← testcontainers 容器配置" and "src-tauri/tests/{driver}_stateful_command_integration.rs"); update each opening triple-backtick to include an appropriate language (for these snippets use ```text) so the blocks become fenced with ```text to restore syntax highlighting and satisfy the linter.src-tauri/src/db/drivers/mod.rs (1)
29-33: Consider adding Windows path hint for Oracle Instant Client.The hint covers macOS (
DYLD_LIBRARY_PATH) and Linux (LD_LIBRARY_PATH), but DbPaw is a cross-platform application. On Windows, users need to add the Oracle Instant Client directory to thePATHenvironment variable.💡 Suggested enhancement
let hint = if lower.contains("dpi-1047") || lower.contains("cannot locate a 64-bit oracle client") { "hint: Oracle Instant Client is not installed — download it from \ https://www.oracle.com/database/technologies/instant-client/downloads.html \ and add the directory containing libclntsh to your library path \ - (macOS: DYLD_LIBRARY_PATH; Linux: LD_LIBRARY_PATH)" + (Windows: PATH; macOS: DYLD_LIBRARY_PATH; Linux: LD_LIBRARY_PATH)"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src-tauri/src/db/drivers/mod.rs` around lines 29 - 33, The hint text in mod.rs (the block that builds the variable named hint when matching lower.contains("dpi-1047") || lower.contains("cannot locate a 64-bit oracle client")) only mentions macOS and Linux library vars; update that message to also mention Windows users should add the Oracle Instant Client directory to their PATH environment variable (e.g., "Windows: add the Instant Client directory to PATH") so the hint covers all platforms and references the same hint variable and matching condition.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.claude/commands/new-db.md:
- Around line 417-425: Update the "Final Verification" checklist under the
"Final Verification" heading to include the repo's Rust verification steps: add
the commands `bun run lint:rust` (to run Rust linting via bun) and `cargo test
--lib` (to run Rust unit tests) in addition to the existing `cargo check
--manifest-path src-tauri/Cargo.toml`; ensure they appear in the same
three-check list so the final steps run in order: `bun run typecheck`, `bun run
lint`, `bun run lint:rust`, `cargo check --manifest-path src-tauri/Cargo.toml`,
and `cargo test --lib`.
In `@src-tauri/src/db/drivers/oracle.rs`:
- Around line 125-129: When SSH is enabled (check form.ssh_enabled) wrap
failures from crate::ssh::start_ssh_tunnel(form) into the driver's standard
connection failure error by mapping the returned Err to conn_failed_error(...)
instead of propagating raw SSH errors; replace the current ? propagation in the
block that sets effective_form.host/port and ssh_tunnel so that any Err from
start_ssh_tunnel is converted with conn_failed_error (include context like "SSH
tunnel setup failed") before returning, keeping effective_form and ssh_tunnel
assignment logic the same on success.
- Around line 549-553: The code uppercases schema/table names before quoting
(e.g., in the table_ref construction and other dictionary/query sites) which
breaks quoted mixed-case identifiers; update calls so quote_ident(...) and
escape_literal(...) receive the original schema and table variables (not
schema.to_uppercase() / table.to_uppercase()), e.g., change uses around
table_ref and the dictionary metadata query sites (the occurrences you saw near
the functions that build queries) to pass schema and table unchanged; leave any
explicit uppercasing only where unquoted comparisons to Oracle metadata require
it, but do not uppercase before calling quote_ident or escape_literal.
In `@src-tauri/src/db/drivers/postgres.rs`:
- Around line 1112-1125: The `_INT8` match arm currently maps array elements to
serde_json::Value::Number causing precision loss; update that arm (the match on
"_INT8" in postgres.rs) to mirror the scalar `INT8` handling by converting each
Some(i64) element to serde_json::Value::String(i64.to_string()) and leaving None
as Value::Null, preserving the outer Array and the existing
unwrap_or(serde_json::Value::Null) behavior so bigints in arrays retain full
precision for JS clients.
In `@src-tauri/tests/common/oracle_context.rs`:
- Line 15: The doc says ORACLE_PASSWORD is required but the code uses
env_or("ORACLE_PASSWORD", "") which supplies an empty default; change the code
so ORACLE_PASSWORD is truly required (remove the empty-string default and use
the required env lookup or fail-fast helper) instead of env_or with "" — update
the lookup around the env_or("ORACLE_PASSWORD", "") call in
tests/common/oracle_context.rs (the ORACLE_PASSWORD retrieval) to panic/error if
the variable is missing so docs and code match.
In `@src-tauri/tests/oracle_command_integration.rs`:
- Around line 17-20: The tests (prepare_test_table and callers using
oracle_form_from_test_context) are connecting to an external Oracle instance;
change them to use testcontainers so each integration test runs against an
isolated DB container: update prepare_test_table to accept a ConnectionForm
produced from a testcontainers-managed Oracle container, initialize that
container in the test setup (using the testcontainers crate, appropriate Oracle
image and wait strategy), and replace oracle_form_from_test_context() calls in
the tests with the container-derived form so all connections (e.g., in
OracleDriver::connect and subsequent test logic) target the testcontainer
instance; ensure container lifecycle is tied to the test scope to avoid leaking
external state.
In `@src-tauri/tests/oracle_integration.rs`:
- Around line 19-24: The test currently relies on
oracle_context::oracle_form_from_test_context() and an external ORACLE_SCHEMA
env var; instead provision an Oracle database via testcontainers inside the test
harness, replacing the external dependency with a container-backed instance:
create a test helper (or use existing testcontainers utilities) that starts an
Oracle container, waits until reachable, creates the required schema/user, and
returns a populated form struct (replace oracle_form_from_test_context usage) so
form.schema is set locally; ensure the helper injects the connection info into
the form and that the container is stopped/cleaned up at test teardown.
- Around line 8-13: unique_table_name() currently uses millisecond precision
which can collide when tests run in parallel; change it to append a
collision-proof suffix (e.g., a UUID v4 or a combination of
SystemTime::now().as_nanos() plus a short random hex) to the "DBPAW_ORA_IT_"
prefix to guarantee uniqueness; update the unique_table_name() function to
generate and return that stronger suffix (and keep callers/tests unchanged) so
setup/cleanup no longer flakes in parallel runs.
In `@src/App.tsx`:
- Around line 739-753: The placeholder tab created via setTabs (id: tabId,
fields: type "table", title, connection, database, connectionId, driver,
isLoading:true) lacks a table target so on load failure the code path that only
clears isLoading leaves an unusable tab; update the placeholder to include the
actual table identifier fields (e.g., tableName and schema) when creating it, or
alternatively remove/close the placeholder tab inside the failure/catch path
where isLoading is cleared so no target-less tab remains (adjust the same logic
used in the other occurrence around lines 810-812); ensure references to tabId,
isLoading, setTabs and setActiveTab are handled consistently.
- Around line 1155-1163: The current close-tab logic always activates
newTabs[newTabs.length - 1]; change it to pick the closed tab's neighbor
instead: inside the setTabs updater for setTabs and setActiveTab, determine the
index of the tab being closed (use the prev array to find the index of tabId
before filtering), then after computing newTabs choose the adjacent tab by
preferring the previous sibling (prev[index - 1]) if it exists, otherwise the
next sibling (newTabs[index] after removal); call revealSidebarForTab with that
neighbor id and return that id as the new active tab. Reference setTabs,
setActiveTab, tabId, newTabs, and revealSidebarForTab when making the change.
In `@src/components/business/DataGrid/ComplexValueViewer.tsx`:
- Around line 107-120: The component ComplexValueViewer is rendering hard-coded
user-facing strings (e.g., "JSON", "Tree", "Table", "Copy JSON", "Copied",
"key", "value", "array", "object") and must be routed through react-i18next;
import and call useTranslation inside the ComplexValueViewer component, replace
each literal label (menu/tab labels, button text, table headers, and type
labels) with t('namespace.key') lookups (choose or add descriptive keys like
complexViewer.json, complexViewer.tree, complexViewer.table,
complexViewer.copyJson, complexViewer.copied, complexViewer.key,
complexViewer.value, complexViewer.array, complexViewer.object), and ensure
every occurrence noted (including the table header block around keys, plus the
other ranges referenced: lines ~160-161 and ~183-225) is updated to use t(...)
so the strings follow app locale switching; also add corresponding keys to the
i18n locales files.
In `@src/components/business/DataGrid/TableView.tsx`:
- Around line 1863-1880: The placeholders and UI strings for the new
structured-data controls are hard-coded English and the viewer button lacks an
accessible name and keyboard-focus styling; update ColumnAutocompleteInput
usages (the instances named ColumnAutocompleteInput for WHERE and ORDER BY) to
pass localized strings via t('...') for placeholder text, ensure the viewer
title string is wrapped with t('...'), add an explicit aria-label to the viewer
button element, and add visible keyboard-focus styles (e.g., focus-visible:ring
or focus:outline classes) to the button so it shows on focus as well as hover;
apply the same i18n replacement for the other ColumnAutocompleteInput
occurrences mentioned around the 2100-2103 area.
In `@src/components/business/DataGrid/tableView/ColumnAutocompleteInput.tsx`:
- Around line 51-54: The useEffect that resets activeIndex on [hasSuggestions,
token?.text] misses clamping when the options list changes, causing
filteredOptions[activeIndex] to be undefined in acceptSuggestion; update the
effect (and the similar effect around the other occurrence) to
setActiveIndex(prev => Math.min(prev, Math.max(0, filteredOptions.length - 1)))
or setActiveIndex(0) when filteredOptions is empty so activeIndex is always
within [0, filteredOptions.length-1]; reference the useEffect that calls
setActiveIndex, the filteredOptions/options array, the activeIndex state, and
the acceptSuggestion function to ensure activeIndex is clamped whenever
options/filteredOptions change.
In `@src/components/business/DataGrid/tableView/utils.ts`:
- Around line 125-126: The search is using formatCellValue which returns an
abbreviated preview for large objects, causing false negatives; replace the call
to formatCellValue(value).toLowerCase() with the full serializer
cellValueToString(value).toLowerCase() so searches run against the complete
serialized cell value (ensure cellValueToString is imported/available where used
and keep the normalizedSearchKeyword comparison logic intact).
In `@src/components/business/Sidebar/Sidebar.tsx`:
- Around line 15-17: Export the existing SidebarRevealRequest type from
Sidebar.tsx (the interface extending ActiveTableTarget with id:number) and
update the other module that currently redefines it (the file where
SidebarRevealRequest was duplicated) to import that exported type instead; then
remove the duplicate local interface definition. Ensure the export uses the
exact symbol name SidebarRevealRequest and that the consumer file imports it and
uses the same type, leaving ActiveTableTarget references unchanged.
In `@src/lib/driver-registry.tsx`:
- Around line 167-168: getDriverConfig currently uses a non-null assertion and
will throw if given non-canonical IDs; update it to normalize and/or safely
handle mismatched IDs instead of crashing. Remove the "!" non-null assert in
getDriverConfig and either (a) introduce a normalization step (e.g.,
normalizeDriverId) that maps aliases like "postgresql" -> "postgres", "pgsql" ->
"postgres", "sqlite3" -> "sqlite", and case-folds values like "Oracle" before
lookup in DRIVER_REGISTRY, or (b) change getDriverConfig's return type to
DriverConfig | undefined and have callers handle missing configs gracefully;
reference getDriverConfig and DRIVER_REGISTRY when applying the normalization or
safe-return logic so runtime/imported values don't throw during render.
In `@src/lib/driver-registry.unit.test.ts`:
- Around line 110-205: Tests for helper functions omit the new "oracle" driver
causing blind spots; update the test cases for getDefaultPort,
isFileBasedDriver, isMysqlFamilyDriver, supportsSSLCA, and
supportsCreateDatabase to include assertions for "oracle" (e.g.,
expect(getDefaultPort("oracle")).toBe(<expected_port>),
expect(isFileBasedDriver("oracle")).toBe(false) or true as appropriate, and
corresponding expectations for isMysqlFamilyDriver, supportsSSLCA, and
supportsCreateDatabase) so the new driver is covered by these helper-unit tests.
In `@src/lib/keyboard.unit.test.ts`:
- Line 2: Replace the relative import in the unit test with the project path
alias: instead of importing isModKey and isEditableTarget from "./keyboard"
update the import to use "@/lib/keyboard" so the test imports isModKey and
isEditableTarget via the alias consistent with the frontend path policy.
In `@src/services/mocks.ts`:
- Around line 763-776: The current isArrayQuery/isComplexQuery logic matches any
substring "array"/"json" which misroutes queries; update those checks to detect
the actual table name (e.g., pg_arrays and json_test) using
word-boundary/table-name-aware matching on the normalized SQL (lower) instead of
plain substring checks, handling quoted/dotted identifiers; then keep the rest
of the selection that picks mockArrayTypeData, mockComplexTypeData or
mockQueryResult unchanged (refer to isArrayQuery, isComplexQuery,
mockArrayTypeData, mockComplexTypeData, mockQueryResult).
---
Outside diff comments:
In `@src/components/business/Sidebar/ConnectionList.tsx`:
- Around line 883-890: In ensureDatabaseTablesLoaded: don't return early when
targetDatabase is undefined; instead if targetConnection exists but its
databases are not loaded (targetDatabase === undefined and targetConnection is
present), call the existing fetchAndSetDatabases() and/or connectConnection()
for that connectionId to hydrate the connection tree, then re-resolve
targetDatabase and proceed to reveal the table; reference
ensureDatabaseTablesLoaded, activeTableTarget, connections, connectionId,
databaseName, fetchAndSetDatabases, and connectConnection when making the
change.
- Around line 872-882: The effect that expands nodes currently calls
setExpandedConnections and setExpandedDatabases but misses expanding the
intermediate "databases" group when showSavedQueriesInTree is true; update the
logic (around setExpandedConnections / setExpandedDatabases in
ConnectionList.tsx) to also add the composite parent key
`${connectionId}::databases` to the expanded connections set when
showSavedQueriesInTree is enabled so the database group mounts before attempting
to reveal the table (use the same Set update pattern as setExpandedConnections
to add `${connectionId}::databases` alongside connectionId and ensure dbKey is
added to setExpandedDatabases).
---
Nitpick comments:
In `@ADD_NEW_DB.md`:
- Around line 26-32: The docs currently point to exact line numbers for the
DatabaseDriver trait and connect(), which will become stale; update the text to
reference the actual symbol names (e.g., the DatabaseDriver trait and the
connect() function/impl) instead of file:line ranges and add a short note that
line numbers are approximate and may shift as the codebase changes; ensure you
mention the source file (mod.rs) by name and list the required trait methods
(test_connection, get_databases, get_table_names, get_table_structure,
get_table_info, get_table_data, execute_query, cancel_query, get_schema_names,
get_table_ddl, get_schema_overview, close) so readers can locate them by name.
- Around line 28-32: Several fenced code blocks lack language specifiers (e.g.,
the block containing "test_connection, get_databases, get_table_names,
get_table_structure, get_table_info, get_table_data, execute_query,
cancel_query, get_schema_names, get_table_ddl, get_schema_overview, close" and
the blocks listing file paths like "src-tauri/tests/common/{driver}_context.rs
← testcontainers 容器配置" and
"src-tauri/tests/{driver}_stateful_command_integration.rs"); update each opening
triple-backtick to include an appropriate language (for these snippets use
```text) so the blocks become fenced with ```text to restore syntax highlighting
and satisfy the linter.
In `@docs/table-selection-optimization.md`:
- Around line 238-240: Replace the inline arrow handlers on each cell with
memoized handler factories or an event-delegation approach to avoid recreating
functions on every render; specifically, create useCallback factories like
createCellMouseDownHandler, createCellMouseEnterHandler and
createCellClickHandler that accept rowIndex/colIndex/column and return the
actual event handler which calls the existing handleCellMouseDown,
handleCellMouseEnter and handleCellClick, or alternatively move handlers to the
table/tbody and implement event delegation using event.target to resolve the
cell indices—then update the JSX to reference the stable handlers instead of
inline arrows.
- Around line 358-365: The "Optional Enhancements" section needs explicit
guidance on accessibility, performance, touch support, and testing: document
adding ARIA attributes (e.g., use aria-selected on table cells and describe how
screen readers should announce range selections), recommend performance
strategies for setSelectedRange (throttle/debounce updates or use CSS-only
visual feedback during drag and update state on mouseup), describe
touch-specific patterns (long-press to begin selection and touch-move handling),
and add testing guidance (unit tests for selection logic and E2E tests for
drag/touch/keyboard interactions like Shift+Click and Ctrl/Cmd+Click). Ensure
the doc references the selection update entry point setSelectedRange and the
cell attribute aria-selected so implementers can find where to apply these
changes.
- Around line 1-365: The document uses fragile absolute line-number references
(e.g., "第 543-561 行"); update all such occurrences to refer to stable code
anchors instead — use function/handler names and markup identifiers like
handleCellClick, handleIndexMouseDown, handleIndexMouseEnter, the cell rendering
<td> block, the selected-range state declarations (selectedRange,
selectedRangeRef), the mouseup useEffect, getSelectedRangeCopyText and the
ContextMenuItem copy handler in TableView.tsx — replace each "第 X-Y 行" note with
a descriptive reference to the function/section or JSX element so readers can
locate the code even if lines shift.
- Around line 285-309: In getSelectedRangeCopyText, add defensive bounds
checking before using columns[c] (e.g., const col = columns[c]; if (!col)
continue) to avoid runtime errors if selectedRange is inconsistent, and reduce
unnecessary recomputation by removing currentData from the useCallback
dependency array—keep [selectedRange, columns, getCellDisplayValue] so the memo
only updates when the selection, column set, or cell-display logic changes
rather than on every currentData mutation.
- Around line 163-169: handleCellClick currently has an empty body but remains
bound to the onClick handler in the JSX; either remove the onClick binding where
handleCellClick is used, or keep the handler and make its intent explicit: if
it's required to consume the click (e.g., to stop propagation/accessibility),
replace the empty body with a clear comment and minimal behavior (e.g.,
stopPropagation or blur) and remove unused parameters (rowIndex, col) from the
signature; update any references to handleCellClick in the JSX accordingly
(either delete the onClick prop or keep it and ensure the handler
documents/implements the necessary no-op behavior).
In `@src-tauri/src/db/drivers/mod.rs`:
- Around line 29-33: The hint text in mod.rs (the block that builds the variable
named hint when matching lower.contains("dpi-1047") || lower.contains("cannot
locate a 64-bit oracle client")) only mentions macOS and Linux library vars;
update that message to also mention Windows users should add the Oracle Instant
Client directory to their PATH environment variable (e.g., "Windows: add the
Instant Client directory to PATH") so the hint covers all platforms and
references the same hint variable and matching condition.
🪄 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: 1f482542-3f97-4746-bf42-408311b6844f
⛔ Files ignored due to path filters (2)
package-lock.jsonis excluded by!**/package-lock.jsonsrc-tauri/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (45)
.claude/commands/new-db.md.github/workflows/release.ymlADD_NEW_DB.mdCLAUDE.mddocs/table-selection-optimization.mdpackage.jsonscripts/test-integration.shsrc-tauri/Cargo.tomlsrc-tauri/src/commands/transfer.rssrc-tauri/src/db/drivers/mod.rssrc-tauri/src/db/drivers/mysql.rssrc-tauri/src/db/drivers/oracle.rssrc-tauri/src/db/drivers/postgres.rssrc-tauri/src/ssh.rssrc-tauri/tauri.conf.jsonsrc-tauri/tests/common/oracle_context.rssrc-tauri/tests/oracle_command_integration.rssrc-tauri/tests/oracle_integration.rssrc-tauri/tests/postgres_integration.rssrc/App.tsxsrc/components/business/DataGrid/ComplexValueViewer.tsxsrc/components/business/DataGrid/TableView.tsxsrc/components/business/DataGrid/tableView/ColumnAutocompleteInput.tsxsrc/components/business/DataGrid/tableView/columnAutocomplete.tssrc/components/business/DataGrid/tableView/columnAutocomplete.unit.test.tssrc/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/SavedQueriesList.tsxsrc/components/business/Sidebar/Sidebar.tsxsrc/components/business/Sidebar/connection-list/helpers.tsxsrc/components/business/Sidebar/connection-list/helpers.unit.test.tssrc/lib/connection-form/rules.tssrc/lib/connection-form/rules.unit.test.tssrc/lib/connection-form/validate.unit.test.tssrc/lib/driver-registry.tsxsrc/lib/driver-registry.unit.test.tssrc/lib/keyboard.unit.test.tssrc/lib/sqlEditorDatabase.unit.test.tssrc/lockfile.unit.test.tssrc/services/api.tssrc/services/api.unit.test.tssrc/services/mocks.tssrc/types/bun-test.d.ts
Comment on lines
+417
to
+425
| ## Final Verification | ||
|
|
||
| Run these three checks and fix any errors before declaring done: | ||
|
|
||
| ```bash | ||
| bun run typecheck | ||
| bun run lint | ||
| cargo check --manifest-path src-tauri/Cargo.toml | ||
| ``` |
There was a problem hiding this comment.
Add the repo's Rust verification steps to the final checklist.
This currently stops at cargo check, but new drivers under src-tauri/src/**/*.rs are also expected to pass bun run lint:rust and cargo test --lib.
Suggested checklist update
bun run typecheck
bun run lint
+bun run lint:rust
cargo check --manifest-path src-tauri/Cargo.toml
+cargo test --lib --manifest-path src-tauri/Cargo.tomlAs per coding guidelines "Use cargo test --lib to run Rust unit tests defined with #[test] in source files" and "Run bun run lint:rust with cargo check to validate Rust code before committing".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.claude/commands/new-db.md around lines 417 - 425, Update the "Final
Verification" checklist under the "Final Verification" heading to include the
repo's Rust verification steps: add the commands `bun run lint:rust` (to run
Rust linting via bun) and `cargo test --lib` (to run Rust unit tests) in
addition to the existing `cargo check --manifest-path src-tauri/Cargo.toml`;
ensure they appear in the same three-check list so the final steps run in order:
`bun run typecheck`, `bun run lint`, `bun run lint:rust`, `cargo check
--manifest-path src-tauri/Cargo.toml`, and `cargo test --lib`.
Comment on lines
+125
to
+129
| if let Some(true) = form.ssh_enabled { | ||
| let tunnel = crate::ssh::start_ssh_tunnel(form)?; | ||
| effective_form.host = Some("127.0.0.1".to_string()); | ||
| effective_form.port = Some(tunnel.local_port as i64); | ||
| ssh_tunnel = Some(tunnel); |
There was a problem hiding this comment.
Wrap SSH tunnel setup failures as connection failures too.
Right now SSH auth/network errors bypass the driver's normal connection-error path, so the Oracle connect flow returns a different class of message than a direct socket failure.
As per coding guidelines "Use conn_failed_error() to provide context-aware error messages for connection failures (TLS issues, auth failures, network problems)".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src-tauri/src/db/drivers/oracle.rs` around lines 125 - 129, When SSH is
enabled (check form.ssh_enabled) wrap failures from
crate::ssh::start_ssh_tunnel(form) into the driver's standard connection failure
error by mapping the returned Err to conn_failed_error(...) instead of
propagating raw SSH errors; replace the current ? propagation in the block that
sets effective_form.host/port and ssh_tunnel so that any Err from
start_ssh_tunnel is converted with conn_failed_error (include context like "SSH
tunnel setup failed") before returning, keeping effective_form and ssh_tunnel
assignment logic the same on success.
Comment on lines
+549
to
+553
| let table_ref = format!( | ||
| "{}.{}", | ||
| quote_ident(&schema.to_uppercase()), | ||
| quote_ident(&table.to_uppercase()) | ||
| ); |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In Oracle Database, are quoted identifiers case-sensitive and stored with their exact case, such that uppercasing a quoted schema or table name changes which object is referenced?
💡 Result:
Yes.
- Unquoted identifiers are case-insensitive in Oracle: Oracle folds them to uppercase when interpreting them (so
hr.empandHR.EMPrefer to the same object). [1] - Double-quoted identifiers are case-sensitive and Oracle treats different casing as different names: e.g.,
employees,"employees","Employees", and"EMPLOYEES"can be different objects in the same namespace. Therefore, changing the case inside the quotes (such as uppercasing"MyTable"to"MYTABLE") changes which object is referenced (or can cause “object does not exist” if only the original-cased object exists). [1] - Oracle’s docs also state that a quoted user-defined identifier is case-sensitive (with a separate exception about omitting quotes in some PL/SQL contexts when the name is otherwise a valid ordinary identifier). [2]
Sources: [1] Oracle SQL Reference (“Nonquoted identifiers … interpreted as uppercase. Quoted identifiers are case sensitive.”) [2] Oracle Database PL/SQL Language Reference (“A quoted user-defined identifier is case-sensitive …”).
🏁 Script executed:
# First, check the oracle.rs file structure and the specific lines mentioned
wc -l src-tauri/src/db/drivers/oracle.rsRepository: codeErrorSleep/dbpaw
Length of output: 102
🏁 Script executed:
# Read the context around lines 549-553 and look for quote_ident usage
sed -n '530,570p' src-tauri/src/db/drivers/oracle.rsRepository: codeErrorSleep/dbpaw
Length of output: 1476
🏁 Script executed:
# Search for other occurrences of to_uppercase() with quote_ident in oracle.rs
rg -n 'to_uppercase.*quote_ident|quote_ident.*to_uppercase' src-tauri/src/db/drivers/oracle.rs -A 2 -B 2Repository: codeErrorSleep/dbpaw
Length of output: 249
🏁 Script executed:
# Look for dictionary filters pattern above line 549
sed -n '500,549p' src-tauri/src/db/drivers/oracle.rs | grep -n 'dictionary\|filter\|where\|to_uppercase\|quote_ident'Repository: codeErrorSleep/dbpaw
Length of output: 284
🏁 Script executed:
# Find where quote_ident is defined
rg -n 'fn quote_ident|quote_ident =' src-tauri/src/db/drivers/oracle.rs -A 5Repository: codeErrorSleep/dbpaw
Length of output: 594
🏁 Script executed:
# Look at the broader context of the dictionary filters (lines 1-50)
head -50 src-tauri/src/db/drivers/oracle.rsRepository: codeErrorSleep/dbpaw
Length of output: 1561
🏁 Script executed:
# Search for all to_uppercase() calls in oracle.rs to see the full pattern
rg -n 'to_uppercase()' src-tauri/src/db/drivers/oracle.rsRepository: codeErrorSleep/dbpaw
Length of output: 878
Do not uppercase schema and table names before quoting them for Oracle.
Oracle only folds unquoted identifiers to uppercase. When users create "CamelCaseTable" or "MixedSchema", uppercasing before quoting references a different object. For example, quote_ident(&"MyTable".to_uppercase()) produces "MYTABLE", not the originally created "MyTable".
This pattern exists across multiple methods: dictionary metadata queries (lines 305–306, 339–340, 399–400, 464–465, 511–512) and table data operations (lines 551–552). Pass the original schema and table names to quote_ident() and escape_literal() without modification.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src-tauri/src/db/drivers/oracle.rs` around lines 549 - 553, The code
uppercases schema/table names before quoting (e.g., in the table_ref
construction and other dictionary/query sites) which breaks quoted mixed-case
identifiers; update calls so quote_ident(...) and escape_literal(...) receive
the original schema and table variables (not schema.to_uppercase() /
table.to_uppercase()), e.g., change uses around table_ref and the dictionary
metadata query sites (the occurrences you saw near the functions that build
queries) to pass schema and table unchanged; leave any explicit uppercasing only
where unquoted comparisons to Oracle metadata require it, but do not uppercase
before calling quote_ident or escape_literal.
Comment on lines
+1112
to
+1125
| "_INT8" => row | ||
| .try_get::<Vec<Option<i64>>, _>(name) | ||
| .ok() | ||
| .map(|v| { | ||
| serde_json::Value::Array( | ||
| v.into_iter() | ||
| .map(|o| match o { | ||
| Some(n) => serde_json::Value::Number(n.into()), | ||
| None => serde_json::Value::Null, | ||
| }) | ||
| .collect(), | ||
| ) | ||
| }) | ||
| .unwrap_or(serde_json::Value::Null), |
There was a problem hiding this comment.
Inconsistent precision handling for _INT8 (BIGINT[]) causes data loss.
Scalar INT8 (lines 989-998) is correctly converted to String to preserve precision for JavaScript clients (values > 2^53 - 1 lose precision as JSON numbers). However, _INT8 array elements are converted to Number, causing precision loss for large BIGINT values.
This should match the scalar behavior for consistency and correctness.
🔧 Proposed fix to preserve BIGINT precision in arrays
"_INT8" => row
.try_get::<Vec<Option<i64>>, _>(name)
.ok()
.map(|v| {
serde_json::Value::Array(
v.into_iter()
.map(|o| match o {
- Some(n) => serde_json::Value::Number(n.into()),
+ Some(n) => serde_json::Value::String(n.to_string()),
None => serde_json::Value::Null,
})
.collect(),
)
})
.unwrap_or(serde_json::Value::Null),📝 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
| "_INT8" => row | |
| .try_get::<Vec<Option<i64>>, _>(name) | |
| .ok() | |
| .map(|v| { | |
| serde_json::Value::Array( | |
| v.into_iter() | |
| .map(|o| match o { | |
| Some(n) => serde_json::Value::Number(n.into()), | |
| None => serde_json::Value::Null, | |
| }) | |
| .collect(), | |
| ) | |
| }) | |
| .unwrap_or(serde_json::Value::Null), | |
| "_INT8" => row | |
| .try_get::<Vec<Option<i64>>, _>(name) | |
| .ok() | |
| .map(|v| { | |
| serde_json::Value::Array( | |
| v.into_iter() | |
| .map(|o| match o { | |
| Some(n) => serde_json::Value::String(n.to_string()), | |
| None => serde_json::Value::Null, | |
| }) | |
| .collect(), | |
| ) | |
| }) | |
| .unwrap_or(serde_json::Value::Null), |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src-tauri/src/db/drivers/postgres.rs` around lines 1112 - 1125, The `_INT8`
match arm currently maps array elements to serde_json::Value::Number causing
precision loss; update that arm (the match on "_INT8" in postgres.rs) to mirror
the scalar `INT8` handling by converting each Some(i64) element to
serde_json::Value::String(i64.to_string()) and leaving None as Value::Null,
preserving the outer Array and the existing unwrap_or(serde_json::Value::Null)
behavior so bigints in arrays retain full precision for JS clients.
| /// ORACLE_HOST – defaults to "localhost" | ||
| /// ORACLE_PORT – defaults to 1521 | ||
| /// ORACLE_USER – defaults to "system" | ||
| /// ORACLE_PASSWORD – no default (required) |
There was a problem hiding this comment.
Documentation and code inconsistency for ORACLE_PASSWORD.
The doc comment at line 15 states ORACLE_PASSWORD has "no default (required)", but line 34 uses env_or("ORACLE_PASSWORD", "") which provides an empty string as the default. This inconsistency could lead to confusing test failures when connecting to Oracle instances that require a password.
🔧 Option A: Make password truly required (matches docs)
fn oracle_form_from_local_env() -> ConnectionForm {
ConnectionForm {
driver: "oracle".to_string(),
host: Some(shared::env_or("ORACLE_HOST", "localhost")),
port: Some(shared::env_i64("ORACLE_PORT", 1521)),
username: Some(shared::env_or("ORACLE_USER", "system")),
- password: Some(shared::env_or("ORACLE_PASSWORD", "")),
+ password: Some(
+ std::env::var("ORACLE_PASSWORD")
+ .expect("ORACLE_PASSWORD environment variable is required")
+ ),
database: Some(shared::env_or("ORACLE_SERVICE", "FREE")),
schema: Some(shared::env_or("ORACLE_SCHEMA", "SYSTEM")),
..Default::default()
}
}🔧 Option B: Update docs to reflect empty default
-/// ORACLE_PASSWORD – no default (required)
+/// ORACLE_PASSWORD – defaults to "" (empty string)Also applies to: 34-34
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src-tauri/tests/common/oracle_context.rs` at line 15, The doc says
ORACLE_PASSWORD is required but the code uses env_or("ORACLE_PASSWORD", "")
which supplies an empty default; change the code so ORACLE_PASSWORD is truly
required (remove the empty-string default and use the required env lookup or
fail-fast helper) instead of env_or with "" — update the lookup around the
env_or("ORACLE_PASSWORD", "") call in tests/common/oracle_context.rs (the
ORACLE_PASSWORD retrieval) to panic/error if the variable is missing so docs and
code match.
Comment on lines
+15
to
+17
| interface SidebarRevealRequest extends ActiveTableTarget { | ||
| id: number; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if SidebarRevealRequest is constructed or typed elsewhere
rg -n "SidebarRevealRequest" --type=ts --type=tsxRepository: codeErrorSleep/dbpaw
Length of output: 91
🏁 Script executed:
rg "SidebarRevealRequest" -t ts -t tsxRepository: codeErrorSleep/dbpaw
Length of output: 91
🏁 Script executed:
rg "SidebarRevealRequest" --type-list | grep -i tsxRepository: codeErrorSleep/dbpaw
Length of output: 453
🏁 Script executed:
rg "SidebarRevealRequest" --color=neverRepository: codeErrorSleep/dbpaw
Length of output: 483
🏁 Script executed:
cat -n src/App.tsx | head -100Repository: codeErrorSleep/dbpaw
Length of output: 3559
🏁 Script executed:
sed -n '100,150p' src/App.tsxRepository: codeErrorSleep/dbpaw
Length of output: 1340
SidebarRevealRequest is duplicated in two places and should be unified.
The type is defined both in src/App.tsx (line ~119) and in src/components/business/Sidebar/Sidebar.tsx (lines 15-17). App.tsx should import the type from Sidebar instead of redefining it locally. Export SidebarRevealRequest from Sidebar.tsx and remove the duplicate definition from App.tsx.
🤖 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 15 - 17, Export the
existing SidebarRevealRequest type from Sidebar.tsx (the interface extending
ActiveTableTarget with id:number) and update the other module that currently
redefines it (the file where SidebarRevealRequest was duplicated) to import that
exported type instead; then remove the duplicate local interface definition.
Ensure the export uses the exact symbol name SidebarRevealRequest and that the
consumer file imports it and uses the same type, leaving ActiveTableTarget
references unchanged.
Comment on lines
+167
to
+168
| export const getDriverConfig = (driver: Driver): DriverConfig => | ||
| DRIVER_REGISTRY.find((d) => d.id === driver)!; |
There was a problem hiding this comment.
Don't let the central registry lookup crash on non-canonical driver IDs.
getDriverConfig() now sits underneath several UI helpers, but the ! assumes every runtime dbType is already canonical. A persisted/imported value like postgresql, pgsql, sqlite3, or mixed-case Oracle will now throw during render instead of being normalized or rejected gracefully.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/lib/driver-registry.tsx` around lines 167 - 168, getDriverConfig
currently uses a non-null assertion and will throw if given non-canonical IDs;
update it to normalize and/or safely handle mismatched IDs instead of crashing.
Remove the "!" non-null assert in getDriverConfig and either (a) introduce a
normalization step (e.g., normalizeDriverId) that maps aliases like "postgresql"
-> "postgres", "pgsql" -> "postgres", "sqlite3" -> "sqlite", and case-folds
values like "Oracle" before lookup in DRIVER_REGISTRY, or (b) change
getDriverConfig's return type to DriverConfig | undefined and have callers
handle missing configs gracefully; reference getDriverConfig and DRIVER_REGISTRY
when applying the normalization or safe-return logic so runtime/imported values
don't throw during render.
Comment on lines
+110
to
+205
| describe("getDefaultPort", () => { | ||
| test("returns correct ports for network drivers", () => { | ||
| expect(getDefaultPort("postgres")).toBe(5432); | ||
| expect(getDefaultPort("mysql")).toBe(3306); | ||
| expect(getDefaultPort("mariadb")).toBe(3306); | ||
| expect(getDefaultPort("tidb")).toBe(4000); | ||
| expect(getDefaultPort("clickhouse")).toBe(8123); | ||
| expect(getDefaultPort("mssql")).toBe(1433); | ||
| }); | ||
|
|
||
| test("returns null for file-based drivers", () => { | ||
| expect(getDefaultPort("sqlite")).toBeNull(); | ||
| expect(getDefaultPort("duckdb")).toBeNull(); | ||
| }); | ||
| }); | ||
|
|
||
| // ─── isFileBasedDriver ──────────────────────────────────────────────────────── | ||
|
|
||
| describe("isFileBasedDriver", () => { | ||
| test("returns true for file-based drivers", () => { | ||
| expect(isFileBasedDriver("sqlite")).toBe(true); | ||
| expect(isFileBasedDriver("duckdb")).toBe(true); | ||
| }); | ||
|
|
||
| test("returns false for network drivers", () => { | ||
| const networkDrivers: Driver[] = [ | ||
| "postgres", | ||
| "mysql", | ||
| "mariadb", | ||
| "tidb", | ||
| "clickhouse", | ||
| "mssql", | ||
| ]; | ||
| for (const d of networkDrivers) { | ||
| expect(isFileBasedDriver(d)).toBe(false); | ||
| } | ||
| }); | ||
| }); | ||
|
|
||
| // ─── isMysqlFamilyDriver ────────────────────────────────────────────────────── | ||
|
|
||
| describe("isMysqlFamilyDriver", () => { | ||
| test("returns true for MySQL-family drivers", () => { | ||
| expect(isMysqlFamilyDriver("mysql")).toBe(true); | ||
| expect(isMysqlFamilyDriver("mariadb")).toBe(true); | ||
| expect(isMysqlFamilyDriver("tidb")).toBe(true); | ||
| }); | ||
|
|
||
| test("returns false for non-MySQL drivers", () => { | ||
| const others: Driver[] = [ | ||
| "postgres", | ||
| "sqlite", | ||
| "duckdb", | ||
| "clickhouse", | ||
| "mssql", | ||
| ]; | ||
| for (const d of others) { | ||
| expect(isMysqlFamilyDriver(d)).toBe(false); | ||
| } | ||
| }); | ||
| }); | ||
|
|
||
| // ─── supportsSSLCA ──────────────────────────────────────────────────────────── | ||
|
|
||
| describe("supportsSSLCA", () => { | ||
| test("returns true for drivers with SSL CA support", () => { | ||
| expect(supportsSSLCA("postgres")).toBe(true); | ||
| expect(supportsSSLCA("mysql")).toBe(true); | ||
| expect(supportsSSLCA("mariadb")).toBe(true); | ||
| expect(supportsSSLCA("tidb")).toBe(true); | ||
| }); | ||
|
|
||
| test("returns false for drivers without SSL CA support", () => { | ||
| expect(supportsSSLCA("sqlite")).toBe(false); | ||
| expect(supportsSSLCA("duckdb")).toBe(false); | ||
| expect(supportsSSLCA("clickhouse")).toBe(false); | ||
| expect(supportsSSLCA("mssql")).toBe(false); | ||
| }); | ||
| }); | ||
|
|
||
| // ─── supportsCreateDatabase ─────────────────────────────────────────────────── | ||
|
|
||
| describe("supportsCreateDatabase", () => { | ||
| test("returns true for drivers that can create databases", () => { | ||
| expect(supportsCreateDatabase("postgres")).toBe(true); | ||
| expect(supportsCreateDatabase("mysql")).toBe(true); | ||
| expect(supportsCreateDatabase("mariadb")).toBe(true); | ||
| expect(supportsCreateDatabase("tidb")).toBe(true); | ||
| expect(supportsCreateDatabase("clickhouse")).toBe(true); | ||
| expect(supportsCreateDatabase("mssql")).toBe(true); | ||
| }); | ||
|
|
||
| test("returns false for file-based drivers", () => { | ||
| expect(supportsCreateDatabase("sqlite")).toBe(false); | ||
| expect(supportsCreateDatabase("duckdb")).toBe(false); | ||
| }); |
There was a problem hiding this comment.
The helper coverage still skips Oracle.
This suite adds Oracle to the registry, but these helper sections never assert Oracle's default port or the basic capability helpers. A regression in the new driver's connection defaults would still ship green.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/lib/driver-registry.unit.test.ts` around lines 110 - 205, Tests for
helper functions omit the new "oracle" driver causing blind spots; update the
test cases for getDefaultPort, isFileBasedDriver, isMysqlFamilyDriver,
supportsSSLCA, and supportsCreateDatabase to include assertions for "oracle"
(e.g., expect(getDefaultPort("oracle")).toBe(<expected_port>),
expect(isFileBasedDriver("oracle")).toBe(false) or true as appropriate, and
corresponding expectations for isMysqlFamilyDriver, supportsSSLCA, and
supportsCreateDatabase) so the new driver is covered by these helper-unit tests.
| @@ -0,0 +1,29 @@ | |||
| import { describe, expect, test } from "bun:test"; | |||
| import { isModKey, isEditableTarget } from "./keyboard"; | |||
There was a problem hiding this comment.
Use @/ alias instead of relative import
Please switch this import to the project alias to stay consistent with the frontend path policy.
As per coding guidelines, "Use path alias @/ to reference files from ./src/ directory".
Suggested change
-import { isModKey, isEditableTarget } from "./keyboard";
+import { isModKey, isEditableTarget } from "@/lib/keyboard";📝 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 { isModKey, isEditableTarget } from "./keyboard"; | |
| import { isModKey, isEditableTarget } from "@/lib/keyboard"; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/lib/keyboard.unit.test.ts` at line 2, Replace the relative import in the
unit test with the project path alias: instead of importing isModKey and
isEditableTarget from "./keyboard" update the import to use "@/lib/keyboard" so
the test imports isModKey and isEditableTarget via the alias consistent with the
frontend path policy.
Comment on lines
+763
to
776
| // Dedicated array-type dataset: SELECT * FROM pg_arrays | ||
| const isArrayQuery = | ||
| lower.includes("pg_arrays") || | ||
| lower.includes("array"); | ||
| // Dedicated complex-type dataset: SELECT * FROM json_test | ||
| const isComplexQuery = | ||
| !isArrayQuery && | ||
| (lower.includes("json_test") || | ||
| lower.includes("json") || | ||
| lower.includes("jsonb") || | ||
| lower.includes("complex")); | ||
| const result = { | ||
| ...mockQueryResult, | ||
| ...(isArrayQuery ? mockArrayTypeData : isComplexQuery ? mockComplexTypeData : mockQueryResult), | ||
| timeTakenMs: Math.floor(Math.random() * 100) + 20, |
There was a problem hiding this comment.
Match the special mock datasets on table name, not any array/json substring.
With this logic, unrelated queries like SELECT array_agg(...), SELECT json_agg(...), or even statements with those words in literals/comments get routed to the fixed pg_arrays/json_test fixtures. In mock mode that produces the wrong columns and row shape for otherwise normal query-editor flows.
More precise routing example
- const isArrayQuery =
- lower.includes("pg_arrays") ||
- lower.includes("array");
+ const isArrayQuery = /\bfrom\s+(?:\w+\.)?pg_arrays\b/.test(lower);
@@
- !isArrayQuery &&
- (lower.includes("json_test") ||
- lower.includes("json") ||
- lower.includes("jsonb") ||
- lower.includes("complex"));
+ !isArrayQuery && /\bfrom\s+(?:\w+\.)?json_test\b/.test(lower);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/services/mocks.ts` around lines 763 - 776, The current
isArrayQuery/isComplexQuery logic matches any substring "array"/"json" which
misroutes queries; update those checks to detect the actual table name (e.g.,
pg_arrays and json_test) using word-boundary/table-name-aware matching on the
normalized SQL (lower) instead of plain substring checks, handling quoted/dotted
identifiers; then keep the rest of the selection that picks mockArrayTypeData,
mockComplexTypeData or mockQueryResult unchanged (refer to isArrayQuery,
isComplexQuery, mockArrayTypeData, mockComplexTypeData, mockQueryResult).
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
New Features
Improvements
Documentation
Chores