Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,12 @@ See the `guide/` directory for detailed documentation:

- TDD: Red -> Green -> Refactor
- Tidy First: separate structural changes from behavioral changes
- Use GORM queries only (no raw SQL)
- Use GORM's model layer for queries. Raw SQL is allowed only where GORM has no
form for the statement — full-text operators, FTS5 virtual-table DDL and
writes, schema introspection its migrator cannot do, connection pragmas — and
only inside `internal/adapters/outbound/searchsql` and `internal/db`.
Identifiers come from package constants, values are always bound parameters.
Details and the evidence for each exemption: `guide/development.md` §Raw SQL
- Tests: `CGO_ENABLED=1 go test -tags "fts5" ./... -count=1`
- Integration test: `./scripts/integration-test.sh` (full Gitea + PostgreSQL + ccg Docker pipeline)

Expand Down
2 changes: 1 addition & 1 deletion CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ Graceful shutdown: SIGINT/SIGTERM 시 진행 중인 clone/build에 context cance

- TDD: Red → Green → Refactor
- Tidy First: 구조적 변경과 행위 변경 분리
- GORM 쿼리만 사용 (raw SQL 금지)
- 쿼리는 GORM 모델 계층으로 작성한다. Raw SQL은 GORM에 대응 형태가 없는 경우에만 허용한다 — 전문 검색 연산자, FTS5 가상 테이블 DDL·쓰기, migrator가 못 하는 스키마 introspection, 연결 pragma — 그리고 `internal/adapters/outbound/searchsql`와 `internal/db` 안에서만이다. 식별자는 패키지 상수에서 오고, 값은 항상 bound parameter다 (상세와 각 예외의 근거: `guide/development.md` §Raw SQL)
- 코드 정렬: 종류별 그룹화가 아니라 "타입 + 그 타입의 생성자·메소드"를 붙여 두는 응집 관례를 따른다 (상세: `guide/development.md` §Declaration order)
- 테스트: `CGO_ENABLED=1 go test -tags "fts5" ./... -count=1`
- Integration test: `./scripts/integration-test.sh` (Gitea + PostgreSQL + ccg Docker 전체 파이프라인)
Expand Down
47 changes: 46 additions & 1 deletion guide/development.md
Original file line number Diff line number Diff line change
Expand Up @@ -205,11 +205,56 @@ go test ./internal/adapters/inbound/cli -run TestProjectSkills -count=1

- TDD: Red → Green → Refactor
- Tidy First: Separate structural changes from behavioral changes
- Use GORM queries only (no raw SQL)
- Use GORM's model layer for queries; raw SQL only where GORM has no form for
the statement (see [Raw SQL](#raw-sql))
- Logging: `slog`
- CLI: `cobra` framework
- Build flags: `CGO_ENABLED=1 -tags "fts5"`

### Raw SQL

Write queries through GORM's model layer — `Model`, `Where`, `FindInBatches`,
`Migrator` — wherever GORM has a form for the statement. That is the default, and
outside the two packages named below a raw `Raw`/`Exec` is a review stop.

Raw SQL is allowed only where GORM has no form at all. That is not a matter of
taste; each category below names something GORM's builder or migrator cannot
express:

- **Full-text operators and index maintenance** — SQLite FTS5 `MATCH` and its
`rank` column; PostgreSQL `to_tsvector`, `to_tsquery`, `ts_rank`, `@@`. GORM
has no builder form for a match operator or a rank expression.
- **DDL on the FTS5 virtual tables** — `CREATE VIRTUAL TABLE … USING fts5`,
and the `DROP`/`ALTER TABLE … RENAME TO` pairs the legacy upgrade needs.
`AutoMigrate` does not model a virtual table.
- **Writes into the FTS5 virtual tables** — the namespace-scoped deletes and the
bulk inserts. These tables have no GORM model and are not in `AutoMigrate`;
routing them through `Table("search_fts")` would name the table in a string
either way, and the bulk insert is one statement on purpose so a rebuild does
not pay a round trip per row.
- **Schema introspection GORM's migrator cannot do** — `PRAGMA table_info`,
`sqlite_master`, `information_schema.columns`, `pg_indexes`, `pg_trigger`.
- **Connection pragmas** — `PRAGMA journal_mode`, `PRAGMA busy_timeout`.

These live in `internal/adapters/outbound/searchsql` and `internal/db` only.

Two constraints hold inside an exempt statement. Table and column names come
from package constants, never from caller input. Every value is a bound
parameter — `Exec("DELETE FROM "+sqliteFTSTable+" WHERE namespace = ?", ns)` is
correct; concatenating `ns` into the string is not, and no amount of exemption
makes it correct.

The introspection exemption is the one worth checking rather than trusting,
because GORM does ship `Migrator().HasTable`, `HasColumn` and `ColumnTypes`.
`searchsql/migrator_limits_test.go` runs all three against a real FTS5 table and
records what happens: `HasTable` works, `ColumnTypes` fails with `invalid DDL`,
and `HasColumn` matches the DDL text rather than the schema, so it can report
`false` for a column that exists. `HasTable` works but returns no error, while
every caller of `sqliteTableExists` propagates one — swallowing it would turn a
transient failure into "the table is absent" in the upgrade path. If a GORM
upgrade makes that test fail, the exemption should be reconsidered, not the
test relaxed.

### Declaration order within a file

Follow the standard-library convention of **cohesion over kind-grouping**: keep a
Expand Down
43 changes: 42 additions & 1 deletion guide/ko/development.md
Original file line number Diff line number Diff line change
Expand Up @@ -207,7 +207,48 @@ go test ./internal/adapters/inbound/cli -run TestProjectSkills -count=1

- TDD: Red → Green → Refactor
- Tidy First: 구조적 변경과 행동 변경의 분리
- GORM 쿼리만 사용 (Raw SQL 사용 금지)
- 쿼리는 GORM 모델 계층으로 작성. Raw SQL은 GORM에 대응 형태가 없는 경우에만 허용
([Raw SQL](#raw-sql) 참고)
- 로깅: `slog`
- CLI: `cobra` 프레임워크
- 빌드 플래그: `CGO_ENABLED=1 -tags "fts5"`

### Raw SQL

GORM에 대응 형태가 있는 문장은 모두 GORM 모델 계층(`Model`, `Where`,
`FindInBatches`, `Migrator`)으로 작성합니다. 이것이 기본이고, 아래 두 패키지
밖에서 나타나는 `Raw`/`Exec`는 리뷰에서 멈춰야 합니다.

Raw SQL은 GORM에 대응 형태가 아예 없는 경우에만 허용합니다. 취향 문제가 아니라,
아래 각 항목은 GORM builder나 migrator가 표현할 수 없는 것을 가리킵니다.

- **전문 검색 연산자와 인덱스 관리** — SQLite FTS5의 `MATCH`와 `rank` 컬럼,
PostgreSQL의 `to_tsvector`, `to_tsquery`, `ts_rank`, `@@`. match 연산자나 rank
표현식에 해당하는 builder 형태가 GORM에 없습니다.
- **FTS5 가상 테이블 DDL** — `CREATE VIRTUAL TABLE … USING fts5`, 그리고 legacy
업그레이드가 필요로 하는 `DROP` / `ALTER TABLE … RENAME TO` 쌍. `AutoMigrate`는
가상 테이블을 모델링하지 않습니다.
- **FTS5 가상 테이블 쓰기** — namespace 범위 delete와 bulk insert. 이 테이블들은
GORM 모델이 없고 `AutoMigrate` 대상도 아니어서, `Table("search_fts")`를 거쳐도
테이블 이름은 결국 문자열로 남습니다. bulk insert가 한 문장인 것은 의도이며,
rebuild가 행마다 round trip을 내지 않게 합니다.
- **GORM migrator가 못 하는 스키마 introspection** — `PRAGMA table_info`,
`sqlite_master`, `information_schema.columns`, `pg_indexes`, `pg_trigger`.
- **연결 pragma** — `PRAGMA journal_mode`, `PRAGMA busy_timeout`.

허용 범위는 `internal/adapters/outbound/searchsql`와 `internal/db` 두 패키지뿐입니다.

허용된 문장 안에서도 두 제약이 유지됩니다. 테이블·컬럼 이름은 패키지 상수에서만
오고, 호출자 입력에서 오지 않습니다. 값은 항상 bound parameter입니다 —
`Exec("DELETE FROM "+sqliteFTSTable+" WHERE namespace = ?", ns)`는 맞고, `ns`를
문자열에 이어 붙이는 것은 틀립니다. 어떤 예외도 이것을 맞게 만들지 않습니다.

introspection 예외는 믿지 말고 확인해야 하는 항목입니다. GORM에도
`Migrator().HasTable`, `HasColumn`, `ColumnTypes`가 있기 때문입니다.
`searchsql/migrator_limits_test.go`가 실제 FTS5 테이블을 상대로 셋 다 실행하고
결과를 기록합니다: `HasTable`은 동작하고, `ColumnTypes`는 `invalid DDL`로
실패하며, `HasColumn`은 스키마가 아니라 DDL 텍스트를 매칭하므로 존재하는 컬럼에도
`false`를 반환할 수 있습니다. `HasTable`은 동작하지만 error를 돌려주지 않고,
`sqliteTableExists`의 모든 호출자는 error를 전파합니다. 그것을 삼키면 업그레이드
경로에서 일시적 실패가 "테이블 없음"으로 바뀝니다. GORM 업그레이드로 그 테스트가
깨지면 테스트를 느슨하게 만들 것이 아니라 예외 자체를 다시 판단해야 합니다.
7 changes: 7 additions & 0 deletions internal/adapters/outbound/searchsql/backend.go
Original file line number Diff line number Diff line change
@@ -1,4 +1,11 @@
// @index Shared search backend interface and errors for SQLite FTS5 and PostgreSQL tsvector implementations.
//
// The raw SQL in this package is bounded and deliberate: full-text operators,
// DDL and writes against the FTS5 virtual tables, and the schema introspection
// GORM's migrator cannot do. guide/development.md §Raw SQL states the rule and
// what falls under it; migrator_limits_test.go holds the evidence for the
// introspection part. Anything GORM's model layer can express belongs there
// instead, here as much as anywhere else.
package searchsql

import (
Expand Down
100 changes: 100 additions & 0 deletions internal/adapters/outbound/searchsql/migrator_limits_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,100 @@
//go:build fts5

package searchsql

import (
"testing"

"gorm.io/driver/sqlite"
"gorm.io/gorm"
"gorm.io/gorm/logger"
)

// This file is the evidence behind one line of guide/development.md §Raw SQL:
// that the schema introspection in this package cannot go through GORM's
// migrator. GORM does ship HasTable, HasColumn and ColumnTypes, so the exemption
// would be a bare assertion without a test that runs all three against a real
// FTS5 virtual table and records what they do.
//
// If a GORM upgrade makes one of these fail, the right response is to
// reconsider the exemption for that call, not to relax the test.

// newVirtualTableProbeDB opens an in-memory database holding the FTS5 table this
// package creates in production, plus a second one shaped like the pre-namespace
// legacy table.
func newVirtualTableProbeDB(t *testing.T) *gorm.DB {
t.Helper()
db, err := gorm.Open(sqlite.Open(":memory:"), &gorm.Config{Logger: logger.Discard})
if err != nil {
t.Fatalf("open sqlite: %v", err)
}
if err := createSQLiteFTSTable(db, sqliteFTSTable, true); err != nil {
t.Fatalf("create %s: %v", sqliteFTSTable, err)
}
if err := db.Exec(
"CREATE VIRTUAL TABLE legacy_probe USING fts5(node_id UNINDEXED, content, language)",
).Error; err != nil {
t.Fatalf("create legacy_probe: %v", err)
}
return db
}

// TestMigratorHasTableSeesFTS5VirtualTables records that table presence is the
// one introspection GORM answers correctly here — and why sqliteTableExists
// keeps its raw statement anyway: HasTable returns a bool with no error, while
// every caller of sqliteTableExists propagates one. Swallowing that error would
// read a transient failure as "the table is absent", which in the legacy-upgrade
// path is the difference between stopping and rebuilding.
func TestMigratorHasTableSeesFTS5VirtualTables(t *testing.T) {
db := newVirtualTableProbeDB(t)

if !db.Migrator().HasTable(sqliteFTSTable) {
t.Errorf("HasTable(%q) = false, want true: the virtual table was just created", sqliteFTSTable)
}
if db.Migrator().HasTable("no_such_table") {
t.Error("HasTable(\"no_such_table\") = true, want false")
}

exists, err := sqliteTableExists(db, sqliteFTSTable)
if err != nil || !exists {
t.Errorf("sqliteTableExists(%q) = (%v, %v), want (true, nil)", sqliteFTSTable, exists, err)
}
}

// TestMigratorColumnTypesRejectsFTS5VirtualTables records that ColumnTypes
// cannot describe a virtual table at all: it parses the stored DDL, and
// CREATE VIRTUAL TABLE is not a shape it can parse.
func TestMigratorColumnTypesRejectsFTS5VirtualTables(t *testing.T) {
db := newVirtualTableProbeDB(t)

types, err := db.Migrator().ColumnTypes(sqliteFTSTable)
if err == nil {
t.Fatalf("ColumnTypes(%q) succeeded with %d columns, want an error", sqliteFTSTable, len(types))
}
if got := err.Error(); got != "invalid DDL" {
t.Errorf("ColumnTypes(%q) error = %q, want %q", sqliteFTSTable, got, "invalid DDL")
}
}

// TestMigratorHasColumnMisreadsFTS5VirtualTables is the reason PRAGMA table_info
// stays. HasColumn matches patterns against the stored DDL text rather than
// asking the schema, so whether it finds a column depends on how that column
// happens to be spelled in the CREATE statement. `content` is a real column on
// legacy_probe and HasColumn says it is not there.
func TestMigratorHasColumnMisreadsFTS5VirtualTables(t *testing.T) {
db := newVirtualTableProbeDB(t)

present, err := sqliteColumnExists(db, "legacy_probe", "content")
if err != nil {
t.Fatalf("sqliteColumnExists: %v", err)
}
if !present {
t.Fatal("PRAGMA table_info does not see legacy_probe.content; the fixture is wrong, not GORM")
}

if db.Migrator().HasColumn("legacy_probe", "content") {
t.Error("HasColumn now agrees with the schema on this table. GORM may have gained real " +
"virtual-table introspection: reconsider the exemption in guide/development.md §Raw SQL " +
"rather than relaxing this test")
}
}
Loading