feat(postgresql): Add interval column type for schema.TimeInterval#2167
feat(postgresql): Add interval column type for schema.TimeInterval#2167candiduslynx merged 4 commits intomainfrom
schema.TimeInterval#2167Conversation
87ce135 to
1e13e3a
Compare
yevgenypats
left a comment
There was a problem hiding this comment.
Looks good. Can you also add a default for text and maybe also log a warning that it doesn't support a specific type.
|
@yevgenypats I think we need to carefully consider whether to default to text here--it can result in changed behaviour whenever the SDK adds a new type. For example, intervals were previously stored as Maybe a solution is to add a test that iterates over all types in the SDK, checking they are all handled? Then you wouldn't be able to upgrade the SDK to a version with a new type without also updating the handled types. |
2b13de8 to
29a53ce
Compare
ce5648b to
2ad5045
Compare
Squashed commits: [adacabd] add interval col impl
2ad5045 to
8e807b9
Compare
hermanschaaf
left a comment
There was a problem hiding this comment.
LGTM, just wondering why we chose to rely on defaults for the cockroachdb case
Maybe we can have an option then. because let's say a community plugin is not up to date, it will be pretty hard requirement always staying up to date with all latest fields. maybe we can ignore this column as well? I think let's wait and see what issues this will cause. |
🤖 I have created a release *beep* *boop* --- ## [1.3.0](plugins-destination-postgresql-v1.2.2...plugins-destination-postgresql-v1.3.0) (2022-10-13) ### Features * **postgresql:** Add interval column type for `schema.TimeInterval` ([#2167](#2167)) ([f2e6a53](f2e6a53)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
Add interval column type for
schema.TimeInterval(#2062)