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

builtin: add random-bytes function #2927

Merged
merged 7 commits into from
Mar 28, 2017
Merged

Conversation

bom-d-van
Copy link
Contributor

No description provided.

@coocood
Copy link
Member

coocood commented Mar 26, 2017

@bom-d-van
The test is failing because that test case is unstable. I've submitted a PR to fix it.
You don't need to worry about it for now.

@@ -249,6 +249,7 @@ func (ts *testTypeInferrerSuite) TestInferType(c *C) {
{`sha1(123)`, mysql.TypeVarString, "utf8"},
{`sha(123)`, mysql.TypeVarString, "utf8"},
{`sha2(123, 256)`, mysql.TypeVarString, charset.CharsetUTF8},
{`random_bytes(32)`, mysql.TypeInt24, charset.CharsetBin},
Copy link
Member

Choose a reason for hiding this comment

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

Should be mysql.TypeVarString, charset.CharsetBin.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed! :)

@coocood
Copy link
Member

coocood commented Mar 26, 2017

@bom-d-van
You just change the test case, but the typeinfer didn't change, so the test case failed.

FAIL: typeinferer_test.go:38: testTypeInferrerSuite.TestInferType
typeinferer_test.go:287:
    c.Assert(tp, Equals, ca.tp, Commentf("Tp for %s", ca.expr))
... obtained uint8 = 0x9
... expected uint8 = 0xfd
... Tp for random_bytes(32)

@bom-d-van
Copy link
Contributor Author

@coocood Sorry for that, it should be alright now. Thanks!

@coocood
Copy link
Member

coocood commented Mar 26, 2017

LGTM

@coocood coocood added status/LGT1 Indicates that a PR has LGTM 1. contribution This PR is from a community contributor. labels Mar 26, 2017
f, err = fc.getFunction(datumsToConstants([]types.Datum{types.NewDatum(1025)}), s.ctx)
c.Assert(err, IsNil)
_, err = f.eval(nil)
c.Assert(err, Not(IsNil))
Copy link
Member

Choose a reason for hiding this comment

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

You can use NotNil instead of Not(IsNil).

return d, nil
}
size := arg.GetInt64()
if size < 1 || size > 1024 {
Copy link
Member

Choose a reason for hiding this comment

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

MySQL returns the following error code and error message
"mysql> select RANDOM_BYTES(2048);
ERROR 1690 (22003): length value is out of range in 'random_bytes'"

Please use standard error code or add a TODO comment here (Then we will fix it latter.).

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 have changed it as you suggested. Thanks. :)

@tiancaiamao
Copy link
Contributor

LGTM
Thanks for contribute! @bom-d-van

@tiancaiamao tiancaiamao merged commit c83bdda into pingcap:master Mar 28, 2017
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. status/LGT1 Indicates that a PR has LGTM 1.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants