Skip to content

Add comment for pgtype.Interval struct#2113

Merged
jackc merged 1 commit into
jackc:masterfrom
kowaltek:master
Aug 26, 2024
Merged

Add comment for pgtype.Interval struct#2113
jackc merged 1 commit into
jackc:masterfrom
kowaltek:master

Conversation

@kowaltek

Copy link
Copy Markdown
Contributor

Comment explicitly states that pgtype.Interval is a nullable postgres type.

@kowaltek kowaltek changed the title Add comment for pgtype.Interval struct Add comment for pgtype.Interval struct Closes #2111 Aug 22, 2024
@kowaltek kowaltek changed the title Add comment for pgtype.Interval struct Closes #2111 Add comment for pgtype.Interval struct Aug 22, 2024

@jackc jackc left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Sorry, but there are several errors in the comments.

But beyond that, I really don't think that this is the place to explain Valid. Otherwise, we would need more or less duplicate comments on all the types. A package level comment on the pgtype package would be more reasonable.

Comment thread pgtype/interval.go Outdated

// Interval represents an interval that may be null.
// Interval implements the [Scanner] interface so
// it can be used as a scan destination:

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Implementing Scanner is not why it can be used as a destination in pgx. That's only when used through database/sql.

Comment thread pgtype/interval.go Outdated
// // NULL value
// }
//
// When using as a parameter for prepared statement

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Whether or not a prepared statement is used makes no difference.

Comment thread pgtype/interval.go Outdated
// }
//
// When using as a parameter for prepared statement
// the [Valid] field has to be explicitly set to [true] by the user.

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Valid has to be set if it is not NULL. It is perfectly reasonable for Valid to be false when representing a NULL.

Additional information warns about using nullable types being
used as parameters to query with Valid set to false.
@kowaltek

Copy link
Copy Markdown
Contributor Author

Many thanks for your comments. Made me have a deeper look into pgtype. I added a short notice to the doc.go.
Anyway I get your point about the behavior of pgtype types being similar to database/sql and why it might be a bit redundant to add another explanation in pgtype, so feel free to simply reject changes.

Anyhow, wanted to thank you for your awesome work!

@jackc jackc merged commit 603f233 into jackc:master Aug 26, 2024
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.

2 participants