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

Enum Array doesn't work with nativeEnumName #5322

Closed
5 tasks done
NeelDigonto opened this issue Mar 9, 2024 · 6 comments
Closed
5 tasks done

Enum Array doesn't work with nativeEnumName #5322

NeelDigonto opened this issue Mar 9, 2024 · 6 comments

Comments

@NeelDigonto
Copy link

NeelDigonto commented Mar 9, 2024

Describe the bug

Enum Array doesn't work with nativeEnumName

Reproduction

  @Enum({
    items: () => TenantFeatureFlag,
    array: true,
  })
  tenantFeatureFlags: TenantFeatureFlag[];

This creates

"tenantFeatureFlags" text[] not null
  @Enum({
    items: () => TenantFeatureFlag,
    array: true,
    nativeEnumName: 'TenantFeatureFlag',
  })
  tenantFeatureFlags: TenantFeatureFlag[];

But this produces

"tenantFeatureFlags" "TenantFeatureFlag" not null

It's missing the [] after TenantFeatureFlag.

The update query then fails with

DriverException ... '{feat1, feat2}' ... - invalid input value for enum "TenantFeatureFlag": "{feat1,feat2}"

Using Reflect Metadata, and swc with the recommended settings.

{
  "jsc": {
    "parser": {
      "syntax": "typescript",
      "tsx": false,
      "decorators": true,
      "dynamicImport": true
    },
    "transform": {
      "legacyDecorator": true,
      "decoratorMetadata": true
    },
    "target": "esnext",
    "baseUrl": ".",
    "paths": {
      "@/*": ["./src/*"]
    }
  },
  "module": {
    "type": "commonjs"
  },
  "minify": false,
  "sourceMaps": true
}

I can provide more reproduction, if the bug is not obvious.

What driver are you using?

@mikro-orm/postgresql

MikroORM version

6.1.6

Node.js version

20.11.1 LTS

Operating system

Ubuntu 23.04

Validations

@B4nan
Copy link
Member

B4nan commented Mar 10, 2024

This might be a bit tricky, it feels like knex does not support native enum arrays at all, haven't found a single issue on their end, and I don't see a way to even set up such a column (only via table. specificType(), which is kinda the same as using raw queries...).

This does not mean there is no way around it, but it complicates things - we will need to implement some things that knex is doing (but better, lol), so we don't use table.enum() at all for native enum arrays.

Edit: turns out its pretty simple change, will finalize it later today

@B4nan B4nan closed this as completed in c2e362b Mar 10, 2024
@NeelDigonto
Copy link
Author

NeelDigonto commented Mar 10, 2024

@B4nan Thank you so much for the quick updates.
I will test them tomorrow.

@NeelDigonto
Copy link
Author

NeelDigonto commented Mar 12, 2024

@B4nan
I tried it now,

  1. With getCreateSchemaSQL() as in your mock, the returned SQL is correct and as expected.
  2. With getUpdateSchemaSQL() the enum creation SQL is itself not present, but it is correctly references for ex. as TenantFeatureFlag[].
  3. With migration create from Migrator, again enum creation is itself not present in the SQL but referenced correctly.

@B4nan B4nan reopened this Mar 12, 2024
@B4nan B4nan closed this as completed in 7c8be79 Mar 12, 2024
@B4nan
Copy link
Member

B4nan commented Mar 12, 2024

I knew it will be more tricky :D

@NeelDigonto
Copy link
Author

NeelDigonto commented Mar 12, 2024

@B4nan Will test again once it's released.
Again, thank you so much for you time. I never expected any project maintainer to be so active.
You are one heck of a maintainer.

Again, thank you so much for your efforts.

@B4nan
Copy link
Member

B4nan commented Mar 13, 2024

Btw you can test the dev version already, every commit to master gets published.

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

No branches or pull requests

2 participants