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

Set database on socket string connection #484

Closed
aurium opened this issue Dec 15, 2013 · 2 comments · Fixed by #487
Closed

Set database on socket string connection #484

aurium opened this issue Dec 15, 2013 · 2 comments · Fixed by #487

Comments

@aurium
Copy link
Contributor

aurium commented Dec 15, 2013

node-postgres allows to connect using sockets in a connection string like this:
pg.connect('/some/path', callback)
That is nice, but does not allows to set another database but one with the same user name.

I propose to accept a special protocol named socket to enable the database selection like this:
pg.connect('socket:/some/path?database', callback)
The url.parse('socket:/some/path?database') returns:

{ protocol: 'socket:',
  host: '',
  query: 'database',
  pathname: '/some/path',
  ...}

That data is enough to make the magic, with a small change on function parse at connection-parameters.js. (and also my favorite solution)

That can not change the current behavior for sockets when the first char is "/".


I also try a change on current sockets behavior, splitting the string on spaces. That makes easy to set a database by this way:
pg.connect('/some/path database', callback)

pg/lib/connection-parameters.js
*** 14,20 ****
  var parse = function(str) {
    //unix socket
    if(str.charAt(0) === '/') {
-     return { host: str };
    }
--- 14,21 ----
  var parse = function(str) {
    //unix socket
    if(str.charAt(0) === '/') {
+     var config = str.split(' ');
+     return { host: config[0], database: config[1] };
    }

What you think? node-postgres can use the solution 1, 2, or a mix of both?

I can also code it, if you like.

@brianc
Copy link
Owner

brianc commented Dec 19, 2013

I think that is awesome! 💃 If you code it & it is tested I will merge it! 👍

@aurium
Copy link
Contributor Author

aurium commented Dec 19, 2013

Ok! Done! :-)
See pull request #487

@aurium aurium closed this as completed Dec 19, 2013
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 a pull request may close this issue.

2 participants