Inconsistent constraint names in PostgreSQL

[sebastian-nowak]

Environment

Knex version: 0.95.1 Database + version: PostgreSQL 13 OS: n/a

Bug

Run the code:

  knex.schema.createTable('TestTable', (table) => {
    table.integer('otherId').primary().notNull().references('Other.id');
  }).debug();

It generates the following queries:

[
  {
    sql: 'create table "TestTable" ("otherId" integer not null)',
    bindings: []
  },
  {
    sql: 'alter table "TestTable" add constraint "TestTable_pkey" primary key ("otherId")',
    bindings: []
  },
  {
    sql: 'alter table "TestTable" add constraint "testtable_otherid_foreign" foreign key ("otherId") references "Other" ("id")',
    bindings: []
  }
]

Notice how one constraint name correctly preserves the PascalCase (TestTable_pkey) while the other one ignores it (testtable_otherid_foreign). Knex should be consistent and either always keep the original formatting or always convert it to lower case. I'd definitely prefer the former.

[kibertoad]

@sebastian-nowak Is this a new behaviour in 0.95? I checked the code and it is likely to be pretty old.

[kibertoad]

@elhigu We have this code in generic Tablecompiler:

  // If no name was specified for this index, we will create one using a basic
  // convention of the table name, followed by the columns, followed by an
  // index type, such as primary or index, which makes the index unique.
  _indexCommand(type, tableName, columns) {
    if (!Array.isArray(columns)) columns = columns ? [columns] : [];
    const table = tableName.replace(/\.|-/g, '_');
    const indexName = (
      table +
      '_' +
      columns.join('_') +
      '_' +
      type
    ).toLowerCase();
    return this.formatter.wrap(indexName);
  }

Can you think of a good reason for lowercasing there? PG Tablecompiler doesn't lowercase anything.

[sebastian-nowak]

You're right it was like this for a while, I haven't used Knex before.

Can you think of a good reason for lowercasing there?

Maybe whoever added the .toLowerCase() there simply wasn't sure whether he can keep the original formatting? It's a generic compiler so it was a safer bet. Also, this module defaults to lower case for SQL keywords (select instead of SELECT etc) so it could have been a personal preference too.

I think we can rather safely assume that all databases can handle mixed case correctly, as long as the constraint name is in quotes. It would be very surprising if dropping .toLowerCase() could cause a query failure.

[kibertoad]

@sebastian-nowak I'm mostly concerned that at this point changing logic would be something of a breaking change, because it is possible that someone wrote constraint-manipulating SQL (e. g. for dropping some of the constraints by name) taking the knex behaviour into consideration, and changing observable behaviour outside of semver major is not a good thing to do. And considering that original logic was there at least from 2016, and it didn't come up until now, I think it's not that big of an issue for the majority of the users.

[sebastian-nowak]

That's a good point, it could affect existing migrations. One possible solution is to make the new behavior opt-in in knex config, and it would become enabled by default only when there's a major release.

And considering that original logic was there at least from 2016, and it didn't come up until now, I think it's not that big of an issue for the majority of the users.

I think the majority of users prefer snake case for columns names, so they were not affected. Bigger projects would usually set key and constraint names manually to avoid surprises like this, so they wouldn't notice it too.

[mercmobily]

This likely needs a design decision and probably an opt‑in to avoid a breaking change. Constraint names are generated differently in two places, leading to inconsistent casing on PostgreSQL: primary key name preserves table case, while foreign key name is lower‑cased by the generic helper.

Code evidence:

• Generic _indexCommand lowercases the generated name: lib/dialects/oracle/schema/oracle-tablecompiler.js:93–110 shows the pattern; the generic helper in other compilers uses .toLowerCase() on table + '_' + columns + '_' + type.
• PostgreSQL‑specific paths don’t lower case names the same way. Hence "TestTable_pkey" vs "testtable_otherid_foreign" in your example.

Changing this across the board would be a breaking change as users may have written migrations that refer to the lower‑cased names. A safe path might be:

• Add a configuration flag to preserve original casing in generated constraint names (i.e. drop the .toLowerCase()), defaulting to current behaviour until a major release.
• Document best practice: explicitly name constraints when casing matters.