v4.2.0 may have broken sqlite support

[maphe]

Describe the bug
I attempted upgrading our project from 4.1.1 to 4.2 and our test suite running on sqlite is all broken. It looks like the schema generator omits some columns when creating the db. Omitted columns are all tagged @ManyToOne, but not all @ManyToOne columns get omitted. 🙃

Is there any breaking changes I should have applied in the configuration?

Stack trace

SQLITE_ERROR: table <some_table> has no column named <ref_table>_id

To Reproduce
I tried my best isolating the issue to provide easy reproduction steps but I couldn't manage to get anything. My little tests with small dummy entities worked fine. Our production DB has 45 tables and references all across, the schema generator seems to be missing about 5 to 10 columns spread in different tables.

There is also no obvious difference between a @ManyToOne column that gets created, and another that doesn't. I just can't put my finger on what's going on, all I can tell is that toggling between 4.1 and 4.2 makes it or breaks it.

Additional context
Let me know if there'a any relevant log or data I could provide without flooding you with a pile of entities and configuration.

[B4nan]

Well, without a reproduction I dont really have a way to help. My tests are working just fine and you are the only one with such issue so far.

So please provide something I can try, otherwise will close it.

Maybe you could share at least the problematic entity definition and ORM bootstrap script/config.

Also verify the latest dev version (4.2.4-dev.81), might be something already fixed.

My guess is that it is caused by order of entities in which they get discovered, but without a repro... :]

[B4nan]

Any updates? This should be extremely easy to replicate, as it is just about entity definition.

[maphe]

I'll try the last dev version tomorrow.

After that I'll try to reduce the scope to isolate the issue but as I mention we have a lot of entities and a lot of relationships that make it hard to trim down. Worst case I might have to grant you a temporary read access to the repo. I'll update you tomorrow or Tuesday.

[maphe]

@B4nan I ended up creating a separate repo where it's easy to replicate, and added you as a collaborator. I added instructions in the readme but it's pretty straightforward. Let me know if you have any questions.

I can also do some slack or discord if ever you need some more direct messaging.

Thanks

[B4nan]

I can't even install the native dependencies. Please turn that into a reproduction, I am not interested in your app :] It is issue with schema generation, so include only entity definitions and script that prints the dump, literally no 3rd party modules are needed for this. And don't forget to remove all entities that are working fine.

[maphe]

All right this is as small as I can make it: https://gist.github.com/maphe/33e6bc2a59aedbc4afa79b8cf75498ae

Also worth noting this one fails with both 4.1 and 4.2 so it might not be specific to 4.2.

[B4nan]

Perfect, thanks, will look into this probably later today!

[B4nan]

Please test it via 4.2.4-dev.96

[maphe]

I tried with the following in package.json:

    "@mikro-orm/core": "4.2.4-dev.97",
    "@mikro-orm/mysql": "4.2.4-dev.97",
    "@mikro-orm/nestjs": "4.2.0",
    "@mikro-orm/sqlite": "4.2.4-dev.97",
    "@mikro-orm/knex": "4.2.4-dev.97",

Still seeing the same issue:

insert into brand_site_restrictions (site_id) values (1) - SQLITE_ERROR: table brand_site_restrictions has no column named site_id

[B4nan]

Your test case is included in the CI now and is passing, so if this does not work for you, adjust the reproduction. Also double check the changes are really pulled in your project, maybe you have some dependency resolution issue (like multiple versions or the orm installed), so try wiping node_modules too.

[maphe]

I finally have it working but I had to force the following:

  "resolutions": {
    "@mikro-orm/knex": "4.2.4-dev.97"
  },

I think there's an issue somewhere in your dependencies where ^4.2.4-dev.97 actually loads 4.2.4-dev.499 which is older. I don't know if it's specific to yarn or if npm would do the same.

Anyway I think that should clean up when you release a final version.

[merceyz]

I think there's an issue somewhere in your dependencies where ^4.2.4-dev.97 actually loads 4.2.4-dev.499 which is older. I don't know if it's specific to yarn or if npm would do the same.

That's just how semver works

[maphe]

Fair enough. Might be a good idea to clean up the .499 from the list then.

[B4nan]

That is unfortunately no longer possible (and when I had this issue before, unpublishing actually did not help anyway). Just NPM things... 🤷

➜  mikro-orm git:(master) ✗ npm unpublish @mikro-orm/core@4.2.4-dev.499
npm ERR! code E405
npm ERR! 405 Method Not Allowed - PUT https://registry.npmjs.org/@mikro-orm%2fcore/-rev/46-0ce22c767df87b6da96c0626ba6b2e76 - You can no longer unpublish this package.
npm ERR! Failed criteria:
npm ERR! has dependent packages in the registry, has too many downloads
npm ERR!
npm ERR! Please deprecate it instead:
npm ERR! npm deprecate -f '@mikro-orm/core@4.2.4-dev.499' "this package has been deprecated"
npm ERR! To learn more about our unpublish policies, see https://www.npmjs.com/policies/unpublish

npm ERR! A complete log of this run can be found in:
npm ERR!     /Users/adamek/.npm/_logs/2020-11-05T07_19_36_870Z-debug.log

(I did mark it as deprecated, guess what, it makes no difference at least that warns when installing now)

It was caused by lerna --canary that was quite unpredictable for me (especially in the CI) and the lerna docs are in general not very verbose when it comes to advanced stuff :/ But this will go away when 4.3 is out, so within few days.

[maphe]

All right, thanks for the details, and thanks for the fix 👍 I can wait until last stable version.