-
Notifications
You must be signed in to change notification settings - Fork 7
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
Add example project using TypeORM #187
base: main
Are you sure you want to change the base?
Conversation
The code has been scaffolded like this: npx typeorm init --name . --database postgres --express This commit adds the result unmodified.
"scripts": { | ||
"start": "ts-node src/index.ts", | ||
"typeorm": "typeorm-ts-node-commonjs" | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Problem
The web server does not start, otherwise Express server has started on port 3000
would have been displayed on stdout. It feels like the program just freezes before being able to conclude any significant work.
sink:javascript-typeorm amo$ npm start
> [email protected] start
> ts-node src/index.ts
query: SELECT * FROM current_schema()
query: SELECT version();
Do you have any other idea what could go wrong here? In the CrateDB logs, there is zero indication about any error, effectively no message at all.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thoughts
CrateDB will respond with a version string like that:
CrateDB 5.2.2 (built a33862e/2023-02-09T10:26:09Z, Linux 5.10.124-linuxkit amd64, OpenJDK 64-Bit Server VM 19.0.2+7)
Not all applications/dialects/drivers like that, because they expect it to be compatible with PostgreSQL, and run feature discovery procedures on it. Does this trip TypeORM as well?
In Python lands, in order to make psycopg and asyncpg compatible with the CrateDB SQLAlchemy dialect, this was one of the details needed to be made compatible.
-- https://github.com/daq-tools/sqlalchemy-postgresql-relaxed/blob/main/sqlalchemy_postgresql_relaxed/base.py#L7-L22
-- crate/crate-python#532
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With crate-workbench/typeorm@fcd3841918, the example program advances to a type error, which is natural.
driverError: error: Cannot find data type: serial
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With ce929ef, which adjusts the @PrimaryGeneratedColumn
to use the UUID strategy, this SQL DDL is generated.
CREATE TABLE "user" (
"id" uuid NOT NULL DEFAULT uuid_generate_v4(),
"firstName" character varying NOT NULL,
"lastName" character varying NOT NULL,
"age" integer NOT NULL,
CONSTRAINT "PK_cace4a159ff9f2512dd42373760"
PRIMARY KEY ("id")
)
PostgresConnectionOptions says:
/**
* The Postgres extension to use to generate UUID columns. Defaults to uuid-ossp.
* If pgcrypto is selected, TypeORM will use the gen_random_uuid() function from this extension.
* If uuid-ossp is selected, TypeORM will use the uuid_generate_v4() function from this extension.
*/
readonly uuidExtension?: "pgcrypto" | "uuid-ossp"
- Reference: Add a uuid type crate#11032
- CrateDB only has gen_random_text_uuid()
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To improve the situation, crate-workbench/typeorm#1 uses a dialect for CrateDB to provide CrateDB's gen_random_text_uuid() function as an uuidGenerator
to TypeORM.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Within the SQL DDL statement above, the careful reader already spotted that the NOT NULL
constraint precedes the DEFAULT
clause, and wondered if that would work with CrateDB.
It turns out, it does not.
Now that the dialect does the right thing on the DEFAULT
definition, ...
CREATE TABLE "user" ("id" uuid NOT NULL DEFAULT gen_random_text_uuid() ...
... it would be so sweet if CrateDB could be supportive on that end.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is a patch for CrateDB already. Thanks, @seut.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In case the output of version() were still an issue, please note that since 5.1.1 it is possible to do the following:
cr> set search_path='public,pg_catalog';
SET OK, 0 rows affected (0.006 sec)
cr> show search_path;
+--------------------+
| setting |
+--------------------+
| public, pg_catalog |
+--------------------+
SHOW 1 row in set (0.007 sec)
cr> create function public.version() RETURNS TEXT
LANGUAGE JAVASCRIPT
AS $$function version() {return 'test';}$$;
CREATE OK, 1 row affected (0.057 sec)
cr> select version();
+--------+
| 'test' |
+--------+
| test |
+--------+
SELECT 1 row in set (0.912 sec)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you. It will also be interesting for https://github.com/daq-tools/sqlalchemy-postgresql-relaxed.
@hlcianfagna: Thank you very much for supporting this patch by adding further refinements. |
* Check PK data type to support uuid * Reflect that id is now an uuid and not a number
About
Verify TypeORM works well with CrateDB.
Details
The code has been scaffolded like this:
npx typeorm init --name . --database postgres --express
References
This example uses that other dialect patch needed to make TypeORM compatible with CrateDB.
/cc @proddata, @hammerhead, @hlcianfagna