Skip to content
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

Add ScanLocation to Timestamp and Timestamptz #1948

Merged
merged 2 commits into from
May 8, 2024
Merged

Conversation

jackc
Copy link
Owner

@jackc jackc commented Mar 16, 2024

This setting allows configuring the time.Location of time.Time values scanned from PostgreSQL timestamp and timestamptz.

For timestamptz it changes the display location but the instant in time is unchanged. That is, regardless of the value of ScanLocation, the results of time.Time.Equal() will be the same.

For timestamp it changes the location the time is assumed to have been in. That is, changing the value of ScanLocation will change the results of time.Time.Equal().

If ScanLocation is set, it will be used to convert the time to the given
location when scanning from the database.

The Codec interface is now implemented by *pgtype.TimestamptzCodec
instead of pgtype.TimestamptzCodec. This is technically a breaking
change, but it is extremely unlikely that anyone is depending on this,
and if there is downstream breakage it is trivial to fix.

#1195
#1945
If ScanLocation is set, the timestamps will be assumed to be in the
given location when scanning from the database.

The Codec interface is now implemented by *pgtype.TimestampCodec instead
of pgtype.TimestampCodec. This is technically a breaking change, but it
is extremely unlikely that anyone is depending on this, and if there is
downstream breakage it is trivial to fix.

#1195
#1945
Copy link

@alexrjones alexrjones left a comment

Choose a reason for hiding this comment

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

Hi @jackc, I saw you requested feedback on #1195 (comment). Though I'm not experienced with pgx's internals, FWIW this change did address the issues we were facing in our application.


func (TimestampCodec) FormatSupported(format int16) bool {
func (*TimestampCodec) FormatSupported(format int16) bool {

Choose a reason for hiding this comment

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

Question: Is it strictly necessary for the new codec implementations to use a pointer receiver? In both cases the only member is the *time.Location field which is itself a pointer. As you mentioned in your commit descriptions, this is technically a breaking change, albeit a very minor one.

Copy link
Owner Author

Choose a reason for hiding this comment

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

Is it strictly necessary? No. But as I understand it, it is usually better for interfaces implemented by structs to use methods that receive the pointer rather than the value.

@jackc jackc merged commit 8649231 into master May 8, 2024
14 checks passed
@jackc jackc deleted the pgtype-time-zones branch May 8, 2024 13:35
@fantapop
Copy link

Hi! Is there a way to use the ScanLocation with pgx/v4?

@jackc
Copy link
Owner Author

jackc commented May 29, 2024

@fantapop Only if someone were to backport it.

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.

3 participants