Fix case-sensitive VISUALISE column reference validation#143
Open
cpsievert wants to merge 1 commit intoposit-dev:mainfrom
Open
Fix case-sensitive VISUALISE column reference validation#143cpsievert wants to merge 1 commit intoposit-dev:mainfrom
cpsievert wants to merge 1 commit intoposit-dev:mainfrom
Conversation
c495a46 to
eece4f2
Compare
eece4f2 to
b949f97
Compare
The VISUALISE parser preserves the exact casing from the user's query (e.g., `VISUALISE ROOM_TYPE AS x` stores "ROOM_TYPE"). However, DuckDB lowercases unquoted identifiers in query results, so the schema column is "room_type". Since ggsql quotes column names in generated SQL (making them case-sensitive in DuckDB), this mismatch caused validation errors like: "aesthetic 'x' references non-existent column 'ROOM_TYPE'" Add a normalize_column_names() step early in the prepare_data pipeline that resolves VISUALISE column references to match the actual schema column names using case-insensitive matching. This is reader-agnostic: it normalizes to whatever the reader returns, not specifically to lowercase. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
b949f97 to
b467129
Compare
thomasp85
reviewed
Feb 27, 2026
Comment on lines
+51
to
+52
| // Normalize global mappings using the first layer's schema (global mappings | ||
| // are merged into all layers, so any layer's schema suffices for normalization) |
Collaborator
There was a problem hiding this comment.
This assertion is wrong. Each layer only takes from the global mapping what their data source and aesthetic requirements dictates. It's better to wait until global mapping has been merged into the layers and then do the normalisation only for the layers
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
SQL treats unquoted identifiers as case-insensitive —
SELECT REGION FROM tworks even when the column is stored asregion. However, ggsql's VISUALISE clause matches column references case-sensitively against the result schema. Since DuckDB lowercases unquoted identifiers in query results, writingVISUALISE REGION AS xfails when the result column isregion:Fix
Adds a
normalize_column_names()step early inprepare_data_with_reader(before merge, validation, and query building). It uses case-insensitive matching to resolve VISUALISE column references to the actual column names in the result schema. This is reader-agnostic — it normalizes to whatever the reader returns, not specifically to lowercase.Reproducing
CLI:
Python:
Test plan
test_case_insensitive_column_referencestest