Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Composite Types in Conjunction with RowToStructByName #2180

Open
jacobmikesell opened this issue Nov 26, 2024 · 3 comments
Open

Composite Types in Conjunction with RowToStructByName #2180

jacobmikesell opened this issue Nov 26, 2024 · 3 comments

Comments

@jacobmikesell
Copy link

jacobmikesell commented Nov 26, 2024

Is your feature request related to a problem? Please describe.
I'd like to be able to maintain only a stable struct mapping (with no special interfaces) even when using a composite type in the query.

Example (the names aren't great sorry):

type myOtherTable struct {
      SomeId int64
      SomeBool bool
}
type myTable struct {
     SomeField string
     TableValue []myOtherTable
}


func (x *myTable) ScanRow(rows pgx.Rows) error {
	val, err := pgx.RowToStructByName[myTable](rows)
	if err != nil {
		return err
	}
	*x = val
	return nil
}
create table my_table (pk bigserial primary key, some_field text);
create table my_other_table (pk bigserial primary key, my_fk bigint, some_bool bool);

select some_field, array_agg(mo) as table_value from
my_table inner m join my_other_table mo on m.pk = mo.my_fk

I'd like to be able to

var result myTable
pool.QueryRow("query").Scan(&myTable)

Describe the solution you'd like
I think this should be reasonable. The composite pgtype appears to have access to the column names, given that a "default" behavior (or new interface to meet) seems like it could use reflection to match to struct fields (by tag/default) instead of requiring field index matching.

Describe alternatives you've considered
I've implemented by meeting the existing interface, but it ends up creating friction when the rest of the struct maps by struct tag.

Additional context
I'm willing to put up a PR for this if the design/idea is valid and fits the ecosystem here!

@jackc
Copy link
Owner

jackc commented Nov 29, 2024

This is already work given my_other_table and _my_other_table are registered. See pgx.Conn.LoadType and pgtype.Map.RegisterType.

  • Your myOtherTable struct would need to exactly match the columns in the table.

@WhiskeyJack96
Copy link

Hey @jackc and @jacobmikesell I stumbled across this while preparing to open an issue for the same, but I wanted to be able to remap the struct field names with the db tag (similar to how RowToStructByName handles it) for the composite types.

I put up #2230 as a possible method for handling this, would this be acceptable? Or is there another (perhaps better) way to handle this?

@jacobmikesell
Copy link
Author

Another Option would be exposing a factory for "type specific" composite codec, which is one direction we're considering.

This has the benefit of not needing a magic interface, and giving the db tag the same meaning regardless of scan context.

While this can be done without any change to PGX I believe the feature would pair well with the existing "struct" scanning, though I'm not sure how it could be added in a backwards compatible way. Perhaps as a fallback when the "CompositeIndexScanner" interface is not met?
This would require moving the logic for field mapping into PlanScan but it could easily be cached similar to RowToStruct* methods

Example:

func NewCompositeCodec(t reflect.Type, fields ...pgtype.CompositeCodecField) *CompositeCodec {
	//Map fields to struct fields of T to get the indexes we need
	switch t.Kind() {
	case reflect.Struct:
	default:
		panic(fmt.Sprintf("unsupported type %v", t.Kind()))
	}
	indexToField := make(map[int][]int)
	for i := range t.NumField() {
		field := t.Field(i)
		if !field.IsExported() {
			continue
		}
		value, ok := field.Tag.Lookup("db")
		if !ok {
			value = field.Name
		}
		if value == "-" {
			continue
		}
		for i, codecField := range fields {
			if strings.EqualFold(codecField.Name, value) {
				indexToField[i] = field.Index
			}
		}
	}

        //Wrap the pgtype composite codec to since most of the methods are the same
	return &CompositeCodec{CompositeCodec: &pgtype.CompositeCodec{Fields: fields}, indexToField: indexToField}
}


func (plan *scanPlanTextCompositeToCompositeIndexScanner) Scan(src []byte, target any) error {
	targetScanner := reflect.ValueOf(target).Elem()
	if src == nil {
		targetScanner.Set(reflect.Zero(targetScanner.Type()))
		return nil
	}

	scanner := plan.s.NewCompositeScanner(plan.m, src)
	for i, field := range plan.cc.Fields {
		if scanner.Next() {
			index, ok := plan.cc.indexToField[i]
			if !ok {
				//Skip if lax/error if not
				continue
			}
			fieldTarget := targetScanner.FieldByIndex(index).Addr().Interface()
			if fieldTarget != nil {
				fieldPlan := plan.m.PlanScan(field.Type.OID, plan.s.Format(), fieldTarget)
//abreviated

//Usage

		t, err := conn.LoadType(ctx, "my_custom_type")
		if err != nil {
			return err
		}
		t.Codec = NewCompositeCodec(reflect.TypeOf(MyCustomType{}), t.Codec.(*pgtype.CompositeCodec).Fields...)
		conn.TypeMap().RegisterType(t)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants