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.

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

vtgate: Gen4 only_full_group_by regression #10011

Closed
derekperkins opened this issue Mar 30, 2022 · 8 comments · Fixed by #10074
Closed

vtgate: Gen4 only_full_group_by regression #10011

derekperkins opened this issue Mar 30, 2022 · 8 comments · Fixed by #10074

Comments

@derekperkins
Copy link
Member

derekperkins commented Mar 30, 2022

I think the Gen4 planner is erroring with using aliases in group by columns.

(1055, "Expression of SELECT list is not in GROUP BY clause and contains nonaggregated column 'Count' which is not functionally dependent on columns in GROUP BY clause; this is incompatible with sql_mode=only_full_group_by")

This query has worked previously on vtgate and still works on v13 with the V3 planner

SELECT
  latest_error as `Latest Error`,
  count(*) AS `Count`,
  sum(if(latest_tried_at BETWEEN FROM_UNIXTIME(1648578304) AND FROM_UNIXTIME(1648664704), 1, 0)) AS `Last Errored in Period`
FROM (
  select
    JSON_UNQUOTE(JSON_EXTRACT(attributes, '$.latestError')) as latest_error,
    TIMESTAMP(JSON_EXTRACT(attributes, '$.latestTriedAt')) as latest_tried_at
  from searches__extractor__msgs
  where time_next is not null
) as msgs_errors
WHERE latest_error IS NOT NULL
GROUP BY latest_error
ORDER BY `Count` desc
LIMIT 30;

I'm rolling out #10007 today, maybe the subquery is fixed there, otherwise that'll get me back to the Gen4 planner where I can do an explain format=vitess.

@derekperkins
Copy link
Member Author

I rolled out the subquery fix and this issue was not resolved. In fact, I can't even run explain format=vitess because that throws the same error.

cc @systay

@systay
Copy link
Collaborator

systay commented Mar 31, 2022

Hi @derekperkins!

We have worked on the gen4 aggregation planning a lot in main, and this query seems to work well there. I'll check how difficult this would be to fix on R13.

@derekperkins
Copy link
Member Author

derekperkins commented Apr 1, 2022

I just rolled out vtgate based on main and this problem still exists. I forgot to mention that it works on unsharded keyspaces, but fails on sharded keyspaces. The sharding key is not referenced in the query - it's just a hash on a tenant id int.

@systay
Copy link
Collaborator

systay commented Apr 4, 2022

Ah, isn't the GROUP BY missing a column? You are returning three columns, and one of them is an aggregation function, so the other two should be in the grouping definition, right?

@derekperkins
Copy link
Member Author

derekperkins commented Apr 4, 2022

No, it's the group by column + count & sum. I tested using the column name and the alias as the group by, and got the same error both times. This query has worked for several releases prior to v13, and currently works on unsharded keyspaces.

@systay
Copy link
Collaborator

systay commented Apr 6, 2022

Oh, now I see. That is weird! I'll see what I can do.

@msolters
Copy link
Contributor

msolters commented Apr 8, 2022

lol #9889

@frouioui
Copy link
Member

Hey @derekperkins, this issue should be fixed with #10074. Let us know if you're still observing the issue

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants