Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUG] If the index name is the same as the generated name, it will be empty and a type error will occur #3420

Closed
Karibash opened this issue May 13, 2024 · 6 comments · Fixed by #2733
Labels
bug Something isn't working db/mysql drizzle/kit priority Will be worked on next

Comments

@Karibash
Copy link
Contributor

Karibash commented May 13, 2024

What version of drizzle-orm are you using?

0.30.10

What version of drizzle-kit are you using?

0.21.0

What type of database are you using?

MySQL

Describe the Bug

When generating a schema from an existing DB using the introspect command, the index name will be empty if it is identical to the generated name.

createTableIndexes2 = (tableName, idxs, casing2) => {
  let statement = "";
  idxs.forEach((it) => {
    let idxKey = it.name.startsWith(tableName) && it.name !== tableName ? it.name.slice(tableName.length + 1) : it.name;
    idxKey = idxKey.endsWith("_index") ? idxKey.slice(0, -"_index".length) + "_idx" : idxKey;
    idxKey = casing2(idxKey);
    const indexGeneratedName = indexName(tableName, it.columns);
    const escapedIndexName = indexGeneratedName === it.name ? "" : `"${it.name}"`;
    statement += `		${idxKey}: `;
    statement += it.isUnique ? "uniqueIndex(" : "index(";
    statement += `${escapedIndexName})`;
    statement += `.on(${it.columns.map((it2) => `table.${casing2(it2)}`).join(", ")}),`;
    statement += `
    `;
  });
  return statement;
};

However, since the index and uniqueIndex functions must take a string as an argument, a type error occurs.

export function index(name: string): IndexBuilderOn {
return new IndexBuilderOn(name, false);
}
export function uniqueIndex(name: string): IndexBuilderOn {
return new IndexBuilderOn(name, true);
}

Expected behavior

To avoid type errors, you must always pass the index name.

Patch

The following patch can be applied as a workaround until this issue is resolved.

diff --git a/bin.cjs b/bin.cjs
index fef2511d2e2d69c877fc925c281d88aa828f150f..b2bf7b5982518fda91b1ce21efd87e4eb41c58e3 100755
--- a/bin.cjs
+++ b/bin.cjs
@@ -108063,11 +108063,9 @@ import { sql } from "drizzle-orm"
         let idxKey = it.name.startsWith(tableName) && it.name !== tableName ? it.name.slice(tableName.length + 1) : it.name;
         idxKey = idxKey.endsWith("_index") ? idxKey.slice(0, -"_index".length) + "_idx" : idxKey;
         idxKey = casing2(idxKey);
-        const indexGeneratedName = indexName(tableName, it.columns);
-        const escapedIndexName = indexGeneratedName === it.name ? "" : `"${it.name}"`;
         statement += `		${idxKey}: `;
         statement += it.isUnique ? "uniqueIndex(" : "index(";
-        statement += `${escapedIndexName})`;
+        statement += `"${it.name}")`;
         statement += `.on(${it.columns.map((it2) => `table.${casing2(it2)}`).join(", ")}),`;
         statement += `
 `;
@L-Mario564 L-Mario564 transferred this issue from drizzle-team/drizzle-kit-mirror Nov 4, 2024
@L-Mario564 L-Mario564 added bug Something isn't working drizzle/kit db/mysql priority Will be worked on next labels Nov 4, 2024
@AndriiSherman
Copy link
Member

should not be an issue on latest(0.30.0+), if it's still an issue - please reopen

@Karibash
Copy link
Contributor Author

@AndriiSherman
I checked the latest source code and it is not fixed.
If the set index name is the same as the auto-generated name, the index function is set with no arguments.
However, mysql-core's index function requires an argument, so a type error occurs.
reference:

const escapedIndexName = indexGeneratedName === it.name ? '' : `"${it.name}"`;

export function index(name: string): IndexBuilderOn {

@Karibash
Copy link
Contributor Author

Please check the following PR: #2733

@AndriiSherman AndriiSherman reopened this Dec 16, 2024
@AndriiSherman
Copy link
Member

thanks! I was wrong about this issue, I'll review this PR and merge!

@AndriiSherman
Copy link
Member

Available in [email protected]

Release Notes

@Karibash
Copy link
Contributor Author

Image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working db/mysql drizzle/kit priority Will be worked on next
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants