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

Adding support for pg-native #4327

Merged
merged 6 commits into from Sep 6, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/coverage.yml
Expand Up @@ -39,7 +39,7 @@ jobs:
run: npm run test:coverage
env:
CI: true
DB: "postgres mysql mysql2 mssql sqlite3"
DB: "postgres pgnative mysql mysql2 mssql sqlite3"
KNEX_TEST_TIMEOUT: 60000

- name: Stop Databases
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/integration-tests.yml
Expand Up @@ -19,7 +19,7 @@ jobs:
fail-fast: false
matrix:
node-version: [16.x, 14.x, 12.x, 10.x]
database-type: [postgres, mysql, mssql, sqlite3]
database-type: [postgres, pgnative, mysql, mssql, sqlite3]

steps:
- name: Checkout Repository
Expand Down
2 changes: 2 additions & 0 deletions lib/constants.js
Expand Up @@ -12,6 +12,7 @@ const SUPPORTED_CLIENTS = Object.freeze(
'mysql2',
'oracledb',
'postgres',
'pgnative',
'redshift',
'sqlite3',
].concat(Object.keys(CLIENT_ALIASES))
Expand All @@ -23,6 +24,7 @@ const DRIVER_NAMES = Object.freeze({
MySQL2: 'mysql2',
Oracle: 'oracledb',
PostgreSQL: 'pg',
PgNative: 'pgnative',
Redshift: 'pg-redshift',
SQLite: 'sqlite3',
});
Expand Down
59 changes: 59 additions & 0 deletions lib/dialects/pgnative/index.js
@@ -0,0 +1,59 @@
// PostgreSQL Native Driver (pg-native)
// -------
const Client_PG = require('../postgres');
machuga marked this conversation as resolved.
Show resolved Hide resolved

class Client_PgNative extends Client_PG {
constructor(...args) {
super(...args);
this.driverName = 'pgnative';
this.canCancelQuery = true;
}

_driver() {
return require('pg').native;
}

_stream(connection, obj, stream, options) {
if (!obj.sql) throw new Error('The query is empty');

const client = this;
return new Promise((resolver, rejecter) => {
stream.on('error', rejecter);
stream.on('end', resolver);

return client
._query(connection, obj)
.then((obj) => obj.response)
.then(({ rows }) => rows.forEach((row) => stream.write(row)))
.catch(function (err) {
stream.emit('error', err);
})
.then(function () {
stream.end();
});
});
}

async cancelQuery(connectionToKill) {
try {
return await this._wrappedCancelQueryCall(null, connectionToKill);
} catch (err) {
this.logger.warn(`Connection Error: ${err}`);
}
}

_wrappedCancelQueryCall(emptyConnection, connectionToKill) {
return new Promise(function (resolve, reject) {
connectionToKill.native.cancel(function (err) {
if (err) {
reject(err);
return;
}

resolve(true);
});
});
}
}

module.exports = Client_PgNative;
30 changes: 18 additions & 12 deletions lib/dialects/postgres/index.js
Expand Up @@ -62,34 +62,40 @@ class Client_PG extends Client {
return `"${value.replace(/"/g, '""')}"${arrayAccessor}`;
}

_acquireOnlyConnection() {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why was introduction of this method and modification of acquireRawConnection necessary if pgnative doesn't override either?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This goes back to connect using the deprecated (at least according to docs) callback signature where the same connection is returned: https://github.com/brianc/node-postgres/blob/master/packages/pg/lib/client.js#L279

The native client doesn't do this. The mocking was dependent on this behavior, which is now undocumented, so I moved it toward being compliant with the current node-postgres documentation + compatible with pg-native

Copy link
Contributor Author

Choose a reason for hiding this comment

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

const connection = new this.driver.Client(this.connectionSettings);

return connection.connect().then(() => connection);
}

// Get a raw connection, called by the `pool` whenever a new
// connection needs to be added to the pool.
acquireRawConnection() {
const client = this;
return new Promise(function (resolver, rejecter) {
const connection = new client.driver.Client(client.connectionSettings);
connection.connect(function (err, connection) {
if (err) {
return rejecter(err);
}

return this._acquireOnlyConnection()
.then(function (connection) {
connection.on('error', (err) => {
connection.__knex__disposed = err;
});

connection.on('end', (err) => {
connection.__knex__disposed = err || 'Connection ended unexpectedly';
});

if (!client.version) {
return client.checkVersion(connection).then(function (version) {
client.version = version;
resolver(connection);
return connection;
});
}
resolver(connection);

return connection;
})
.then(function setSearchPath(connection) {
client.setSchemaSearchPath(connection);
return connection;
});
}).then(function setSearchPath(connection) {
client.setSchemaSearchPath(connection);
return connection;
});
}

// Used to explicitly close a connection, called internally by the pool
Expand Down
8 changes: 8 additions & 0 deletions package.json
Expand Up @@ -29,13 +29,16 @@
"test:oracledb": "cross-env DB=oracledb npm run test:db",
"test:sqlite": "cross-env DB=sqlite3 npm run test:db",
"test:postgres": "cross-env DB=postgres npm run test:db",
"test:pgnative": "cross-env DB=pgnative npm run test:db",
"test:tape": "node test/tape/index.js | tap-spec",
"test:cli": "cross-env KNEX_PATH=../knex.js KNEX=bin/cli.js jake -f test/jake/Jakefile",
"db:start": "docker-compose -f scripts/docker-compose.yml up --build -d mysql oracledbxe postgres mssql && docker-compose -f scripts/docker-compose.yml up waitmssql waitmysql waitpostgres waitoracledbxe",
"db:start:no-oracle": "docker-compose -f scripts/docker-compose.yml up --build -d mysql postgres mssql && docker-compose -f scripts/docker-compose.yml up waitmssql waitmysql waitpostgres",
"db:stop": "docker-compose -f scripts/docker-compose.yml down",
"db:start:postgres": "docker-compose -f scripts/docker-compose.yml up --build -d postgres && docker-compose -f scripts/docker-compose.yml up waitpostgres",
"db:stop:postgres": "docker-compose -f scripts/docker-compose.yml down",
"db:start:pgnative": "docker-compose -f scripts/docker-compose.yml up --build -d pgnative && docker-compose -f scripts/docker-compose.yml up waitpgnative",
"db:stop:pgnative": "docker-compose -f scripts/docker-compose.yml down",
"db:start:mysql": "docker-compose -f scripts/docker-compose.yml up --build -d mysql && docker-compose -f scripts/docker-compose.yml up waitmysql",
"db:stop:mysql": "docker-compose -f scripts/docker-compose.yml down",
"db:start:oracle": "docker-compose -f scripts/docker-compose.yml up --build -d oracledbxe && docker-compose -f scripts/docker-compose.yml up waitoracledbxe",
Expand Down Expand Up @@ -72,6 +75,9 @@
"pg": {
"optional": true
},
"pg-native": {
"optional": true
},
"sqlite3": {
"optional": true
}
Expand Down Expand Up @@ -105,6 +111,7 @@
"nyc": "^15.1.0",
"oracledb": "^5.2.0",
"pg": "^8.7.1",
"pg-native": "^3.0.0",
"pg-query-stream": "^4.2.1",
"prettier": "2.3.2",
"rimraf": "^3.0.2",
Expand Down Expand Up @@ -177,6 +184,7 @@
"mysql": false,
"mysql2": false,
"pg": false,
"pg-native": false,
"pg-query-stream": false,
"oracle": false,
"sqlite3": false,
Expand Down
21 changes: 21 additions & 0 deletions scripts/docker-compose.yml
Expand Up @@ -84,6 +84,27 @@ services:
- -c
- 'until /usr/local/bin/psql postgres://testuser:knextest@postgres/knex_test -c "SELECT 1"; do sleep 5; done'

pgnative:
image: postgres:alpine
# see https://www.postgresql.org/docs/current/non-durability.html
command: '-c full_page_writes=off -c fsync=off -c synchronous_commit=off'
ports:
- '25433:5432'
environment:
- POSTGRES_USER=testuser
- POSTGRES_PASSWORD=knextest
- POSTGRES_DB=knex_test
waitpgnative:
image: postgres:alpine
links:
- pgnative
depends_on:
- pgnative
entrypoint:
- bash
- -c
- 'until /usr/local/bin/psql postgres://testuser:knextest@pgnative/knex_test -c "SELECT 1"; do sleep 5; done'

oracledbxe:
image: quillbuilduser/oracle-18-xe
container_name: oracledbxe_container
Expand Down
10 changes: 10 additions & 0 deletions scripts/stress-test/docker-compose.yml
Expand Up @@ -7,11 +7,13 @@ services:
- "8474:8474"
- "23306:23306"
- "25432:25432"
- "25433:25433"
- "21521:21521"
- "21433:21433"
links:
- "mysql"
- "postgresql"
- "pgnative"
- "oracledbxe"
- "mssql"

Expand All @@ -31,6 +33,14 @@ services:
- POSTGRES_PASSWORD=postgresrootpassword
- POSTGRES_USER=postgres

pgnative:
image: mdillon/postgis
Copy link
Collaborator

Choose a reason for hiding this comment

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

what's the advantage of using this image over vanilla pg?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not too sure if there are technical advantages, but the postgresql strategy uses it above on line 27. I assumed there was a good reason for it. For benefit of pg-native development, it makes debugging easier by ensuring there aren't any odd bugs because of configuration issues across two different images.

ports:
- "35433:5432"
environment:
- POSTGRES_PASSWORD=postgresrootpassword
- POSTGRES_USER=postgres

oracledbxe:
image: wnameless/oracle-xe-11g
ports:
Expand Down
20 changes: 16 additions & 4 deletions scripts/stress-test/knex-stress-test.js
Expand Up @@ -13,6 +13,13 @@ const pg = Knex({
pool: { max: 50 },
});

const pgnative = Knex({
client: 'pgnative',
connection:
'postgres://postgres:postgresrootpassword@localhost:25433/postgres',
pool: { max: 50 },
});

const mysql2 = Knex({
client: 'mysql2',
connection:
Expand Down Expand Up @@ -91,10 +98,10 @@ setInterval(() => {
lastCounters = _.cloneDeep(counters);
}, 2000);

async function killConnectionsPg() {
return pg.raw(`SELECT pg_terminate_backend(pg_stat_activity.pid)
async function killConnectionsPg(client) {
return client.raw(`SELECT pg_terminate_backend(pg_stat_activity.pid)
FROM pg_stat_activity
WHERE pg_stat_activity.datname = 'postgres'
WHERE pg_stat_activity.datname = 'postgres'
AND pid <> pg_backend_pid()`);
}

Expand Down Expand Up @@ -154,6 +161,7 @@ async function main() {
// create TCP proxies for simulating bad connections etc.
async function recreateProxies() {
await recreateProxy('postgresql', 25432, 5432);
await recreateProxy('postgresql', 25433, 5433);
await recreateProxy('mysql', 23306, 3306);
await recreateProxy('oracledbxe', 21521, 1521);
await recreateProxy('mssql', 21433, 1433);
Expand All @@ -164,6 +172,9 @@ async function main() {
loopQueries('PSQL:', pg.raw('select 1'));
loopQueries('PSQL TO:', pg.raw('select 1').timeout(20));

loopQueries('PGNATIVE:', pgnative.raw('select 1'));
loopQueries('PGNATIVE TO:', pgnative.raw('select 1').timeout(20));

loopQueries('MYSQL:', mysql.raw('select 1'));
loopQueries('MYSQL TO:', mysql.raw('select 1').timeout(20));

Expand All @@ -181,7 +192,8 @@ async function main() {
await delay(20); // kill everything every quite often from server side
try {
await Promise.all([
killConnectionsPg(),
killConnectionsPg(pg),
killConnectionsPg(pgnative),
killConnectionsMyslq(mysql),
// killConnectionsMyslq(mysql2),
killConnectionsMssql(),
Expand Down