refactor: extract CQL into feature/cql (part of #778)

[apstndb]

Summary

PR2 of #778: extract the CQL (Cassandra interface) statement family out of the core internal/mycli package into internal/mycli/feature/cql, behind the Feature registration seam. After this change core no longer imports github.com/gocql/gocql or github.com/googleapis/go-spanner-cassandra. Follows the BIGQUERY extraction pattern (#781).

Scope

• New package internal/mycli/feature/cql: moves CQLStatement, the fail-closed firstCQLKeyword/cqlStatementMutates classifier, formatCassandraTypeName, and Execute. The zapcore.WarnLevel.String() usage moves with it (zap stays a core dep for gRPC-interceptor logging; only the gocql/spancql import lines leave core).
• Marker wiring: CQLStatement embeds mycli.MutationClassifier with Classify wired in newCQLStatement to cqlStatementMutates over its own CQL text (mirrors newBigQueryStatement), carries its own var _ mycli.ConditionallyMutatingStatement = (*CQLStatement)(nil) assertion, and the core assertion is removed. It is deliberately not MarksDetachedCompatible (the Cassandra adapter binds to the Spanner database, so CQL can't run in detached mode).
• Feature(): contributes the "CQL" def only (EARLY EXPERIMENTAL note preserved), no vars, no flags, fresh instances per call, no package-level mutable state.
• Session surgery: removed cqlCluster/cqlSession fields and the cqlSession Close block. The lazy CQL session is reimplemented on mycli.FeatureState (key cql.session) as a mutex-guarded, build-once io.Closer holder that closes the gocql session (the cluster is not separately closed, same as today), preserving today's spancql.NewCluster build (DatabaseUri: session.DatabasePath(), Timeout = 5s) and error messages exactly. No rebuild-on-key-change rule (DatabasePath can't change on a live Session).
• Registration: feature/all.All() returns {cql.Feature(), bigquery.Feature()}; merged order stays LLM, CQL, BIGQUERY (LLM still in core at the end of the core table).
• Tests: classifier (TestCQLStatementMutates) and dispatch/classification tests move into feature/cql; the dispatch-level READONLY guard for mutating CQL moves into an external package mycli_test file (cql_readonly_guard_test.go).

Byte-identity proof

make docs-update then git diff --exit-code README.md docs/system_variables.md -> exit 0 (byte-identical). The CQL statement-help row keeps its position because the merged order (llm, cql, bigquery) matches the pre-extraction ordering already established by #780/#781.

Dependency check

$ go list -deps ./internal/mycli | grep -E 'gocql|go-spanner-cassandra'
(no output)

Root main still pulls them via feature/all (correct):

$ go list -deps . | grep -E 'gocql|go-spanner-cassandra'
github.com/gocql/gocql/internal/lru
github.com/gocql/gocql/internal/murmur
github.com/gocql/gocql/internal/streams
github.com/gocql/gocql
github.com/googleapis/go-spanner-cassandra/logger
github.com/googleapis/go-spanner-cassandra/adapter
github.com/googleapis/go-spanner-cassandra/cassandra/gocql

Validation

• make check — pass (0 lint issues, formatting correct)
• make check-race — pass
• go vet ./... — clean
• docs byte-identity — no diff

Deviation from the task spec (allowed-SELECT guard test)

The task asked the external guard test to mirror bigquery_readonly_guard_test.go "exactly", including running an allowed CQL SELECT through Session.ExecuteStatement and expecting it to "fail later for a non-READONLY reason". That is not safe for CQL and is intentionally not done:

• BigQuery's Execute has a pre-network guard (effectiveProject returns "" -> immediate "project not configured" error), so the allowed SELECT fails fast with no I/O.
• CQL's Execute has no such guard; a passing SELECT reaches spancql.NewCluster, which panics (NewCluster panics when NewTCPProxy/newAdapterClient fails) whenever the Cassandra adapter client cannot be built — i.e. always in unit tests, with or without ADC. Running it through Execute would crash the test binary, not return an error. This is the same reason the original in-core TestReadOnlyGuardAllowsReadOnlyCQL asserted classification directly instead of calling Execute.

Coverage is preserved without weakening the #757 guarantees:

• cql_readonly_guard_test.go runs the blocked mutating cases through real dispatch + ExecuteStatement (short-circuit at the guard, no adapter built) and asserts the READONLY sentinel; the allowed SELECT is asserted at dispatch level to be a non-static MutationStatement.
• feature/cql/cql_test.go (TestCQLStatementConditionalMutation) asserts the wired classifier directly (read-only SELECT -> non-mutating, DELETE -> mutating) via the exported Classify field.
• feature/cql/classifier_test.go (TestCQLStatementMutates) keeps the exhaustive fail-closed keyword coverage.

Part of #778

🤖 Generated with Claude Code

https://claude.ai/code/session_01V28qKRVwUCkpT5pepTksgr

[gemini-code-assist]

Code Review

This pull request extracts the CQL (Cassandra Query Language) statement family and its Cassandra adapter integration from the core mycli package into a dedicated optional feature package under internal/mycli/feature/cql. This refactoring decouples the core package from Cassandra-specific dependencies such as gocql and go-spanner-cassandra. The PR registers the new CQL feature, migrates the statement classification and execution logic, and moves the corresponding unit and integration tests (including the READONLY guard tests) to the new package structure. There are no review comments provided, so I have no additional feedback to offer.

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.

[github-actions]

Code Metrics Report

📊 View detailed coverage report (available for 7 days)

	main (09e4d81)	#782 (e579ec3)	+/-
Coverage	73.3%	73.2%	-0.2%
Code to Test Ratio	1:1.3	1:1.3	+0.0
Test Execution Time	1m38s	1m31s	-7s

Details

  |                     | main (09e4d81) | #782 (e579ec3) |  +/-  |
  |---------------------|----------------|----------------|-------|
- | Coverage            |          73.3% |          73.2% | -0.2% |
  |   Files             |             91 |             93 |    +2 |
  |   Lines             |           7917 |           7933 |   +16 |
+ |   Covered           |           5805 |           5808 |    +3 |
+ | Code to Test Ratio  |          1:1.3 |          1:1.3 |  +0.0 |
  |   Code              |          18473 |          18517 |   +44 |
+ |   Test              |          25062 |          25128 |   +66 |
+ | Test Execution Time |          1m38s |          1m31s |   -7s |

Code coverage of files in pull request scope (73.5% → 72.7%)

Files	Coverage	+/-	Status
internal/mycli/client_side_statement_def.go	86.1%	-0.1%	modified
internal/mycli/feature/all/all.go	100.0%	0.0%	modified
internal/mycli/feature/cql/classifier.go	100.0%	+100.0%	added
internal/mycli/feature/cql/cql.go	9.3%	+9.3%	added
internal/mycli/session.go	69.8%	+0.1%	modified
internal/mycli/statement_processing.go	90.6%	0.0%	modified
internal/mycli/statements.go	66.1%	+9.1%	modified

---

Reported by octocov

[apstndb]

Internal review results (orchestrator line review + independent adversarial review): both conclude SAFE TO MERGE.

Verified under attack: classifier and formatCassandraTypeName byte-identical to the pre-move code; Execute identical modulo exported wrappers (including the empty-header toTableHeader(headers) vs NewTableHeader(headers...) equivalence — both return nil); READONLY coverage equal-or-better (mutating CQL now blocked through real dispatch, #757 keyword table preserved byte-identically, plus a new DELETE-classify assertion); docs zero-diff; core dependency graph free of gocql/go-spanner-cassandra; merged order LLM, CQL, BIGQUERY.

Two honest notes for the record, neither blocking:

1. Failure-retry nuance: the old code cached the Cassandra cluster before CreateSession(), so a transient CreateSession failure retried on the same cluster/proxy; the new holder rebuilds the cluster on retry. Unobservable in practice because NewCluster panics on proxy-setup failure before a failing CreateSession is reachable (go-spanner-cassandra v0.7.0, spanner.go:88-92, 116-120).
2. That NewCluster panic is PRE-EXISTING production behavior (the old in-core Execute called the same function): a CQL statement on a session where the adapter cannot be built panics the REPL. Recorded as a post-series follow-up candidate (recover in feature/cql or an upstream report) — same class as the parse-panic findings already queued.

[apstndb]

APPROVE-WITH-NIT: I found no blocking behavior issues in the CQL extraction.

One non-blocking test coverage gap is worth tightening before or after merge:

• [P3] internal/mycli/cql_readonly_guard_test.go:79: the dispatch-built SELECT allow-path test only asserts that the returned statement is not a static MutationStatement. The wired classifier is asserted in feature/cql through newCQLStatement, and the production HandleGroups currently does call that constructor, so the implementation is correct. Still, the dispatch test (or TestFeatureDefDispatch) should also assert that the built *cql.CQLStatement classifies SELECT as non-mutating. That would catch a future HandleGroups regression that accidentally returns a zero-value CQLStatement and causes READONLY mode to fail-closed for CQL SELECT.

Validation I ran:

• go test ./internal/mycli ./internal/mycli/feature/cql -run 'TestCQL|TestReadOnlyGuard.*CQL|TestFeatureDefDispatch|TestClientSideStatementDefsNonShadowing|TestClientSideStatementDefsCompletionConsistency|TestFeature'
• go list -deps ./internal/mycli contains no gocql / go-spanner-cassandra imports
• go list -deps . still contains the CQL dependencies through the full binary feature set

[apstndb]

Addressed the APPROVE-WITH-NIT [P3] in 2bdf95d: TestReadOnlyGuardAllowsReadOnlyCQL now asserts the conditional classification on the DISPATCH-BUILT statement (via a test-only ClassifyForTest bridge, since the marker method is unexported): SELECT must classify non-mutating and DELETE mutating on statements produced by the real def table. A HandleGroups regression returning a zero-value CQLStatement (nil Classify fails closed) now surfaces as a test failure instead of silently blocking CQL SELECT in READONLY mode.