Skip to content

[usage] Handle empty value varchar time, use ISO8601 #10490

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

Merged
merged 1 commit into from
Jun 7, 2022

Conversation

easyCZ
Copy link
Member

@easyCZ easyCZ commented Jun 6, 2022

Description

Hardens handling of Varchar Time records in the DB. Here, we ensure that empty value of VarChar is equivalent to the empty string and therefore not a valid Timestamp, but a valid value to store in the DB.

Related Issue(s)

How to test

Unit tests (run by CI)

Release Notes

NONE

Documentation

NONE

@easyCZ easyCZ force-pushed the mp/usage-varchar-time-serialize-des branch from 8a86688 to bb6a4c2 Compare June 6, 2022 20:45
@@ -80,9 +79,21 @@ func (n VarcharTime) IsSet() bool {

// Value implements the driver Valuer interface.
func (n VarcharTime) Value() (driver.Value, error) {
Copy link
Member Author

Choose a reason for hiding this comment

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

This is what will be inserted into SQL statements. Here we use the ISO8601 format to be consistent with existing records.

if n.IsSet() {
return TimeToISO8601(n.t), nil
}
return "", nil
}

func (n VarcharTime) String() string {
Copy link
Member Author

Choose a reason for hiding this comment

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

This is what is used to print this field for humans, when we're converting the struct to a string. We align with the DB value and also print ISO8601

@easyCZ easyCZ changed the title [usage] Handle empty value varchar time [usage] Handle empty value varchar time, use ISO8601 Jun 6, 2022
@easyCZ easyCZ requested a review from a team June 7, 2022 07:10
@github-actions github-actions bot added the team: webapp Issue belongs to the WebApp team label Jun 7, 2022
@roboquat roboquat merged commit 4a5f8ec into main Jun 7, 2022
@roboquat roboquat deleted the mp/usage-varchar-time-serialize-des branch June 7, 2022 11:21
@roboquat roboquat added deployed: webapp Meta team change is running in production deployed Change is completely running in production labels Jun 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
deployed: webapp Meta team change is running in production deployed Change is completely running in production release-note-none size/L team: webapp Issue belongs to the WebApp team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants