Conversation
| // to avoid problems when used for recovery_target_time | ||
| func ConvertToPostgresFormat(timestamp string) string { | ||
| formatWithoutZ := func(t time.Time) string { | ||
| formatted := t.Format("2006-01-02 15:04:05.000000Z07:00") |
There was a problem hiding this comment.
Here, questions. In the ParseTargetTime function in pkg/types/time.go , we make use of pq.ParseTimestamp().
Why not use pq.FormatTimestamp() here, regardless of input formats?
Moreover, why not validate the input in this function? We validated RFC3339 and RFC3339Micro, but standalone, this function, given "foo", will assume it's a valid timestamp and just return it.
|
There is an E2E test exercising the conversion function: cloudnative-pg/cloudnative-pg#8981 |
b43eeb3 to
0af1ca3
Compare
|
Regarding I tested it and it still outputs So using Additionally, So we need Go's Regarding validation: You're right that given However, I've pushed two additional commits that improve this:
If we want stricter validation (return error for invalid input), that would be a breaking API change. We could consider it for a future version. |
|
Thanks Marco, that makes good sense. Good changes. |
|
Hey @jsilvela! Merry Christmas and Happy New Year to you too! This PR has been expanded to address a related consistency issue discovered while investigating. The original fix avoids the trailing Now both functions consistently treat RFC3339-like timestamps without timezone as UTC, and Related PRs: cloudnative-pg/cloudnative-pg#8937 and cloudnative-pg/plugin-barman-cloud#700 |
Signed-off-by: Jaime Silvela <jaime.silvela@mailfence.com>
Use +00:00 instead of +00 for UTC timezone offset to maintain consistency with non-UTC timestamps which use the full offset format (e.g., +03:00). Signed-off-by: Marco Nenciarini <marco.nenciarini@enterprisedb.com>
Add support for converting timestamps in RFC3339-like format without timezone (e.g., "2023-07-06T08:00:39") to PostgreSQL format. This format is accepted by ParseTargetTime in pkg/types/time.go but was previously passed through unchanged, causing PostgreSQL to reject the timestamp due to the 'T' separator. Signed-off-by: Marco Nenciarini <marco.nenciarini@enterprisedb.com>
RFC3339-like timestamps without timezone (e.g., "2006-01-02T15:04:05") are now interpreted as UTC and output with explicit +00:00 suffix. This ensures consistency between: - ParseTargetTime (used by barman-cloud for backup selection) - ConvertToPostgresFormat (used for PostgreSQL recovery_target_time) Both functions now treat such timestamps as UTC, avoiding potential timezone mismatches where backup selection and recovery could target different points in time. Signed-off-by: Marco Nenciarini <marco.nenciarini@enterprisedb.com>
|
That's a good catch |
|
Also, on a separate PR I have an E2E to exercise PITR specified in RFC3339, cloudnative-pg/cloudnative-pg#8981 |
Signed-off-by: Armando Ruocco <armando.ruocco@enterprisedb.com>
Let's move the discussion about the E2E on cloudnative-pg/cloudnative-pg#8981 |
…39 format (#8981) Exercises the machinery fix (cloudnative-pg/machinery#168) that allows RFC3339 timestamps in recovery_target_time. The existing PITR snapshot test is extended to also restore with an RFC3339-formatted targetTime, ensuring both formats work end-to-end. Ref: #8937, cloudnative-pg/machinery#167 Signed-off-by: Jaime Silvela <jaime.silvela@mailfence.com>
…39 format (#8981)%0A%0AExercises the machinery fix (cloudnative-pg/machinery#168) that allows%0ARFC3339 timestamps in recovery_target_time. The existing PITR snapshot%0Atest is extended to also restore with an RFC3339-formatted targetTime,%0Aensuring both formats work end-to-end.%0A%0ARef: cloudnative-pg/cloudnative-pg#8937: Jaime Silvela <jaime.silvela@mailfence.com> cloudnative-pg/cloudnative-pg@refs/heads/main
Closes #167
Avoid the trailing Z in RFC3339 / ISO 8601 in UTC zone, which does not play as a recovery target time.