Skip to content
This repository has been archived by the owner on Jan 28, 2021. It is now read-only.

Implement group by index #525

Merged
merged 2 commits into from
Nov 7, 2018
Merged

Implement group by index #525

merged 2 commits into from
Nov 7, 2018

Conversation

kuba--
Copy link
Contributor

@kuba-- kuba-- commented Nov 1, 2018

Signed-off-by: kuba-- kuba@sourced.tech

Closes: #520, #524, #535

@kuba-- kuba-- added bug Something isn't working wip work in progress labels Nov 1, 2018
sql/parse/parse.go Outdated Show resolved Hide resolved
@kuba--
Copy link
Contributor Author

kuba-- commented Nov 3, 2018

This is not nice to have solution, but at least it fixes: #524, #520, so I'm totally open for any alternative approaches.

So far I tested against following combination of queries:

SELECT commit_hash AS `commit_hash` FROM commits GROUP BY commit_hash;
SELECT commit_hash AS `commit_hash` FROM commits GROUP BY 1;
SELECT SUBSTRING(`commits`.`commit_hash`, 1, 5) AS `commit_hash` FROM commits GROUP BY commit_hash;
SELECT SUBSTRING(`commits`.`commit_hash`, 1, 5) AS `commit_hash` FROM commits GROUP BY 1;
SELECT count(commit_hash),  repository_id FROM commits GROUP BY 2;

SELECT SUBSTRING(`repositories`.`repository_id`, 1, 1024) AS `repository_id`
FROM `repositories`
  INNER JOIN `files` ON (`repositories`.`repository_id` = SUBSTRING(`files`.`repository_id`, 1, 1024))
GROUP BY 1;

@kuba-- kuba-- removed the wip work in progress label Nov 3, 2018
@kuba-- kuba-- requested a review from a team November 3, 2018 02:01
@@ -243,7 +243,7 @@ func TestTime_DayOfWeek(t *testing.T) {
{"null date", sql.NewRow(nil), nil, false},
{"invalid type", sql.NewRow([]byte{0, 1, 2}), nil, false},
{"date as string", sql.NewRow(stringDate), int32(3), false},
{"date as time", sql.NewRow(time.Now()), int32(time.Now().UTC().Weekday()+1) % 7, false},
{"date as time", sql.NewRow(time.Now()), int32(time.Now().UTC().Weekday() + 1), false},
Copy link
Contributor

Choose a reason for hiding this comment

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

if weekday == 6 it would return 7, MySQL weekday index range is [0, 6] https://dev.mysql.com/doc/refman/8.0/en/date-and-time-functions.html#function_weekday

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I changed it because tests were failing

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This test is testing a DayOfWeek (not weekday): https://dev.mysql.com/doc/refman/8.0/en/date-and-time-functions.html#function_dayofweek

just expected value was specified by go's weekday, so there is no need for %

@ajnavarro
Copy link
Contributor

@erizocosmico could you have a look?

kuba-- added 2 commits November 6, 2018 15:41
Signed-off-by: kuba-- <kuba@sourced.tech>
Signed-off-by: kuba-- <kuba@sourced.tech>
@ajnavarro ajnavarro merged commit 7ce74fb into src-d:master Nov 7, 2018
@kuba-- kuba-- deleted the tableau-520/group-by branch November 7, 2018 11:25
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants