-
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
expression/expressions: replace iconv-go with text/transform #28
Conversation
Remove CGO requirement for compiling iconv-go currently is using. Replace it with native implementation of Go. Signed-off-by: Unknwon <u@gogs.io>
Thank god, passed this time. |
LGTM. But I cannot press the merge button. :( |
Godep is killing GitHub diff. So, you can't even find where did I changed. |
@unknwon I usually keep them into separate commits:
|
godep save doesn't work with tidb out of box, so I changed everything manually 😂 really don't want to go through hell again. |
OK... there is the diff: diff --git a/expression/expressions/convert.go b/expression/expressions/convert.go
index b636cb5..8fc7918 100644
--- a/expression/expressions/convert.go
+++ b/expression/expressions/convert.go
@@ -17,11 +17,12 @@ import (
"fmt"
"strings"
- iconv "github.com/djimenez/iconv-go"
"github.com/juju/errors"
"github.com/ngaut/log"
"github.com/pingcap/tidb/context"
"github.com/pingcap/tidb/expression"
+ "golang.org/x/net/html/charset"
+ "golang.org/x/text/transform"
)
// FunctionConvert provides a way to convert data between different character sets.
@@ -74,7 +75,13 @@ func (f *FunctionConvert) Eval(ctx context.Context, args map[interface{}]interfa
} else if strings.ToLower(f.Charset) == "utf8mb4" {
return value, nil
}
- target, err := iconv.ConvertString(str, "utf-8", f.Charset)
+
+ encoding, _ := charset.Lookup(f.Charset)
+ if encoding == nil {
+ return nil, fmt.Errorf("unknow char decoder %s", f.Charset)
+ }
+
+ target, _, err := transform.String(encoding.NewDecoder(), str)
if err != nil {
log.Errorf("Convert %s to %s with error: %v", str, f.Charset, err)
return nil, errors.Trace(err) |
@unknwon this PR introduced too much dependencies, can you remove the depedency of "golang.org/x/net/html/charset", all we need is a map from charset name to encoding. |
* add version subcommand Signed-off-by: Neil Shen <overvenus@gmail.com> * restore: fix typo Signed-off-by: Neil Shen <overvenus@gmail.com> * Init cmd on subcommands Signed-off-by: Neil Shen <overvenus@gmail.com> * tests: clean up tikv-importer Signed-off-by: Neil Shen <overvenus@gmail.com>
* set resource group ID for read request * replace group id with name
fix write metric
…cap#28) close pingcap#48899 Co-authored-by: Naman Gupta <naman.gupta@gmail.com>
Remove CGO requirement for compiling iconv-go currently is using.
Replace it with native implementation of Go.
make
test has passed, but test cases don't look very detailed onexpression
, so not sure if everything will go well.