Skip to content

Apply PR #110's Read index doc comment fix to the v2 legacy index module - #207

Merged
sinmetal merged 1 commit into
cloudspannerecosystem:masterfrom
sinmetal:fix-read-index-comment-v2-legacy
Aug 3, 2026
Merged

Apply PR #110's Read index doc comment fix to the v2 legacy index module#207
sinmetal merged 1 commit into
cloudspannerecosystem:masterfrom
sinmetal:fix-read-index-comment-v2-legacy

Conversation

@sinmetal

@sinmetal sinmetal commented Aug 2, 2026

Copy link
Copy Markdown
Collaborator

Summary

v2/module/builtin/templates/legacy_index.go.tpl hardcodes the doc comment on every generated Read{{ .LegacyFuncName }} as // Generated from unique index '...'., even when the underlying index is not unique. Users of the legacy index module (--use-legacy-index-module) therefore still see misleading doc comments on non-unique-index ReadXxx functions.

This is the same issue that #110 (merged 2022-12-07) fixed for module/builtin/templates/index.go.tpl, and that #205 backported to the v1 templates. legacy_index.go.tpl was missed by both, so after #205 the v1 output and the v2 legacy output disagree on the same comment:

// v1 (after #205)                      // v2 legacy (today)
// Generated from index '...'.          // Generated from unique index '...'.
func ReadCompositePrimaryKeysByError(   func ReadCompositePrimaryKeysByError(

Change

Mirror #110 and #205 exactly — drop the word unique from the Read{{ .LegacyFuncName }} doc comment so it reads // Generated from index '...'. regardless of uniqueness:

+// Generated from index '{{ .IndexName }}'.                                                                              func Read{{ .LegacyFuncName }}(ctx context.Cont.KeySet) ([]*{{ .Type.Name }}, error) {

v2/test/testmodels/legacy_default/**/*.yo.go is regenerated via make -C v2 testdata-from-ddl to reflect the template change.

The Find{{ .LegacyFuncName }} comment is left nside an {{ if .IsUnique }} branch, so uniqueindex is accurate there.

Scope

  • Comment-only change. No runtime behavior change.
  • Touches only v2/module/builtin/templates/legaegenerated v2/test/testmodels/legacy_default/`(5 files, 17 lines).
  • v2/test/testmodels/default/ and `dump_types/legacy index module was already fixed by Fix typo in index.go.tpl #110.
  • v2 embeds templates with go:embed (v2/module/builtin/module.go), so there is no v1-style tplbin regeneration step.
  • The v1 tree is untouched (already fixed by use handy-spanner for tests #20

@sinmetal
sinmetal marked this pull request as ready for review August 3, 2026 06:47
@sinmetal
sinmetal requested a review from vvakame August 3, 2026 06:47
@sinmetal
sinmetal merged commit 91c4f85 into cloudspannerecosystem:master Aug 3, 2026
11 checks passed
@sinmetal
sinmetal deleted the fix-read-index-comment-v2-legacy branch August 3, 2026 07:03
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.

2 participants