feat(postgres to_timestamp): Implementing Postgres to_timestamp funct… #263
Conversation
| java.time.ZonedDateTime | ||
| .ofInstant( | ||
| column.fold(resultSet.getTimestamp(_), resultSet.getTimestamp(_)).toInstant, | ||
| ZoneId.of(ZoneOffset.UTC.getId) |
There was a problem hiding this comment.
I'm not sure how we'd like to handle this, having it default to utc seems problematic, but I'm not sure how we should go from java.sql.Timestamp -> java.time.ZonedDateTime.
There was a problem hiding this comment.
looking at the javadoc of getTimestamp, I'm pretty sure that it's converting to UTC, so I believe this is correct. It'd probably be a good idea to add a round trip test to make sure (e.g. insert a timestamp from some other time zone, grab it back out, then compare to ensure that time is the same (accounting for the difference in time zones)
There was a problem hiding this comment.
Yeah that's a good idea, I'll add that in!
| import zio.sql.Jdbc | ||
|
|
||
| /** | ||
| */ | ||
| trait PostgresModule extends Jdbc { self => | ||
|
|
||
| object PostgresFunctionDef { | ||
| val Sind = FunctionDef[Double, Double](FunctionName("sind")) | ||
| val Sind = FunctionDef[Double, Double](FunctionName("sind")) | ||
| val ToTimestamp = FunctionDef[Double, ZonedDateTime](FunctionName("to_timestamp")) |
There was a problem hiding this comment.
I think this should take a Long rather than a Double.
There was a problem hiding this comment.
That makes sense I was mostly copying from Postgres docs, I'll make that change!
| java.time.ZonedDateTime | ||
| .ofInstant( | ||
| column.fold(resultSet.getTimestamp(_), resultSet.getTimestamp(_)).toInstant, | ||
| ZoneId.of(ZoneOffset.UTC.getId) |
There was a problem hiding this comment.
looking at the javadoc of getTimestamp, I'm pretty sure that it's converting to UTC, so I believe this is correct. It'd probably be a good idea to add a round trip test to make sure (e.g. insert a timestamp from some other time zone, grab it back out, then compare to ensure that time is the same (accounting for the difference in time zones)
|
@robmwalsh I've added in some additional tests per your suggestion and I've brought in latest master. Ready for rereview. |
postgres/src/test/scala/zio/sql/postgresql/FunctionDefSpec.scala
Outdated
Show resolved
Hide resolved
|
So is this ready? It looks ready... |
Yep this is ready now! |
|
Awesome progress! |
|
@brbrown25 thanks! |
feat(postgres to_timestamp): Implementing Postgres to_timestamp funct…
…ion. #215