Skip to content

Comments

fix: add proper quoting for camelCase columns in CREATE INDEX#23

Closed
screenfluent wants to merge 3 commits intopgplex:mainfrom
screenfluent:fix/quote-camelcase-index-columns
Closed

fix: add proper quoting for camelCase columns in CREATE INDEX#23
screenfluent wants to merge 3 commits intopgplex:mainfrom
screenfluent:fix/quote-camelcase-index-columns

Conversation

@screenfluent
Copy link
Contributor

Problem

CREATE INDEX statements were generating column names without quotes, causing PostgreSQL to lowercase them. This breaks indexes on tables with camelCase columns.

Example of the bug:

-- Input schema:
CREATE INDEX "idx_invite_assignedTo" ON "invite"("assignedTo");

-- pgschema generates (WRONG):
CREATE INDEX CONCURRENTLY IF NOT EXISTS idx_invite_assignedTo ON invite (assignedTo);
-- Error: column "assignedto" does not exist

-- Should generate (CORRECT):
CREATE INDEX CONCURRENTLY IF NOT EXISTS idx_invite_assignedTo ON invite ("assignedTo");

Solution

Use util.QuoteIdentifier() for column names in index generation. This ensures:

  • camelCase columns keep their quotes: "assignedTo"
  • lowercase columns don't get unnecessary quotes: email
  • reserved words get quoted: "order", "user"

Changes

  • internal/diff/index.go: Fixed regular CREATE INDEX statements
  • internal/plan/rewrite.go: Fixed CREATE INDEX CONCURRENTLY statements
  • Added comprehensive tests for camelCase, mixed case, and reserved words

Testing

All new tests pass:

go test ./internal/diff -run TestGenerateIndexSQL -v
# PASS all 7 test cases

Tested on real project with camelCase columns - confirmed working correctly.

Related Issues

This is a different issue from #17/#22 (constraint column ordering). This one specifically fixes missing quotes in index column names.

Copilot AI review requested due to automatic review settings September 13, 2025 09:31
Copy link
Contributor

Copilot AI left a 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 fixes a bug where CREATE INDEX statements were not properly quoting camelCase column names, causing PostgreSQL to lowercase them and resulting in "column does not exist" errors.

  • Added util.QuoteIdentifier() calls to properly quote column names in index generation
  • Fixed both regular CREATE INDEX and CREATE INDEX CONCURRENTLY statements
  • Added comprehensive test coverage for camelCase columns, mixed case scenarios, and reserved words

Reviewed Changes

Copilot reviewed 7 out of 8 changed files in this pull request and generated no comments.

File Description
internal/diff/index.go Fixed column name quoting in regular CREATE INDEX statements
internal/plan/rewrite.go Fixed column name quoting in CREATE INDEX CONCURRENTLY statements
internal/diff/index_test.go Added comprehensive test cases for camelCase column handling
testdata/diff/online/add_camelcase_index/* Test data files demonstrating the fix for camelCase index creation

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@screenfluent screenfluent force-pushed the fix/quote-camelcase-index-columns branch 2 times, most recently from df777ec to 5759a93 Compare September 13, 2025 09:45
@screenfluent screenfluent marked this pull request as draft September 13, 2025 10:04
@screenfluent screenfluent force-pushed the fix/quote-camelcase-index-columns branch 2 times, most recently from 719fc98 to 26b6049 Compare September 13, 2025 13:04
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please re-sync the main branch. I have removed those files there. Thanks for catching this.

part := col.Name
var part string
// Don't quote functional expressions
if strings.Contains(col.Name, "(") || strings.Contains(col.Name, ")") {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am wondering if we can handle quotation in the ir module

https://github.com/pgschema/pgschema/blob/main/internal/ir/ir.go#L201

Problem: CREATE INDEX statements were generating column names without quotes,
causing PostgreSQL to lowercase them (assignedTo -> assignedto).

Solution: Use util.QuoteIdentifier() for simple column names, but avoid
quoting functional expressions like lower(email) or upper(firstName).

Changes:
- internal/diff/index.go: Fixed regular CREATE INDEX statements
- internal/plan/rewrite.go: Fixed CREATE INDEX CONCURRENTLY statements
- Added comprehensive tests for camelCase, functional indexes, and reserved words

This ensures:
- camelCase columns are quoted: "assignedTo"
- lowercase columns remain unquoted: email
- reserved words are quoted: "order", "user"
- functional expressions remain unquoted: lower(email)
…TE INDEX

Problem: CREATE INDEX statements were missing quotes for:
1. CamelCase column names (assignedTo -> assignedto)
2. CamelCase index names (idx_invite_assignedTo -> idx_invite_assignedto)

Solution: Use util.QuoteIdentifier() for:
- Index names with camelCase
- Simple column names (but not functional expressions)

Changes:
- internal/diff/index.go: Quote index names and column names
- internal/plan/rewrite.go: Quote index names for CONCURRENTLY
- Added comprehensive tests for all scenarios
- Fixed test data to expect quoted index names

This ensures proper quoting while preserving functional indexes like lower(email)
@screenfluent screenfluent force-pushed the fix/quote-camelcase-index-columns branch from 26b6049 to 99583a6 Compare September 13, 2025 13:45
All CREATE INDEX CONCURRENTLY tests now use lower() function in wait queries
to properly handle case-insensitive index name comparisons in PostgreSQL
@tianzhou tianzhou force-pushed the main branch 4 times, most recently from eb7c30c to 0f15948 Compare September 24, 2025 03:30
@tianzhou
Copy link
Contributor

The codebase has been changed significantly and make this PR not mergeable.

@tianzhou tianzhou closed this Oct 30, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants