Merge pull request #48 from PostHog/fix/regclass-metabase-support

Fix ::regclass cast for Metabase column discovery

Author: fuziontech

tests/integration/TEST_RESULTS.md

@@ -47,7 +47,7 @@ To run without DuckLake: `DUCKGRES_TEST_NO_DUCKLAKE=1 go test ./tests/integratio
 | **Fivetran Queries** | 4 | 4 | 0 | 100% |
 | **Airbyte Queries** | 3 | 3 | 0 | 100% |
 | **dbt Queries** | 7 | 7 | 0 | 100% |
-| **Metabase Queries** | 5 | 4 | 1 | 80% |
+| **Metabase Queries** | 5 | 5 | 0 | 100% |
 | **DBeaver Queries** | 4 | 4 | 0 | 100% |
 | **Prepared Statements** | 3 | 3 | 0 | 100% |
 | **Transactions** | 2 | 2 | 0 | 100% |
@@ -100,7 +100,7 @@ DuckLake has additional constraints compared to vanilla DuckDB:
 | Airbyte Queries | 100% |
 | dbt Queries | 100% |
 | DBeaver Queries | 100% |
-| Metabase Queries | 80% (1 `::regclass` limitation) |
+| Metabase Queries | 100% |
 
 ## Passing Test Categories (Vanilla DuckDB)
 
@@ -145,12 +145,12 @@ All pg_catalog views and functions work correctly:
 - Column metadata ✓
 - Server version ✓
 
-**Metabase** (80%):
+**Metabase** (100%):
 - Get schemas ✓
 - Get tables ✓
 - Connection test ✓
 - Version check ✓
-- Get columns ✗ (`::regclass` cast not supported)
+- Get columns ✓
 
 ### ETL Tools Compatibility
 
@@ -194,18 +194,13 @@ DuckDB: `10 / 3 = 3.333...` (float)
 **Impact**: Arithmetic tests
 **Solution**: Use explicit `CAST` or `::integer`
 
-### 4. `::regclass` Cast (Metabase)
-Metabase uses `'table_name'::regclass` for column lookup. The transpiler converts this to `::varchar` which doesn't work for attrelid lookups.
-
-**Workaround**: Join with pg_class by relname instead
-
-### 5. RETURNING Clause (DuckLake)
+### 4. RETURNING Clause (DuckLake)
 DuckLake does not support `INSERT/UPDATE/DELETE ... RETURNING`.
 
 **Impact**: Tests using RETURNING clause fail in DuckLake mode
 **Workaround**: Use separate SELECT after mutation
 
-### 6. Per-Connection Database (Vanilla DuckDB)
+### 5. Per-Connection Database (Vanilla DuckDB)
 Each new database connection gets a fresh in-memory DuckDB database. This is by design but affects tests requiring data persistence across connections.
 
 **Note**: This is not an issue in DuckLake mode where metadata persists.
@@ -227,6 +222,9 @@ Each new database connection gets a fresh in-memory DuckDB database. This is by
 - **Session commands** (`transpiler/transform/setshow.go`): Added RESET ALL, DISCARD ALL/PLANS/SEQUENCES/TEMP support
 - **Transaction modes** (`transpiler/transform/setshow.go`): Strip ISOLATION LEVEL and READ ONLY/WRITE from BEGIN statements
 
+### Regclass Cast Fix (PR #48)
+- **`::regclass` subquery rewrite** (`transpiler/transform/typecast.go`): Convert `'tablename'::regclass` to `(SELECT oid FROM pg_class WHERE relname = 'tablename')` for proper OID lookup. This fixes Metabase column discovery queries.
+
 ### Test Harness Fixes
 - **`compare.go`**: Use `sql.RawBytes` to avoid driver parsing issues
 - **`compare.go`**: Add `IgnoreColumnNames` option (DuckDB names anonymous columns differently)
@@ -297,10 +295,9 @@ go test ./tests/integration/clients/... -v -run "TestGrafana"
 
 ## Recommendations for Improving Compatibility
 
-1. **Fix `::regclass` handling** in the transpiler for better Metabase support
-2. **Transpile regex operators** (`~`, `~*`) to `regexp_matches()`
-3. **Handle integer division** to match PostgreSQL behavior
-4. **Add RETURNING emulation** for DuckLake mode (separate SELECT)
+1. **Transpile regex operators** (`~`, `~*`) to `regexp_matches()`
+2. **Handle integer division** to match PostgreSQL behavior
+3. **Add RETURNING emulation** for DuckLake mode (separate SELECT)
 
 ## CI Configuration
 

transpiler/transform/typecast.go

@@ -8,6 +8,7 @@ import (
 
 // TypeCastTransform converts PostgreSQL-specific type casts to DuckDB equivalents.
 // For example: ::pg_catalog.regtype -> ::VARCHAR
+// Special case: 'tablename'::regclass -> (SELECT oid FROM pg_class WHERE relname = 'tablename')
 type TypeCastTransform struct {
 	// TypeMappings maps pg_catalog types to DuckDB types
 	TypeMappings map[string]string
@@ -17,16 +18,16 @@ type TypeCastTransform struct {
 func NewTypeCastTransform() *TypeCastTransform {
 	return &TypeCastTransform{
 		TypeMappings: map[string]string{
-			"regtype":      "varchar",
-			"regclass":     "varchar",
-			"regnamespace": "varchar",
-			"regproc":      "varchar",
-			"regoper":      "varchar",
-			"regoperator":  "varchar",
-			"regprocedure": "varchar",
-			"regconfig":    "varchar",
+			"regtype":       "varchar",
+			"regnamespace":  "varchar",
+			"regproc":       "varchar",
+			"regoper":       "varchar",
+			"regoperator":   "varchar",
+			"regprocedure":  "varchar",
+			"regconfig":     "varchar",
 			"regdictionary": "varchar",
-			"text":         "varchar",
+			"text":          "varchar",
+			// Note: regclass is handled specially - converted to subquery lookup
 		},
 	}
 }
@@ -58,35 +59,70 @@ func (t *TypeCastTransform) walkAndTransform(node *pg_query.Node, changed *bool)
 		if n.TypeCast != nil && n.TypeCast.TypeName != nil {
 			// Check if this is a pg_catalog type cast
 			typeName := n.TypeCast.TypeName
+			typeLower := ""
+
 			if len(typeName.Names) >= 2 {
 				// Check for pg_catalog.typename pattern
 				if first := typeName.Names[0]; first != nil {
 					if str := first.GetString_(); str != nil && strings.EqualFold(str.Sval, "pg_catalog") {
 						if second := typeName.Names[1]; second != nil {
 							if typeStr := second.GetString_(); typeStr != nil {
-								typeLower := strings.ToLower(typeStr.Sval)
-								if newType, ok := t.TypeMappings[typeLower]; ok {
-									// Replace with just the DuckDB type name
-									typeName.Names = []*pg_query.Node{
-										{Node: &pg_query.Node_String_{String_: &pg_query.String{Sval: newType}}},
-									}
-									*changed = true
-								}
+								typeLower = strings.ToLower(typeStr.Sval)
 							}
 						}
 					}
 				}
 			} else if len(typeName.Names) == 1 {
-				// Single name, check if it needs mapping
+				// Single name
 				if first := typeName.Names[0]; first != nil {
 					if str := first.GetString_(); str != nil {
-						typeLower := strings.ToLower(str.Sval)
-						if newType, ok := t.TypeMappings[typeLower]; ok {
+						typeLower = strings.ToLower(str.Sval)
+					}
+				}
+			}
+
+			// Special handling for ::regclass with string literal argument
+			// Convert 'tablename'::regclass to (SELECT oid FROM pg_class WHERE relname = 'tablename')
+			if typeLower == "regclass" {
+				if sublink := t.createRegclassSubquery(n.TypeCast.Arg); sublink != nil {
+					// Replace the TypeCast node with the SubLink
+					node.Node = &pg_query.Node_SubLink{SubLink: sublink}
+					*changed = true
+					return
+				}
+				// For non-string arguments (like oid::regclass), fall through to varchar mapping
+				typeLower = "regclass_fallback"
+			}
+
+			// Standard type mapping for other types
+			if typeLower == "regclass_fallback" {
+				// Fallback for regclass with non-string arguments
+				if len(typeName.Names) >= 2 {
+					typeName.Names = []*pg_query.Node{
+						{Node: &pg_query.Node_String_{String_: &pg_query.String{Sval: "varchar"}}},
+					}
+				} else if len(typeName.Names) == 1 {
+					if first := typeName.Names[0]; first != nil {
+						if str := first.GetString_(); str != nil {
+							str.Sval = "varchar"
+						}
+					}
+				}
+				*changed = true
+			} else if newType, ok := t.TypeMappings[typeLower]; ok {
+				if len(typeName.Names) >= 2 {
+					// Replace pg_catalog.typename with just the new type
+					typeName.Names = []*pg_query.Node{
+						{Node: &pg_query.Node_String_{String_: &pg_query.String{Sval: newType}}},
+					}
+				} else if len(typeName.Names) == 1 {
+					if first := typeName.Names[0]; first != nil {
+						if str := first.GetString_(); str != nil {
 							str.Sval = newType
-							*changed = true
 						}
 					}
 				}
+				*changed = true
 			}
 			// Recurse into the argument
 			t.walkAndTransform(n.TypeCast.Arg, changed)
@@ -245,3 +281,107 @@ func (t *TypeCastTransform) walkSelectStmt(stmt *pg_query.SelectStmt, changed *b
 		t.walkSelectStmt(stmt.Rarg, changed)
 	}
 }
+
+// createRegclassSubquery creates a SubLink for 'tablename'::regclass
+// This converts: 'users'::regclass
+// To: (SELECT oid FROM pg_class WHERE relname = 'users')
+func (t *TypeCastTransform) createRegclassSubquery(arg *pg_query.Node) *pg_query.SubLink {
+	if arg == nil {
+		return nil
+	}
+
+	// Get the string value from the argument
+	var tableName string
+	if aConst := arg.GetAConst(); aConst != nil {
+		if sval := aConst.GetSval(); sval != nil {
+			tableName = sval.Sval
+		}
+	}
+
+	if tableName == "" {
+		return nil
+	}
+
+	// Build: SELECT oid FROM pg_class WHERE relname = 'tablename'
+	// Create the SELECT target: oid
+	targetList := []*pg_query.Node{
+		{
+			Node: &pg_query.Node_ResTarget{
+				ResTarget: &pg_query.ResTarget{
+					Val: &pg_query.Node{
+						Node: &pg_query.Node_ColumnRef{
+							ColumnRef: &pg_query.ColumnRef{
+								Fields: []*pg_query.Node{
+									{Node: &pg_query.Node_String_{String_: &pg_query.String{Sval: "oid"}}},
+								},
+							},
+						},
+					},
+				},
+			},
+		},
+	}
+
+	// Create the FROM clause: pg_class
+	fromClause := []*pg_query.Node{
+		{
+			Node: &pg_query.Node_RangeVar{
+				RangeVar: &pg_query.RangeVar{
+					Relname:        "pg_class",
+					Inh:            true,
+					Relpersistence: "p",
+				},
+			},
+		},
+	}
+
+	// Create the WHERE clause: relname = 'tablename'
+	whereClause := &pg_query.Node{
+		Node: &pg_query.Node_AExpr{
+			AExpr: &pg_query.A_Expr{
+				Kind: pg_query.A_Expr_Kind_AEXPR_OP,
+				Name: []*pg_query.Node{
+					{Node: &pg_query.Node_String_{String_: &pg_query.String{Sval: "="}}},
+				},
+				Lexpr: &pg_query.Node{
+					Node: &pg_query.Node_ColumnRef{
+						ColumnRef: &pg_query.ColumnRef{
+							Fields: []*pg_query.Node{
+								{Node: &pg_query.Node_String_{String_: &pg_query.String{Sval: "relname"}}},
+							},
+						},
+					},
+				},
+				Rexpr: &pg_query.Node{
+					Node: &pg_query.Node_AConst{
+						AConst: &pg_query.A_Const{
+							Val: &pg_query.A_Const_Sval{
+								Sval: &pg_query.String{Sval: tableName},
+							},
+						},
+					},
+				},
+			},
+		},
+	}
+
+	// Build the SelectStmt
+	// Note: If multiple tables with the same name exist in different schemas,
+	// the subquery may return multiple rows and fail. This matches PostgreSQL's
+	// behavior where ::regclass uses search_path to resolve ambiguity.
+	selectStmt := &pg_query.SelectStmt{
+		TargetList:  targetList,
+		FromClause:  fromClause,
+		WhereClause: whereClause,
+	}
+
+	// Create the SubLink (scalar subquery)
+	return &pg_query.SubLink{
+		SubLinkType: pg_query.SubLinkType_EXPR_SUBLINK,
+		Subselect: &pg_query.Node{
+			Node: &pg_query.Node_SelectStmt{
+				SelectStmt: selectStmt,
+			},
+		},
+	}
+}