Repository navigation
feat: implement PostgreSQL DDL to SQLite DDL converter - #1
Conversation
Replace the yaml-lint codebase with a full pg2sqlc implementation that converts PostgreSQL 16 DDL schemas to SQLite3-compatible DDL. Pipeline: Parse (sqlparser) → Normalize → Plan → Transform → Render Key capabilities: - Type mapping (40+ PG types → SQLite affinity types) - Expression conversion (now()→CURRENT_TIMESTAMP, bool→0/1, cast stripping) - ALTER TABLE constraint merging into CREATE TABLE - SERIAL/BIGSERIAL → INTEGER PRIMARY KEY (rowid alias) - Foreign key support with optional PRAGMA foreign_keys - Topological sort for FK dependency ordering - Schema filtering and collision handling - Strict mode with comprehensive diagnostics (25+ warning codes) - Golden test suite (6 fixtures) + 70 unit tests Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Summary of ChangesHello @hiromaily, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a complete rewrite of the project, transforming it from a YAML linter into a dedicated tool for converting PostgreSQL 16 DDL schemas to SQLite3-compatible DDL. The core functionality is built around a robust, multi-stage pipeline designed for offline text-to-text conversion, ensuring accurate and comprehensive schema translation while providing detailed diagnostics for any lossy or unsupported features. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
The pull request successfully implements a PostgreSQL 16 DDL to SQLite3 DDL converter, replacing the previous YAML linter codebase. The architecture follows a clear 7-stage pipeline and includes comprehensive unit and golden tests. The transformation logic handles type mapping, expression conversion, and topological sorting for foreign key dependencies. My feedback focuses on improving the robustness of name resolution in multi-schema scenarios, optimizing enum resolution, and ensuring that cycle detection in topological sorting is reported to the user.
| let alters = std::mem::take(&mut model.alter_constraints); | ||
|
|
||
| for alter in alters { | ||
| let target_table = model | ||
| .tables | ||
| .iter_mut() | ||
| .find(|t| t.name.name.normalized == alter.table.name.normalized); |
There was a problem hiding this comment.
Merging ALTER TABLE constraints currently only compares the table name, ignoring the schema. In multi-schema conversions (include_all_schemas: true), this can lead to constraints being merged into the wrong table if multiple schemas contain tables with the same name. The comparison should use the full QualifiedName.
| } => Some(TableConstraint::ForeignKey { | ||
| name: name.as_ref().map(|n| Ident::new(&n.value)), | ||
| columns: columns.iter().map(|c| Ident::new(&c.value)).collect(), | ||
| ref_table: convert_object_name(foreign_table), | ||
| ref_columns: referred_columns | ||
| .iter() | ||
| .map(|c| Ident::new(&c.value)) | ||
| .collect(), | ||
| on_delete: on_delete.as_ref().and_then(convert_referential_action), | ||
| on_update: on_update.as_ref().and_then(convert_referential_action), | ||
| deferrable: false, | ||
| }), |
There was a problem hiding this comment.
The parser hardcodes deferrable: false for foreign key constraints. PostgreSQL supports DEFERRABLE characteristics which should be extracted from the sqlparser AST to ensure accurate transformation and warning emission later in the pipeline.
There was a problem hiding this comment.
just added comments for now
| let type_name = name.to_string().to_lowercase(); | ||
| match type_name.as_str() { | ||
| "serial" => PgType::Serial, | ||
| "bigserial" => PgType::BigSerial, | ||
| "smallserial" => PgType::SmallSerial, | ||
| "inet" => PgType::Inet, | ||
| "cidr" => PgType::Cidr, | ||
| "macaddr" | "macaddr8" => PgType::MacAddr, | ||
| "money" => PgType::Money, | ||
| "xml" => PgType::Xml, | ||
| "point" => PgType::Point, | ||
| "line" => PgType::Line, | ||
| "lseg" => PgType::Lseg, | ||
| "box" => PgType::Box, | ||
| "path" => PgType::Path, | ||
| "polygon" => PgType::Polygon, | ||
| "circle" => PgType::Circle, | ||
| "int4range" => PgType::Int4Range, | ||
| "int8range" => PgType::Int8Range, | ||
| "numrange" => PgType::NumRange, | ||
| "tsrange" => PgType::TsRange, | ||
| "tstzrange" => PgType::TsTzRange, | ||
| "daterange" => PgType::DateRange, | ||
| _ => PgType::Other { name: type_name }, | ||
| } | ||
| } |
There was a problem hiding this comment.
The mapping for DataType::Custom uses the full string representation of the type name. If a custom type like serial is schema-qualified (e.g., pg_catalog.serial), the current match logic will fail to recognize it. It is safer to check the last part of the ObjectName.
| /// Resolve enum columns: replace PgType::Other with PgType::Enum where a matching enum exists. | ||
| fn resolve_enums(model: &mut SchemaModel, _warnings: &mut [Warning]) { | ||
| let enum_names: Vec<(String, Vec<String>)> = model | ||
| .enums | ||
| .iter() | ||
| .map(|e| (e.name.name.normalized.clone(), e.values.clone())) | ||
| .collect(); | ||
|
|
||
| for table in &mut model.tables { | ||
| for col in &mut table.columns { | ||
| if let PgType::Other { name } = &col.pg_type { | ||
| if let Some((_, _values)) = enum_names.iter().find(|(n, _)| n == name) { | ||
| col.pg_type = PgType::Enum { name: name.clone() }; | ||
| } | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
The enum resolution logic has O(T * C * E) complexity, where T is tables, C is columns, and E is enums. This can be optimized to O(T * C) by pre-collecting enum names into a HashSet for faster lookups.
fn resolve_enums(model: &mut SchemaModel, _warnings: &mut [Warning]) {
let enum_names: std::collections::HashSet<String> = model
.enums
.iter()
.map(|e| e.name.name.normalized.clone())
.collect();
for table in &mut model.tables {
for col in &mut table.columns {
if let PgType::Other { name } = &col.pg_type {
if enum_names.contains(name) {
col.pg_type = PgType::Enum { name: name.clone() };
}
}
}
}
}|
|
||
| /// Sort tables in dependency order (tables referenced by FKs come first). | ||
| /// Falls back to alphabetical order if cycles are detected. | ||
| pub fn topological_sort(tables: &mut Vec<Table>) { |
There was a problem hiding this comment.
When a cycle is detected in the foreign key dependency graph, the topological sort falls back to alphabetical ordering without notifying the user. This can result in SQLite DDL that fails to execute if foreign keys are enabled. A warning should be emitted when a cycle is detected.
1 participant
Summary
make ci)Architecture
Key Capabilities
now()→CURRENT_TIMESTAMP,true/false→1/0, cast stripping,nextval()removalschema__tableprefixingpg2sqlcwith--input,--output,--schema,--enable-foreign-keys,--strict,--emit-warningsTest plan
make cipasses (fmt-check + clippy -D warnings + all tests)🤖 Generated with Claude Code