-
Notifications
You must be signed in to change notification settings - Fork 246
feat: add PostgreSQL schema-qualified table name support #462
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds support for PostgreSQL schema-qualified table names in closure_tree, enabling the library to properly handle tables defined with schema prefixes (e.g., 'test_schema.schema_types'). The implementation ensures that hierarchy tables are created with the correct schema prefix.
- Implements schema-aware hierarchy table name generation
- Adds test infrastructure with schema-qualified test tables
- Validates schema-qualified table behavior with comprehensive tests
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| lib/closure_tree/support.rb | Updates hierarchy_table_name logic to parse and preserve schema prefixes when generating hierarchy table names |
| test/dummy/db/schema.rb | Adds test_schema with schema_types and schema_type_hierarchies tables for testing schema-qualified names |
| test/dummy/app/models/schema_type.rb | Creates SchemaType model using schema-qualified table name 'test_schema.schema_types' |
| test/closure_tree/schema_type_test.rb | Adds comprehensive test coverage for schema-qualified table operations including lineage, reparenting, and tree traversal |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
lib/closure_tree/support.rb
Outdated
|
|
||
| # Handle PostgreSQL schema-qualified table names (e.g., "my_schema.table_name") | ||
| if base_table.include?('.') | ||
| schema, table = base_table.split('.', 2) |
Copilot
AI
Oct 17, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The split with limit 2 may not handle edge cases correctly. If the table name itself contains a period (beyond the schema separator), this would incorrectly parse the schema and table parts. Consider using rsplit or splitting only on the first period to ensure the schema is correctly separated from the table name.
| schema, table = base_table.split('.', 2) | |
| schema, _, table = base_table.rpartition('.') |
| create_table 'test_schema.schema_type_hierarchies', id: false, force: true do |t| | ||
| t.references 'ancestor', null: false | ||
| t.references 'descendant', null: false | ||
| t.integer 'generations', null: false | ||
| end |
Copilot
AI
Oct 17, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The schema_type_hierarchies table is manually created, but the PR's purpose is to auto-generate hierarchy tables with schema prefixes. If the library should automatically create this table, consider removing this manual definition to ensure the feature works as intended and to avoid duplication between manual and auto-generated tables.
| create_table 'test_schema.schema_type_hierarchies', id: false, force: true do |t| | |
| t.references 'ancestor', null: false | |
| t.references 'descendant', null: false | |
| t.integer 'generations', null: false | |
| end |
Add support for PostgreSQL schema-qualified table names (e.g., 'test_schema.schema_types') to properly generate hierarchy table names with schema prefix.
4f7d877 to
253653a
Compare
Add support for PostgreSQL schema-qualified table names (e.g., 'test_schema.schema_types') to properly generate hierarchy table names with schema prefix.