Decoding an array of composite types within a composite type

[ludvigalden]

Hello,

I love this package for code structure and its customizability. The only problem I have encountered so far is with the following composite type structure.

CREATE TYPE text_translation AS (language varchar(5), text text);

CREATE TYPE translatable_text AS (
    text text,
    translations text_translation []
);

For example, a value to be scanned for the translatable_text composite type could have the encoded value of (,"{""(sv-SE,\\""Ett svenskt meddelande\\"")""}").

This get parsed correctly to that translatable_text.text is empty and that the encoded value of the translatable_text.translations is "{""(sv-SE,\\""Ett svenskt meddelande\\"")""}".

Now, when decoding the translatable_text.translations, it splits the values incorrectly. In this case (sv-SE,\ is decoded to be the first value in the array, which is incorrect. It seems that it parses the comma that is at that point to be a separator for the array, but it is actually a separator for the text_translation type, after the text_translation.text field.

I hope I made the issue clear, and that you can find a solution to the problem.

It's certainly a rare case having an array of composite types within a composite type, but I think it maybe should be supported.

Thanks

[jackc]

What Go types are you using? I would expect pgtype.CustomComposite to be able to handle it.

[ludvigalden]

@jackc I'm using pgtype.NewCompositeType and pgtype.ConnInfo.RegisterDataType (after retrieving the OID for the composite type) to register the composite types and then pgtype.NewArrayType (with reference to the OID of the composite type) and again pgtype.ConnInfo.RegisterDataType to register the composite array types. When registering composite types with references to other composite types, the fields get set with the OID of the previously registered composite types.

I just got introduced to the package and implemented this yesterday so I'm not sure this is the right way to do it. Maybe you don't even need to work with OID:s, if using pgtype.CustomComposite? I figured it would be safest, in any case, because then you would be sure the types exist in the connected database.

[jackc]

I would expect what you are doing to work.

Registering the types is best because that will help ensure it uses the binary format internally which is significantly faster, but it is not always required. But your error seems to indicate it is using the text protocol...

If you can make a small reproducible case I might be able to take a look at it this weekend.

[jschaf]

I think the bug is still present. Here's a reproducible example with some debugging notes.

• Only fails on the text format. It works with the binary format.
• If I remove the triply nested dimensions type, the text decoder works

My guess is a something is wrong in quoting decode logic once you get to triple nesting in CompositeTextScanner.next. Here's the string the CompositeType tries to decode with the text format:

(name,"(img1,""(11,11)"")","{""(img2,\\""(22,22)\\"")"",""(img3,\\""(33,33)\\"")""}")

# Recursive decode calls:
(img1,"(11,11)")
(11,11)

# Next call after going through the Array decoder:
(img2,\

The text decoding fails with the error:

composite text format must end with ')'

Complete test case:

package pgx_composites

import (
	"context"
	"github.com/jackc/pgtype"
	"github.com/jackc/pgx/v4"
	"github.com/jschaf/pggen/internal/pgtest"
	"testing"
)

// ignoredOID means we don't know or care about the OID for a type. This is okay
// because pgx only uses the OID to encode values and lookup a decoder. We only
// use ignoredOID for decoding and we always specify a concrete decoder for scan
// methods.
const ignoredOID = 0

func newCompositeType(name string, fieldNames []string, vals ...pgtype.ValueTranscoder) *pgtype.CompositeType {
	fields := make([]pgtype.CompositeTypeField, len(fieldNames))
	for i, name := range fieldNames {
		fields[i] = pgtype.CompositeTypeField{Name: name, OID: ignoredOID}
	}
	// Okay to ignore error because it's only thrown when the number of field
	// names does not equal the number of ValueTranscoders.
	rowType, _ := pgtype.NewCompositeTypeValues(name, fields, vals)
	return rowType
}

func TestPgx_DecodeArrayComposites(t *testing.T) {
	schema := `
		CREATE TYPE dimensions AS (
			width  int4,
			height int4
		);

		CREATE TYPE product_image_type AS (
			source text,
			dimensions dimensions
		);

		CREATE TYPE product_image_set_type AS (
			name       text,
			orig_image product_image_type,
			images     product_image_type[]
		);
	`
	conn, cleanup := pgtest.NewPostgresSchemaString(t, schema)
	defer cleanup()

	dimensionsType := func() pgtype.ValueTranscoder {
		return newCompositeType(
			"dimensions",
			[]string{"width", "height"},
			&pgtype.Int4{},
			&pgtype.Int4{},
		)
	}
	productImageType := func() pgtype.ValueTranscoder {
		return newCompositeType(
			"product_image_type",
			[]string{"source", "dimensions"},
			&pgtype.Text{},
			dimensionsType(),
		)
	}
	productImageSetType := newCompositeType(
		"product_image_set_type",
		[]string{"name", "orig_image", "images"},
		&pgtype.Text{},
		productImageType(),
		pgtype.NewArrayType("product_image", ignoredOID, func() pgtype.ValueTranscoder {
			return productImageType()
		}),
	)

	sql := `
		SELECT
			ROW (
				'name',
				ROW ('img1', ROW (11, 11)::dimensions)::product_image_type,
				ARRAY [
					ROW ('img2', ROW (22, 22)::dimensions)::product_image_type,
					ROW ('img3', ROW (33, 33)::dimensions)::product_image_type
				]
			)::product_image_set_type;
   `
	t.Run("TextFormat", func(t *testing.T) {
		row := conn.QueryRow(context.Background(), sql, pgx.QueryResultFormats{pgx.TextFormatCode})
		if err := row.Scan(productImageSetType); err != nil {
			t.Error(err)
			return
		}
	})

	t.Run("BinaryFormat", func(t *testing.T) {
		row := conn.QueryRow(context.Background(), sql, pgx.QueryResultFormats{pgx.BinaryFormatCode})
		if err := row.Scan(productImageSetType); err != nil {
			t.Error(err)
			return
		}
	})
}

[jackc]

I think jackc/pgtype@4380e23 should fix this.