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.

Sign up for GitHub

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

Fix week number for date_format evalengine function #17432

Merged
merged 3 commits into from
Dec 26, 2024
Merged

Fix week number for date_format evalengine function #17432

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
44c1dc2
test: add a failing test
GuptaManan100 Dec 26, 2024
a2e1ffb
feat: fix the problem
GuptaManan100 Dec 26, 2024
e9f9d0b
Use the documented modes for the rest of the formatters
dbussink Dec 26, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 1 addition & 4 deletions go/mysql/datetime/spec.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -374,10 +374,7 @@ func (u fmtWeek0) parse(t *timeparts, bytes string) (string, bool) {
type fmtWeek1 struct{}

func (fmtWeek1) format(dst []byte, t DateTime, prec uint8) []byte {
year, week := t.Date.ISOWeek()
if year < t.Date.Year() {
week = 0
}
week := t.Date.Week(1)
Comment on lines 373 to +374
Copy link
Member Author

Choose a reason for hiding this comment

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

I can make the same change for the other fmtWeek functions. Haven't done that yet, but i think it makes sense to just use the same implementation for everything.

Copy link
Contributor

Choose a reason for hiding this comment

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

@GuptaManan100 i think we should do that indeed. Makes it less error prone and the week function afaik is already better tested too.

Copy link
Contributor

Choose a reason for hiding this comment

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

@GuptaManan100 Pushed up the fix to always used the documented mode number.

return appendInt(dst, week, 2)
}

Expand Down
20 changes: 20 additions & 0 deletions go/vt/vtgate/evalengine/compiler_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -740,6 +740,26 @@ func TestCompilerSingle(t *testing.T) {
expression: `cast(_utf32 0x0000FF as binary)`,
result: `VARBINARY("\x00\x00\x00\xff")`,
},
{
expression: `DATE_FORMAT(timestamp '2024-12-30 10:34:58', "%u")`,
result: `VARCHAR("53")`,
},
{
expression: `WEEK(timestamp '2024-12-30 10:34:58', 0)`,
result: `INT64(52)`,
},
{
expression: `WEEK(timestamp '2024-12-30 10:34:58', 1)`,
result: `INT64(53)`,
},
{
expression: `WEEK(timestamp '2024-01-01 10:34:58', 0)`,
result: `INT64(0)`,
},
{
expression: `WEEK(timestamp '2024-01-01 10:34:58', 1)`,
result: `INT64(1)`,
},
}

tz, _ := time.LoadLocation("Europe/Madrid")
Expand Down
1 change: 1 addition & 0 deletions go/vt/vtgate/evalengine/testcases/inputs.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,7 @@ var inputConversions = []string{
"time '10:04:58'", "time '31:34:58'", "time '32:34:58'", "time '130:34:58'", "time '5 10:34:58'",
"time '10:04:58.1'", "time '31:34:58.4'", "time '32:34:58.5'", "time '130:34:58.6'", "time '5 10:34:58.9'", "date '2000-01-01'",
"timestamp '2000-01-01 10:34:58'", "timestamp '2000-01-01 10:34:58.123456'", "timestamp '2000-01-01 10:34:58.978654'",
"timestamp '2024-12-30 10:34:58'",
"20000101103458", "20000101103458.1234", "20000101103458.123456", "20000101", "103458", "103458.123456",
"'20000101103458'", "'20000101103458.1234'", "'20000101103458.123456'", "'20000101'", "'103458'", "'103458.123456'",
"'20000101103458foo'", "'20000101103458.1234foo'", "'20000101103458.123456foo'", "'20000101foo'", "'103458foo'", "'103458.123456foo'",
Expand Down
Loading