Merge pull request #2327 from dolthub/fulghum/record_type_fix

bugfix: support records containing non-DoltgresType values

Author: fulghum

server/functions/framework/interpreted_function.go

@@ -138,6 +138,10 @@ func (iFunc InterpretedFunction) QuerySingleReturn(ctx *sql.Context, stack plpgs
 		}
 		fromType, ok := sch[0].Type.(*pgtypes.DoltgresType)
 		if !ok {
+			// TODO: We ensure we have a DoltgresType, but we should also convert the value to
+			//       ensure it's in the correct form for the DoltgresType. This logic lives in
+			//       pgexpressions.GMSCast, but need to be extracted to avoid a dependency cycle
+			//       so it can be used here and from server.plpgsql.
 			fromType, err = pgtypes.FromGmsTypeToDoltgresType(sch[0].Type)
 			if err != nil {
 				return nil, err

server/plpgsql/interpreter_logic.go

@@ -273,7 +273,11 @@ func call(ctx *sql.Context, iFunc InterpretedFunction, stack InterpreterStack) (
 			if err != nil {
 				return nil, err
 			}
-			stack.BufferReturnQueryResults(convertRowsToRecords(schema, rows))
+			records, err := convertRowsToRecords(schema, rows)
+			if err != nil {
+				return nil, err
+			}
+			stack.BufferReturnQueryResults(records)
 
 		case OpCode_ScopeBegin:
 			stack.PushScope()
@@ -292,15 +296,26 @@ func call(ctx *sql.Context, iFunc InterpretedFunction, stack InterpreterStack) (
 
 // convertRowsToRecords iterates overs |rows| and converts each field in each row
 // into a RecordValue. |schema| is specified for type information.
-func convertRowsToRecords(schema sql.Schema, rows []sql.Row) [][]pgtypes.RecordValue {
+func convertRowsToRecords(schema sql.Schema, rows []sql.Row) ([][]pgtypes.RecordValue, error) {
 	records := make([][]pgtypes.RecordValue, 0, len(rows))
 	for _, row := range rows {
 		record := make([]pgtypes.RecordValue, len(row))
 		for i, field := range row {
 			t := schema[i].Type
 			doltgresType, ok := t.(*pgtypes.DoltgresType)
 			if !ok {
-				panic("expected Doltgres type")
+				// non-Doltgres types are still used in analysis, but we only support disk serialization
+				// for Doltgres types, so we must convert the GMS type to the nearest Doltgres type here.
+				// TODO: this conversion isn't fully accurate. expression.GMSCast has additional logic in
+				//       its Eval() method to handle types more exactly and also handles converting the
+				//       value to ensure it is well formed for the returned DoltgresType. We can't
+				//       currently use GMSCast directly here though, because of a dependency cycle, so
+				//       that conversion logic needs to be extracted into a package both places can import.
+				var err error
+				doltgresType, err = pgtypes.FromGmsTypeToDoltgresType(t)
+				if err != nil {
+					return nil, err
+				}
 			}
 
 			record[i] = pgtypes.RecordValue{
@@ -311,7 +326,7 @@ func convertRowsToRecords(schema sql.Schema, rows []sql.Row) [][]pgtypes.RecordV
 		records = append(records, record)
 	}
 
-	return records
+	return records, nil
 }
 
 // applyNoticeOptions adds the specified |options| to the |noticeResponse|.

testing/go/create_function_plpgsql_test.go

@@ -654,6 +654,56 @@ $$ LANGUAGE plpgsql;`},
 				},
 			},
 		},
+		{
+			Name: "RETURNS TABLE with join query",
+			SetUpScript: []string{
+				`CREATE TABLE customers (
+					id INT PRIMARY KEY,
+					name TEXT
+				);`,
+				`CREATE TABLE orders (
+					id SERIAL PRIMARY KEY,
+					customer_id INT,
+					amount INT
+				);`,
+				`INSERT INTO customers VALUES (1, 'John'), (2, 'Jane');`,
+				`INSERT INTO orders VALUES (1, 1, 100), (2, 2, 10);`,
+				`CREATE OR REPLACE FUNCTION func2(n INT) RETURNS TABLE (c_id INT, c_name TEXT, c_total_spent INT) 
+					LANGUAGE plpgsql
+					AS $$
+					BEGIN
+						RETURN QUERY
+						SELECT c.id,
+							   c.name,
+							   SUM(o.amount) AS total_spent
+						FROM customers c
+						JOIN orders o ON o.customer_id = c.id
+						GROUP BY c.id, c.name
+						HAVING SUM(o.amount) >= n
+						;
+					END;
+					$$;`,
+			},
+			Assertions: []ScriptTestAssertion{
+				{
+					Query: "SELECT func2(1);",
+					Expected: []sql.Row{
+						{"(1,John,100)"},
+						{"(2,Jane,10)"},
+					},
+				},
+				{
+					Query: "SELECT func2(11);",
+					Expected: []sql.Row{
+						{"(1,John,100)"},
+					},
+				},
+				{
+					Query:    "SELECT func2(111);",
+					Expected: []sql.Row{},
+				},
+			},
+		},
 		{
 			Name: "RETURNS SETOF with composite param",
 			SetUpScript: []string{