Merge pull request #23 from PostHog/fix/nested-begin-warning

Handle nested BEGIN with warning instead of error

Author: EDsCODE

server/conn.go

@@ -292,6 +292,16 @@ func (c *clientConn) handleQuery(body []byte) error {
 
 	// For non-SELECT queries, use Exec
 	if cmdType != "SELECT" {
+		// Handle nested BEGIN: PostgreSQL issues a warning but continues,
+		// while DuckDB throws an error. Match PostgreSQL behavior.
+		if cmdType == "BEGIN" && c.txStatus == txStatusTransaction {
+			c.sendNotice("WARNING", "25001", "there is already a transaction in progress")
+			writeCommandComplete(c.writer, "BEGIN")
+			writeReadyForQuery(c.writer, c.txStatus)
+			c.writer.Flush()
+			return nil
+		}
+
 		result, err := c.db.Exec(query)
 		if err != nil {
 			c.sendError("ERROR", "42000", err.Error())
@@ -902,6 +912,11 @@ func (c *clientConn) sendError(severity, code, message string) {
 	c.writer.Flush()
 }
 
+func (c *clientConn) sendNotice(severity, code, message string) {
+	writeNoticeResponse(c.writer, severity, code, message)
+	// Don't flush here - let the caller decide when to flush
+}
+
 // Extended query protocol handlers
 
 func (c *clientConn) handleParse(body []byte) {
@@ -1228,6 +1243,14 @@ func (c *clientConn) handleExecute(body []byte) {
 	log.Printf("[%s] Execute %q with %d params: %s", c.username, portalName, len(args), p.stmt.query)
 
 	if cmdType != "SELECT" {
+		// Handle nested BEGIN: PostgreSQL issues a warning but continues,
+		// while DuckDB throws an error. Match PostgreSQL behavior.
+		if cmdType == "BEGIN" && c.txStatus == txStatusTransaction {
+			c.sendNotice("WARNING", "25001", "there is already a transaction in progress")
+			writeCommandComplete(c.writer, "BEGIN")
+			return
+		}
+
 		// Non-SELECT: use Exec with converted query
 		result, err := c.db.Exec(p.stmt.convertedQuery, args...)
 		if err != nil {

server/conn_test.go

@@ -332,3 +332,29 @@ func TestTransactionErrorStatus(t *testing.T) {
 		t.Errorf("after ROLLBACK from error txStatus = %c, want %c", c.txStatus, txStatusIdle)
 	}
 }
+
+func TestNestedBeginDetection(t *testing.T) {
+	// Test that we can detect when a nested BEGIN would occur
+	// The actual warning is sent in handleQuery, but we test the detection logic here
+	c := &clientConn{txStatus: txStatusIdle}
+
+	// First BEGIN should work normally
+	c.updateTxStatus("BEGIN")
+	if c.txStatus != txStatusTransaction {
+		t.Errorf("after first BEGIN txStatus = %c, want %c", c.txStatus, txStatusTransaction)
+	}
+
+	// At this point, a second BEGIN should trigger warning behavior
+	// In handleQuery, when cmdType == "BEGIN" && c.txStatus == txStatusTransaction,
+	// we send a warning and return success without calling DuckDB
+	isNestedBegin := c.txStatus == txStatusTransaction
+	if !isNestedBegin {
+		t.Error("expected nested BEGIN to be detected")
+	}
+
+	// Transaction status should remain 'T' (not change to 'I' or 'E')
+	// The warning is sent but the transaction continues
+	if c.txStatus != txStatusTransaction {
+		t.Errorf("txStatus should still be %c after nested BEGIN detection, got %c", txStatusTransaction, c.txStatus)
+	}
+}

server/protocol.go

@@ -223,6 +223,32 @@ func writeErrorResponse(w io.Writer, severity, code, message string) error {
 	return writeMessage(w, msgErrorResponse, data)
 }
 
+// writeNoticeResponse sends a notice/warning to the client
+// Unlike errors, notices are informational and don't terminate the command
+func writeNoticeResponse(w io.Writer, severity, code, message string) error {
+	var data []byte
+
+	// Severity (WARNING, NOTICE, INFO, DEBUG, LOG)
+	data = append(data, 'S')
+	data = append(data, []byte(severity)...)
+	data = append(data, 0)
+
+	// SQLSTATE code
+	data = append(data, 'C')
+	data = append(data, []byte(code)...)
+	data = append(data, 0)
+
+	// Message
+	data = append(data, 'M')
+	data = append(data, []byte(message)...)
+	data = append(data, 0)
+
+	// Terminator
+	data = append(data, 0)
+
+	return writeMessage(w, msgNoticeResponse, data)
+}
+
 // writeCommandComplete sends a command completion message
 func writeCommandComplete(w io.Writer, tag string) error {
 	data := []byte(tag)