Skip to content

Commit 8ab4017

Browse files
fuziontechclaude
fuziontech
and
claude
authored
Fix typmod defaulting to 0 instead of -1 for all types (#269)
* Fix typmod defaulting to 0 instead of -1 for all types (#JDBC) PostgreSQL uses typmod=-1 to mean "no type modifier". Go's zero value for int32 is 0, so every TypeInfo without an explicit Typmod was sending typmod=0 on the wire. JDBC clients (pgjdbc) interpret this differently from -1: - INTERVAL typmod=0 → getScale() returns 0 (precision 0, no fractional seconds) instead of 6 (default microsecond precision) - TIME/TIMESTAMP typmod=0 → similar precision mismatch - Other types: typmod=0 can trigger unexpected metadata behavior This likely causes Metabase/JDBC NoSuchElementException errors when querying INTERVAL-returning functions like uptime(). Also adds missing rows.Err() check in handleExecute (extended query protocol). The simple query path checked for streaming errors but the extended protocol path did not, meaning gRPC stream errors from workers would be silently swallowed instead of reported to the client. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Add tests for unknown type typmod and extended query error recovery - Add STRING, TIME WITH TIME ZONE, TIMESTAMP WITH TIME ZONE, and unknown type (SOMECUSTOMTYPE) cases to TestMapDuckDBTypeTypmod - Add TestExtendedQueryErrorHandling integration test with two subtests: single error recovery and multi-cycle error/success on same connection Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Fix staticcheck SA4016: remove redundant |0 in typmod expressions Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 85dc1a7 commit 8ab4017

File tree

4 files changed

+157
-22
lines changed

4 files changed

+157
-22
lines changed

‎server/conn.go‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4542,6 +4542,17 @@ func (c *clientConn) handleExecute(body []byte) {
45424542
rowCount++
45434543
}
45444544

4545+
if err := rows.Err(); err != nil {
4546+
if isQueryCancelled(err) {
4547+
c.sendError("ERROR", "57014", "canceling statement due to user request")
4548+
} else {
4549+
slog.Error("Row iteration error.", "user", c.username, "error", err)
4550+
c.sendError("ERROR", "42000", err.Error())
4551+
}
4552+
c.setTxError()
4553+
return
4554+
}
4555+
45454556
c.updateTxStatus(cmdType)
45464557
tag := buildCommandTagFromRowCount(cmdType, int64(rowCount))
45474558
_ = writeCommandComplete(c.writer, tag)

‎server/types.go‎

Lines changed: 22 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -156,65 +156,65 @@ func mapDuckDBType(typeName string) TypeInfo {
156156
return TypeInfo{OID: arrayOID, Size: -1, Typmod: elemInfo.Typmod}
157157
}
158158
// Unknown element type — fall through to text
159-
return TypeInfo{OID: OidText, Size: -1}
159+
return TypeInfo{OID: OidText, Size: -1, Typmod: -1}
160160
}
161161

162162
switch {
163163
case upper == "BOOLEAN" || upper == "BOOL":
164-
return TypeInfo{OID: OidBool, Size: 1}
164+
return TypeInfo{OID: OidBool, Size: 1, Typmod: -1}
165165
case upper == "TINYINT" || upper == "INT1":
166-
return TypeInfo{OID: OidInt2, Size: 2} // PostgreSQL doesn't have int1
166+
return TypeInfo{OID: OidInt2, Size: 2, Typmod: -1} // PostgreSQL doesn't have int1
167167
case upper == "SMALLINT" || upper == "INT2":
168-
return TypeInfo{OID: OidInt2, Size: 2}
168+
return TypeInfo{OID: OidInt2, Size: 2, Typmod: -1}
169169
case upper == "INTEGER" || upper == "INT4" || upper == "INT":
170-
return TypeInfo{OID: OidInt4, Size: 4}
170+
return TypeInfo{OID: OidInt4, Size: 4, Typmod: -1}
171171
case upper == "BIGINT" || upper == "INT8":
172-
return TypeInfo{OID: OidInt8, Size: 8}
172+
return TypeInfo{OID: OidInt8, Size: 8, Typmod: -1}
173173
case upper == "HUGEINT" || upper == "INT128":
174174
// Map to NUMERIC(38,0) so postgres_scanner reads it as DECIMAL(38,0) → INT128,
175175
// matching the HUGEINT physical type. Typmod = ((38 << 16) | 0) + 4 = 2490372.
176176
return TypeInfo{OID: OidNumeric, Size: -1, Typmod: 2490372}
177177
case upper == "UTINYINT" || upper == "USMALLINT":
178-
return TypeInfo{OID: OidInt4, Size: 4}
178+
return TypeInfo{OID: OidInt4, Size: 4, Typmod: -1}
179179
case upper == "UINTEGER":
180-
return TypeInfo{OID: OidOid, Size: 4} // PostgreSQL oid type for pg_catalog columns
180+
return TypeInfo{OID: OidOid, Size: 4, Typmod: -1} // PostgreSQL oid type for pg_catalog columns
181181
case upper == "UBIGINT":
182182
// Map to NUMERIC(20,0) so postgres_scanner reads it as DECIMAL(20,0) → INT128.
183183
// UBIGINT max (2^64-1 = 18446744073709551615) is 20 digits.
184184
// Without typmod, the extension can't determine buffer size and fails with
185185
// "out of buffer in ReadInteger". Typmod = ((20 << 16) | 0) + 4 = 1310724.
186186
return TypeInfo{OID: OidNumeric, Size: -1, Typmod: 1310724}
187187
case upper == "REAL" || upper == "FLOAT4" || upper == "FLOAT":
188-
return TypeInfo{OID: OidFloat4, Size: 4}
188+
return TypeInfo{OID: OidFloat4, Size: 4, Typmod: -1}
189189
case upper == "DOUBLE" || upper == "FLOAT8":
190-
return TypeInfo{OID: OidFloat8, Size: 8}
190+
return TypeInfo{OID: OidFloat8, Size: 8, Typmod: -1}
191191
case strings.HasPrefix(upper, "DECIMAL") || strings.HasPrefix(upper, "NUMERIC"):
192192
return TypeInfo{OID: OidNumeric, Size: -1, Typmod: parseNumericTypmod(typeName)}
193193
case upper == "VARCHAR" || strings.HasPrefix(upper, "VARCHAR("):
194-
return TypeInfo{OID: OidVarchar, Size: -1}
194+
return TypeInfo{OID: OidVarchar, Size: -1, Typmod: -1}
195195
case upper == "TEXT" || upper == "STRING":
196-
return TypeInfo{OID: OidText, Size: -1}
196+
return TypeInfo{OID: OidText, Size: -1, Typmod: -1}
197197
case upper == "BLOB" || upper == "BYTEA":
198-
return TypeInfo{OID: OidBytea, Size: -1}
198+
return TypeInfo{OID: OidBytea, Size: -1, Typmod: -1}
199199
case upper == "DATE":
200-
return TypeInfo{OID: OidDate, Size: 4}
200+
return TypeInfo{OID: OidDate, Size: 4, Typmod: -1}
201201
case upper == "TIME":
202-
return TypeInfo{OID: OidTime, Size: 8}
202+
return TypeInfo{OID: OidTime, Size: 8, Typmod: -1}
203203
case upper == "TIME WITH TIME ZONE" || upper == "TIMETZ":
204-
return TypeInfo{OID: OidTimetz, Size: 12}
204+
return TypeInfo{OID: OidTimetz, Size: 12, Typmod: -1}
205205
case upper == "TIMESTAMP":
206-
return TypeInfo{OID: OidTimestamp, Size: 8}
206+
return TypeInfo{OID: OidTimestamp, Size: 8, Typmod: -1}
207207
case upper == "TIMESTAMP WITH TIME ZONE" || upper == "TIMESTAMPTZ":
208-
return TypeInfo{OID: OidTimestamptz, Size: 8}
208+
return TypeInfo{OID: OidTimestamptz, Size: 8, Typmod: -1}
209209
case upper == "INTERVAL":
210-
return TypeInfo{OID: OidInterval, Size: 16}
210+
return TypeInfo{OID: OidInterval, Size: 16, Typmod: -1}
211211
case upper == "UUID":
212-
return TypeInfo{OID: OidUUID, Size: 16}
212+
return TypeInfo{OID: OidUUID, Size: 16, Typmod: -1}
213213
case upper == "JSON":
214-
return TypeInfo{OID: OidJSON, Size: -1}
214+
return TypeInfo{OID: OidJSON, Size: -1, Typmod: -1}
215215
default:
216216
// Default to text for unknown types
217-
return TypeInfo{OID: OidText, Size: -1}
217+
return TypeInfo{OID: OidText, Size: -1, Typmod: -1}
218218
}
219219
}
220220

‎server/types_test.go‎

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,67 @@ func TestMapDuckDBType(t *testing.T) {
9595
}
9696
}
9797

98+
func TestMapDuckDBTypeTypmod(t *testing.T) {
99+
// PostgreSQL uses typmod=-1 to mean "no modifier". JDBC clients (pgjdbc)
100+
// interpret typmod=0 differently from -1. For example, INTERVAL with typmod=0
101+
// means "second precision 0" (no fractional seconds), while typmod=-1 means
102+
// default precision (microseconds). Sending the wrong typmod breaks JDBC
103+
// metadata and can cause client-side errors.
104+
tests := []struct {
105+
typeName string
106+
wantTypmod int32
107+
}{
108+
// Types without modifiers should have typmod=-1
109+
{"BOOLEAN", -1},
110+
{"TINYINT", -1},
111+
{"SMALLINT", -1},
112+
{"INTEGER", -1},
113+
{"BIGINT", -1},
114+
{"REAL", -1},
115+
{"DOUBLE", -1},
116+
{"VARCHAR", -1},
117+
{"TEXT", -1},
118+
{"BYTEA", -1},
119+
{"DATE", -1},
120+
{"TIME", -1},
121+
{"TIMETZ", -1},
122+
{"TIMESTAMP", -1},
123+
{"TIMESTAMPTZ", -1},
124+
{"INTERVAL", -1},
125+
{"UUID", -1},
126+
{"JSON", -1},
127+
{"UTINYINT", -1},
128+
{"USMALLINT", -1},
129+
{"UINTEGER", -1},
130+
// NUMERIC without precision should have typmod=-1
131+
{"NUMERIC", -1},
132+
{"DECIMAL", -1},
133+
// NUMERIC with precision should have a positive typmod
134+
{"DECIMAL(10,2)", int32((10<<16)|2) + 4},
135+
// HUGEINT and UBIGINT have specific typmods for postgres_scanner
136+
{"HUGEINT", int32(38<<16) + 4},
137+
{"UBIGINT", int32(20<<16) + 4},
138+
// Aliases and long-form type names
139+
{"STRING", -1},
140+
{"TIME WITH TIME ZONE", -1},
141+
{"TIMESTAMP WITH TIME ZONE", -1},
142+
// Array types inherit element typmod
143+
{"INTEGER[]", -1},
144+
{"INTERVAL[]", -1},
145+
// Unknown types should default to typmod=-1
146+
{"SOMECUSTOMTYPE", -1},
147+
}
148+
149+
for _, tt := range tests {
150+
t.Run(tt.typeName, func(t *testing.T) {
151+
info := mapDuckDBType(tt.typeName)
152+
if info.Typmod != tt.wantTypmod {
153+
t.Errorf("mapDuckDBType(%q).Typmod = %d, want %d", tt.typeName, info.Typmod, tt.wantTypmod)
154+
}
155+
})
156+
}
157+
}
158+
98159
func TestEncodeBool(t *testing.T) {
99160
tests := []struct {
100161
name string

‎tests/integration/clients/clients_test.go‎

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package clients
22

33
import (
4+
"context"
45
"database/sql"
56
"fmt"
67
"os"
@@ -564,6 +565,68 @@ func TestPreparedStatements(t *testing.T) {
564565
})
565566
}
566567

568+
// TestExtendedQueryErrorHandling verifies that errors during the extended query
569+
// protocol (prepared statements) are properly reported to the client and don't
570+
// corrupt the connection state.
571+
func TestExtendedQueryErrorHandling(t *testing.T) {
572+
t.Run("error_during_prepared_execution", func(t *testing.T) {
573+
// Use a single connection so we can verify the SAME connection survives errors.
574+
conn, err := testDB.Conn(context.Background())
575+
if err != nil {
576+
t.Fatalf("Conn failed: %v", err)
577+
}
578+
defer func() { _ = conn.Close() }()
579+
580+
// CAST($1 AS INTEGER) with a non-numeric string uses the extended query
581+
// protocol (Parse/Bind/Execute) and produces a conversion error at execution time.
582+
var val int
583+
err = conn.QueryRowContext(context.Background(), "SELECT CAST($1 AS INTEGER)", "not-a-number").Scan(&val)
584+
if err == nil {
585+
t.Fatal("Expected conversion error, got none")
586+
}
587+
588+
// The SAME connection should still be usable after the error.
589+
// If handleExecute didn't properly handle the error (e.g., protocol desync),
590+
// this would fail or hang.
591+
var result int
592+
err = conn.QueryRowContext(context.Background(), "SELECT $1::int", 42).Scan(&result)
593+
if err != nil {
594+
t.Fatalf("Connection unusable after error: %v", err)
595+
}
596+
if result != 42 {
597+
t.Errorf("Expected 42, got %d", result)
598+
}
599+
})
600+
601+
t.Run("error_recovery_multi_cycle", func(t *testing.T) {
602+
// Run multiple error/success cycles on the same connection.
603+
conn, err := testDB.Conn(context.Background())
604+
if err != nil {
605+
t.Fatalf("Conn failed: %v", err)
606+
}
607+
defer func() { _ = conn.Close() }()
608+
609+
for i := 0; i < 3; i++ {
610+
// Failing query via extended protocol
611+
var badVal int
612+
err := conn.QueryRowContext(context.Background(), "SELECT CAST($1 AS INTEGER)", "bad").Scan(&badVal)
613+
if err == nil {
614+
t.Fatalf("Expected error on iteration %d", i)
615+
}
616+
617+
// Successful query via extended protocol on the same connection
618+
var val int
619+
err = conn.QueryRowContext(context.Background(), "SELECT $1::int + 1", i).Scan(&val)
620+
if err != nil {
621+
t.Fatalf("Success query failed on iteration %d: %v", i, err)
622+
}
623+
if val != i+1 {
624+
t.Errorf("Expected %d, got %d on iteration %d", i+1, val, i)
625+
}
626+
}
627+
})
628+
}
629+
567630
// TestTransactions tests transaction handling
568631
func TestTransactions(t *testing.T) {
569632
t.Run("commit", func(t *testing.T) {

0 commit comments

Comments
 (0)