drizzle-team / drizzle-orm

Headless TypeScript ORM with a head. Runs on Node, Bun and Deno. Lives on the Edge and yes, it's a JavaScript ORM too 😅
https://orm.drizzle.team
Apache License 2.0
24.79k stars 658 forks source link

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

Open EamonHeffernan opened 8 months ago

EamonHeffernan commented 8 months ago

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

jonatandorozco commented 8 months ago

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

gmanavarro commented 7 months ago

Just ran into the exact same issue

driwand commented 7 months ago

@EamonHeffernan did you figured out anything for this?

EamonHeffernan commented 7 months ago

@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.

nick-kang commented 6 months ago

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.

jonatandorozco commented 6 months ago

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

EamonHeffernan commented 6 months ago

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.

jonatandorozco commented 6 months ago

@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

noobmaster19 commented 5 months ago

Anyone found a workaround for this issue?

stabildev commented 4 months ago

Same issue here, any fixes?

nagy135 commented 3 months ago

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.

LocatedInSpace commented 3 months ago

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?

stabildev commented 2 months ago

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

sukeshpabolu commented 2 months ago

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 commented 2 months ago

typeorm fixed this here

sukeshpabolu commented 2 months ago

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`, `
       )})`;
nagy135 commented 2 months ago

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

sukeshpabolu commented 2 months ago

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

jonatandorozco commented 2 months ago

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?

sukeshpabolu commented 2 months ago

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

grundmanise commented 4 weeks ago

Hi, is there any progress on merging #3041 to address this issue?

artem-simutin commented 2 weeks ago

Facing this issue... It should be fixed ASAP