Support for logical streaming replication #1271
Conversation
lib/connection-parameters.js
Outdated
| @@ -88,6 +89,9 @@ ConnectionParameters.prototype.getLibpqConnectionString = function(cb) { | |||
| if(this.database) { | |||
| params.push("dbname='" + this.database + "'"); | |||
| } | |||
| if(this.replication) { | |||
| params.push("replication='" + (this.database === true ? "true" : this.replication) + "'"); | |||
There was a problem hiding this comment.
"Replication" can have "database", "true" and "false" values.
This is checking this.database === true, though – not this.replication === true. Is that really intended? I’m not sure why dbname='true' (not replication='true') would have any special meaning beyond a database named “true”.
There was a problem hiding this comment.
Oh, sorry. I understand. You are right.
I'll change that code
There was a problem hiding this comment.
I changed this issue in last commit. Check please :)
lib/connection-parameters.js
Outdated
| @@ -88,6 +89,9 @@ ConnectionParameters.prototype.getLibpqConnectionString = function(cb) { | |||
| if(this.database) { | |||
| params.push("dbname='" + this.database + "'"); | |||
| } | |||
| if(this.replication) { | |||
| params.push("replication='" + (this.replication === true ? "true" : this.replication) + "'"); | |||
There was a problem hiding this comment.
Since true stringifies to "true", this can be "replication='" + this.replication + "'".
There was a problem hiding this comment.
I push the commit that accepted your advice :)
lib/client.js
Outdated
| @@ -222,6 +223,9 @@ Client.prototype.getStartupConf = function() { | |||
| if (appName) { | |||
| data.application_name = appName; | |||
| } | |||
| if (params.replication) { | |||
| data.replication = params.replication === true ? 'true' : params.replication; | |||
There was a problem hiding this comment.
Similarly: '' + params.replication
|
Hi @brianc , |
|
@kibae I'm excited for you to get your module out into the wild!! Don't forget to add it to the wiki! I'll go ahead & push a minor version bump right now with the changes in this PR. |
|
published @ 6.2.0! |
|
@brianc I changed dependencies of pg to 6.2.0 npm ERR! notarget No compatible version found: pg@6.2.0 |
|
@brainc It looks like 6.1.5 in npm yet. Can you check it? |
|
@brianc @charmander Please publish to npm 6.2.0~ |
Hello, @brianc @charmander
Please check the codes associated with this request. #1254
After merging, I will create a new project for logical replication.
Thanks