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

Add procedure support #41

Merged
merged 35 commits into from
May 11, 2016
Merged

Add procedure support #41

merged 35 commits into from
May 11, 2016

Conversation

calebmer
Copy link
Collaborator

@calebmer calebmer commented May 6, 2016

This PR adds support for PostgreSQL procedure execution. This will lead to a lot of awesome things like custom search functionality (#31) and authentication (#40) to name a few things that currently have open issues. This also adds a PostgreSQL native extension mechanism.

This PR is almost done, it has support for mutative procedures and query procedures, all that's missing right now is support computed column procedures.

There are a whole bunch of edge cases I'm probably missing so if you guys could try this out in your projects and report back the results that would be awesome 👍. I've published a prerelease on npm at version 1.3.0-beta.0.

Relevant files to get an understanding of how this include:

@calebmer
Copy link
Collaborator Author

calebmer commented May 8, 2016

Ok, the code for this is pretty much done! 🎉

Computed columns work great. Before I merge and publish this I'm going to write for comments from anybody, and write some docs.

If anyone is interested in trying this out, the code is published under 1.3.0-beta.1.

@calebmer
Copy link
Collaborator Author

calebmer commented May 8, 2016

One thing I haven’t decided on, is should procedures like forum_example.search_posts return a connection like postNodes or not? If so a user could paginate as normal over a set returned by the procedure which could be nice.

I'm also thinking about deprecating arguments testing equality in fields like postNodes (so deprecate the headline, topic, etc. arguments on that field). Thoughs?

@ghost
Copy link

ghost commented May 9, 2016

Re: whether procedures should return a connection: Yes, definitely, but how do you distinguish between procedures that behave like tables / return rows and others that are helper functions?

Other than that, it looks good!

@calebmer
Copy link
Collaborator Author

calebmer commented May 9, 2016

returns setof is the differentiator I think.

@calebmer
Copy link
Collaborator Author

Ok, implementing connections from setof procedures was more difficult than I thought. I'm going to leave this open so people can review if they want and I can write docs. Let’s get this merged quickly so we can iterate 👍

@calebmer
Copy link
Collaborator Author

Just finished writing documentation, so if you want to read to learn more about this feature/spell check take a look!

I'm super excited about this feature guys 😊

hasNextPage
}
totalCount
nodes {
Copy link

Choose a reason for hiding this comment

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

Should this be edges { node { headline, body } }?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

nodes is a little simpler and less intimidating for the README I think.

Copy link

Choose a reason for hiding this comment

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

Ah, but returns the same? I've not checked the implementation yet

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yep, it only returns the list, it's a shortcut if you're not interested in the cursors.

@calebmer calebmer merged commit 62f3f51 into master May 11, 2016
@calebmer calebmer deleted the feat/procedures branch May 11, 2016 11:30
@calebmer
Copy link
Collaborator Author

Released in 1.3.0 🎉

benjie added a commit that referenced this pull request Jan 27, 2020
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.

2 participants