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
feat(postgres): dyanmic oids(#9863) #10077
Conversation
Codecov Report
@@ Coverage Diff @@
## master #10077 +/- ##
==========================================
+ Coverage 96.3% 96.3% +<.01%
==========================================
Files 63 63
Lines 9413 9429 +16
==========================================
+ Hits 9065 9081 +16
Misses 348 348
Continue to review full report at Codecov.
|
a11975c
to
2e707b9
Compare
652da77
to
19b04d4
Compare
Take a look at travis tests, I pass all except for one test on postgres 10. The specific test I fail looks fishy. I belive it is not related to the work I have done. That specific test is run with |
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.
Looks good, some code style related changes
oids: [1083], | ||
array_oids: [1183] | ||
}; | ||
BaseTypes.UUID.types.postgres = ['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.
This mapping doesn't serves any purpose, right? but leave it for now
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.
You are right, as of now it doesn't have any purpose. Originally I had two cases in mind. The first one was to allow overriding the default type parser(this.lib.types.typeParser
) within sequelize. In this case you need the mapping to get the OID and set the parser. The second case was range autodetection, which requires base types to be mapped.
Thanks @javiertury 👍 |
Pull Request check-list
npm run test
ornpm run test-DIALECT
pass with this change (including linting)?Description of change
Related to issue #9863. This pull request makes postgres
DataTypes
OID agnostic, all OID information is contained inConnectionManager
. They are refreshed automatically.I've added some tests but I didn't run them. I will wait for this pull request to run them on travis.
EDIT: Autodetection of Ranges dropped, reasons in #9863