Skip to content

Commit 638f53a

Browse files
committed
fix: address code review findings (18-item tracker)
Correctness: - #3: Restore 3k-row cap in Quick Start step 3 - #6: Add PostgreSQL parser caveat to Workflow 7 (MySQL syntax → parse error → fallback) - #10: Fix ORM pattern to present fixed_with_warning to user (not auto-accept) - #12: Unfixable rewrites MUST present to user before substituting Docs accuracy: - #7: Rails: use db:structure:dump with schema_format = :sql - #8: Prisma: add required --from-empty --to-schema-datamodel flags Error handling: - #16: Add Error Handling section for dsql_lint failures (MCP unavailable, parse error, timeout) Evals: - #5: Add evals 102/103 to results summary table - #9: Fix model metadata (remove specific version) - #11: Add missing category_id column to eval 103 prompt Already fixed in previous commit: - #17: dsql-lint removed from description (93e9c7b) PR body items (1, 2, 4) will be updated separately.
1 parent 93e9c7b commit 638f53a

4 files changed

Lines changed: 25 additions & 11 deletions

File tree

plugins/databases-on-aws/skills/dsql/SKILL.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -181,7 +181,7 @@ See [scripts/README.md](../../scripts/README.md) for usage and hook configuratio
181181

182182
1. **Explore:** Use `readonly_query` with `information_schema` to list tables. Use `get_schema` for table structure.
183183
2. **Query:** Use `readonly_query` for SELECT queries. **MUST** include `tenant_id` in WHERE for multi-tenant apps. **MUST** build SQL with `safe_query.build()`.
184-
3. **Schema changes:** Use `transact` with one DDL per transaction; multiple DML statements **MAY** share a transaction. **MUST** use `CREATE INDEX ASYNC` in a separate call. Use `dsql_lint` to validate first.
184+
3. **Schema changes:** Use `transact` with one DDL per transaction. **MUST** batch DML under 3,000 rows. **MUST** use `CREATE INDEX ASYNC` in a separate call. Use `dsql_lint` to validate first.
185185

186186
---
187187

@@ -241,7 +241,7 @@ MUST load [ddl-migrations/overview.md](references/ddl-migrations/overview.md) be
241241

242242
### Workflow 7: Validate and Migrate to DSQL
243243

244-
Run `dsql_lint(sql=source_sql, fix=true)` to validate and auto-convert SQL from any source (PostgreSQL, MySQL, ORM-generated). MUST load [dsql-lint.md](references/dsql-lint.md) for the full workflow, ORM-specific guidance, and unfixable error resolution. MUST load [mysql-migrations/type-mapping.md](references/mysql-migrations/type-mapping.md) for MySQL-specific types not covered by auto-fix.
244+
Run `dsql_lint(sql=source_sql, fix=true)` to validate and auto-convert PostgreSQL-compatible SQL. For MySQL-specific syntax (SET, ENGINE, PARTITION BY), `dsql_lint` returns a parse error — fall back to [mysql-migrations/type-mapping.md](references/mysql-migrations/type-mapping.md) for manual conversion. MUST load [dsql-lint.md](references/dsql-lint.md) for the full workflow, ORM-specific guidance, and unfixable error resolution.
245245

246246
### Workflow 8: Query Plan Explainability
247247

plugins/databases-on-aws/skills/dsql/references/dsql-lint.md

Lines changed: 18 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ Use for any migration scenario: pg_dump imports, ORM migration files (Django, Ra
5454
3. For each diagnostic in the response:
5555
- `fixed`: Accept — safe mechanical transformation
5656
- `fixed_with_warning`: Present to user — explain application-layer implications
57-
- `unfixable`: Rewrite manually using skill knowledge (Table Recreation for `unsupported_alter_table_op`, DELETE for `truncate`, omit for `partition_by`)
57+
- `unfixable`: Present to user with proposed alternative, confirm before substituting
5858
4. Take `fixed_sql` from the response
5959
5. If `fixed_sql` contains multiple DDL statements, issue each as a separate `transact` call
6060
6. Execute each DDL with `transact(["<single DDL statement>"])`
@@ -70,8 +70,8 @@ Use for any migration scenario: pg_dump imports, ORM migration files (Django, Ra
7070
**ORM-specific guidance:**
7171

7272
- **Django:** Run `python manage.py sqlmigrate <app> <migration>` to get raw SQL, then lint
73-
- **Rails:** Export with `rails db:schema:dump` (SQL format), then lint
74-
- **Prisma:** Use `prisma migrate diff` to get SQL, then lint
73+
- **Rails:** Set `config.active_record.schema_format = :sql`, then run `rails db:structure:dump` to get SQL output, then lint
74+
- **Prisma:** Use `prisma migrate diff --from-empty --to-schema-datamodel ./prisma/schema.prisma --script` to get SQL, then lint
7575
- **TypeORM/Sequelize:** Generate migration SQL, then lint
7676
- **SQLAlchemy:** Use `metadata.create_all()` with `echo=True` to capture SQL, then lint
7777

@@ -99,10 +99,11 @@ Use for any migration scenario: pg_dump imports, ORM migration files (Django, Ra
9999
### ORM migration validation
100100

101101
```
102-
1. Obtain ORM-generated SQL (Django sqlmigrate, Prisma migrate, Rails schema dump)
102+
1. Obtain ORM-generated SQL (Django sqlmigrate, Prisma migrate diff, Rails db:structure:dump)
103103
2. dsql_lint(sql=orm_sql, fix=true)
104104
3. For each diagnostic:
105-
- fixed/fixed_with_warning → accept the fix
105+
- fixed → accept the fix
106+
- fixed_with_warning → present to user, explain app-layer implications before proceeding
106107
- unfixable → rewrite using skill knowledge (Table Recreation, app-layer patterns)
107108
4. Split fixed_sql into one-DDL-per-transaction calls
108109
5. Execute each with transact
@@ -112,7 +113,8 @@ Use for any migration scenario: pg_dump imports, ORM migration files (Django, Ra
112113

113114
## Handling Unfixable Errors
114115

115-
When `dsql_lint` reports unfixable errors, use skill knowledge to resolve:
116+
When `dsql_lint` reports unfixable errors, **MUST** present the proposed alternative to the user
117+
and obtain confirmation before substituting. Use skill knowledge to resolve:
116118

117119
| Rule | Resolution |
118120
| ---------------------------- | ---------------------------------------------------------------- |
@@ -143,6 +145,16 @@ The MCP tool handles exit codes internally. Agents receive structured JSON regar
143145

144146
---
145147

148+
## Error Handling
149+
150+
If `dsql_lint` is unavailable (MCP server not configured) or returns a parse error:
151+
152+
- **MCP unavailable:** Fall back to manual validation using [development-guide.md](development-guide.md) DDL rules and type constraints. Note to user that deterministic validation was not performed.
153+
- **Parse error (`parse_error` rule):** The SQL contains syntax the PostgreSQL parser cannot handle (e.g., MySQL-specific syntax). Fall back to [mysql-migrations/type-mapping.md](mysql-migrations/type-mapping.md) for manual conversion.
154+
- **Timeout:** Retry once. If persistent, split large SQL into smaller statements and lint individually.
155+
156+
---
157+
146158
## Additional Resources
147159

148160
- [dsql-lint on PyPI](https://pypi.org/project/dsql-lint/)

tools/evals/databases-on-aws/dsql/dsql_lint_eval_results.md

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,18 @@
11
# dsql_lint Eval Results — With-Skill vs Baseline
22

3-
**Date:** 2026-05-06
3+
**Date:** 2026-05-08
44
**MCP Server:** awslabs.aurora-dsql-mcp-server (local build, feature/dsql-lint-mcp-tool merged to main)
55
**dsql-lint version:** 0.1.3
6-
**Model:** Claude Opus 4.6 (subagent execution)
6+
**Model:** Claude (subagent execution)
77

88
## Summary
99

1010
| Eval | Scenario | With Skill | Baseline | Delta |
1111
| ---- | ------------------------- | ---------- | --------------- | --------------------------------------------------------------- |
1212
| 100 | pg_dump PostgreSQL schema | **PASS** | FAIL (3 errors) | Skill corrects JSON, index, transaction handling |
1313
| 101 | Django ORM migration | **PASS** | FAIL (3 errors) | Skill corrects JSON, index, provides actionable Django guidance |
14+
| 102 | Clean DSQL-compatible SQL | **PASS** | N/A | Tool correctly reports no issues; agent does not execute |
15+
| 103 | MySQL unsupported syntax | **PASS** | N/A | Tool returns parse error; agent falls back to mysql-migrations |
1416

1517
The skill demonstrably changes agent behavior. The baseline agent hallucinates incorrect
1618
DSQL constraints (JSONB support, synchronous indexes) while the skill-guided agent uses

tools/evals/databases-on-aws/dsql/dsql_lint_evals.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@
3838
},
3939
{
4040
"id": 103,
41-
"prompt": "I need to migrate this MySQL table to DSQL:\n\nCREATE TABLE products (\n id INT AUTO_INCREMENT PRIMARY KEY,\n name VARCHAR(100),\n tags SET('electronics','clothing','food'),\n details JSON,\n FOREIGN KEY (category_id) REFERENCES categories(id)\n) ENGINE=InnoDB PARTITION BY HASH(id) PARTITIONS 4;",
41+
"prompt": "I need to migrate this MySQL table to DSQL:\n\nCREATE TABLE products (\n id INT AUTO_INCREMENT PRIMARY KEY,\n name VARCHAR(100),\n category_id INT,\n tags SET('electronics','clothing','food'),\n details JSON,\n FOREIGN KEY (category_id) REFERENCES categories(id)\n) ENGINE=InnoDB PARTITION BY HASH(id) PARTITIONS 4;",
4242
"expected_output": "Calls dsql_lint with fix=true, identifies multiple issues including unfixable ones (PARTITION BY), presents what can be auto-fixed vs what needs manual rewrite",
4343
"files": [],
4444
"expectations": [

0 commit comments

Comments
 (0)