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

feat: support geometry #3449

Open
wants to merge 64 commits into
base: main
Choose a base branch
from

Conversation

jayy-lmao
Copy link
Contributor

@jayy-lmao jayy-lmao commented Aug 21, 2024

fixes #166

If we're happy with the way I'm implementing point (and maybe line/lseg), I will continue with other types. (Got carried away a little, fun problem. Any changes requested with one type I'm more than happy to copy it across to others)

@jayy-lmao jayy-lmao marked this pull request as ready for review August 21, 2024 09:56
@jayy-lmao jayy-lmao changed the title feaet: support geometry feat: support geometry Aug 21, 2024
Copy link
Contributor

@CommanderStorm CommanderStorm left a comment

Choose a reason for hiding this comment

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

Hi, I have commented a few things inline ^^
Overall, looks quite similar to PgCube

sqlx-postgres/src/types/point.rs Outdated Show resolved Hide resolved
sqlx-postgres/src/types/point.rs Outdated Show resolved Hide resolved
sqlx-postgres/src/types/point.rs Outdated Show resolved Hide resolved
@jayy-lmao
Copy link
Contributor Author

Hi, I have commented a few things inline ^^ Overall, looks quite similar to PgCube

Ty for taking the time to review!
Haha that'd be because I wrote PgCube

@jayy-lmao
Copy link
Contributor Author

jayy-lmao commented Aug 24, 2024

Just noticed a bunch of changes to sqlx-postgres/src/types/cube.rs.

As this was based on cube.rs before changes, I will backport it to these changes for consistency.
Until then I will make this a draft again.

@jayy-lmao jayy-lmao force-pushed the feat/support-geometry-postgres branch from 2d7b88d to d16c98e Compare August 24, 2024 10:42
@jayy-lmao jayy-lmao marked this pull request as draft August 24, 2024 10:53
@jayy-lmao jayy-lmao marked this pull request as ready for review August 28, 2024 11:21
@jayy-lmao
Copy link
Contributor Author

Hi, I have left a few suggestions below. For such an humongous PR, reviewing is quite difficult

General disclaimer: I am not a maintainer, just a fellow contributor

I agree it is pretty large, appreciate your patience with reviewing it.

@abonander if you prefer I can split it up into chunks to review instead. Realise my cube work may have left you with some headache.

@martinhamel
Copy link

Is that still alive? I would really need it for a project.

Thanks for the work !

@CommanderStorm
Copy link
Contributor

The PR has been raised a month ago and is a 2k lines humongous PR.
If this PR takes longer, that is to be expected.

You can always patch your cargo dependencys (docs: https://doc.rust-lang.org/cargo/reference/overriding-dependencies.html), but that comes (obviously) at some risk

@jayy-lmao
Copy link
Contributor Author

I'll break up the size and do a component at a time for reviewers, will carry over feedback that's been received so far.
Will close / migrate to draft once the other PRs are up.

@abonander
Copy link
Collaborator

I'll break up the size and do a component at a time for reviewers, will carry over feedback that's been received so far.
Will close / migrate to draft once the other PRs are up.

That would be great, thank you. 2k lines is a lot to go through with the time I have to spend on SQLx.

@jayy-lmao jayy-lmao mentioned this pull request Oct 30, 2024
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.

Support geometric types for postgres
4 participants