Skip to content

Fix CLI bugs: DML classification, trace timeout, partition timestamp#65

Merged
apstndb merged 4 commits into
mainfrom
fix/cli-bugs-and-go125
Jun 28, 2026
Merged

Fix CLI bugs: DML classification, trace timeout, partition timestamp#65
apstndb merged 4 commits into
mainfrom
fix/cli-bugs-and-go125

Conversation

@apstndb

@apstndb apstndb commented Jun 28, 2026

Copy link
Copy Markdown
Owner

Summary

Rationale

Consolidate on memefish lexer-based utilities for statement classification and splitting rather than maintaining parallel ad-hoc parsers.

Test plan

  • go test ./...
  • TestWithCancelPreservesParentDeadline documents trace context wiring
  • Emulator integration still passes with memefish-split fixtures

Fixes #57, #58, #60, #61

Strip leading SQL comments before DML classification (#58).
Derive trace context from the user timeout instead of Background (#57).
Honor read timestamp bound in --try-partition-query (#60).
Align README and ko publish workflow with go 1.25 (#61).

Co-authored-by: Cursor <cursoragent@cursor.com>

@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 replaces regex-based DML detection with a custom SQL classifier (isDML) that strips leading comments and whitespace, updates Go version requirements to 1.25, refactors telemetry shutdown logic to prevent fatal crashes, and updates batch transaction options. Feedback points out that strings.Cut(rest, " ") fails to isolate the keyword if it is followed by other whitespace characters like newlines or tabs, and suggests using strings.IndexAny along with adding corresponding test cases.

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 sqlclassify.go Outdated
Comment thread sqlclassify_test.go Outdated
Replace hand-rolled DML detection with gsqlutils/stmtkind.IsDMLLexical
and drop gsqlsep in favor of memefish.SplitRawStatements for emulator
fixtures. Keeps trace timeout, partition timestamp bound, and Go 1.25 fixes.

Co-authored-by: Cursor <cursoragent@cursor.com>
@apstndb

apstndb commented Jun 28, 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 project to Go 1.25, replaces regex-based DML detection with stmtkind.IsDMLLexical from gsqlutils, refactors telemetry shutdown handling, and updates the batch transaction initialization. It also adds a new context deadline test and migrates integration tests to use memefish for statement splitting. Feedback on the changes suggests removing the unused sqlclassify.go and sqlclassify_test.go files, and deferring bt.Cleanup(ctx) to prevent potential resource leaks during batch read-only transactions.

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 sqlclassify.go Outdated
Comment thread main.go
apstndb and others added 2 commits June 29, 2026 02:59
Delete unused local DML classifier superseded by stmtkind.IsDMLLexical.
Ensure BatchReadOnlyTransaction.Cleanup runs on PartitionQuery errors.

Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
@apstndb apstndb merged commit c27c8a5 into main Jun 28, 2026
2 checks passed
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.

Respect --timeout when Cloud Trace export is enabled

1 participant