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 support for describing prepared statements #1968

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

Conversation

akheron
Copy link

@akheron akheron commented Sep 6, 2019

Describing a prepared statement is useful determining inferred input parameter types and result column types for prepared statements.

Usage:

client.query({
  text: 'your statement here',
  describe: true
}).then(result => {
  // result.fields contains the output fields as usual
  // result.params (added in this PR) contains the OIDs of input parameters
})

When describe is given, the query is not bound or executed at all. The only commands sent to Posgres are Parse, Describe and Flush. Sync will be sent after the ParameterDescription message is obtained .

@akheron
Copy link
Author

akheron commented Sep 6, 2019

Tests fail because it seems I still need to add support to native.

Before doing that, it would be nice to know whether this is a desirable feature or not in the first place :)

@brianc
Copy link
Owner

brianc commented Nov 11, 2019

Sorry for the delay - I like this approach. I'm cool releasing functionality in the non-native bindings w/o requiring it to be released in the native bindings as well...though it's always nice to have as much parity as possible. I'm down with merging this & doing a new minor version bump w/ the new functionality later this week. Mind rebasing on master? The tests should pass now.

@akheron
Copy link
Author

akheron commented Nov 12, 2019

Rebased on master, but the native tests still seem to fail 🤔 Did you do something that should prevent them from failing?

@charmander
Copy link
Collaborator

I think a more explicit and restricted API would be better (Client#describe(…)), since there’s not enough overlap with querying with regard to what it expects and how it behaves to justify making it an option. Plus it doesn’t get ignored by pg-native/older versions of pg and do something completely different. If it has to be implemented as a secret _describe: true option on query, that’s okay.

@charmander
Copy link
Collaborator

(It doing something completely different on pg-native is exactly why the tests are failing, too.)

@akheron akheron force-pushed the describe-prepared branch from ad03785 to 5a3994a Compare May 8, 2020 04:41
@akheron akheron force-pushed the describe-prepared branch from 5a3994a to a776c84 Compare May 9, 2020 04:36
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.

3 participants