Skip to content

Conversation

@gabegorelick
Copy link
Contributor

@gabegorelick gabegorelick commented Jun 19, 2018

Pull Request check-list

  • Does npm run test or npm run test-DIALECT pass with this change (including linting)?
  • Does the description below contain a link to an existing issue (Closes #[issue]) or a description of the issue you are solving?
  • Have you added new tests to prevent regressions?
  • Is a documentation update included (if this change modifies existing APIs, or introduces new ones)?
  • Did you follow the commit message conventions explained in CONTRIBUTING.md?

Description of change

Adds support for Postgres network address types.

This is a continuation of work started by @tadman in #8154.

Closes #8154

@codecov
Copy link

codecov bot commented Jun 19, 2018

Codecov Report

Merging #9567 into master will decrease coverage by 0.08%.
The diff coverage is 66.66%.

"toposort-class": "^1.0.1",
"uuid": "^3.2.1",
"validator": "^10.0.0",
"validator": "^10.4.0",
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Needed for isIPRange.

@gabegorelick
Copy link
Contributor Author

Tests are now passing (the appveyor failure looks to be an existing flaky test). Let me know if more tests make sense. I'll also try to add some relevant docs to model-definitions.md.

}
inherits(CIDR, BaseTypes.CIDR);

CIDR.parse = function parse(value) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No need to add parse if we are not modifying this value, I think just defining oids should be enough for all three of these types

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

array_oids: []
};

function CIDR() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Even these function definitions are not required, as there is no special handling required for Postgres initializing of these datatypes.

Just define OIDs for these types at the top with others

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

@sushantdhiman sushantdhiman merged commit 53ec6af into sequelize:master Jun 20, 2018
@gabegorelick gabegorelick deleted the pg-inet branch June 20, 2018 16:08
@gabegorelick
Copy link
Contributor Author

@sushantdhiman Thanks for shepherding this. Any chance it could get merged into v4?

@sushantdhiman
Copy link
Contributor

@gabegorelick Please open a new PR targeting v4

@gabegorelick
Copy link
Contributor Author

Thanks @sushantdhiman. I opened #9571 for that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Postgres support for cidr, init, macaddr column types

3 participants