Add support for postgres schema selection #373
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the 📝 WalkthroughWalkthroughThe changes add PostgreSQL schema support throughout the application stack. A new schema parsing method extracts the default schema from connection URLs, updated extraction methods accept and respect a schema parameter, the UI provides schema input for PostgreSQL connections, and comprehensive tests and documentation cover the new functionality. Changes
Sequence DiagramsequenceDiagram
participant User as User
participant Modal as DatabaseModal
participant URL as URL Constructor
participant Loader as PostgresLoader
participant Parser as _parse_schema_from_url
participant DB as PostgreSQL
User->>Modal: Select PostgreSQL + Input Schema
Modal->>URL: Append schema to connection URL
URL-->>Modal: Connection URL with options=-csearch_path=<schema>
Modal->>Loader: load(prefix, connection_url)
Loader->>Parser: _parse_schema_from_url(connection_url)
Parser->>Parser: Extract schema from options parameter
Parser-->>Loader: schema (or 'public' default)
Loader->>DB: Connect & extract_tables_info(cursor, schema)
Loader->>DB: extract_columns_info(cursor, table, schema)
Loader->>DB: extract_foreign_keys(cursor, table, schema)
Loader->>DB: extract_relationships(cursor, schema)
DB-->>Loader: Filtered schema-specific metadata
Loader-->>Modal: Schema-constrained database structure
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
app/src/components/modals/DatabaseModal.tsx (1)
98-103: URL encoding approach is correct.The schema option construction follows PostgreSQL's
optionsparameter format. The use of hardcoded%3DwithencodeURIComponentfor the schema value ensures proper encoding.One minor consideration: schema names with spaces or special characters might need additional validation or quoting. PostgreSQL schema names containing spaces require double quotes, but this is an edge case rarely encountered in practice.
docs/postgres_loader.md (1)
277-289: Reconsider documenting private method usage.The verification example calls
PostgresLoader._parse_schema_from_url(), which is a private method (indicated by the leading underscore). Documenting private methods in user-facing documentation can create maintenance issues if the internal implementation changes.Consider one of these alternatives:
- Make the method public if it's intended for external use
- Document verification through connection testing instead
- Remove this verification section if it's primarily for internal debugging
api/loaders/postgres_loader.py (1)
100-147: Schema parsing logic is generally sound with minor edge case.The implementation correctly handles most common scenarios including URL-encoded formats, case-insensitive matching, comma-separated schemas, and the
$userspecial variable.Minor edge case: The regex pattern
([^\s]+)at line 128 will not correctly capture quoted schema names containing spaces (e.g.,search_path="my schema"). This is an uncommon edge case since PostgreSQL schema names with spaces are rare, but it's worth noting.Regarding the static analysis hints: The broad exception handler at line 145 is reasonable for a parser that should gracefully degrade, though it could optionally log parse failures for debugging. The return statement structure (TRY300 hint) is a style preference and functionally equivalent to using an
elseblock.tests/test_postgres_loader.py (1)
153-239: LGTM! Comprehensive test coverage for schema parsing.The test suite thoroughly covers the main scenarios including defaults, format variations, special cases ($user), quoted values, case insensitivity, and error handling. The tests validate the expected behavior of
_parse_schema_from_urlacross a wide range of inputs.Optional enhancement: Consider adding a test case for quoted schemas containing spaces (e.g.,
search_path="my schema"), which would verify handling of the edge case where PostgreSQL schema names include spaces. This is uncommon in practice but would complete the test coverage.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
README.mdapi/loaders/postgres_loader.pyapp/src/components/modals/DatabaseModal.tsxdocs/postgres_loader.mdtests/test_postgres_loader.py
🧰 Additional context used
📓 Path-based instructions (4)
app/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Frontend source changes in app/**/*.{ts,tsx} must be compiled before production runs
Files:
app/src/components/modals/DatabaseModal.tsx
README.md
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
When changing MCP wiring or adding integrations, update README.md with usage and configuration details
Files:
README.md
**/*.py
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
All Python code should pass pylint (use make lint)
Files:
api/loaders/postgres_loader.pytests/test_postgres_loader.py
tests/test_*.py
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Unit test files should be named tests/test_*.py and runnable with pytest
Files:
tests/test_postgres_loader.py
🧬 Code graph analysis (2)
api/loaders/postgres_loader.py (2)
api/loaders/base_loader.py (1)
load(12-22)api/loaders/mysql_loader.py (5)
load(155-202)extract_tables_info(205-252)extract_relationships(372-411)extract_columns_info(255-332)extract_foreign_keys(335-369)
tests/test_postgres_loader.py (1)
api/loaders/postgres_loader.py (1)
_parse_schema_from_url(101-147)
🪛 LanguageTool
docs/postgres_loader.md
[style] ~269-~269: This adverb was used twice in the sentence. Consider removing one of them or replacing them with a synonym.
Context: ...ted - Ensure the schema name is spelled correctly (schema names are case-sensitive) 4. **...
(ADVERB_REPETITION_PREMIUM)
🪛 Ruff (0.14.10)
api/loaders/postgres_loader.py
143-143: Consider moving this statement to an else block
(TRY300)
145-145: Do not catch blind exception: Exception
(BLE001)
🔇 Additional comments (13)
README.md (1)
219-219: LGTM! Formatting fix.The closing code fence properly completes the Python streaming example code block.
app/src/components/modals/DatabaseModal.tsx (3)
31-31: LGTM! Schema state follows existing patterns.The state initialization is consistent with other form fields in the component.
185-185: LGTM! Schema state reset follows existing pattern.Consistent with other form field resets after successful connection.
394-412: LGTM! Schema UI field is well-implemented.The conditional rendering, optional labeling, placeholder, helper text, and test ID all follow best practices and provide clear UX guidance.
docs/postgres_loader.md (3)
94-123: LGTM! Comprehensive schema configuration documentation.The Custom Schema Configuration section accurately describes the feature with clear examples, URL encoding notes, default behavior, permission requirements, and UI guidance. The PostgreSQL documentation reference is helpful.
256-256: LGTM! Limitation accurately described.The updated limitation correctly states that extraction is limited to one schema at a time, with clear indication of the default and configuration method.
266-276: LGTM! Enhanced troubleshooting guidance.The expanded troubleshooting section provides actionable guidance for schema-related issues, including verification steps and permission checks with correct PostgreSQL GRANT syntax.
api/loaders/postgres_loader.py (6)
8-8: LGTM! Appropriate imports for URL parsing.The urllib.parse imports are standard library components suitable for extracting schema from connection URLs.
150-201: LGTM! Schema integration in load() is well-implemented.The schema is correctly parsed before connection and propagated to extraction methods. The connection URL is passed unchanged to psycopg2, allowing native handling of the
optionsparameter, while the parsed schema constrains metadata queries. The updated docstring provides helpful usage guidance.
204-258: LGTM! extract_tables_info correctly filters by schema.The method signature with default parameter maintains backward compatibility. The SQL query properly filters both the main query and the comment subquery by schema. Schema propagation to column and foreign key extraction is correct.
260-350: LGTM! extract_columns_info properly filters by schema.The method signature maintains backward compatibility. All SQL subqueries and the main query correctly filter by schema, including the pg_namespace join and the PRIMARY KEY/FOREIGN KEY lookups.
352-392: LGTM! extract_foreign_keys correctly scoped to schema.The method signature with default parameter and the SQL query properly constrain foreign key extraction to the specified schema while maintaining backward compatibility.
394-442: LGTM! extract_relationships properly scoped to schema.The method filters all foreign key relationships by the specified schema. The default parameter ensures backward compatibility. The SQL query correctly constrains the results to the target schema.
|
@sirudog Thanks for the contribution |
| // Append schema option for PostgreSQL if provided | ||
| if (selectedDatabase === 'postgresql' && schema.trim()) { | ||
| const schemaOption = `options=-csearch_path%3D${encodeURIComponent(schema.trim())}`; | ||
| dbUrl += (dbUrl.includes('?') ? '&' : '?') + schemaOption; |
There was a problem hiding this comment.
better to use:
const url = new URL(dbUrl);
url.searchParams.set('options', -csearch_path=${schema.trim()});
dbUrl = url.toString();
|
@sirudog I left single comment. Please fix the spellcheck |
|
@galshubeli I'll revisit the spell checks... There are still a few :-( |
@galshubeli, it is done, I hope I did not miss any this time :-) |
2 participants
Currently the schema is hardcoded to
'public'.This PR adds support for postgres schema selection by using the options parameter in the connection string, which is the proper Postgres-native way.
The connection string would look like (before URL encoding):
postgresql://user:pass@host:port/database?options=-c search_path=custom_schemaHow it works:
psycopg2/libpqprocesses the options parameter directly, sosearch_pathis set automatically on connectionsearch_pathvalue from the options parameter to use ininformation_schemaqueriessearch_pathspecified, fall back to'public'Why this approach:
'public'if not specifiedSummary by CodeRabbit
New Features
Documentation
Tests
✏️ Tip: You can customize this high-level summary in your review settings.