Fix double-sync crash on postgres 9.x

packages/pg/lib/query.js

@@ -96,39 +96,28 @@ class Query extends EventEmitter {
     }
   }
 
-  handleCommandComplete(msg, con) {
+  handleCommandComplete(msg, connection) {
     this._checkForMultirow()
     this._result.addCommandComplete(msg)
     // need to sync after each command complete of a prepared statement
-    if (this.isPreparedStatement) {
-      con.sync()
+    // if we were using a row count which results in multiple calls to _getRows
+    if (this.rows) {
+      connection.sync()
     }
   }
 
   // if a named prepared statement is created with empty query text
   // the backend will send an emptyQuery message but *not* a command complete message
-  // execution on the connection will hang until the backend receives a sync message
-  handleEmptyQuery(con) {
-    if (this.isPreparedStatement) {
-      con.sync()
-    }
-  }
-
-  handleReadyForQuery(con) {
-    if (this._canceledDueToError) {
-      return this.handleError(this._canceledDueToError, con)
-    }
-    if (this.callback) {
-      this.callback(null, this._results)
+  // since we pipeline sync immediately after execute we don't need to do anything here
+  // unless we have rows specified, in which case we did not pipeline the intial sync call
+  handleEmptyQuery(connection) {
+    if (this.rows) {
+      connection.sync()
     }
-    this.emit('end', this._results)
   }
 
   handleError(err, connection) {
     // need to sync after error during a prepared statement
-    if (this.isPreparedStatement) {
-      connection.sync()
-    }
     if (this._canceledDueToError) {
       err = this._canceledDueToError
       this._canceledDueToError = false
@@ -141,6 +130,16 @@ class Query extends EventEmitter {
     this.emit('error', err)
   }
 
+  handleReadyForQuery(con) {
+    if (this._canceledDueToError) {
+      return this.handleError(this._canceledDueToError, con)
+    }
+    if (this.callback) {
+      this.callback(null, this._results)
+    }
+    this.emit('end', this._results)
+  }
+
   submit(connection) {
     if (typeof this.text !== 'string' && typeof this.name !== 'string') {
       return new Error('A query must have either text or a name. Supplying neither is unsupported.')
@@ -173,7 +172,14 @@ class Query extends EventEmitter {
       portal: this.portal,
       rows: rows,
     })
-    connection.flush()
+    // if we're not reading pages of rows send the sync command
+    // to indicate the pipeline is finished
+    if (!rows) {
+      connection.sync()
+    } else {
+      // otherwise flush the call out to read more rows
+      connection.flush()
+    }
   }
 
   // http://developer.postgresql.org/pgdocs/postgres/protocol-flow.html#PROTOCOL-FLOW-EXT-QUERY

packages/pg/test/integration/client/prepared-statement-tests.js

@@ -174,5 +174,15 @@ var suite = new helper.Suite()
     checkForResults(query)
   })
 
+  suite.testAsync('with no data response and rows', async function () {
+    const result = await client.query({
+      name: 'some insert',
+      text: '',
+      values: [],
+      rows: 1,
+    })
+    assert.equal(result.rows.length, 0)
+  })
+
   suite.test('cleanup', () => client.end())
 })()

packages/pg/test/integration/gh-issues/1105-tests.js

@@ -0,0 +1,20 @@
+const pg = require('../../../lib')
+const helper = require('../test-helper')
+const suite = new helper.Suite()
+
+suite.testAsync('timeout causing query crashes', async () => {
+  const client = new helper.Client()
+  await client.connect()
+  await client.query('CREATE TEMP TABLE foobar( name TEXT NOT NULL, id SERIAL)')
+  await client.query('BEGIN')
+  await client.query("SET LOCAL statement_timeout TO '1ms'")
+  let count = 0
+  while (count++ < 5000) {
+    try {
+      await client.query('INSERT INTO foobar(name) VALUES ($1)', [Math.random() * 1000 + ''])
+    } catch (e) {
+      await client.query('ROLLBACK')
+    }
+  }
+  await client.end()
+})

packages/pg/test/integration/gh-issues/2085-tests.js

@@ -4,6 +4,12 @@ var assert = require('assert')
 
 const suite = new helper.Suite()
 
+// allow skipping of this test via env var for
+// local testing when you don't have SSL set up
+if (process.env.PGTESTNOSSL) {
+  return
+}
+
 suite.testAsync('it should connect over ssl', async () => {
   const ssl = helper.args.native
     ? 'require'