add test and improve doc

Author: songgao

README.md

@@ -276,18 +276,20 @@ Default:        false
 ```
 
 
-RejectreadOnly causes mysql driver to reject read-only connections. This is
-specifically for AWS Aurora: During a failover, there seems to be a race
-condition on Aurora, where we get connected to the [old master before
-failover], i.e. the [new read-only slave after failover].
-
-Note that this should be a fairly rare case, as automatic failover normally
-happens when master is down, and the race condition shouldn't happen unless it
-comes back up online as soon as the failover is kicked off. But it's pretty
-easy to reproduce using a manual failover. In case this happens, we should
-reconnect to the Aurora cluster by returning a driver.ErrBadConnection.
-
-tl;dr: Set this if you are using Aurora.
+RejectreadOnly causes the driver to reject read-only connections. This is for a
+possible race condition during an automatic failover, where the mysql client
+gets connected to a read-only replica after the failover. 
+
+Note that this should be a fairly rare case, as an automatic failover normally
+happens when the primary is down, and the race condition shouldn't happen
+unless it comes back up online as soon as the failover is kicked off. On the
+other hand, when this happens, an mysql application can get stuck on a
+read-only connection until restarted. It is however fairly easy to reproduce,
+for example, using a manual failover on AWS Aurora's MySQL-compatible cluster.
+
+If you are not relying on read-only transactions to reject writes that aren't
+supposed to happen, setting this on some MySQL providers (such as AWS Aurora)
+is safer for failovers.
 
 
 ##### `strict`

driver_test.go

@@ -171,6 +171,17 @@ func (dbt *DBTest) mustQuery(query string, args ...interface{}) (rows *sql.Rows)
 	return rows
 }
 
+func maybeSkip(t *testing.T, err error, skipErrno uint16) {
+	mySQLErr, ok := err.(*MySQLError)
+	if !ok {
+		return
+	}
+
+	if mySQLErr.Number == skipErrno {
+		t.Skipf("skipping test as SET SESSION TRANSACTION is not supported")
+	}
+}
+
 func TestEmptyQuery(t *testing.T) {
 	runTests(t, dsn, func(dbt *DBTest) {
 		// just a comment, no query
@@ -1168,11 +1179,9 @@ func TestStrict(t *testing.T) {
 	if conn != nil {
 		conn.Close()
 	}
-	if me, ok := err.(*MySQLError); ok && me.Number == 1231 {
-		// Error 1231: Variable 'sql_mode' can't be set to the value of 'ALLOW_INVALID_DATES'
-		// => skip test, MySQL server version is too old
-		return
-	}
+	// Error 1231: Variable 'sql_mode' can't be set to the value of
+	// 'ALLOW_INVALID_DATES' => skip test, MySQL server version is too old
+	maybeSkip(t, err, 1231)
 	runTests(t, relaxedDsn, func(dbt *DBTest) {
 		dbt.mustExec("CREATE TABLE test (a TINYINT NOT NULL, b CHAR(4))")
 
@@ -1949,3 +1958,35 @@ func TestColumnsReusesSlice(t *testing.T) {
 		t.Fatalf("expected columnNames to be set, got nil")
 	}
 }
+
+func TestRejectReadOnly(t *testing.T) {
+	runTests(t, dsn, func(dbt *DBTest) {
+		// Create Table
+		dbt.mustExec("CREATE TABLE test (value BOOL)")
+		// Set the session to read-only. We didn't set the `rejectReadOnly`
+		// option, so any writes after this should fail.
+		_, err := dbt.db.Exec("SET SESSION TRANSACTION READ ONLY")
+		// Error 1193: Unknown system variable 'TRANSACTION'
+		maybeSkip(t, err, 1193)
+		if _, err := dbt.db.Exec("DROP TABLE test"); err == nil {
+			t.Fatalf("writing to DB in read-only session without " +
+				"rejectReadOnly did not error")
+		}
+		// Set the session back to read-write so runTests() can properly clean
+		// up the table `test`.
+		dbt.mustExec("SET SESSION TRANSACTION READ WRITE")
+	})
+
+	// Enable the `rejectReadOnly` option.
+	runTests(t, dsn+"&rejectReadOnly=true", func(dbt *DBTest) {
+		// Create Table
+		dbt.mustExec("CREATE TABLE test (value BOOL)")
+		// Set the session to read only. Any writes after this should error on
+		// a driver.ErrBadConn, and cause `database/sql` to initiate a new
+		// connection.
+		dbt.mustExec("SET SESSION TRANSACTION READ ONLY")
+		// This would error, but `database/sql` should automatically retry on a
+		// new connection which is not read-only, and eventually succeed.
+		dbt.mustExec("DROP TABLE test")
+	})
+}

packets.go

@@ -556,8 +556,8 @@ func (mc *mysqlConn) handleErrorPacket(data []byte) error {
 		// Oops; we are connected to a read-only connection, and won't be able
 		// to issue any write statements. Since RejectReadOnly is configured,
 		// we throw away this connection hoping this one would have write
-		// permission. This is specifically for an AWS Aurora behavior during
-		// failover. See README.md for more.
+		// permission. This is specifically for a possible race condition
+		// during failover (e.g. on AWS Aurora). See README.md for more.
 		//
 		// We explicitly close the connection before returning
 		// driver.ErrBadConn to ensure that `database/sql` purges this