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

Feature request: access field names and types even when NO rows are returned #209

Merged
merged 1 commit into from
Jun 5, 2013

Conversation

brianc
Copy link
Owner

@brianc brianc commented Jun 3, 2013

A common trick to get information about the structure of a query result is to run the query with a LIMIT 0 thus only getting the field names and types.

This trick seems to be impossible to do with the node-postgresql module because the only place where field names are available is within a result row, so no rows means no such info.

I hope I'm missing something. If not, this would be a really useful addition.
I've been thinking the information could be passed as part of the .end() event, with information being filed names and data types.

@brianc
Copy link
Owner

brianc commented Oct 25, 2012

I like this idea. Do you think you could write a simple failing test w/ the api you imagined? Usually helps me get a head start on new features. Doing this in javascript will be trivial. The trickier part will be having the native driver do the same thing, but should be doable.

@strk
Copy link
Contributor Author

strk commented Oct 26, 2012

I was about to look into this when I found a "rowDescription" event
in "simple-query-tests.js". Is a query going to send such event ?
Would seem to be what I was looking for, if it did

@strk
Copy link
Contributor Author

strk commented Oct 26, 2012

Actually, since we're at it, it would be also good to get field types too !

strk pushed a commit to CartoDB/node-postgres that referenced this pull request Oct 26, 2012
@strk
Copy link
Contributor Author

strk commented Oct 26, 2012

@brianc: check out git://github.com/CartoDB/node-postgres.git types
Haven't added a test for the event-based API, for that one there could be a new event, maybe the "rowDescription" I saw above (but haven't found tested)

@brianc
Copy link
Owner

brianc commented Nov 3, 2012

awesome - just saw this update. will check this out asap - been a crazy past week

@brianc
Copy link
Owner

brianc commented Dec 11, 2012

this is a pretty beefy change, but I like it. Will work on this when I get a bit more time. Just giving it a little bump. 💡

@whitelynx
Copy link
Contributor

👍

This change would make it possible to actually write a generic frontend for Postgres using Node.js, similar to phpPGAdmin.

The same API presented in that failing test can actually be used in conjunction with the row event as well, since the Result object is passed as the second parameter there, though the rowDescription event is also useful for the case where you're not getting any rows back.

@strk
Copy link
Contributor Author

strk commented May 17, 2013

Any news on this ?

@brianc
Copy link
Owner

brianc commented May 17, 2013

On accessing field names when no rows are returned?

@strk
Copy link
Contributor Author

strk commented May 17, 2013

both field names and field types

@jatorre
Copy link

jatorre commented May 17, 2013

Cant be more supportive of the idea. Needed for lot of instrospection

@brianc
Copy link
Owner

brianc commented May 17, 2013

k - working on it now

@strk
Copy link
Contributor Author

strk commented May 20, 2013

@brian let me know when this is ready for test, or if you need help, thank you

@brianc
Copy link
Owner

brianc commented May 20, 2013

@strk thanks! I got held up this weekend with a bunch of family coming in to town. This is next on my list for this week. ❤️

@strk
Copy link
Contributor Author

strk commented May 27, 2013

@brianc any progress here ?

@strk
Copy link
Contributor Author

strk commented May 30, 2013

Ok, I'm giving this a try, within today I should have a pull request ready.

@strk
Copy link
Contributor Author

strk commented May 30, 2013

In order to fix the test I produced earlier we should maintain a mapping between oid and type names as type names are not in the protocol (only oid). But now I'm not sure it is ok to maintain such mapping internally. The mapping could be different across different databases (especially for custom types). Maybe the library could expose a function to update the typeName database and provide a basic one...

@strk
Copy link
Contributor Author

strk commented May 30, 2013

or (cleaner) we just pass over the type oid and let the calling code do whatever they want with it

@strk
Copy link
Contributor Author

strk commented May 30, 2013

For some background info, my development database has ~750 types. Note that any table also becomes a type.

strk pushed a commit to CartoDB/node-postgres that referenced this pull request May 30, 2013
Adds a 'rowDescription' event in query object.
Add test for accessing field info via both event and result object
@whitelynx
Copy link
Contributor

Just passing the type's OID back to the user would work fine, as long as we provided a way to look up the type name that corresponds to a given OID... basically, provide lazily-loaded/cached type information whenever the user requests it.

Another possibility would be to provide an option which would cause node-postgres to automatically look up type names for any type OIDs it encounters in query results that it doesn't already have cached, but only if the user requests it. (since in the case of a cache miss, that would incur an extra query's worth of overhead, we wouldn't want that to be default behavior)

@strk
Copy link
Contributor Author

strk commented Jun 3, 2013

The cleanest way would be to just return oid and let caller do any lookup/caching...
@brianc : news about your half-working branch ?

@brianc
Copy link
Owner

brianc commented Jun 3, 2013

Yeah returning just the OID is the way to go. There's a query which can be run to pull the mapping from type to name out of the database if a client is interested. I'll push it when it's finished.

Closes #209
Native implementation requires significant refactor and so I wont work on this
if/until there is an issue for it
@brianc
Copy link
Owner

brianc commented Jun 3, 2013

@strk this should solve your issue, yes?

@strk
Copy link
Contributor Author

strk commented Jun 3, 2013

Yes, I think that's good.

@brianc
Copy link
Owner

brianc commented Jun 5, 2013

💃 yay! merging & pushing new version now

brianc added a commit that referenced this pull request Jun 5, 2013
Feature request: access field names and types even when NO rows are returned
@brianc brianc merged commit 4f8cce4 into master Jun 5, 2013
@brianc brianc deleted the issues/209 branch June 5, 2013 02:16
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.

4 participants