drizzle-orm icon indicating copy to clipboard operation
drizzle-orm copied to clipboard

[BUG]: Deeply nested queries fail due to table name length

Open EamonHeffernan opened this issue 11 months ago • 8 comments

What version of drizzle-orm are you using?

0.30.1

What version of drizzle-kit are you using?

0.20.14

Describe the Bug

When creating a query with deeply nested data, the sql generated uses names based on the relation names that have been set up, with more text being appended to the name for each level of depth. "table1" "table1_table2" "table1_table2_table3" etc...

If this reaches the limit for the length of a table name, the database will truncate the names to the number of allowed characters, and fail with the error error: table name "documentVersions_workflowStep_workflow_contract_entities_userEn" specified more than once because there were multiple more tables in the name that are cut off. They instead all get truncated to the same string which causes an error.

Expected behavior

Drizzle should either use shorter names upon reaching the limit for a table name, or use aliases within the request that get turned back into the correct names upon response. This ideally should not be something the drizzle user has to worry about, as the query builder should preferably work without having to manually set table names for all the joins.

Environment & setup

This occurs using postgresql

EamonHeffernan avatar Mar 26 '24 04:03 EamonHeffernan

I'm experiencing a similar issue, where the query says the column does not exist due name truncating

jonatandorozco avatar Mar 27 '24 16:03 jonatandorozco

Just ran into the exact same issue

gmanavarro avatar Apr 09 '24 20:04 gmanavarro

@EamonHeffernan did you figured out anything for this?

driwand avatar Apr 19 '24 01:04 driwand

@EamonHeffernan did you figured out anything for this?

I'm not a maintainer, the only workaround I've found in my own code is to either do it without query which defeats the whole point of having the query system, or to instead break it into small enough queries that don't run into this issue. I've attempted to put this issue into the discord to get some attention about it. But this seems like such a simple case to run into because it doesn't take that much nesting to run into it if you have decently long table names.

EamonHeffernan avatar Apr 19 '24 02:04 EamonHeffernan

This is because Postgres has a default max table name length of 63 characters (https://www.postgresql.org/docs/current/sql-syntax-lexical.html#SQL-SYNTAX-IDENTIFIERS) which is set by NAMEDATALEN. Any table name (or table alias) that is longer than 63 characters will get truncated to 63 characters. Updating NAMEDATALEN is not recommended.

Being able to set a table alias in drizzle queries would be helpful.

nick-kang avatar May 10 '24 09:05 nick-kang

What about the table names being hashed? With that we can ensure the length is short and the names are unique to avoid collisions

jonatandorozco avatar May 14 '24 17:05 jonatandorozco

What about the table names being hashed? With that we can ensure the length is short and the names are unique to avoid collisions

Yeah, I don't think there is any reason this would need to be opt in, this should be just automatically handled because the names are not relevant to the consumer of the query. The only downside would be that it would be slightly harder to debug because the query wouldn't have useful names, but that is better than the query not running at all.

EamonHeffernan avatar May 16 '24 01:05 EamonHeffernan

@EamonHeffernan I believe we can still have good debugging even with hashed names, if they're kept in memory in a map whenever a warning/error occurs we can fallback to that map to retrieve the original names and show the query accordingly

jonatandorozco avatar May 16 '24 16:05 jonatandorozco

Anyone found a workaround for this issue?

noobmaster19 avatar Jun 27 '24 07:06 noobmaster19

Same issue here, any fixes?

stabildev avatar Jul 11 '24 22:07 stabildev

This stops me from doing most basic queries with "with" keyword ...how is this not a biggest priority bug fix :(

To clarify, this bug makes drizzle totally unusable on any bigger project. Because if you have 4-5 levels deep relations you will absolutely hit this limit (and even error message isnt really clear that this is the issue). Your only option then is to rename your tables to something like "a", "b", but not the db table name, but the name of the instance of a table so it makes your whole codebase unreadable.

nagy135 avatar Aug 18 '24 15:08 nagy135

Like previous commenters are saying, this completely prevents using Drizzle with any small amount of nesting :( Is there an update from the Drizzle team about this being a priority?

LocatedInSpace avatar Sep 02 '24 05:09 LocatedInSpace

An easy workaround is just giving your relations shorter names, especially for join tables, e. g.

export const customerRelations = relations(customers ({ one, many }) => ({
  o: one(organizations, {
    fields: [customers.orgId],
    references: [organizations.id],
  }),
  mgrs: many(managers),
  ma: ...
  pm: ...
}));

and use these short names in your with queries

stabildev avatar Sep 02 '24 11:09 stabildev

Something has to be done to these lines https://github.com/drizzle-team/drizzle-orm/blob/c8359a16fff4b05aff09445edd63fc65a7430ce9/drizzle-orm/src/pg-core/dialect.ts#L1228 and https://github.com/drizzle-team/drizzle-orm/blob/c8359a16fff4b05aff09445edd63fc65a7430ce9/drizzle-orm/src/pg-core/dialect.ts#L1284

sukeshpabolu avatar Sep 17 '24 18:09 sukeshpabolu

typeorm fixed this here

sukeshpabolu avatar Sep 17 '24 18:09 sukeshpabolu

For now you can do patch-package with the below patch to fix the issue

diff --git a/node_modules/drizzle-orm/pg-core/dialect.js b/node_modules/drizzle-orm/pg-core/dialect.js
index a8dfc1f..560a664 100644
--- a/node_modules/drizzle-orm/pg-core/dialect.js
+++ b/node_modules/drizzle-orm/pg-core/dialect.js
@@ -35,6 +35,7 @@ import { ViewBaseConfig } from "../view-common.js";
 import { PgViewBase } from "./view-base.js";
 class PgDialect {
   static [entityKind] = "PgDialect";
+  nameMap = {};
   async migrate(migrations, session, config) {
     const migrationsTable = typeof config === "string" ? "__drizzle_migrations" : config.migrationsTable ?? "__drizzle_migrations";
     const migrationsSchema = typeof config === "string" ? "drizzle" : config.migrationsSchema ?? "drizzle";
@@ -73,6 +74,15 @@ class PgDialect {
   escapeString(str) {
     return `'${str.replace(/'/g, "''")}'`;
   }
+  makeHash(length = 5) {
+    let result = '';
+    const characters = 'ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789';
+    const charactersLength = characters.length;
+    for (let i = 0; i < length; i++) {
+      result += characters.charAt(Math.floor(Math.random() * charactersLength));
+    }
+    return result;
+  }
   buildWithCTE(queries) {
     if (!queries?.length)
       return void 0;
@@ -973,7 +983,9 @@ class PgDialect {
         const normalizedRelation = normalizeRelation(schema, tableNamesMap, relation);
         const relationTableName = getTableUniqueName(relation.referencedTable);
         const relationTableTsName = tableNamesMap[relationTableName];
-        const relationTableAlias = `${tableAlias}_${selectedRelationTsKey}`;
+        // const relationTableAlias = `${tableAlias}_${selectedRelationTsKey}`;
+        const relationTableAlias = this.makeHash();
+        this.nameMap[ `${tableAlias}_${selectedRelationTsKey}`] = relationTableAlias;
         const joinOn2 = and(
           ...normalizedRelation.fields.map(
             (field2, i) => eq(
@@ -1019,7 +1031,7 @@ class PgDialect {
     if (nestedQueryRelation) {
       let field = sql`json_build_array(${sql.join(
         selection.map(
-          ({ field: field2, tsKey, isJson }) => isJson ? sql`${sql.identifier(`${tableAlias}_${tsKey}`)}.${sql.identifier("data")}` : is(field2, SQL.Aliased) ? field2.sql : field2
+          ({ field: field2, tsKey, isJson }) => isJson ? sql`${sql.identifier(this.nameMap[`${tableAlias}_${tsKey}`])}.${sql.identifier("data")}` : is(field2, SQL.Aliased) ? field2.sql : field2
         ),
         sql`, `
       )})`;

sukeshpabolu avatar Sep 17 '24 19:09 sukeshpabolu

thanks for giving this an effort! You should definitely file a PR, you are the hero we need ...but i m pretty sure there will be friction, because this would make reading queries harder with hashed table names, so probably some flag or just take few first letters or sth would be way to go, but creating PR could start a debate

nagy135 avatar Sep 17 '24 21:09 nagy135

I havent contributed to that many projects. Is it ok to inspire from this? Like how they used crypto and other things

sukeshpabolu avatar Sep 18 '24 08:09 sukeshpabolu

thanks for giving this an effort! You should definitely file a PR, you are the hero we need ...but i m pretty sure there will be friction, because this would make reading queries harder with hashed table names, so probably some flag or just take few first letters or sth would be way to go, but creating PR could start a debate

What about to only enable it if the table/field length exceeds a certain limit?

jonatandorozco avatar Sep 19 '24 04:09 jonatandorozco

thanks for giving this an effort! You should definitely file a PR, you are the hero we need ...but i m pretty sure there will be friction, because this would make reading queries harder with hashed table names, so probably some flag or just take few first letters or sth would be way to go, but creating PR could start a debate

What about to only enable it if the table/field length exceeds a certain limit?

yes we can do that too. I created a PR

sukeshpabolu avatar Sep 20 '24 09:09 sukeshpabolu