fix(editor): quote PostgreSQL table completion identifiers - #791
fix(editor): quote PostgreSQL table completion identifiers#791mikevillari wants to merge 3 commits into
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
|
I need your local test results regarding the following points:
|
|
Verified locally against a real PostgreSQL 18.4 process, using the exact PR head Results: 36 autocomplete-generated queries passed, plus 21 control queries with the expected outcomes. I created each of these in
The important distinction in point 1 is how the table was created: unquoted Example queries actually generated and executed: SELECT marker FROM "lowercase_table";
SELECT marker FROM "uppercase_table";
SELECT marker FROM public."MixedCaseTable";
SELECT marker FROM "authschema"."MixedCaseTable";
SELECT marker FROM "AuthSchema"."QUOTED_UPPER_TABLE";For both Method: create isolated fixture tables containing distinct marker rows; read their names from Server reported: |
|
What I meant to say was this: |
For Assuming the table was created externally with the screaming case, ofc, and the auto-completion is in libredb-studio. |
|
Understood — the previous implementation added unnecessary quotes to ordinary names, and my earlier test report only established that the quoted SQL executed. I have revised the implementation to preserve ordinary unquoted names. The completion now follows PostgreSQL's conservative
For example, ordinary schema qualification now inserts Verified locally on native PostgreSQL 18.4: 45 queries generated by the revised completion provider across The focused regressions pass (43/43); the complete Pushed as |
|
We can discuss this; so they should only be used for table names containing UPPPERlowercase letters("UserAuthority"), special characters, spaces, etc.("User Authority", "user's authority" etc...) |
|
Thanks @cevheri — I agree that ordinary table names should stay unquoted. The latest revision ( SELECT * FROM authschema.user_authority;
SELECT * FROM authschema."UserAuthority";
SELECT * FROM "User Authority";
SELECT * FROM "user's authority";The uppercase case needs one distinction: a table created with unquoted I agree that quoting the wrong spelling could change which table a query refers to. This implementation uses the exact name returned by the database, rather than turning the user's typed uppercase text into a quoted name. It also handles names that are SQL keywords, such as One detail to agree on: it follows PostgreSQL's conservative |
|
how did you handle the code-completion functionality in other SQL databases? |
|
I checked the shared completion code in response to your question. The table-name quoting was already PostgreSQL-only, but the new quoted-name column lookup also affected other dialects. I have now restricted that lookup to PostgreSQL too. I compared the complete suggestions with the code before this PR: all 544 checks across the other database settings matched, including inserted text and replacement ranges. I also added regression tests for those settings and for switching between PostgreSQL and MySQL, SQLite, DuckDB, SQL Server, and Oracle. For live checks, I used the real SQLite and DuckDB providers: 28 completion-generated queries returned the expected rows. I have not run live MySQL, SQL Server, or Oracle checks, so the tests for those are editor-level tests only. The aim is to fix PostgreSQL quoting while preserving the other databases' existing behavior. Their existing special-name completion limitations remain outside this fix; I recorded those in the PR description. Pushed as |
|
I’m working on PR #811 right now, which includes some big changes to the schema/objects explorer(nosql databases test remaining) After that, I’ll do a detailed test of all SQL database providers. And thanks again for the clean and disciplined work. It’s really appreciated. |
PostgreSQL table suggestions insert catalog names verbatim, so accepting
My_Schema_With_Caps.My_Table_With_Capsproduces unquoted SQL that resolves to lowercase names. This also reproduces with the completion provider from before #715 (base4c5f6577). Follow-up to the report in #705: #705 (comment).Pass the editor's connection dialect into the completion provider and format PostgreSQL table-name components using PostgreSQL 18's conservative
quote_identrule. Ordinary lowercase names remain unquoted (authschema.user_authority); case-sensitive names retain their required quotes (authschema."UserAuthority","USER_AUTHORITY"); keywords and special characters are escaped. The keyword categories come from PostgreSQL's parser keyword list, separate from the editor's suggestion list. The shared unconditional identifier-quoter keeps its existing semantics.Keep display labels and filtering separate from inserted SQL, preserve qualified replacement ranges and bare-label fallback, recognize quoted table names when offering columns after a dot only for PostgreSQL, and re-register when the connection dialect changes. Other dialects keep their existing insertion behavior.
The quoting decision uses the catalog name, not the case of the typed prefix. PostgreSQL stores
CREATE TABLE USER_AUTHORITY (...)asuser_authority; its completion is bare. A table explicitly created as"USER_AUTHORITY"is a distinct object and its completion retains quotes.Testing
1a63657a): 45 autocomplete-generated queries acrosspublic,authschema, and quoted"AuthSchema", with distinct marker rows verifying the selected object. 9 unquoted spelling controls also passed. Cache built from realinformation_schemanames using the adapter's existing display-label convention; completion provider bundled from the revised source, Monaco word/range modeled for this matrix.quote_identfor all 494 server keywords and 13 additional identifier cases.1a63657a): accepted suggestions and executed the resulting SQL against the same PostgreSQL instance for 7 cases: lowercase, uppercase typed prefix, mixed-case table, quoted uppercase table, quoted schema, keyword, and schema-dot trigger. All returned the intended marker. This is a real Monaco/provider interaction test, not full-application E2E.attw) passed.bun run test: 15,122 pass, 0 fail, including all 35 isolated component groups. Temporary local Helm 4.1.3, 7-Zip 26.03, and the locked chart dependency were installed before the successful complete run.The implementation retains the provider's existing dot-delimited table-label representation. It does not redesign schema metadata, handle literal dots within individual identifier components, add completion of partially quoted input, or quote column suggestions. Full application E2E and the repository-wide coverage gate were not run locally; CI remains the merge gate.
AI-assisted implementation and validation.
Cross-database follow-up
Both new quoted table insertion and new quoted-name column lookup are restricted to PostgreSQL. The latter was initially shared; this revision restores the pre-PR bare-name lookup for every other dialect and callers without a dialect.
37fada54across all 16 non-PostgreSQL registration settings plus the unspecified-dialect fallback (32 inputs each). This includes defensive registrations for non-SQL type IDs, not a claim that all 16 use the SQL editor. Insertion text, ranges, labels and other suggestion fields were compared. A separate 32-case PostgreSQL comparison matches prior revision1a63657a. These use a modeled Monaco interface and a synthetic cache.getSchema()/query()methods. 28 completion-generated queries returned expected marker rows (12 SQLite, 16 DuckDB), covering ordinary lowercase, mixed/uppercase names, direct/alias column completion, and DuckDB non-default-schema tables. Before/after suggestion objects matched. The cache mirrors QueryEditor and Monaco word/range is modeled; these are not browser UI checks.