-
Notifications
You must be signed in to change notification settings - Fork 5.9k
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
dev row constructor #90
Conversation
@@ -557,6 +558,7 @@ sys_var "@@"(({global}".")|({session}".")|{local}".")?{ident} | |||
{right} return right | |||
{rollback} lval.item = string(l.val) | |||
return rollback | |||
{row} return row |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
row is an unreserved keyword. Please treat is as other unreserved keyword, like "rollback".
And also update the test case in parser_test.go.
PTAL |
// CheckOneColumn checks whether expression e has only one column for the evaluation result. | ||
// Now most of the expressions have one column except Row expression. | ||
func CheckOneColumn(e expression.Expression) error { | ||
_, ok := e.(*Row) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not need to check if len(e.values) == 1?
Or why not rename this function to "IsRowExpr" and return bool?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no need, row(1) is valid.
because later we may check subselect expression, not only row.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
row(1) will be parsed failed in parser
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok
PTAL @shenli |
LGTM |
if err != nil { | ||
return | ||
} | ||
|
||
return Val == nil != is.Not, nil | ||
return val == nil != is.Not, nil |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should this line split into two lines?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe we just need a parenthesis
LGTM |
1 similar comment
LGTM |
* *: added support for storing checkpoint at local filesystem * config, tests: change default checkpoint driver to "file" * mydump: regenerate the parser to avoid golang/go#27350 during coverage * checkpoints: addressed comments
Signed-off-by: disksing <i@disksing.com> Signed-off-by: disksing <i@disksing.com> Co-authored-by: ystaticy <y_static_y@sina.com>
close pingcap#53037 Co-authored-by: Michael Deng <33045922+michaelmdeng@users.noreply.github.com>
base support row constructor like
select row(1, 1) > row(1,1)
row constructor now only supports in comparison operation.
TODO in other PRs:
select row(1, 1)
is not allowed.