sql: TypeORM support for Node.js Apps

[ThomasLohmann]

Hi there,

does anyone have an idea how to get TypeORM running with CockroachDB? I tried it out, but it doesn’t work. It would be really great if we find a solution, because Sequelize is bad compared to TypeORM. Furthermore, TypeScript is now state of the art on the backend.

TypeORM Link: http://typeorm.io/#/

Here is what I have tried so far and my error:

My TypeORM config:

I followed just the instructions by CockroachDB from here:

https://www.cockroachlabs.com/docs/stable/build-a-nodejs-app-with-cockroachdb.html

And my folder structure is nothing more than the TypeORM starter project:

npm instruction: typeorm init --name TypeORM --database postgres --express

If I just switch the config to my postgres database everything works.

[knz]

Thanks Thomas!

[knz]

@ThomasLohmann I'm fixing the proximate problem (the one in the error message you observe) in #22314. Once that is merged, we'd welcome the results of any further testing -- in case of further errors please post them here in follow-up comments.

[nvanbenschoten]

I just tested this out and with #22323 in place, the TypeORM demo seems to be working! There's probably more work to be done with this, but that's a good start.

[nvanbenschoten]

.... and now we're stuck on #9851.

[ThomasLohmann]

Hi there,

any news about that issue?

Kind regards,
Thomas

[knz]

cc @awoods187 perhaps you can chime in?

[awoods187]

We are investigating pg compatibility errors like this for 2.1. More to come.

[ThomasLohmann]

@awoods187 what are your release plans for 2.1?

[knz]

@awoods187 the issue here is specifically the ability to change the data type of a column with ALTER.

[awoods187]

@ThomasLohmann we are in the process of finalizing it now. Do you have any requests that you'd like for me to be aware of?

[ThomasLohmann]

@awoods187 not for the moment. It would just be great if we find a solution for this problem as fast as possible.

[awwong1]

CockroachDB 2.0 has just been released
I think there is a lot of community interest in getting this added to TypeORM, especially now that JSON is supported.

[ThomasLohmann]

@awoods187 and @knz any news on that issue?

Kind regards,
Thomas

[knz]

Hi Thomas!
We have solved a small proximate issue introduced in the 2.0 release, fix will be available in 2.0.1 or 2.0.2 (#24475).
The next main roadblock is the ability to change the type of an existing column with ALTER. We have initiated work on this feature (#24703); we are currently targeting the 2.1 release to publish this support, although you may have access to it in earlier alpha releases.

[ThomasLohmann]

Sounds good!

[knz]

Update: @nvanbenschoten and I looked into this. The requirement to use ALTER SET TYPE was a red herring. The reason why TypeORM wants ALTER SET TYPE is that it sees a discrepancy between the schema specified in the entity and the schema reported by information_schema, so it wants to correct this discrepancy with ALTER COLUMN SET TYPE.

So adding support for ALTER SET TYPE would not be a good solution: a TypeORM app would then cause an expensive, wasteful column type change on every startup, whereas the correct solution is to make it satisfied that the current schema in-db is the one expected for the entities.

Will investigate further to see where the discrepancy crops up.

[knz]

Oh I see. Thanks for explaining.

[knz]

I filed this separate issue for that: #34831

[AlexMesser]

btw, I discovered that pg_tables in addition to tables, also stores sequences

CREATE SEQUENCE "post_key_seq"

CREATE TABLE "post" ("key" INT DEFAULT nextval('"post_key_seq"') NOT NULL)

SELECT * FROM "pg_tables" WHERE "schemaname" = 'public'

Is it a bug or it is by design ?

[knz]

This is not by design, no. I filed #34856 and fixing in #34857 and #34858. Feel free to pick either of those patches in your crdb branch.

[ThomasLohmann]

@knz Hi everyone,

TypeORM released an offical example with CockroachDB a few hours ago as you can see here:

https://github.com/typeorm/cockroachdb-example

or here is the closed issue ("pleerock" is the founder of TypeORM):

typeorm/typeorm#1040

Maybe you can add the example to your offical Docs as an alternative to Sequelize.js here:

https://www.cockroachlabs.com/docs/stable/build-a-nodejs-app-with-cockroachdb-sequelize.html

It would be really nice.

I think after more than one year, we can finally close this issue.

Kind regards,
Thomas

[knz]

@ThomasLohmann we're still iterating on QAing these changes. We'll come back to this when we have asserted the result is of sufficient maturity to be advertised. Thanks!

[knz]

Hi @AlexMesser,

upon advice from Umed we downloaded a copy of https://github.com/typeorm/cockroachdb-example

We ran the following.

1. npm i
2. npm start - this runs fine, the first time
3. Ctrl+C to stop
4. npm start again - this fails with:

Error:  { QueryFailedError: cannot drop primary key
    at new QueryFailedError (/var/home/kena/src/cockroachdb-example/node_modules/typeorm/error/QueryFailedError.js:11:28)
    at Query.callback (/var/home/kena/src/cockroachdb-example/node_modules/typeorm/driver/cockroachdb/CockroachQueryRunner.js:174:38)
    at Query.handleError (/var/home/kena/src/cockroachdb-example/node_modules/pg/lib/query.js:142:17)
    at Connection.connectedErrorMessageHandler (/var/home/kena/src/cockroachdb-example/node_modules/pg/lib/client.js:160:17)
    at emitOne (events.js:116:13)
    at Connection.emit (events.js:211:7)
    at Socket.<anonymous> (/var/home/kena/src/cockroachdb-example/node_modules/pg/lib/connection.js:125:12)
    at emitOne (events.js:116:13)
    at Socket.emit (events.js:211:7)
    at addChunk (_stream_readable.js:263:12)
  message: 'cannot drop primary key',
  name: 'QueryFailedError',
  length: 44,
  severity: 'ERROR',
  code: 'XX000',
  detail: undefined,
  hint: undefined,
  position: undefined,
  internalPosition: undefined,
  internalQuery: undefined,
  where: undefined,
  schema: undefined,
  table: undefined,
  column: undefined,
  dataType: undefined,
  constraint: undefined,
  file: undefined,
  line: undefined,
  routine: undefined,
  query: 'ALTER TABLE "category" DROP CONSTRAINT "PK_9c4e4a89e3674fc9f382d733f03"',
  parameters: [] }

Can you clarify further what's happening here?

I have used the latest version in our main repository master, build v19.1.0-beta.20190225-213-g71681f6003.

Thank you

[pleerock]

@knz can you please run against 2.1.5? Since its the latest stable version we were developing against. It feels like master has some breaking changes causing you this error.

[knz]

Good idea, that indeed yields a difference in behavior.

@bobvawter could you get the SQL exec traces on both crdb versions and point out where they diverge?

[pleerock]

Hey @knz, did you have a chance to make a complete test of TypeORM with a stable 2.1.5 version?

[knz]

Can you explain what "a complete test" is? At this point we're just looking at the example provided above: https://github.com/typeorm/cockroachdb-example

Do you have a more exhaustive test you'd like us to look at instead?

[pleerock]

okay, let me rephrase it.

We have landed CockroachDB support in TypeORM. Everything works except something we have postponed for the future (email discussion). Example I provided is just a start point in the case if you want to start checking results. What's left to do from our side?

[knz]

@pleerock you have pointed out a difference in behavior between CockroachDB 2.1 and 19.1. I confirm we also see a difference, and we are still determining where it comes from.

There are really two situations possible:

1. either TypeORM relies on a stabilized API, and we have incorrectly introduced an error in the API at some point in the 19.1 development. In this case, we need to fix CockroachDB.
2. or, TypeORM relies on an internal API or, perhaps, relies on a bug in CockroachDB (for example, a wrong implementation of pg_catalog tables) which got subsequently fixed. In this case, we'll request you to change TypeORM to use the stable API or to not depend on CockroachDB bugs.

Unfortunately as of today I do not know which of the two points applies. Give us a little more time to investigate further.

[maddyblue]

I've confirmed that 2.1.5 works correctly with the most recent release of typeorm. The current beta of cockroach fails due to the addition of the domain_catalog, domain_schema, and domain_name columns in the information_schema.columns table, which we added since the 2.1 release.

[maddyblue]

I take it back. While that is a difference, it isn't the cause of this problem. So far in my testing it's non-deterministic, and sometimes errors with the above error.

[maddyblue]

Some progress: I think this is caused by our handling of the int type. The above error is caused because on the second run, typeorm sees a column type of columns.id as int8 but wants int. Since the type is wrong it tries to drop and add the column with the correct type, but we don't support changing the PK. In cockroach int is int8, but we are going to make int be int4 in the release after the upcoming release (see #32848). Still unsure why this was working with cockroach 2.1.

It's possible that the best solution for this is for typeorm to run set default_int_size = 4 at the beginning of each session.

[bobvawter]

In 2.1, int is a wholly separate type from int4 or int8. If you ask for int, you'll see int in the reflection and it happens to be an 8-byte value.

In 19.1, we completely removed the "naked" int type by treating it, at parse time, as an alias either for int8 or int4. This is how we handle the float type, it's always resolved as float8. We chose the aliasing approach so that it's unambiguous as to what size size int you're using in cluster that have clients using different sizes. Otherwise, the introspection would have to switch the presentation based on the user's original intent and the client's default_int_size setting. This would have created a combinatorial mess.

I guess the question is whether or not it's correct for a client to want to change a column from intX to int, given that the int size has an implementation-dependent size, IIRC. If a client sets default_int_size=4, then it would still see int4 for the column type. Is TypeORM looking for int specifically, or is it switching on the declared vs. expected size? What does it do for the 'float' type since we always resolve that to float8?

[maddyblue]

Ok now I understand fully. TypeORM issues a create table with type int. In cockroach 2.1 the int acted like an int8 but would display itself as the type int, which appeased typeorm. In 19.1 we always change int to something. Since the default is int8, typeorm complains upon second invocation. Some possible solutions:

1. Execute set default_int_size = 4 at the start of each session started by typeorm.
2. Change typeorm to only return int8 or int4 and never int during its type stuff (see https://github.com/typeorm/typeorm/blob/a12e72d417638ea8fba2ca672d5acfaaaaf691c9/src/driver/cockroachdb/CockroachDriver.ts#L421 for example).

Cockroach is going to fix this, but not until the release in later half of this year, because we decided to allow people 1 release to deal with this if they wanted (see the PR I linked above for more).

[pleerock]

We have applied suggested fix in the latest typeorm version (0.2.15) and now everything seemed to work and tests are passing under latest master version.

You can continue testing TypeORM's CockroachDB support with the latest cockroachdb's master now.

We have one issue however. For some reason CI randomly fails on a random test with following error:

restart transaction: TransactionRetryWithProtoRefreshError: TransactionRetryError: retry txn (RETRY_SERIALIZABLE): "sql txn" id=bc254db7 key=/Table/SystemConfigSpan/Start rw=true pri=0.08168281 stat=PENDING epo=0 ts=1552496631.167807787,1 orig=1552496631.114934486,0 max=1552496631.114934486,0 wto=false seq=80`

Sometimes tests successfully pass, sometimes they throw this error. Fail example on CI. Any hint on why we have this error is appreciated. We are using the latest docker image from CRDB-unstable hub.

Also side note. We have found your latest docker development image doesn't match the latest master and some changes are missing in there:

(not sure how often master is released for docker, but what I imagine they should go in parallel)

[awoods187]

Hi all--you will need to setup a transaction retry loop as covered by these docs https://www.cockroachlabs.com/docs/v2.1/transactions.html#transaction-retries. Let us know if you have questions.

[pleerock]

We applied transaction retries to the latest version (I released changes in typeorm@0.2.16-rc.1). All tests are green now.

[awoods187]

Amazing news! We will run a final set of internal tests tomorrow--thanks for all your hard work!

I also filed #35876 to make sure we don't accidentally change anything that would impact TypeORM.

[awoods187]

Everything looks good on our end--we will try it out with some customers. I'm going to close this issue and will file new ones if they run into any problems!