Skip to content

Commit 5a8a949

Browse files
committed
fix(sqlite): use column references in length CHECK and add tests
- Fix bug where SQLite CHECK constraints used the column name as a string literal (e.g. length('col')) instead of referencing the column value (length("col")). - Introduce sqliteLengthCheck helper in sqlite/schema_utils.nim and import Column. - Replace direct stringified CHECK generation in create/add column code paths with sqliteLengthCheck for char, string and uuid columns. - Update README and documents/rdb/schema_builder.md examples to correct CHECK quoting. - Add tests (tests/sqlite/test_schema_length_check.nim) to verify both CREATE TABLE and ALTER TABLE ... ADD COLUMN paths and validate runtime constraint behavior. - Keep change confined to SQLite driver implementation; no public API changes.
1 parent e33182d commit 5a8a949

8 files changed

Lines changed: 245 additions & 8 deletions

File tree

Lines changed: 130 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,130 @@
1+
---
2+
description: 318-bug-incorrect-sqlite-check-constraint-for-columnchar-columnstring-and-columnuuidブランチでの開発時に読み込む
3+
alwaysApply: false
4+
---
5+
SQLite CHECK Constraint 引用誤り修正 ブランチルール
6+
===
7+
8+
このブランチで実装することは以下の通りです。
9+
- SQLite の schema builder において、`Column.char`, `Column.string`, `Column.uuid` が生成する `CHECK` 制約の列参照が文字列リテラル扱いになっている不具合を修正する。
10+
- `CREATE TABLE` と `ALTER TABLE ... ADD COLUMN` の両方で、`length('column_name')` ではなく列値を参照する `length(column_name)` 系の SQL を生成するように整える。
11+
- SQLite driver の修正を公開 API へ漏らさず、driver 内部の SQL 生成責務に閉じ込める。
12+
- 長さ制約の既定値あり・既定値なしの両ケースで、`char` `string` `uuid` の意味論が崩れないことを確認できるテストを整備する。
13+
- 必要に応じて README / `documents/rdb/schema_builder.md` の SQLite 例を同期し、誤った `CHECK (length('email') <= 255)` 表記を残さない。
14+
15+
## 進捗
16+
- [x] `.cursor/rules/project.mdc` と `.cursor/rules/branch.mdc` を確認した
17+
- [x] 現在のブランチ名に対応するブランチルールファイルが未作成であることを確認した
18+
- [x] SQLite の `CHECK` 生成箇所を調査し、`create_column_query.nim` と `add_column_query.nim` の両方に同種の問題があることを確認した
19+
- [x] 対象カラム型が `Column.char`, `Column.string`, `Column.uuid` の 3 系統であることを確認した
20+
- [x] ドキュメントに同じ誤表記が残っていることを確認した
21+
- [x] ブランチルールに改修方針・進捗粒度・参考資料を整理した
22+
- [x] SQLite schema builder の `CHECK` 制約生成を、列名を文字列リテラル化しない形式へ修正する
23+
- [x] `CREATE TABLE` 経路のテストを追加または更新し、生成 SQL が列値長を検査することを確認する
24+
- [x] `ADD COLUMN` 経路のテストを追加または更新し、生成 SQL が列値長を検査することを確認する
25+
- [x] `char` `string` `uuid` の既定長と明示長の両ケースで回帰確認する
26+
- [x] 必要な README / `documents/rdb/schema_builder.md` の記述を同期する
27+
- [ ] 実装途中で設計認識に誤りが見つかった場合は、本ファイルの `調査結果・設計まとめ` を更新する
28+
29+
## 参考資料
30+
- `src/allographer/schema_builder/queries/sqlite/sub/create_column_query.nim`
31+
- `src/allographer/schema_builder/queries/sqlite/sub/add_column_query.nim`
32+
- `tests/sqlite/test_create_schema.nim`
33+
- `tests/sqlite/test_schema.nim`
34+
- `documents/rdb/schema_builder.md`
35+
- `README.md`
36+
- SQLite Documentation: CHECK constraints
37+
- SQLite Documentation: core functions `length(...)`
38+
39+
## 調査結果・設計まとめ
40+
41+
### 背景
42+
43+
SQLite 実装では、文字列長を制限するために `CHECK (length(...) <= N)` を列定義へ付与している。
44+
45+
しかし現状の SQL 生成は `length('{column.name}')` となっており、列値ではなく列名そのものを単一引用符付きの文字列リテラルとして評価している。
46+
47+
その結果、たとえば `name` 列では常に `length('name') == 4` と評価されるため、期待した列値バリデーションにならない。
48+
49+
### 現状の問題
50+
51+
1. SQLite の `CREATE TABLE` 用列生成で `CHECK (length('{column.name}') <= {maxLength})` を出力している
52+
2. SQLite の `ADD COLUMN` 用列生成でも同じ誤りを重複して持っている
53+
3. 影響範囲が `Column.char`, `Column.string`, `Column.uuid` にまたがっている
54+
4. ドキュメント例にも同じ誤表記があり、利用者が誤仕様と誤解する余地がある
55+
5. SQL 生成の不具合であり、公開 API ではなく SQLite driver 内部責務として直すべき問題である
56+
57+
### 設計目標
58+
59+
1. SQLite で生成される長さ `CHECK` 制約が常に列値を参照する
60+
2. `char`, `string`, `uuid` の API 意味論を変えずに不具合のみを修正する
61+
3. `create` と `add column` の両経路で SQL 生成規則を揃える
62+
4. 既存の RDB 向け共通 API を汚さず、SQLite 実装へ変更を閉じ込める
63+
5. テストと文書で回帰防止できる状態にする
64+
65+
### 非目標
66+
67+
1. SQLite 以外の driver の長さ制約仕様を同時に変更すること
68+
2. `Column.char`, `Column.string`, `Column.uuid` の既定長そのものを見直すこと
69+
3. schema builder 全体の quoting 戦略をこのブランチで全面再設計すること
70+
4. 公開 API の追加・削除・引数変更を行うこと
71+
72+
### 実装方針
73+
74+
#### 1. SQLite の長さ `CHECK` 制約は列値参照に統一する
75+
76+
変更対象:
77+
- `src/allographer/schema_builder/queries/sqlite/sub/create_column_query.nim`
78+
- `src/allographer/schema_builder/queries/sqlite/sub/add_column_query.nim`
79+
80+
方針:
81+
- `length('{column.name}')` を列値参照の式へ修正する
82+
- `char`, `string`, `uuid` の 3 系統で同じルールを適用する
83+
- `CREATE TABLE` と `ADD COLUMN` で生成式の差異を作らない
84+
85+
#### 2. 列名の扱いは SQLite 列参照として妥当な形を維持する
86+
87+
要点:
88+
- 単一引用符 `'...'` は文字列リテラルなので使用しない
89+
- 既存の SQLite SQL 生成スタイルに合わせ、列参照として解釈される形を採用する
90+
- 必要なら identifier quoting の既存ルールを確認し、単純な文字列置換ではなく SQLite 方言として自然な表現に寄せる
91+
- 共通 helper を `schema_utils` に置き、`CREATE TABLE` / `ADD COLUMN` の両経路で同じ生成規則を使う
92+
93+
#### 3. テストは SQL 文字列と実動作の両面を意識して追加する
94+
95+
候補:
96+
1. schema 生成テストで `CHECK` 句が列値参照になっていることを確認する
97+
2. `Column.string("name")` の既定長ケースを確認する
98+
3. `Column.char("name", 20)` の明示長ケースを確認する
99+
4. `Column.uuid("uuid")` の既定長ケースを確認する
100+
5. `add()` 経路でも同じ `CHECK` 句になることを確認する
101+
102+
### 受け入れ基準
103+
104+
1. SQLite の `char`, `string`, `uuid` が生成する `CHECK` 制約で列名が単一引用符付き文字列になっていない
105+
2. `CREATE TABLE` と `ADD COLUMN` の両方で同じ修正が反映されている
106+
3. 追加または更新したテストで不具合再発を防止できる
107+
4. 必要なドキュメント例が実装と一致している
108+
5. 公開 API 変更なしで SQLite driver 内に修正が閉じ込められている
109+
110+
### 参照仕様
111+
112+
- SQLite Documentation: CHECK constraints
113+
https://www.sqlite.org/lang_createtable.html#ckconst
114+
- SQLite Documentation: Built-In Scalar SQL Functions
115+
https://www.sqlite.org/lang_corefunc.html#length
116+
117+
### リスク
118+
119+
1. 列名参照の quoting 形式を誤ると、今度は予約語や特殊文字を含む列名で不整合が出る可能性がある
120+
2. `create` と `add column` の片方だけ修正すると、経路差による再発が起きる
121+
3. ドキュメント未更新のままだと、修正後も利用者が誤った SQL 例を参照して混乱する
122+
4. SQL 文字列比較系テストだけだと、実際の制約評価の誤りを見逃す可能性がある
123+
124+
### 段階導入案
125+
126+
1. 影響箇所と既存 SQLite テストを整理する
127+
2. `create` / `add column` の SQL 生成を修正する
128+
3. `char` / `string` / `uuid` の回帰テストを追加または更新する
129+
4. 必要な README / documents を同期する
130+
5. 実装差分が設計とずれたら本ファイルを更新する

AGENTS.md

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,3 +8,15 @@
88
- If you output anything other than the final code, the answer is invalid.
99
- git add, git commitのコマンド操作は禁止。
1010
- 常に日本語で回答する。
11+
- "commit message"と入力されたら次のことを行う
12+
- `cd {project_root} && git diff --cached > .diff` を実行して.diffファイルを作成する
13+
- .diffファイルを読み込んで、変更内容を確認する
14+
- 変更内容を確認したら、commit messageを作成する
15+
- commit messageを作成したら、commit messageを返す
16+
- デフォルトは日本語、言語のの指定があればそれに従う
17+
- "PR message"と入力されたら次のことを行う
18+
- mainブランチとの差分を取得する
19+
- 差分を読み込んで、変更内容を確認する
20+
- 変更内容を確認したら、PR messageを作成する
21+
- PR messageを作成したら、PR messageを返す
22+
- デフォルトは日本語、言語のの指定があればそれに従う

README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,7 @@ rdb.alter(
8383
])
8484
)
8585
86-
>> ALTER TABLE "users" ADD COLUMN `email` UNIQUE DEFAULT "" CHECK (length(`email`) <= 255)
86+
>> ALTER TABLE "users" ADD COLUMN 'email' VARCHAR UNIQUE DEFAULT '' CHECK (length("email") <= 255)
8787
>> ALTER TABLE "users" DROP `name`
8888
```
8989

documents/rdb/schema_builder.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ rdb.alter(
6464
])
6565
)
6666
```
67-
`>> ALTER TABLE "users" ADD COLUMN 'email' UNIQUE DEFAULT '' CHECK (length('email') <= 255)`
67+
`>> ALTER TABLE "users" ADD COLUMN 'email' VARCHAR UNIQUE DEFAULT '' CHECK (length("email") <= 255)`
6868

6969

7070
### change column

src/allographer/schema_builder/queries/sqlite/schema_utils.nim

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import ../../../query_builder/models/sqlite/sqlite_types
88
import ../../../query_builder/models/sqlite/sqlite_query
99
import ../../../query_builder/models/sqlite/sqlite_exec
1010
import ../../../query_builder/error
11+
import ../../models/column
1112
import ../../models/table
1213

1314

@@ -20,6 +21,12 @@ proc notAllowedType*(typ:string) =
2021
raise newException(DbError, &"type {typ} is not allowed")
2122

2223

24+
proc sqliteLengthCheck*(column:Column, maxLength:int):string =
25+
## "CHECK (length({column.name}) <= {maxLength})"
26+
let escapedColumnName = column.name.replace("\"", "\"\"")
27+
return &" CHECK (length(\"{escapedColumnName}\") <= {maxLength})"
28+
29+
2330
# ==================================================
2431
# Sqlite
2532
# ==================================================

src/allographer/schema_builder/queries/sqlite/sub/add_column_query.nim

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,7 @@ proc addUuidColumn(table:Table, column:Column):seq[string] =
102102
query.add(&" DEFAULT '{column.defaultString}'")
103103

104104
let maxLength = column.info["maxLength"].getInt
105-
query.add(&" CHECK (length('{column.name}') <= {maxLength})")
105+
query.add(sqliteLengthCheck(column, maxLength))
106106

107107
if column.isUnsigned:
108108
notAllowedOption("unsigned", "varchar", column.name)
@@ -120,7 +120,7 @@ proc addCharColumn(table:Table, column:Column):seq[string] =
120120
query.add(&" DEFAULT '{column.defaultString}'")
121121

122122
let maxLength = column.info["maxLength"].getInt
123-
query.add(&" CHECK (length('{column.name}') <= {maxLength})")
123+
query.add(sqliteLengthCheck(column, maxLength))
124124

125125
if column.isUnsigned:
126126
notAllowedOption("unsigned", "char", column.name)
@@ -138,7 +138,7 @@ proc addVarcharColumn(table:Table, column:Column):seq[string] =
138138
query.add(&" DEFAULT '{column.defaultString}'")
139139

140140
let maxLength = column.info["maxLength"].getInt
141-
query.add(&" CHECK (length('{column.name}') <= {maxLength})")
141+
query.add(sqliteLengthCheck(column, maxLength))
142142

143143
if column.isUnsigned:
144144
notAllowedOption("unsigned", "varchar", column.name)

src/allographer/schema_builder/queries/sqlite/sub/create_column_query.nim

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,7 @@ proc createUuidColumn(column:Column):string =
8282
result.add(&" DEFAULT '{column.defaultString}'")
8383

8484
let maxLength = column.info["maxLength"].getInt
85-
result.add(&" CHECK (length('{column.name}') <= {maxLength})")
85+
result.add(sqliteLengthCheck(column, maxLength))
8686

8787
if column.isUnsigned:
8888
notAllowedOption("unsigned", "varchar", column.name)
@@ -101,7 +101,7 @@ proc createCharColumn(column:Column):string =
101101
result.add(&" DEFAULT '{column.defaultString}'")
102102

103103
let maxLength = column.info["maxLength"].getInt
104-
result.add(&" CHECK (length('{column.name}') <= {maxLength})")
104+
result.add(sqliteLengthCheck(column, maxLength))
105105

106106
if column.isUnsigned:
107107
notAllowedOption("unsigned", "char", column.name)
@@ -120,7 +120,7 @@ proc createVarcharColumn(column:Column):string =
120120
result.add(&" DEFAULT '{column.defaultString}'")
121121

122122
let maxLength = column.info["maxLength"].getInt
123-
result.add(&" CHECK (length('{column.name}') <= {maxLength})")
123+
result.add(sqliteLengthCheck(column, maxLength))
124124

125125
if column.isUnsigned:
126126
notAllowedOption("unsigned", "varchar", column.name)
Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,88 @@
1+
discard """
2+
cmd: "nim c -d:reset $file"
3+
"""
4+
# nim c -r -d:reset tests/sqlite/test_schema_length_check.nim
5+
6+
import std/unittest
7+
import std/asyncdispatch
8+
import std/json
9+
import std/strformat
10+
import std/strutils
11+
import ../../src/allographer/schema_builder
12+
import ../../src/allographer/query_builder
13+
import ./connections
14+
import ../clear_tables
15+
16+
17+
let rdb = sqlite
18+
19+
20+
proc tableSql(tableName:string): string =
21+
let rows = rdb.raw(&"SELECT sql FROM sqlite_master WHERE type = 'table' AND name = '{tableName}'").get().waitFor
22+
return rows[0]["sql"].getStr
23+
24+
25+
suite("Sqlite length check constraint"):
26+
test("create table uses a column reference in CHECK"):
27+
let tableName = "length_check_create"
28+
29+
rdb.create(
30+
table(tableName, [
31+
Column.increments("id"),
32+
Column.char("char", 4).default("ABCD"),
33+
Column.string("string", 4).nullable(),
34+
Column.uuid("uuid").nullable()
35+
])
36+
)
37+
38+
let sql = tableSql(tableName)
39+
check sql.contains("DEFAULT 'ABCD'")
40+
check sql.contains("CHECK (length(\"char\") <= 4)")
41+
check sql.contains("CHECK (length(\"string\") <= 4)")
42+
check sql.contains("CHECK (length(\"uuid\") <= 256)")
43+
44+
expect DbError:
45+
rdb.table(tableName).insert(@[
46+
%*{
47+
"char": "ABCDE",
48+
"string": "ABCDE",
49+
"uuid": repeat("u", 300)
50+
}
51+
]).waitFor
52+
53+
54+
test("add column keeps CHECK on a column reference"):
55+
let tableName = "length_check_add"
56+
57+
rdb.create(
58+
table(tableName, [
59+
Column.increments("id")
60+
])
61+
)
62+
63+
rdb.alter(
64+
table(tableName, [
65+
Column.char("char", 4).nullable().add(),
66+
Column.string("string", 4).default("WXYZ").add(),
67+
Column.uuid("uuid").default("ZZZZ").add()
68+
])
69+
)
70+
71+
let sql = tableSql(tableName)
72+
check sql.contains("CHECK (length(\"char\") <= 4)")
73+
check sql.contains("CHECK (length(\"string\") <= 4)")
74+
check sql.contains("CHECK (length(\"uuid\") <= 256)")
75+
check sql.contains("DEFAULT 'WXYZ'")
76+
check sql.contains("DEFAULT 'ZZZZ'")
77+
78+
expect DbError:
79+
rdb.table(tableName).insert(@[
80+
%*{
81+
"char": "ABCDE",
82+
"string": "ABCDE",
83+
"uuid": repeat("u", 300)
84+
}
85+
]).waitFor
86+
87+
88+
clearTables(rdb).waitFor

0 commit comments

Comments
 (0)