search

ArsenyYankovsky / typeorm-aurora-data-api-driver

A bridge between TypeORM and Aurora Data API
MIT License
175 stars 38 forks source link

Update README.md with field support #79

Closed bdombro closed 3 years ago

bdombro commented 3 years ago

I found that these field types do not work with this Package, I think because Data API doesn't support arrays: json, set

ArsenyYankovsky commented 3 years ago

Please create an issue with a reproducible example. JSON columns are supposed to work and they do work in a couple of test scenarios. Set needs a bit of work but it should be an easy fix.

bdombro commented 3 years ago

Reproducible example: do you have suggestion on how to best share that? Is there a template you'd like me to use?

FWIW, I notice your existing JSON test is on a 'json-simple' field type, which is a little different the the 'json' field type I had issues with.

ArsenyYankovsky commented 3 years ago

The best for me personally would be a failing test in this project, but just about anything that will allow me to create a test out of it will work. I'm testing json and jsonb types, see tests related to JsonEntity

bdombro commented 3 years ago

Ohh, yeah I'm using MySQL not PostGRES, which looks like you only test the json-simple.

Is this accurate?: To run your automated tests, I'll need to create a PR and watch the CI/CD?

This is the error I get btw:

"stack": "Error: 'param_5' is an invalid type
    at error (/var/task/node_modules/typeorm-aurora-data-api-driver/dist/typeorm-aurora-data-api-driver.umd.js:369:39)
    at formatType (/var/task/node_modules/typeorm-aurora-data-api-driver/dist/typeorm-aurora-data-api-driver.umd.js:539:28)
    at formatParam (/var/task/node_modules/typeorm-aurora-data-api-driver/dist/typeorm-aurora-data-api-driver.umd.js:480:48)
    at /var/task/node_modules/typeorm-aurora-data-api-driver/dist/typeorm-aurora-data-api-driver.umd.js:465:24
    at Array.reduce (<anonymous>)
    at processParams (/var/task/node_modules/typeorm-aurora-data-api-driver/dist/typeorm-aurora-data-api-driver.umd.js:449:33)
    at query (/var/task/node_modules/typeorm-aurora-data-api-driver/dist/typeorm-aurora-data-api-driver.umd.js:690:46)
    at Object.query (/var/task/node_modules/typeorm-aurora-data-api-driver/dist/typeorm-aurora-data-api-driver.umd.js:907:26)
    at DataApiDriver.<anonymous> (/var/task/node_modules/typeorm-aurora-data-api-driver/dist/typeorm-aurora-data-api-driver.umd.js:1340:62)
    at step (/var/task/node_modules/typeorm-aurora-data-api-driver/dist/typeorm-aurora-data-api-driver.umd.js:81:27)"
ArsenyYankovsky commented 3 years ago

JSON type was introduced in MySQL 5.7. As far as I know Aurora Serverless only supports MySQL 5.6.

bdombro commented 3 years ago

Ahh, so guess what?!? Aurora Serverless V2 is 5.7. 🤯🤯🤯

Either way, how about we add some info like my suggested on the README so other people don't get confused?

ArsenyYankovsky commented 3 years ago

Good to know, but V2 is still in preview and not released. I'd rather add the support for these types which is just 2 lines of code. Feel free to submit a PR with a fix or create an issue and I'll fix it when I have time.

bdombro commented 3 years ago

I'd rather add the support for these types which is just 2 lines of code.

Since you seem to have a hunch on "2 lines of code", would you mind elaborating? I could make a PR with it if you could help guide me ;-).

ArsenyYankovsky commented 3 years ago

Since you seem to have a hunch on "2 lines of code", would you mind elaborating? I could make a PR with it if you could help guide me ;-).

Sure, we just need to add two clauses similar to these from the original MySQL preparePersistentValue here to the MysqlQueryTransformer here. Make sure you add the test cases as well. Thanks!