Skip to content

Use spaniter ResultSetStats helpers#45

Merged
apstndb merged 3 commits into
mainfrom
use-spaniter-resultsetstats
Jun 26, 2026
Merged

Use spaniter ResultSetStats helpers#45
apstndb merged 3 commits into
mainfrom
use-spaniter-resultsetstats

Conversation

@apstndb

@apstndb apstndb commented Jun 26, 2026

Copy link
Copy Markdown
Owner

Summary

  • Update spaniter to v0.1.1-alpha.3.
  • Use spaniter.Stats as the boundary when rebuilding *sppb.ResultSet for jq inputs.
  • Replace local ResultSetStats construction and empty-stats checks with Stats.ResultSetStats and Stats.IsZero.

Context

This validates the downstream use case that motivated the new spaniter helpers: execspansql already drains/consumes *spanner.RowIterator through spaniter, then needs to expose the captured lifecycle data as a protobuf ResultSet.

Tests

  • go test ./...
  • make test

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Code Review

This pull request updates the github.com/apstndb/spaniter dependency and refactors BuildResultSet and BuildResultSetFromParts to utilize the spaniter.Stats struct instead of passing individual query plan, query stats, and row count parameters. The review feedback suggests adding a defensive nil check for rowIter in BuildResultSet to prevent potential nil pointer dereferences, along with importing the errors package to support this check.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread jqresult/protojson.go
Comment thread jqresult/protojson.go
@apstndb

apstndb commented Jun 26, 2026

Copy link
Copy Markdown
Owner Author

/gemini review

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Code Review

This pull request updates the github.com/apstndb/spaniter dependency to version v0.1.1-alpha.3 and refactors the result set construction logic. Specifically, BuildResultSet and BuildResultSetFromParts in jqresult/protojson.go have been updated to accept and utilize the spaniter.Stats struct instead of passing individual query plan, query stats, and row count parameters. Callers in main.go have been updated accordingly to pass result.Stats directly. There are no review comments, and I have no additional feedback to provide.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

@apstndb

apstndb commented Jun 26, 2026

Copy link
Copy Markdown
Owner Author

/gemini review

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Code Review

This pull request updates the github.com/apstndb/spaniter dependency to v0.1.1-alpha.3 and refactors BuildResultSet and BuildResultSetFromParts in the jqresult package to utilize the spaniter.Stats struct. It also adds a nil check for the row iterator in BuildResultSet along with a corresponding unit test. I have no feedback to provide.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

@apstndb apstndb merged commit 10a8bf1 into main Jun 26, 2026
2 checks passed
@apstndb apstndb deleted the use-spaniter-resultsetstats branch June 26, 2026 05:33
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.

1 participant