Skip to content

allow multiple sql statements#114

Merged
p-hoffmann merged 1 commit into
developfrom
p-hoffmann/multisql
Aug 13, 2025
Merged

allow multiple sql statements#114
p-hoffmann merged 1 commit into
developfrom
p-hoffmann/multisql

Conversation

@p-hoffmann

Copy link
Copy Markdown
Contributor

No description provided.

Copilot AI review requested due to automatic review settings August 13, 2025 03:51

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

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 modifies the SQL query execution mechanism to allow multiple SQL statements to be executed in a single batch. The change switches from executing individual SQL statements to using batch execution, which enables support for multiple statements separated by semicolons.

Key changes:

  • Replace single statement execution with batch execution
  • Update affected rows handling to always return 0

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Comment thread ext/trex/sql/duckdb.rs
.map(|_affected_rows| {
vec![Response::Execution(Tag::new("OK").with_rows(0))]
})
.map_err(|e| PgWireError::ApiError(Box::new(e)))

Copilot AI Aug 13, 2025

Copy link

Choose a reason for hiding this comment

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

Always returning 0 affected rows loses important information. The execute_batch method may not return affected row counts, but this hardcoded 0 could mislead clients expecting accurate row counts for INSERT/UPDATE/DELETE operations.

Copilot uses AI. Check for mistakes.
Comment thread ext/trex/sql/duckdb.rs
.map(|affected_rows| {
vec![Response::Execution(Tag::new("OK").with_rows(affected_rows))]
.execute_batch(_query)
.map(|_affected_rows| {

Copilot AI Aug 13, 2025

Copy link

Choose a reason for hiding this comment

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

The variable _affected_rows is prefixed with underscore but then not used. If execute_batch doesn't return affected rows, consider renaming to _ or removing the variable entirely for clarity.

Suggested change
.map(|_affected_rows| {
.map(|_| {
Copilot uses AI. Check for mistakes.
@coveralls

Copy link
Copy Markdown

Pull Request Test Coverage Report for Build 16927020019

Details

  • 0 of 3 (0.0%) changed or added relevant lines in 1 file are covered.
  • 2 unchanged lines in 2 files lost coverage.
  • Overall coverage increased (+0.003%) to 38.431%

Changes Missing Coverage Covered Lines Changed/Added Lines %
ext/trex/sql/duckdb.rs 0 3 0.0%
Files with Coverage Reduction New Missed Lines %
ext/node/ops/crypto/primes.rs 1 83.96%
ext/runtime/external_memory.rs 1 73.08%
Totals Coverage Status
Change from base Build 16863662250: 0.003%
Covered Lines: 14840
Relevant Lines: 38615

💛 - Coveralls
@p-hoffmann p-hoffmann merged commit 45755f4 into develop Aug 13, 2025
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

3 participants