Skip to content

Conversation

@tianzhou
Copy link
Member

No description provided.

tianzhou and others added 9 commits December 26, 2025 00:00
🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <[email protected]>
🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <[email protected]>
- Backend: Replace SQLResult with StatementResult (includes sql field)
- Backend: All connectors return StatementResult[] array
- Backend: execute-sql tool returns statements array in response
- Frontend: Each statement result becomes a separate tab
- Frontend: Multi-statement tabs labeled as "12:34:05 (1/3)"
- Frontend: Remove redundant executedSql (now in result.sql)

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

Co-Authored-By: Claude Opus 4.5 <[email protected]>
Remove statement index/total suffix from multi-statement tabs.

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

Co-Authored-By: Claude Opus 4.5 <[email protected]>
🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <[email protected]>
- Add ChevronLeft/Right icons
- Show arrow buttons when tabs overflow
- Hide native scrollbar
- Smooth scroll on arrow click

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

Co-Authored-By: Claude Opus 4.5 <[email protected]>
Copilot AI review requested due to automatic review settings December 26, 2025 09:15
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR implements multi-statement SQL execution support, allowing users to execute multiple SQL statements separated by semicolons in a single query. The backend changes the return type from a single SQLResult to an array of StatementResult[], with each statement's results tracked separately. The frontend introduces a tabbed interface to display results from multiple statements, with each tab showing the results of one statement.

  • Refactored all database connectors (Postgres, MySQL, MariaDB, SQLite, SQL Server) to return per-statement results
  • Added frontend tabbed interface for viewing multi-statement results with horizontal scrolling
  • Updated API layer to handle array of statement results instead of single result

Reviewed changes

Copilot reviewed 16 out of 16 changed files in this pull request and generated 14 comments.

Show a summary per file
File Description
src/connectors/interface.ts Renamed SQLResult to StatementResult and added sql field; changed executeSQL return type to array
src/connectors/sqlserver/index.ts Implemented multi-statement support with statement splitting and per-statement result tracking
src/connectors/sqlite/index.ts Implemented multi-statement support with individual statement execution and result collection
src/connectors/postgres/index.ts Implemented multi-statement support with transaction wrapping for consistency
src/connectors/mysql/index.ts Implemented multi-statement support using MySQL2's native multi-statement capability
src/connectors/mariadb/index.ts Implemented multi-statement support with statement splitting and result parsing
src/tools/execute-sql.ts Updated response builder to map statement results into the expected format
frontend/src/components/views/ToolDetailView.tsx Refactored to handle multiple result tabs instead of single result, with tab management logic
frontend/src/components/tool/ResultsTable.tsx Updated to receive StatementResult and display SQL from result object
frontend/src/components/tool/ResultsTabs.tsx New component implementing tabbed interface with horizontal scrolling and tab close functionality
frontend/src/components/tool/types.ts New type definitions for ResultTab interface
frontend/src/components/tool/index.ts Added exports for new ResultsTabs component and ResultTab type
frontend/src/components/icons/XIcon.tsx New close icon component for tab close buttons
frontend/src/components/icons/ChevronRightIcon.tsx New right chevron icon for tab scroll button
frontend/src/components/icons/ChevronLeftIcon.tsx New left chevron icon for tab scroll button
frontend/src/api/tools.ts Updated to parse array of statement results and transform them into StatementResult[]

Comment on lines +517 to +519
const statements = sql.split(';')
.map(statement => statement.trim())
.filter(statement => statement.length > 0);
Copy link

Copilot AI Dec 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Splitting SQL statements on semicolons can break valid SQL that contains semicolons within string literals, comments, or stored procedure bodies. For example, "SELECT 'hello;world'" would be incorrectly split into two statements. Consider using a proper SQL parser or implementing a state machine that respects SQL syntax rules for quoted strings and comments.

Copilot uses AI. Check for mistakes.
Comment on lines 453 to 474
for (let statement of statements) {
const trimmedStatement = statement.toLowerCase().trim();
if (trimmedStatement.startsWith('select') ||
trimmedStatement.startsWith('with') ||
trimmedStatement.startsWith('explain') ||
trimmedStatement.startsWith('analyze') ||
(trimmedStatement.startsWith('pragma') &&
(trimmedStatement.includes('table_info') ||
trimmedStatement.includes('index_info') ||
trimmedStatement.includes('index_list') ||
trimmedStatement.includes('foreign_key_list')))) {
readStatements.push(statement);
const isReadStatement = trimmedStatement.startsWith('select') ||
trimmedStatement.startsWith('with') ||
trimmedStatement.startsWith('explain') ||
trimmedStatement.startsWith('analyze') ||
(trimmedStatement.startsWith('pragma') &&
(trimmedStatement.includes('table_info') ||
trimmedStatement.includes('index_info') ||
trimmedStatement.includes('index_list') ||
trimmedStatement.includes('foreign_key_list')));

if (isReadStatement) {
// Apply maxRows limit to SELECT queries if specified
const processedStatement = SQLRowLimiter.applyMaxRows(statement, options.maxRows);
const rows = this.db.prepare(processedStatement).all();
results.push({ sql: statement, rows, rowCount: rows.length });
} else {
writeStatements.push(statement);
const result = this.db.prepare(statement).run();
results.push({ sql: statement, rows: [], rowCount: result.changes });
}
}
Copy link

Copilot AI Dec 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the multi-statement path, if any statement fails during execution in the for loop, there's no error logging to help with debugging. Unlike the single-statement path which logs detailed error information when parameters are present, the multi-statement path provides no such debugging context about which specific statement failed. Consider adding error logging similar to the single-statement path.

Copilot uses AI. Check for mistakes.
Comment on lines +510 to +512
const statements = sql.split(';')
.map(statement => statement.trim())
.filter(statement => statement.length > 0);
Copy link

Copilot AI Dec 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Splitting SQL statements on semicolons can break valid SQL that contains semicolons within string literals, comments, or stored procedure bodies. For example, "SELECT 'hello;world'" would be incorrectly split into two statements. Consider using a proper SQL parser or implementing a state machine that respects SQL syntax rules for quoted strings and comments.

Copilot uses AI. Check for mistakes.
Comment on lines +582 to 594
const result = await this.connection.request().query(processedStatement);

// Add each statement result to the array
results.push({
sql: statement,
rows: result.recordset || [],
rowCount: result.rowsAffected[0] || 0,
});
}

return results;
}
} catch (error) {
Copy link

Copilot AI Dec 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the multi-statement path, if any statement fails during execution in the for loop, the error is caught by the outer catch block and wrapped in a generic error message. This loses important context about which specific statement failed. Unlike the single-statement path which logs detailed error information (SQL and parameters), the multi-statement path provides no such debugging information. Consider adding similar error logging for multi-statement queries.

Suggested change
const result = await this.connection.request().query(processedStatement);
// Add each statement result to the array
results.push({
sql: statement,
rows: result.recordset || [],
rowCount: result.rowsAffected[0] || 0,
});
}
return results;
}
} catch (error) {
try {
const result = await this.connection.request().query(processedStatement);
// Add each statement result to the array
results.push({
sql: statement,
rows: result.recordset || [],
rowCount: result.rowsAffected[0] || 0,
});
} catch (statementError) {
// Log detailed information about the failing statement before propagating the error
console.error("Error executing SQL Server statement in multi-statement query", {
statement,
processedStatement,
options,
error: statementError,
});
throw statementError;
}
}
return results;
}
} catch (error) {
// Log high-level context for any failure in executeSQL
console.error("Failed to execute SQL Server query", {
sqlQuery,
options,
parameters,
error,
});

Copilot uses AI. Check for mistakes.
Comment on lines +575 to +580
for (let i = 0; i < statements.length; i++) {
const statementResult = firstResult[i];
const rows = parseQueryResults(statementResult);
const rowCount = extractAffectedRows(statementResult);
results.push({ sql: statements[i], rows, rowCount });
}
Copy link

Copilot AI Dec 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's a potential mismatch between the number of statements and results. If the result array from MySQL contains fewer elements than the statements array (which could happen if some statements fail silently or are optimized away), this will push undefined results. Consider adding a guard to ensure i is within bounds of firstResult array.

Copilot uses AI. Check for mistakes.
Comment on lines +511 to +513
const statements = sqlQuery.split(';')
.map(statement => statement.trim())
.filter(statement => statement.length > 0);
Copy link

Copilot AI Dec 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Splitting SQL statements on semicolons can break valid SQL that contains semicolons within string literals, comments, or stored procedure bodies. For example, "SELECT 'hello;world'" would be incorrectly split into two statements. Consider using a proper SQL parser or implementing a state machine that respects SQL syntax rules for quoted strings and comments.

Copilot uses AI. Check for mistakes.
tianzhou and others added 4 commits December 26, 2025 01:21
Replace local isReadStatement logic with shared allowed-keywords utility.

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

Co-Authored-By: Claude Opus 4.5 <[email protected]>
Resolved conflicts in frontend files keeping multi-statement SQL
support with per-statement tabs and arrow navigation.

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

Co-Authored-By: Claude Opus 4.5 <[email protected]>
@tianzhou tianzhou closed this Dec 26, 2025
@tianzhou tianzhou deleted the multi-statement branch December 26, 2025 15:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants