-
-
Notifications
You must be signed in to change notification settings - Fork 6.4k
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
allow lock modes for cockroachdb #8250
Conversation
2d79b7a
to
bb87630
Compare
:/ at this point I have no clue why the test are failing - there should be based on the code not be a change in postgre itself and cockroach compains about a part that should also not be affected. |
I've kicked off the tests again. I think the non-cockroach failures were just flakey tests that we should eventually remove. The cockroach failures seemed to point at some sort of an issue. |
@imnotjames the previous test fails with a non recoverable error in the connection - and then cannot recover before the next test |
can you check if version running in CI match the version where this feature support was added? |
@pleerock I'm using CockroachDB (ver. 21.1.10) locally - and in circleCI cockroachdb/cockroach:latest-v21.1 is beeing run (21.1.10) only difference is that locally I am using node 16 and ci runs with node 12 |
I've restarted CockroachDB tests again, and if they fail... You need to figure out why, because I don't have ideas. Maybe you can try to remove your code and do pushes one by one until it fail? |
I'll try that - hopefully it will yield some result^^ |
Still failing. |
1b716b4
to
5332a3b
Compare
pushed a new change - seems like that at one place when using the exception the transaction is not closed. it("should throw error when specifying a table that is not part of the query", () => Promise.all(connections.map(async connection => {
if (!(connection.driver instanceof PostgresDriver || connection.driver instanceof CockroachDriver))
return;
return connection.manager.transaction(entityManager => {
return Promise.all([
entityManager.createQueryBuilder(Post, "post")
.innerJoin("post.author", "user")
.setLock("pessimistic_write", undefined, ["img"])
.getOne()
// having the should.be here will prevent the transaction from closing.
//.should.be.rejectedWith('relation "img" in FOR UPDATE clause not found in FROM clause'),
]);
// moving the should.be here will allow the transaction to be closed correctly.
}).should.be.rejectedWith('relation "img" in FOR UPDATE clause not found in FROM clause');
}))); :/ not sure why this behaviour happens in the first place as other exceptions do not break like this. I'm curious if this will now run through in circle ci |
Woho :) |
Thank you! |
fixes #8249
Description of change
allow the same lock modes as postgres - https://www.cockroachlabs.com/docs/stable/select-for-update.html
currently an exception is thrown when a lock is used in a transaction
Adds Support for:
Pull-Request Checklist
master
branchnpm run lint
passes with this changenpm run test
passes with this changeFixes #0000