-
Notifications
You must be signed in to change notification settings - Fork 962
use strconv.FormatInt for string id #634
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
use strconv.FormatInt for string id #634
Conversation
Hi @stevenferrer! Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA. If you have received this in error or have any questions, please contact us at cla@fb.com. Thanks! |
Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks! |
Hey @stevenferrer and thanks for your contribution. Can you give more context for this PR? I assume it's related to previous functionality mentioned here - #127 (comment) |
Hi @a8m, likewise, thank you very much for this amazing project! I just want to address a small issue when using The difference demonstrated below: Current: I'm not sure though if #127 is related. |
Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks! |
Can you elaborate how do you use As mentioned in the linked comment:
|
An example is when you use brown := client.User.Create().SetName("brown").SaveX(ctx)
b, _ := json.Marshal(brown)
fmt.Println(string(b)) Outputs: {"id":"\u0001","name":"brown"} |
Thanks for sharing the example @stevenferrer. Can you please answer my previous comment to help me get more context?
|
We're using For example, the below table: CREATE TABLE address (
id bigserial NOT NULL,
unit_no varchar NOT NULL,
barangay varchar NOT NULL,
province varchar NOT NULL,
city varchar NOT NULL,
updated_at timestamp,
created_at timestamp NOT NULL DEFAULT now()
); is mapped to this struct: type Address struct {
ID string `json:"id"`
UnitNo string `json:"unitNo"`
Barangay string `json:"barangay"`
Province string `json:"province"`
City string `json:"city"`
UpdatedAt *time.Time `json:"-"`
CreatedAt *time.Time `json:"createdAt"`
} |
Thanks for sharing this information. There's no way to auto-map numeric value to string type in ent. Your PR fixes one problem with this, but there are so many other places that need to be changed in order to support this. The GoType option is used exactly for cases like this, but it doesn't support ID fields as the moment. Once I'll add the support for this, you will be able to use it as follows: type StringID string // Need to implement the schema/field.ValueScanner interface.
func (User) Fields() []ent.Field {
return []ent.Field{
field.Int("id").
GoType(new(StringID)),
}
} If you don't want to be blocked on this, you can use external-templates for now (to override the existing behavior) and update your code in the future. |
Ok, got it. Will probably use a workaround for now, thanks for the suggestion! Closing this PR then. |
I'll update this PR with the new API soon @stevenferrer. |
Use
strconv.FormatInt
instead of type casting string ID