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

add builtin-reverse func #1224

Merged
merged 2 commits into from
May 13, 2016
Merged

Conversation

mrmiywj
Copy link
Contributor

@mrmiywj mrmiywj commented May 11, 2016

@CLAassistant
Copy link

CLAassistant commented May 11, 2016

CLA assistant check
All committers have signed the CLA.

Expect string
}{
{"abc", "cba"},
{"LIKE", "EKIL"},
Copy link
Member

Choose a reason for hiding this comment

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

Please add more cases such as reverse(null), reverse(123).

@shenli
Copy link
Member

shenli commented May 12, 2016

Please add test case for type inference here: https://github.com/pingcap/tidb/blob/master/optimizer/typeinferer_test.go#L35

@@ -56,6 +56,7 @@ TEMP_FILE = temp_parser_file
parser:
go get github.com/qiuyesuifeng/goyacc
go get github.com/qiuyesuifeng/golex
go get github.com/golang/example/stringutil
Copy link
Member

Choose a reason for hiding this comment

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

We use vendor to solve dependency. Should we add it into .vendor? @coocood

Copy link
Member

Choose a reason for hiding this comment

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

It's an example package, it's not supposed to be imported.

@shenli
Copy link
Member

shenli commented May 12, 2016

@mrmiywj Thanks for your contribute!

@coocood @zimulala @zxylvlp PTAL

@coocood
Copy link
Member

coocood commented May 12, 2016

@mrmiywj
Can you implement the simple function in stringutil directly?
So we don't have to import the package.

@mrmiywj mrmiywj force-pushed the string-function-support branch from 23b56f1 to 7b92882 Compare May 12, 2016 03:55
@mrmiywj
Copy link
Contributor Author

mrmiywj commented May 12, 2016

cc @shenli @coocood

@shenli
Copy link
Member

shenli commented May 12, 2016

LGTM

| "REVERSE" '(' Expression ')'
{
$$ = &ast.FuncCallExpr{FnName: model.NewCIStr($1.(string)), Args: []ast.ExprNode{$3.(ast.ExprNode)}}
}
Copy link
Member

Choose a reason for hiding this comment

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

In parser.y and scanner.y, we use tab to align the code.
Please change spaces to tabs.

@mrmiywj mrmiywj force-pushed the string-function-support branch from 7b92882 to 46029ea Compare May 12, 2016 05:31
@mrmiywj
Copy link
Contributor Author

mrmiywj commented May 12, 2016

cc @coocood
Sorry for that on my emacs configuration, tabs are automatically replaced by spaces

@shenli
Copy link
Member

shenli commented May 13, 2016

PTAL @coocood

@@ -445,6 +445,7 @@ references {r}{e}{f}{e}{r}{e}{n}{c}{e}{s}
regexp {r}{e}{g}{e}{x}{p}
replace {r}{e}{p}{l}{a}{c}{e}
redundant {r}{e}{d}{u}{n}{d}{a}{n}{t}
reverse {r}{e}{v}{e}{r}{s}{e}
Copy link
Member

Choose a reason for hiding this comment

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

Seems it is not aligned.

@mrmiywj mrmiywj force-pushed the string-function-support branch from 46029ea to 6d324ca Compare May 13, 2016 06:31
@mrmiywj
Copy link
Contributor Author

mrmiywj commented May 13, 2016

cc @shenli

@coocood
Copy link
Member

coocood commented May 13, 2016

@mrmiywj
Still use spaces, not aligned.

@mrmiywj mrmiywj force-pushed the string-function-support branch from 6d324ca to eea7690 Compare May 13, 2016 11:01
@mrmiywj
Copy link
Contributor Author

mrmiywj commented May 13, 2016

cc @coocood

@coocood
Copy link
Member

coocood commented May 13, 2016

@mrmiywj
{reverse} lval.item = string(l.val) return reverse
in scanner.l

@coocood
Copy link
Member

coocood commented May 13, 2016

also missed a tab before line 2567

fix nits

change spaces to tab

fix indent

fix indents

fix indent
@mrmiywj mrmiywj force-pushed the string-function-support branch from eea7690 to ee46ebf Compare May 13, 2016 12:10
@mrmiywj
Copy link
Contributor Author

mrmiywj commented May 13, 2016

cc @coocood

@coocood
Copy link
Member

coocood commented May 13, 2016

@mrmiywj
The alignment is ok now.
Please resolve the conflict.

@mrmiywj
Copy link
Contributor Author

mrmiywj commented May 13, 2016

cc @coocood

@coocood
Copy link
Member

coocood commented May 13, 2016

LGTM
Thank you for your contribution!

@coocood coocood merged commit 55ed68c into pingcap:master May 13, 2016
@sre-bot sre-bot added the contribution This PR is from a community contributor. label Dec 18, 2019
xhebox pushed a commit to xhebox/tidb that referenced this pull request Sep 28, 2021
…or GRANT CREATE USER ON <specific db>.* (pingcap#1224)

* add StaticGlobalOnlyPrivs

* fix
xhebox pushed a commit to xhebox/tidb that referenced this pull request Oct 8, 2021
…or GRANT CREATE USER ON <specific db>.* (pingcap#1224)

* add StaticGlobalOnlyPrivs

* fix
ti-chi-bot pushed a commit that referenced this pull request Oct 9, 2021
…or GRANT CREATE USER ON <specific db>.* (#1224)

* add StaticGlobalOnlyPrivs

* fix
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
contribution This PR is from a community contributor.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants