-
-
Notifications
You must be signed in to change notification settings - Fork 6.2k
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
fix: synchronize with typeorm_metadata table only if needed #9175
fix: synchronize with typeorm_metadata table only if needed #9175
Conversation
Creating a view in a migration results in an error: `QueryFailedError: relation "typeorm_metadata" does not exist` (see typeorm#9173). This fix creates the table before a migration runs. Closes: typeorm#9173
…missing-typeorm-metadata
HI, getting same error here. Will this PR be merged soon? |
@nacholupotti the PR is ready for merge, but it's waiting for a reviewer. @AlexMesser can you review it, please? |
We've the same issue :( |
@christian-forgacs the fastest solution is to clone the branch Unfortunately I don't know when and whether this PR is going to be merged or not. |
…missing-typeorm-metadata
@AlexMesser I applied your suggestions from your review. Now my test doesn't succeeds any more, even if it just calls await connection.createQueryRunner().createView(
new View({
name: "test_view",
expression: "SELECT * FROM test_table",
}),
) So this issue isn't one that just happens in migrations, but rather every time So I think there are two options:
I tried to implement option 1, but because this function isn't async, I can't create the meta data table using the schema builder. Option 3 seems the "easiest" one but requires a change to each EDIT: Added Option 3. |
The way Looks like you don't have such classes (decorated by
typeorm_metadata is needed only if you have classes decorated by I suggest to add additional boolean param for such cases: async createView(view: View, syncWithMetadata: boolean = false): Promise<void> {
const upQueries: Query[] = []
const downQueries: Query[] = []
upQueries.push(this.createViewSql(view))
if (syncWithMetadata)
upQueries.push(await this.insertViewDefinitionSql(view))
downQueries.push(this.dropViewSql(view))
if (syncWithMetadata)
downQueries.push(await this.deleteViewDefinitionSql(view))
await this.executeQueries(upQueries, downQueries)
} We can make it |
Yes.
I understand. So, a workaround would be to create a separate View class even if I don't use it.
That is a really short and easy solution, very nice! But wouldn't we have to change all createView() functions for each QueryRunner and change all code occurrences that needs the old behaviour? If we set the default to true, the old behaviour would be the default (no code changes needed) and only in special cases like my one the param is set to false... await connection.createQueryRunner().createView(
new View({
name: "test_view",
expression: "SELECT * FROM test_table",
}),
false
) in order to prevent the synchronization with metadata_table. If you think this is the best solution, I could implement and test it. Please let me know how you decide. |
We only need to supply Cases where users were using code call of |
@pleerock I implemented it as you described. I ran the test and I tested it in my project, it works fine. Thank you! 😄 |
tests seems to fail |
@pleerock most of the tests run successfully but not the cockroachdb one. I can't find the reason, the test fails with my build and with that one from the origin typeorm master repository. Every time it's another error, I don't know what's going on here. 1) "before each" hook for "should load data when additional condition used"
582 passing (11m)
14 pending
1 failing
1) query builder > joins
"before each" hook for "should load data when additional condition used":
Error: Timeout of 120000ms exceeded. For async tests and hooks, ensure "done()" is called; if returning a Promise, ensure it resolves. (/Users/ips/repos/typeorm-orig/typeorm/build/compiled/test/functional/query-builder/join/query-builder-joins.js)
at listOnTimeout (node:internal/timers:559:17)
at processTimers (node:internal/timers:502:7) 1) "before each" hook for "should not override already set properties"
569 passing (16m)
14 pending
1 failing
1) query builder > entity updation
"before each" hook for "should not override already set properties":
QueryFailedError: internal error: deadline below read timestamp is nonsensical; txn has would have no chance to commit. Deadline: 1663672502.269035525,0. Read timestamp: 1663672555.764900000,1 Previous Deadline: 1663672502.269035525,0.
at CockroachQueryRunner.query (src/driver/cockroachdb/CockroachQueryRunner.ts:302:19)
at runMicrotasks (<anonymous>)
at processTicksAndRejections (node:internal/process/task_queues:96:5)
at CockroachQueryRunner.commitTransaction (src/driver/cockroachdb/CockroachQueryRunner.ts:190:25)
at CockroachQueryRunner.clearDatabase (src/driver/cockroachdb/CockroachQueryRunner.ts:2479:46)
at DataSource.dropDatabase (src/data-source/DataSource.ts:356:17)
at DataSource.synchronize (src/data-source/DataSource.ts:315:29)
at async Promise.all (index 3) |
@julianpoemp nevermind cockroachdb is dumb and fails sometimes. This one is a good to merge. Thank you for contribution! |
Description of change
Creating a view by calling
createQueryRunner().createView(...)
results in an error:QueryFailedError: relation "typeorm_metadata" does not exist
(see #9173). This pull request fixes #9173 by creating the metadata table if it doesn't exist.Fixes: #9173
Pull-Request Checklist
master
branchnpm run format
to apply prettier formattingnpm run test
passes with this changeFixes #0000