koxudaxi / local-data-api

Data API for local, you can write unittest for AWS Aurora Serverless's Data API
https://koxudaxi.github.io/local-data-api
MIT License
112 stars 21 forks source link

Error creating foreign keys in version 0.6.0! #106

Open carlocorradini opened 3 years ago

carlocorradini commented 3 years ago

Describe the bug Since yesterday I was using your awesome project without any issue. Today I've upgraded the Docker image to version 0.6.0 and the entire project is not working any more due to UnhandledPromiseRejectionWarning: BadRequestException: Database error code: 0. Message: ERROR: syntax error at or near "RETURNING" when using relations in the database. The error message is received only when trying to use relations. I'm using TypeORM to synchronize my local schema in development.

To Reproduce I've created a super simple repo with a basic example.

LINK: https://github.com/carlocorradini/local-data-api-issue

All instructions are explained in the README.

Expected behavior Working 😅

Screenshots image

Desktop (please complete the following information):

Additional context I'm using the typeorm-aurora-data-api-driver driver.

The database is PostgreSQL.

The docker-compose.yml is the same as the one shown in the README.

Hope this is a simple fix😥

koxudaxi commented 3 years ago

@carlocorradini Thank you for giving detail of the problem 🚀 I have fixed the problem and released it as 0.6.1 The version adds the RETRUN statement on the only INSERT, DELETE, and UPDATE. I guess the three query types need generated key in responses. Otherwise, the generated key is not needed.

If my thought is not correct then please teach me the other cases 😅

carlocorradini commented 3 years ago

@koxudaxi Thanks!🤩🤩🤩🤗🤗🤗

carlocorradini commented 3 years ago

@koxudaxi

I've found another error due to the usage of uuid.

The old version (0.5.8) had no issue, but the new one throw an error message.

error_2

Error message: Database error code: 0. Message: ERROR: operator does not exist: uuid = character varying. Hint: No operator matches the given name and argument type(s). You might need to add explicit type casts

I've updated the basic example with a new testing procedure (CREATE, UPDATE, SELECT, DELETE): https://github.com/carlocorradini/local-data-api-issue

koxudaxi commented 3 years ago

@carlocorradini Thank you for reporting the problem !!

Would you please check a new version 0.6.2?

I have changed the JDBC driver options. JDBC driver will not cast string value to column type on the local-data-api. The process will be run on DB.

https://github.com/carlocorradini/local-data-api-issue

Btw, May I add your test code to the project as an integration-test?

carlocorradini commented 3 years ago

@carlocorradini Thank you for reporting the problem !!

Would you please check a new version 0.6.2?

I have changed the JDBC driver options. JDBC driver will not cast string value to column type on the local-data-api. The process will be run on DB.

https://github.com/carlocorradini/local-data-api-issue

Btw, May I add your test code to the project as an integration-test?

Of course!

I'm glad to help! 🤗🤩

carlocorradini commented 3 years ago

@carlocorradini Thank you for reporting the problem !!

Would you please check a new version 0.6.2?

I have changed the JDBC driver options. JDBC driver will not cast string value to column type on the local-data-api. The process will be run on DB.

https://github.com/carlocorradini/local-data-api-issue

Btw, May I add your test code to the project as an integration-test?

Give me 5 minutes

carlocorradini commented 3 years ago

@carlocorradini Thank you for reporting the problem !!

Would you please check a new version 0.6.2?

I have changed the JDBC driver options. JDBC driver will not cast string value to column type on the local-data-api. The process will be run on DB.

https://github.com/carlocorradini/local-data-api-issue

Btw, May I add your test code to the project as an integration-test?

@koxudaxi

I've found another error due to the save method does not return the full data but only the data sent to the database. For example if we save the user with only the name (id is autogenerated) we receive back only the name and not the id & name.

error_3

I've updated the basic example with a new testing procedure (CONNECT_DB, CREATE, UPDATE, SELECT, DELETE, CLOSE_CONNECTION_DB): https://github.com/carlocorradini/local-data-api-issue

PS: As always the old version works, but I'm supporting the transition to Kotlin!

koxudaxi commented 3 years ago

@carlocorradini I'm sorry for late reply. I was busy yesterday. I have pushed a fixed version as 0.6.3. Thank you for your help every time.🙏

PS: As always the old version works, but I'm supporting the transition to Kotlin!

Thanks!! I have copied the code getting generated fields from python implementation 😁

carlocorradini commented 3 years ago

@carlocorradini I'm sorry for late reply. I was busy yesterday. I have pushed a fixed version as 0.6.3. Thank you for your help every time.🙏

PS: As always the old version works, but I'm supporting the transition to Kotlin!

Thanks!! I have copied the code getting generated fields from python implementation 😁

Don't worry, your reply is a lot faster than the majority of the other maintainers. 😂

I'm honored to help you to improve this awesome project.

Now I'm busy, give me a couple of hours and I let you know.

carlocorradini commented 3 years ago

@koxudaxi

After hours of trial and error I've finally discovered another issue (sorry 😅😅😅) that is a little "bit dangerous" since has not been discovered even in the older versions (pre 0.6).

The value of timestamptz (timestamp with timezone) (See this) is trimmed after 3 digits (of the ms) removing the 'Z' of the timezone.

In javascript trying to parse the value without the 'Z' results in a 'string' type rather than a 'Date' type. However this causes undefined behaviour or parsing exceptions in other languages.

NB: This problem affect both working and not working versions of docker compose.

I've updated the with a new testing procedures (not only for timestamptz): https://github.com/carlocorradini/local-data-api-issue

PS: timestamp works 😎

koxudaxi commented 3 years ago

@koxudaxi

Thank you for your report!!

The value of timestamptz (timestamp with timezone) (See this) is trimmed after 3 digits (of the ms) removing the 'Z' of the timezone.

Is my implementation wrong? I thought Z delimiter is dropped on the response. 🤔 Can you check the real response from AWS DataAPI?

TIMESTAMP – The corresponding String parameter value is sent as an object of TIMESTAMP type to the database. The accepted format is YYYY-MM-DD HH:MM:SS[.FFF].

Note For Amazon Aurora PostgreSQL, the Data API always returns the Aurora PostgreSQL datatype TIMESTAMPTZ in UTC timezone.

https://docs.aws.amazon.com/AmazonRDS/latest/AuroraUserGuide/data-api.html

carlocorradini commented 3 years ago

@koxudaxi

I've found this: image

Info heere: https://docs.aws.amazon.com/redshift/latest/dg/federated-data-types.html

carlocorradini commented 3 years ago

@koxudaxi

Is strange that timestamptz is not supported since the PostgreSQL community suggests to use it instead of the simple timestamp.

Info: https://justatheory.com/2012/04/postgres-use-timestamptz

carlocorradini commented 3 years ago

Can you check the real response from AWS DataAPI?

Unofrtunately, I can't right now 😣

koxudaxi commented 3 years ago

@carlocorradini I ran your test to real AWS DataAPI. I got an error. The problme is same as https://github.com/koxudaxi/local-data-api/issues/106#issuecomment-801871661 ?

query: INSERT INTO "photo"("id", "url", "userId") VALUES (DEFAULT, :param_1, :param_2), (DEFAULT, :param_3, :param_4), (DEFAULT, :param_5, :param_6) RETURNING "id" -- PARAMETERS: [{"param_1":"https://test1.com","param_2":"1b339b6c-77ed-4ba7-bc9b-1ceed3035a7b","param_3":"https://test2.com","param_4":"1b339b6c-77ed-4ba7-bc9b-1ceed3035a7b","param_5":"https://test3.com","param_6":"1b339b6c-77ed-4ba7-bc9b-1ceed3035a7b"}]
ERROR: ERROR: column "userId" is of type uuid but expression is of type character varying
  Hint: You will need to rewrite or cast the expression.
  Position: 65
BadRequestException: ERROR: column "userId" is of type uuid but expression is of type character varying
  Hint: You will need to rewrite or cast the expression.
  Position: 65
carlocorradini commented 3 years ago

@carlocorradini I ran your test to real AWS DataAPI. I got an error. The problme is same as #106 (comment) ?

query: INSERT INTO "photo"("id", "url", "userId") VALUES (DEFAULT, :param_1, :param_2), (DEFAULT, :param_3, :param_4), (DEFAULT, :param_5, :param_6) RETURNING "id" -- PARAMETERS: [{"param_1":"https://test1.com","param_2":"1b339b6c-77ed-4ba7-bc9b-1ceed3035a7b","param_3":"https://test2.com","param_4":"1b339b6c-77ed-4ba7-bc9b-1ceed3035a7b","param_5":"https://test3.com","param_6":"1b339b6c-77ed-4ba7-bc9b-1ceed3035a7b"}]
ERROR: ERROR: column "userId" is of type uuid but expression is of type character varying
  Hint: You will need to rewrite or cast the expression.
  Position: 65
BadRequestException: ERROR: column "userId" is of type uuid but expression is of type character varying
  Hint: You will need to rewrite or cast the expression.
  Position: 65

It seems the same error but uuid is supported both in data-api and TypeORM driver (See https://github.com/ArsenyYankovsky/typeorm-aurora-data-api-driver/releases/tag/v1.4.0)

EDIT: There is an issue: https://github.com/ArsenyYankovsky/typeorm-aurora-data-api-driver/issues/57

carlocorradini commented 3 years ago

However, try using number instead of uuid.

Symply update:

FROM:

@PrimaryGeneratedColumn('uuid')

TO:

@PrimaryGeneratedColumn()

And try for the timestamptz.

Thanks for your time 🤩😎

koxudaxi commented 3 years ago

There is real result.

SELECTING DATA TIMESTAMPTZ...
query: SELECT "User"."id" AS "User_id", "User"."name" AS "User_name", "User"."created_at" AS "User_created_at", "User"."updated_at" AS "User_updated_at", "User_photos"."id" AS "User_photos_id", "User_photos"."url" AS "User_photos_url", "User_photos"."userId" AS "User_photos_userId" FROM "user" "User" LEFT JOIN "photo" "User_photos" ON "User_photos"."userId"="User"."id"
query: SELECT "user_to_classroom"."id" AS "id", "user_to_classroom"."class_id" AS "class_id" FROM "user_to_classroom" "user_to_classroom" WHERE (("user_to_classroom"."class_id" = :param_1) OR ("user_to_classroom"."class_id" = :param_2) OR ("user_to_classroom"."class_id" = :param_3) OR ("user_to_classroom"."class_id" = :param_4) OR ("user_to_classroom"."class_id" = :param_5) OR ("user_to_classroom"."class_id" = :param_6) OR ("user_to_classroom"."class_id" = :param_7) OR ("user_to_classroom"."class_id" = :param_8) OR ("user_to_classroom"."class_id" = :param_9) OR ("user_to_classroom"."class_id" = :param_10) OR ("user_to_classroom"."class_id" = :param_11) OR ("user_to_classroom"."class_id" = :param_12) OR ("user_to_classroom"."class_id" = :param_13) OR ("user_to_classroom"."class_id" = :param_14) OR ("user_to_classroom"."class_id" = :param_15) OR ("user_to_classroom"."class_id" = :param_16) OR ("user_to_classroom"."class_id" = :param_17) OR ("user_to_classroom"."class_id" = :param_18) OR ("user_to_classroom"."class_id" = :param_19)) -- PARAMETERS: [{"param_1":7,"param_2":6,"param_3":5,"param_4":8,"param_5":6,"param_6":5,"param_7":7,"param_8":8,"param_9":7,"param_10":6,"param_11":8,"param_12":9,"param_13":9,"param_14":5,"param_15":9,"param_16":2,"param_17":4,"param_18":1,"param_19":3}]
query: SELECT "User"."id" AS "User_id", "User"."name" AS "User_name", "User"."created_at" AS "User_created_at", "User"."updated_at" AS "User_updated_at", "User_photos"."id" AS "User_photos_id", "User_photos"."url" AS "User_photos_url", "User_photos"."userId" AS "User_photos_userId" FROM "user" "User" LEFT JOIN "photo" "User_photos" ON "User_photos"."userId"="User"."id"
query: SELECT "user_to_classroom"."id" AS "id", "user_to_classroom"."class_id" AS "class_id" FROM "user_to_classroom" "user_to_classroom" WHERE (("user_to_classroom"."class_id" = $1) OR ("user_to_classroom"."class_id" = $2) OR ("user_to_classroom"."class_id" = $3) OR ("user_to_classroom"."class_id" = $4) OR ("user_to_classroom"."class_id" = $5) OR ("user_to_classroom"."class_id" = $6) OR ("user_to_classroom"."class_id" = $7) OR ("user_to_classroom"."class_id" = $8)) -- PARAMETERS: [2,5,8,6,4,1,3,7]
user.createdAt is 2021-03-20 17:36:44.133053
ERROR: User created_at is not an object but a string
Error: User created_at is not an object but a string
    at /Users/koudai/repo/local-data-api-issue/src/test.ts:266:15
koxudaxi commented 3 years ago

Also, I executed a query for timestamptz with aws-cli. the value doesn't have Z and the ms is 6 digits 🤔

"stringValue": "2021-03-20 17:55:00.016735"

carlocorradini commented 3 years ago

@koxudaxi

See https://github.com/ArsenyYankovsky/typeorm-aurora-data-api-driver/issues/62

koxudaxi commented 3 years ago

@carlocorradini Thanks!! I will change the response of timestamptz to 6 digits tomorrow.

carlocorradini commented 3 years ago

Also, I executed a query for timestamptz with aws-cli. the value doesn't have Z and the ms is 6 digits 🤔

"stringValue": "2021-03-20 17:55:00.016735"

Have you have any idea why?

Probably I have too little experience (still a University student) but... How customers of AWS Aurora can deal with it if the database has the recommended date & time "format".

carlocorradini commented 3 years ago

@carlocorradini Thanks!! I will change the response of timestamptz to 6 digits tomorrow.

Again... thanks for your help and time that you are dedicating to me.

koxudaxi commented 3 years ago

@carlocorradini

Note For Amazon Aurora PostgreSQL, the Data API always returns the Aurora PostgreSQL datatype TIMESTAMPTZ in UTC timezone.

I don't know the reason why AWS implement this way. But, They said the timezone is fixed as UTC. We can parse the timestamp correctly. I guess they think the timestamp format should not depend on the database type.

Again... thanks for your help and time that you are dedicating to me.

No problem!! Thank you for your help 😄

carlocorradini commented 3 years ago

@carlocorradini

Note For Amazon Aurora PostgreSQL, the Data API always returns the Aurora PostgreSQL datatype TIMESTAMPTZ in UTC timezone.

I don't know the reason why AWS implement this way. But, They said the timezone is fixed as UTC. We can parse the timestamp correctly. I guess they think the timestamp format should not depend on the database type.

Again... thanks for your help and time that you are dedicating to me.

No problem!! Thank you for your help 😄

@koxudaxi

See https://github.com/ArsenyYankovsky/typeorm-aurora-data-api-driver/issues/57#issuecomment-803439025.

I'll monitor the progress

koxudaxi commented 3 years ago

@carlocorradini

I will change the response of timestamptz to 6 digits tomorrow.

I have pushed a new version 0.6.4

carlocorradini commented 3 years ago

@carlocorradini

I will change the response of timestamptz to 6 digits tomorrow.

I have pushed a new version 0.6.4

There is a new version for typeorm-aurora-data-api-driver. See https://github.com/ArsenyYankovsky/typeorm-aurora-data-api-driver/issues/57#issuecomment-803576828.

I have updated my repository: https://github.com/carlocorradini/local-data-api-issue

Unfortunately the timestamptz is treated as a string (again). However you can try for the support of uuid, etc...

Thanks!

PS: typeorm has not yet merged the commit so we must use a package from git.

koxudaxi commented 3 years ago

I got an error when I ran your test code with real data-api

query: SELECT * FROM "information_schema"."tables" WHERE "table_schema" = current_schema() AND "table_name" = 'typeorm_metadata'
Deprecated as of 10.7.0. highlight(lang, code, ...args) has been deprecated.
Deprecated as of 10.7.0. Please use highlight(code, options) instead.
https://github.com/highlightjs/highlight.js/issues/2277
query: ALTER TABLE "photo" DROP COLUMN "created_at"
Deprecated as of 10.7.0. highlight(lang, code, ...args) has been deprecated.
Deprecated as of 10.7.0. Please use highlight(code, options) instead.
https://github.com/highlightjs/highlight.js/issues/2277
query: ALTER TABLE "photo" DROP CONSTRAINT "FK_4494006ff358f754d07df5ccc87"
Deprecated as of 10.7.0. highlight(lang, code, ...args) has been deprecated.
Deprecated as of 10.7.0. Please use highlight(code, options) instead.
https://github.com/highlightjs/highlight.js/issues/2277
query: ALTER TABLE "photo" DROP CONSTRAINT "PK_723fa50bf70dcfd06fb5a44d4ff"
Deprecated as of 10.7.0. highlight(lang, code, ...args) has been deprecated.
Deprecated as of 10.7.0. Please use highlight(code, options) instead.
https://github.com/highlightjs/highlight.js/issues/2277
query: ALTER TABLE "photo" DROP COLUMN "id"
Deprecated as of 10.7.0. highlight(lang, code, ...args) has been deprecated.
Deprecated as of 10.7.0. Please use highlight(code, options) instead.
https://github.com/highlightjs/highlight.js/issues/2277
query: ALTER TABLE "photo" ADD "id" uuid NOT NULL DEFAULT uuid_generate_v4()
Deprecated as of 10.7.0. highlight(lang, code, ...args) has been deprecated.
Deprecated as of 10.7.0. Please use highlight(code, options) instead.
https://github.com/highlightjs/highlight.js/issues/2277
query: ALTER TABLE "photo" ADD CONSTRAINT "PK_723fa50bf70dcfd06fb5a44d4ff" PRIMARY KEY ("id")
Deprecated as of 10.7.0. highlight(lang, code, ...args) has been deprecated.
Deprecated as of 10.7.0. Please use highlight(code, options) instead.
https://github.com/highlightjs/highlight.js/issues/2277
query: ALTER TABLE "photo" DROP COLUMN "userId"
Deprecated as of 10.7.0. highlight(lang, code, ...args) has been deprecated.
Deprecated as of 10.7.0. Please use highlight(code, options) instead.
https://github.com/highlightjs/highlight.js/issues/2277
query: ALTER TABLE "photo" ADD "userId" uuid NOT NULL
ERROR: ERROR: column "userId" contains null values
BadRequestException: ERROR: column "userId" contains null values
    at Object.extractError (/Users/koudai/repo/local-data-api-issue/node_modules/aws-sdk/lib/protocol/json.js:52:27)
    at Request.extractError (/Users/koudai/repo/local-data-api-issue/node_modules/aws-sdk/lib/protocol/rest_json.js:55:8)
    at Request.callListeners (/Users/koudai/repo/local-data-api-issue/node_modules/aws-sdk/lib/sequential_executor.js:106:20)
    at Request.emit (/Users/koudai/repo/local-data-api-issue/node_modules/aws-sdk/lib/sequential_executor.js:78:10)
    at Request.emit (/Users/koudai/repo/local-data-api-issue/node_modules/aws-sdk/lib/request.js:688:14)
    at Request.transition (/Users/koudai/repo/local-data-api-issue/node_modules/aws-sdk/lib/request.js:22:10)
    at AcceptorStateMachine.runTo (/Users/koudai/repo/local-data-api-issue/node_modules/aws-sdk/lib/state_machine.js:14:12)
    at /Users/koudai/repo/local-data-api-issue/node_modules/aws-sdk/lib/state_machine.js:26:10
    at Request.<anonymous> (/Users/koudai/repo/local-data-api-issue/node_modules/aws-sdk/lib/request.js:38:9)
    at Request.<anonymous> (/Users/koudai/repo/local-data-api-issue/node_modules/aws-sdk/lib/request.js:690:12) {
  code: 'BadRequestException',
  time: 2021-03-23T16:27:25.870Z,
  requestId: '67b1878c-44be-4375-a92a-e2042d98eefb',
  statusCode: 400,
  retryable: false,
  retryDelay: 5.876152538498225
}
koxudaxi commented 3 years ago

I guess the regex match only 3 digits for ms. But, the real response 6 digits...

https://github.com/jeremydaly/data-api-client/blob/587f8f5707b0c6eb077b3420cace1735565f259a/index.js#L237-L243

!treatAsLocalDate && /^\d{4}-\d{2}-\d{2}(\s\d{2}:\d{2}:\d{2}(\.\d{3})?)?$/.test(value) ?

I ran simple test with real data-api

$ aws rds-data execute-statement --database 'test' \
    --resource-arn $RDS_DATA_API_CLIENT_RESOURCE_ARN \
    --secret-arn $RDS_DATA_API_CLIENT_SECRETARN \
    --include-result-metadata \
    --sql $'CREATE TABLE txtx (t timestamptz);'
{
    "generatedFields": [],
    "numberOfRecordsUpdated": 0
}
$ aws rds-data execute-statement --database 'test' \                                                                             
    --resource-arn $RDS_DATA_API_CLIENT_RESOURCE_ARN \
    --secret-arn $RDS_DATA_API_CLIENT_SECRETARN \
        --sql $'INSERT INTO txtx (t) VALUES (\'2021-03-10 22:41:04.123456+02\')'
{
    "generatedFields": [],
    "numberOfRecordsUpdated": 1
}
$ aws rds-data execute-statement --database 'test' \                                                                             ✘ 254
    --resource-arn $RDS_DATA_API_CLIENT_RESOURCE_ARN \
    --secret-arn $RDS_DATA_API_CLIENT_SECRETARN \
    --include-result-metadata \
    --sql $'SELECT * from txtx'
{
    "columnMetadata": [
        {
            "arrayBaseColumnType": 0,
            "isAutoIncrement": false,
            "isCaseSensitive": false,
            "isCurrency": false,
            "isSigned": false,
            "label": "t",
            "name": "t",
            "nullable": 1,
            "precision": 35,
            "scale": 6,
            "schemaName": "",
            "tableName": "txtx",
            "type": 93,
            "typeName": "timestamptz"
        }
    ],
    "numberOfRecordsUpdated": 0,
    "records": [
        [
            {
                "stringValue": "2021-03-10 20:41:04.123456"
            }
        ]
    ]
}
carlocorradini commented 3 years ago

I've updated the repo with the new version of the drives that has fixed the timestamptz problem.

However currently there is the deprecation notice to solve.

That userId is really strange...