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

Can EasyJSON support case insensitive match #237

Open
zhangruiskyline opened this issue Jun 24, 2019 · 12 comments · May be fixed by #372
Open

Can EasyJSON support case insensitive match #237

zhangruiskyline opened this issue Jun 24, 2019 · 12 comments · May be fixed by #372

Comments

@zhangruiskyline
Copy link

According to https://golang.org/pkg/encoding/json/#Unmarshal

Unmarshal matches incoming object keys to the keys used by Marshal (either the struct field name or its tag), preferring an exact match but also accepting a case-insensitive match

However, if we change to easyjson, it seems we need to be case sensitive, otherwise it fails to unmarshall, in our case, we want to make it can insensitive since our upstream may send both capital or lower case, can we do so in easy json?

@zhangruiskyline
Copy link
Author

zhangruiskyline commented Jun 24, 2019

I think the problem is slightly different than #10, what I am looking at is in json tag, like

type User struct {
	DID string `json:"DID ,omitempty"`
	SID string `json:"SID,omitempty"`
}

so if the upstream sends out like "sid" instead of "SID", default JSON can parse correctly but easyjson can not , even use -snake_case flag dose not help. any idea how to make it more unique?

@zhangruiskyline
Copy link
Author

re-surface this issue as it is quite significant blocking for us

@zhangruiskyline
Copy link
Author

@vstarodub saw your comments in #10 , but my question seems to be different, do you know whether easyjson can easily support this?

Thanks
Rui

@zhangruiskyline zhangruiskyline changed the title EasyJSON support case insensitive match Can EasyJSON support case insensitive match Jun 26, 2019
@rvasily
Copy link
Contributor

rvasily commented Jun 26, 2019

stdlib works in runtime, so it cant try to look various name for fields.

easyjson now generates code, which parse only one name per field. camelCase, snake_case or from struct tags.

To support parsing from various name we need to add this feature to generator. PR are welcome

@zhangruiskyline
Copy link
Author

zhangruiskyline commented Jun 26, 2019 via email

@zhangruiskyline
Copy link
Author

any idea when this can be prioritized? I would like to see it is quite high demand feature as lots of JSON situation we can not control counter partener behavior on case sensitive

Thanks
Rui

@shmel1k
Copy link
Contributor

shmel1k commented Aug 2, 2019

hm, case insensitive match is not a high priority feature, because it slows runtime too much. Too much allocations, unfortunately. So its better to use pre-defined json and not to have deal with insensitive case.

@zhangruiskyline
Copy link
Author

I understand, but actually I think you can provide as option, and just convert all keys to lower case in your current match instead keep the original. maybe in this way, the overhead will be minimum?

@shmel1k
Copy link
Contributor

shmel1k commented Aug 4, 2019

It can be achieved with some patches in generator. But the memory overhead is very large)

@zhangruiskyline
Copy link
Author

hmm, I am not seeing why memory overhead is large. in current parser you just get raw key, my suggestion is with this "case insensitive option", to a tolower(key) instead of directly use key.

anyway, if marshall/unmarshall speed can still beat general python, I am not that much worried about memory neither:)

@shmel1k
Copy link
Contributor

shmel1k commented Aug 4, 2019

I think youre always allowed to implement this feature)

cmaglie added a commit to cmaglie/easyjson that referenced this issue May 4, 2022
The standard library `encoding/json` allows it by default.

Fix mailru#237
@cmaglie cmaglie linked a pull request May 4, 2022 that will close this issue
cmaglie added a commit to cmaglie/easyjson that referenced this issue May 4, 2022
The standard library `encoding/json` allows it by default.

Fix mailru#237
@cmaglie
Copy link

cmaglie commented Aug 10, 2022

If someone is interested I've implemented it here: #372

In the arduino-cli project we are using the patch with success, it would be great to see it merged upstream.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants