Skip to content

implementing quarter() function#2258

Merged
jycor merged 10 commits intomainfrom
james/quarter
Jan 17, 2024
Merged

implementing quarter() function#2258
jycor merged 10 commits intomainfrom
james/quarter

Conversation

@jycor
Copy link
Contributor

@jycor jycor commented Jan 13, 2024

This PR adds support for the QUARTER() function, which tells the quarter from the provided date.

MySQL Docs: https://dev.mysql.com/doc/refman/8.0/en/date-and-time-functions.html#function_quarter

Copy link
Contributor

@fulghum fulghum left a comment

Choose a reason for hiding this comment

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

Code looks alright, but a couple of test cases look wrong to me. Once those match MySQL's results, this looks good to go though.

Comment on lines +110 to +124
{
name: "1",
row: sql.NewRow(1),
expected: int32(1),
},
{
name: "1.1",
row: sql.NewRow(1.1),
expected: int32(1),
},
{
name: "invalid type",
row: sql.NewRow([]byte{0, 1, 2}),
expected: int32(1),
},
Copy link
Contributor

Choose a reason for hiding this comment

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

Seems like all of these should return NULL, right?

Here's the MySQL behavior:

mysql> select quarter("1"), quarter("1.1");
+--------------+----------------+
| quarter("1") | quarter("1.1") |
+--------------+----------------+
|         NULL |           NULL |
+--------------+----------------+

Comment on lines +196 to +198
name: "date as time",
row: sql.NewRow(time.Now()),
expected: int32(time.Now().UTC().Month()),
Copy link
Contributor

Choose a reason for hiding this comment

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

Seems like this test will only pass in the month of January? 🤔

@jycor jycor merged commit 5fe9d26 into main Jan 17, 2024
@jycor jycor deleted the james/quarter branch January 17, 2024 07:41
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