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

Made SPI query arguments type safe #1858

Merged

Conversation

YohDeadfall
Copy link
Contributor

No description provided.

@eeeebbbbrrrr
Copy link
Contributor

&[][..] is not something you see every day. Why is it that &[] doesn't work?

@YohDeadfall
Copy link
Contributor Author

I guess it's leftover from the time when I tried to play with generics and avoid a breaking change. That was required by the compiler for the type deduction, but can be reduced now.

@eeeebbbbrrrr
Copy link
Contributor

I appreciate your efforts on this area of pgrx, btw.

Spi is incredibly hard to get right in C, and it’s not any easier in Rust.

Copy link
Contributor

@eeeebbbbrrrr eeeebbbbrrrr left a comment

Choose a reason for hiding this comment

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

This is really nice, @YohDeadfall. Really nice.

Very clean.

Like @workingjubilee said somewhere, I think we'd want to hold this for v0.13, but I think this PR is a motivator for getting there quickly.

@workingjubilee
Copy link
Member

@YohDeadfall Can you resolve the conflicts?

@YohDeadfall
Copy link
Contributor Author

Sure! Actually, I just wanted to write you about this pull request too (:

Copy link
Member

@workingjubilee workingjubilee left a comment

Choose a reason for hiding this comment

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

Thanks!

@workingjubilee workingjubilee merged commit ae0335b into pgcentralfoundation:develop Oct 28, 2024
4 checks passed
@YohDeadfall YohDeadfall deleted the type-safe-query branch October 28, 2024 09:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants