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

protoc-gen-ent: support timestamp and date #591

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

simon-wenmouth
Copy link

Use of types google.protobuf.Timestamp and google.type.Date currently panics because they are MessageType but not edges.

panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x30 pc=0xae904d]

goroutine 1 [running]:
main.toEdge(0xc0003d1ba0)
... entgo.io/[email protected]/entproto/cmd/protoc-gen-ent/main.go:146 +0x8d

The method isEdge was updated to recognize these types as fields, and the method toField was updated to map them as field.Time.

Use of types `google.protobuf.Timestamp` and `google.type.Date`
currently panics because they are MessageType but not edges.

> panic: runtime error: invalid memory address or nil pointer dereference
> [signal SIGSEGV: segmentation violation code=0x1 addr=0x30 pc=0xae904d]
>
> goroutine 1 [running]:
> main.toEdge(0xc0003d1ba0)
> ... entgo.io/[email protected]/entproto/cmd/protoc-gen-ent/main.go:146 +0x8d

The method `isEdge` was updated to recognize these types as fields, and the
method `toField` was updated to map them as `field.Time`.
@simon-wenmouth
Copy link
Author

The tool protoc-gen-ent does not currently have support for time fields. I added mappings for google.protobuf.timestamp and google.type.date because they have a mapping to field.Time. I did not add a mapping for google.type.timeofday (e.g. SQL time) because ent does not currently support these "civil" date/time types. If that is of interest, I'm willing to implement this feature and discuss the design with your team. At a high level I expect this would mean adding field.CivilDate, field.CivilTime and field.CivilDateTime with the underlying structs coming from either cloud.google.com/go/civil or golang-sql/civil, with corresponding updates to protoc-gen-ent afterwards.

I think this PR duplicates in part the work in #447 (March 2023), which has not received feedback.

I welcome your feedback, and look forward to the possibility of the feature getting merged in the future.

Thanks!

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

Successfully merging this pull request may close these issues.

1 participant