GeezerVison

The World Wide Web, restored to 800 pixels of glory.

GeezerVison is showing github.com — fix(output): flush CSV rows already written when the result stream fails by pcbeingused333 · Pull Request #905 · stackql/stackql

Conversation

pcbeingused333 commented Nov 5, 2026

Copy link

Copy Markdown

Contributor

Description

With --output csv, if the result stream fails midway, the rows already passed to the buffered csv.Writer were lost: CSVWriter.Write returned the stream error without calling Flush(). The table writer already keeps completed rows in this case (TestTableWriterRetainsCompletedRowsOnStreamFailure); this brings CSV in line.

Fix: flush the CSV writer (when it has been created) before returning the stream error.

Type of change

Bug fix (non-breaking change to fix a bug).
Feature (non-breaking change to add functionality).
Breaking change.
Other (eg: documentation change). Please explain.

Issues referenced.

None filed; found while comparing the CSV writer with the table writer's partial-output handling.

Evidence

New unit test TestCSVWriterRetainsCompletedRowsOnStreamFailure (mirrors the table one): fails without the change, passes with it. go test ./internal/stackql/output/ passes; gofmt -l is clean.

Checklist:

A full round of testing has been completed, and there are no test failures as a result of these changes.
The changes are covered with functional and/or integration robot testing.
The changes work on all supported platforms.
Unit tests pass locally, as per the developer guide.
Robot tests pass locally, as per the developer guide.
Linter passes locally, as per the developer guide.

Variations

Only the internal/stackql/output package tests were run; robot tests and the full suite were not run for this three-line change.

Tech Debt

None.

This change was prepared with the assistance of an AI coding tool and reviewed before submission.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions


          fix(output): flush CSV rows already written when the result stream fails

37343d9

Co-Authored-By: Claude Sonnet 5.5 <[email protected]>

jeffreyaven requested a balanced review from Copilot

November 9, 2026 20:25

Copilot started reviewing on behalf of jeffreyaven

November 9, 2026 20:26

View session

Copilot AI reviewed

Nov 9, 2026

View reviewed changes

Copilot AI left a comment

Copy link

Copy Markdown

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

🟡 Changes recommended

The CSV change lacks the required mocked-provider Robot regression verifying that the CLI retains the header and completed rows after a stream failure while still reporting the error.

1 open issue

Add regression test for partial CSV output on stream failure · New

What changed in this PR

This PR preserves buffered CSV rows when a result stream fails, aligning CSV output with the table writer’s partial-output behavior.

Changes:

Flushes an initialized CSV writer before returning the stream error.
Adds a unit regression test checking that the header and completed row survive while the original error is returned.

File	Description
`internal/stackql/output/partial_table_test.go`	Adds CSV partial-output regression coverage.
`internal/stackql/output/output.go`	Flushes buffered CSV output on stream failure.

_{Review effort: Balanced}

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Comment thread

internal/stackql/output/output.go

Comment on lines +402 to +404

		if w != nil {
		w.Flush()
		}

jeffreyaven self-assigned this

Nov 12, 2026

Reviewers

Copilot code review Copilot

Copilot review effort

Applies to this pull request for everyone.Learn more about Copilot code review.

Copilot left review comments

At least 1 approving review is required to merge this pull request.

Uh oh!

There was an error while loading. Please reload this page.

Assignees

jeffreyaven

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

3 participants

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.

You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.