Fix missing RowDescription for result-returning queries

This fixes the Fivetran error "Received resultset tuples, but no
field structure for them" by ensuring consistent query type detection
between handleDescribe and handleExecute.

Changes:
- Add queryReturnsResults() helper function that:
  - Strips leading comments before checking query type
  - Identifies all result-returning query types: SELECT, WITH (CTEs),
    VALUES, SHOW, TABLE, EXECUTE, EXPLAIN, DESCRIBE
- Update handleDescribe (both statement and portal) to use
  queryReturnsResults() instead of simple SELECT prefix check
- Update handleExecute to use queryReturnsResults() for consistency
- Add rewritePgCatalogQuery() to extended query protocol (handleParse)

The root cause was queries with leading comments like
"/*Fivetran*/ SELECT ..." being treated inconsistently:
- handleDescribe saw "/*FIVETRAN*/..." and sent NoData
- handleExecute stripped comments and returned results

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

Author: EDsCODE

server/conn.go

@@ -406,6 +406,45 @@ func stripLeadingComments(query string) string {
 	}
 }
 
+// queryReturnsResults checks if a SQL query returns a result set.
+// This is used to determine whether to send RowDescription or NoData.
+func queryReturnsResults(query string) bool {
+	upper := strings.ToUpper(stripLeadingComments(query))
+	// SELECT is the most common
+	if strings.HasPrefix(upper, "SELECT") {
+		return true
+	}
+	// WITH ... SELECT (CTEs)
+	if strings.HasPrefix(upper, "WITH") {
+		return true
+	}
+	// VALUES clause returns rows
+	if strings.HasPrefix(upper, "VALUES") {
+		return true
+	}
+	// SHOW commands return results
+	if strings.HasPrefix(upper, "SHOW") {
+		return true
+	}
+	// TABLE is shorthand for SELECT * FROM table
+	if strings.HasPrefix(upper, "TABLE") {
+		return true
+	}
+	// EXECUTE can return results if the prepared statement is a SELECT
+	if strings.HasPrefix(upper, "EXECUTE") {
+		return true
+	}
+	// EXPLAIN returns results
+	if strings.HasPrefix(upper, "EXPLAIN") {
+		return true
+	}
+	// DESCRIBE returns results (DuckDB-specific)
+	if strings.HasPrefix(upper, "DESCRIBE") {
+		return true
+	}
+	return false
+}
+
 func (c *clientConn) getCommandType(upperQuery string) string {
 	// Strip leading comments like /*Fivetran*/ before checking command type
 	upperQuery = stripLeadingComments(upperQuery)
@@ -958,15 +997,18 @@ func (c *clientConn) handleParse(body []byte) {
 		}
 	}
 
+	// Rewrite pg_catalog function calls for compatibility (same as simple query protocol)
+	rewrittenQuery := rewritePgCatalogQuery(query)
+
 	// Convert PostgreSQL $1, $2 placeholders to ? for database/sql
-	convertedQuery, numParams := convertPlaceholders(query)
+	convertedQuery, numParams := convertPlaceholders(rewrittenQuery)
 
 	// Close existing statement with same name
 	delete(c.stmts, stmtName)
 
 	c.stmts[stmtName] = &preparedStmt{
-		query:          query,
-		convertedQuery: convertedQuery,
+		query:          query,          // Keep original for logging and Describe
+		convertedQuery: convertedQuery, // Rewritten and placeholder-converted for execution
 		paramTypes:     paramTypes,
 		numParams:      numParams,
 	}
@@ -1106,10 +1148,9 @@ func (c *clientConn) handleDescribe(body []byte) {
 		}
 		c.sendParameterDescription(paramTypes)
 
-		// For SELECT queries, we need to send RowDescription
+		// For queries that return results, we need to send RowDescription
 		// For other queries, send NoData
-		upperQuery := strings.ToUpper(strings.TrimSpace(ps.query))
-		if !strings.HasPrefix(upperQuery, "SELECT") {
+		if !queryReturnsResults(ps.query) {
 			writeNoData(c.writer)
 			return
 		}
@@ -1157,9 +1198,8 @@ func (c *clientConn) handleDescribe(body []byte) {
 			return
 		}
 
-		// For non-SELECT, send NoData
-		upperQuery := strings.ToUpper(strings.TrimSpace(p.stmt.query))
-		if !strings.HasPrefix(upperQuery, "SELECT") {
+		// For queries that don't return results, send NoData
+		if !queryReturnsResults(p.stmt.query) {
 			writeNoData(c.writer)
 			return
 		}
@@ -1239,10 +1279,11 @@ func (c *clientConn) handleExecute(body []byte) {
 
 	upperQuery := strings.ToUpper(strings.TrimSpace(p.stmt.query))
 	cmdType := c.getCommandType(upperQuery)
+	returnsResults := queryReturnsResults(p.stmt.query)
 
 	log.Printf("[%s] Execute %q with %d params: %s", c.username, portalName, len(args), p.stmt.query)
 
-	if cmdType != "SELECT" {
+	if !returnsResults {
 		// Handle nested BEGIN: PostgreSQL issues a warning but continues,
 		// while DuckDB throws an error. Match PostgreSQL behavior.
 		if cmdType == "BEGIN" && c.txStatus == txStatusTransaction {
@@ -1251,7 +1292,7 @@ func (c *clientConn) handleExecute(body []byte) {
 			return
 		}
 
-		// Non-SELECT: use Exec with converted query
+		// Non-result-returning query: use Exec with converted query
 		result, err := c.db.Exec(p.stmt.convertedQuery, args...)
 		if err != nil {
 			c.sendError("ERROR", "42000", err.Error())
@@ -1264,7 +1305,7 @@ func (c *clientConn) handleExecute(body []byte) {
 		return
 	}
 
-	// SELECT: use Query with converted query
+	// Result-returning query: use Query with converted query
 	rows, err := c.db.Query(p.stmt.convertedQuery, args...)
 	if err != nil {
 		c.sendError("ERROR", "42000", err.Error())

server/conn_test.go

@@ -1,6 +1,7 @@
 package server
 
 import (
+	"strings"
 	"testing"
 )
 
@@ -358,3 +359,256 @@ func TestNestedBeginDetection(t *testing.T) {
 		t.Errorf("txStatus should still be %c after nested BEGIN detection, got %c", txStatusTransaction, c.txStatus)
 	}
 }
+
+func TestQueryReturnsResults(t *testing.T) {
+	tests := []struct {
+		name     string
+		query    string
+		expected bool
+	}{
+		// SELECT queries
+		{
+			name:     "simple SELECT",
+			query:    "SELECT * FROM users",
+			expected: true,
+		},
+		{
+			name:     "SELECT with comment",
+			query:    "/*Fivetran*/ SELECT * FROM users",
+			expected: true,
+		},
+		{
+			name:     "SELECT with block and line comment",
+			query:    "/* comment */ -- line\nSELECT 1",
+			expected: true,
+		},
+		// WITH/CTE queries
+		{
+			name:     "WITH clause",
+			query:    "WITH cte AS (SELECT 1) SELECT * FROM cte",
+			expected: true,
+		},
+		{
+			name:     "WITH clause with comment",
+			query:    "/*Fivetran*/ WITH cte AS (SELECT 1) SELECT * FROM cte",
+			expected: true,
+		},
+		// VALUES
+		{
+			name:     "VALUES",
+			query:    "VALUES (1, 2), (3, 4)",
+			expected: true,
+		},
+		// SHOW
+		{
+			name:     "SHOW",
+			query:    "SHOW TABLES",
+			expected: true,
+		},
+		// TABLE
+		{
+			name:     "TABLE command",
+			query:    "TABLE users",
+			expected: true,
+		},
+		// EXPLAIN
+		{
+			name:     "EXPLAIN",
+			query:    "EXPLAIN SELECT * FROM users",
+			expected: true,
+		},
+		// DESCRIBE
+		{
+			name:     "DESCRIBE",
+			query:    "DESCRIBE users",
+			expected: true,
+		},
+		// Non-result queries
+		{
+			name:     "INSERT",
+			query:    "INSERT INTO users VALUES (1)",
+			expected: false,
+		},
+		{
+			name:     "UPDATE",
+			query:    "UPDATE users SET name = 'test'",
+			expected: false,
+		},
+		{
+			name:     "DELETE",
+			query:    "DELETE FROM users",
+			expected: false,
+		},
+		{
+			name:     "CREATE TABLE",
+			query:    "CREATE TABLE test (id INT)",
+			expected: false,
+		},
+		{
+			name:     "CREATE TABLE with comment",
+			query:    "/*Fivetran*/ CREATE TABLE test (id INT)",
+			expected: false,
+		},
+		{
+			name:     "DROP TABLE",
+			query:    "DROP TABLE users",
+			expected: false,
+		},
+		{
+			name:     "BEGIN",
+			query:    "BEGIN",
+			expected: false,
+		},
+		{
+			name:     "COMMIT",
+			query:    "COMMIT",
+			expected: false,
+		},
+		{
+			name:     "ROLLBACK",
+			query:    "ROLLBACK",
+			expected: false,
+		},
+		{
+			name:     "SET",
+			query:    "SET search_path = public",
+			expected: false,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			result := queryReturnsResults(tt.query)
+			if result != tt.expected {
+				t.Errorf("queryReturnsResults(%q) = %v, want %v", tt.query, result, tt.expected)
+			}
+		})
+	}
+}
+
+// TestDescribeExecuteConsistency verifies that handleDescribe and handleExecute
+// use consistent logic to determine if a query returns results.
+//
+// This test reproduces the original bug where:
+// - handleDescribe used: strings.HasPrefix(upperQuery, "SELECT")
+// - handleExecute used: getCommandType(upperQuery) which strips comments
+//
+// For a query like "/* comment */ SELECT * FROM table":
+// - Old handleDescribe: "/* COMMENT */..." doesn't start with "SELECT" → sends NoData
+// - Old handleExecute: getCommandType strips comments → returns "SELECT" → sends results
+//
+// This mismatch caused: "Received resultset tuples, but no field structure for them"
+func TestDescribeExecuteConsistency(t *testing.T) {
+	c := &clientConn{}
+
+	// Queries that caused the bug: result-returning queries with leading comments
+	problematicQueries := []struct {
+		name  string
+		query string
+	}{
+		{"block comment before SELECT", "/* comment */ SELECT * FROM users"},
+		{"block comment before SELECT no space", "/*comment*/SELECT 1"},
+		{"block comment before WITH", "/* query */ WITH cte AS (SELECT 1) SELECT * FROM cte"},
+		{"line comment before SELECT", "-- comment\nSELECT * FROM users"},
+		{"multiple block comments", "/* first */ /* second */ SELECT 1"},
+	}
+
+	for _, tt := range problematicQueries {
+		t.Run(tt.name, func(t *testing.T) {
+			// Simulate the OLD handleDescribe logic (the bug)
+			upperQuery := strings.ToUpper(strings.TrimSpace(tt.query))
+			oldDescribeWouldReturnResults := strings.HasPrefix(upperQuery, "SELECT")
+
+			// Simulate the OLD handleExecute logic
+			cmdType := c.getCommandType(upperQuery)
+			oldExecuteWouldReturnResults := cmdType == "SELECT"
+
+			// The bug: Describe says no results, Execute says results
+			if oldDescribeWouldReturnResults != oldExecuteWouldReturnResults {
+				t.Logf("BUG REPRODUCED: Old logic inconsistent for %q", tt.query)
+				t.Logf("  Old Describe (HasPrefix SELECT): %v", oldDescribeWouldReturnResults)
+				t.Logf("  Old Execute (getCommandType):    %v (cmdType=%s)", oldExecuteWouldReturnResults, cmdType)
+			}
+
+			// Verify the NEW logic is consistent
+			newDescribeWouldReturnResults := queryReturnsResults(tt.query)
+			newExecuteWouldReturnResults := queryReturnsResults(tt.query)
+
+			if newDescribeWouldReturnResults != newExecuteWouldReturnResults {
+				t.Errorf("NEW logic still inconsistent for %q: Describe=%v, Execute=%v",
+					tt.query, newDescribeWouldReturnResults, newExecuteWouldReturnResults)
+			}
+
+			// Verify the query is correctly identified as returning results
+			if !newDescribeWouldReturnResults {
+				t.Errorf("queryReturnsResults(%q) = false, but this query DOES return results", tt.query)
+			}
+
+			// Verify the old logic would have caused the bug
+			if oldDescribeWouldReturnResults == oldExecuteWouldReturnResults {
+				t.Logf("Note: Old logic was consistent for %q (no bug for this case)", tt.query)
+			}
+		})
+	}
+}
+
+// TestRowDescriptionMismatchScenario simulates the exact sequence that caused
+// the "Received resultset tuples, but no field structure for them" error.
+func TestRowDescriptionMismatchScenario(t *testing.T) {
+	// Query with leading comment - common pattern from ETL tools
+	query := "/* etl query */ SELECT * FROM information_schema.tables"
+
+	// Step 1: Client sends Parse (creates prepared statement)
+	// Step 2: Client sends Describe('S') for statement
+	//         Server checks if query returns results
+
+	// OLD BEHAVIOR (the bug):
+	upperQuery := strings.ToUpper(strings.TrimSpace(query))
+	oldWouldSendRowDescription := strings.HasPrefix(upperQuery, "SELECT")
+	// Result: false (query starts with "/* ETL QUERY */", not "SELECT")
+	// Server would send: NoData
+
+	if oldWouldSendRowDescription {
+		t.Error("Expected old logic to NOT detect this as SELECT (reproducing the bug)")
+	}
+
+	// Step 3: Client sends Bind (creates portal)
+	// Step 4: Client sends Execute
+	//         Server executes query and checks if it returns results
+
+	// OLD BEHAVIOR in Execute used getCommandType which strips comments:
+	c := &clientConn{}
+	cmdType := c.getCommandType(upperQuery)
+	oldExecuteWouldQuery := cmdType == "SELECT"
+	// Result: true (getCommandType strips comments, sees "SELECT")
+	// Server would send: RowDescription + DataRows
+
+	if !oldExecuteWouldQuery {
+		t.Error("Expected old Execute logic to detect this as SELECT")
+	}
+
+	// THE BUG: Describe sent NoData, but Execute sent RowDescription + DataRows
+	// Client received DataRows without expecting them → error
+
+	if !oldWouldSendRowDescription && oldExecuteWouldQuery {
+		t.Log("BUG CONFIRMED: Describe sent NoData, Execute sent results")
+		t.Log("This causes: 'Received resultset tuples, but no field structure for them'")
+	}
+
+	// NEW BEHAVIOR (the fix):
+	newWouldSendRowDescription := queryReturnsResults(query)
+	newExecuteWouldQuery := queryReturnsResults(query)
+
+	// Both should now be true and consistent
+	if !newWouldSendRowDescription {
+		t.Error("Fixed Describe should detect this as returning results")
+	}
+	if !newExecuteWouldQuery {
+		t.Error("Fixed Execute should detect this as returning results")
+	}
+	if newWouldSendRowDescription != newExecuteWouldQuery {
+		t.Error("Fixed logic should be consistent between Describe and Execute")
+	}
+
+	t.Log("FIX VERIFIED: Both Describe and Execute now correctly identify result-returning queries")
+}