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

executor: add builtin aggregate function json_arrayagg #1020

Merged
merged 1 commit into from
Sep 22, 2020

Conversation

arthuryangcs
Copy link
Contributor

What problem does this PR solve?

pingcap/tidb#7623

What is changed and how it works?

add an json_arrayagg function

Check List

[] Make sure the replace instruction is changed back to the required instruction and the version is the latest.

Tests

		{`select json_arrayagg(c2) from t group by c1`, true, "SELECT JSON_ARRAYAGG(`c2`) FROM `t` GROUP BY `c1`"},
		{`select json_arrayagg(c1, c2) from t group by c1`, false, ""},
		{`select json_arrayagg(distinct c2) from t group by c1`, false, "SELECT JSON_ARRAYAGG(DISTINCT `c2`) FROM `t` GROUP BY `c1`"},
		{`select json_arrayagg(all c2) from t group by c1`, true, "SELECT JSON_ARRAYAGG(`c2`) FROM `t` GROUP BY `c1`"},

Code changes

  • Add JSON_ARRAYAGG

Related changes

@CLAassistant
Copy link

CLAassistant commented Sep 11, 2020

CLA assistant check
All committers have signed the CLA.

Copy link
Contributor

@kennytm kennytm left a comment

Choose a reason for hiding this comment

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

LGTM

@ti-srebot ti-srebot added the status/LGT1 LGT1 label Sep 14, 2020
Copy link
Contributor

@SunRunAway SunRunAway left a comment

Choose a reason for hiding this comment

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

LGTM

@ti-srebot
Copy link
Contributor

@SunRunAway,Thanks for your review. The bot only counts LGTMs from Reviewers and higher roles, but you're still welcome to leave your comments.See the corresponding SIG page for more information. Related SIG: ddl(slack).

@SunRunAway
Copy link
Contributor

@arthuryangcs Please resolve the confliting files.

@arthuryangcs
Copy link
Contributor Author

@arthuryangcs Please resolve the confliting files.

Resolved.

@arthuryangcs
Copy link
Contributor Author

@SunRunAway @kennytm Please review the code.

@arthuryangcs
Copy link
Contributor Author

@SunRunAway @kennytm Please review the code.

Copy link
Contributor

@djshow832 djshow832 left a comment

Choose a reason for hiding this comment

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

LGTM

@ti-srebot ti-srebot removed the status/LGT1 LGT1 label Sep 22, 2020
@ti-srebot ti-srebot added the status/LGT2 LGT2 label Sep 22, 2020
@djshow832
Copy link
Contributor

/merge

@ti-srebot
Copy link
Contributor

/run-all-tests

@ti-srebot
Copy link
Contributor

@arthuryangcs merge failed.

@arthuryangcs
Copy link
Contributor Author

/merge

@ti-srebot
Copy link
Contributor

@arthuryangcs Oops! auto merge is restricted to Committers of the SIG.See the corresponding SIG page for more information. Related SIG: ddl(slack).

@kennytm kennytm merged commit ef62879 into pingcap:master Sep 22, 2020
@arthuryangcs arthuryangcs deleted the json_arrayagg branch September 23, 2020 03:28
tiancaiamao pushed a commit to tiancaiamao/parser that referenced this pull request Apr 27, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants