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

array_agg(table) returning a []Table? #15

Closed
sentriz opened this issue Mar 4, 2021 · 7 comments
Closed

array_agg(table) returning a []Table? #15

sentriz opened this issue Mar 4, 2021 · 7 comments

Comments

@sentriz
Copy link

sentriz commented Mar 4, 2021

hi thanks for the cool tool!

given a schema

create table if not exists screenshots (
    id bigint primary key               
);                                      

create table if not exists blocks (                            
    id serial primary key,                                     
    screenshot_id bigint not null references screenshots (id), 
    body text not null                                         
);                                                             

and query

-- name: SearchScreenshots :many                        
select                                                  
    screenshots.*,                                      
    array_agg(blocks) blocks                  
from                                                    
    screenshots                                         
    join blocks on blocks.screenshot_id = screenshots.id
where                                                   
    pggen.arg('Body') % blocks.body                    
group by                                                
    screenshots.id                                      
limit pggen.arg('Limit')
offset pggen.arg('Offset');  

('str' % column being a pg_trgm thing)

pggen generates

type SearchScreenshotsParams struct {                                                                            
    Body   string                                                                            
    Limit  int                                                                            
    Offset int                                                                            
}                                                                            
                                                                            
type SearchScreenshotsRow struct {                                                                            
    ID             pgtype.Int8      `json:"id"`                                                                                                                        
    Blocks         []Blocks         `json:"blocks"`                                                                            
}                                                                            
                                                                            
// SearchScreenshots implements Querier.SearchScreenshots.                                                                      
func (q *DBQuerier) SearchScreenshots(ctx context.Context, params SearchScreenshotsParams) ([]SearchScreenshotsRow, error) {    
    // some stuff
    return items, err
}                                                              

however the Blocks in SearchScreenshotsRow's []Blocks is undefined

so my question is,
is it somehow possible, or in the scope of this project, that pggen could generate something like

 Blocks         []Block         `json:"blocks"`

where Block is a struct with fields matching the blocks table?

also postgres shows the type of the array_agg(blocks) column as block[]
image

any help is much appreciated, thanks!

@jschaf
Copy link
Owner

jschaf commented Mar 5, 2021

Oh, interesting. If I understand correctly, the query builds an array of the entire composite type of Block. pggen definitely supports composite types (including create a corresponding Go type def as in https://github.com/jschaf/pggen/blob/main/example/nested/query.sql.go#L71), but I don't think I've tested it with an array of composite types.

Can you provide the pggen command that you used?

I'm able to reproduce the bug. I'll get cracking on fixing it.

@jschaf
Copy link
Owner

jschaf commented Mar 5, 2021

A few slightly off-topic comments not related to pggen.

@sentriz
Copy link
Author

sentriz commented Mar 5, 2021

Can you provide the pggen command that you used?

sure it's pggen gen go --schema-glob sql/schema.sql --query-glob sql/queries.sql --output-dir db --postgres-connection "blah"

I'm able to reproduce the bug. I'll get cracking on fixing it.

😍 brilliant thanks! i'd be very excited to use pggen then, I've just started experimenting with it yesterday and this feature would really seal the deal for me :)

pggen.arg('Body') % blocks.body

this is pg_trgm syntax

order by clause to guarantee stable results

I will do this, thanks 👌

keyset pagination

I was thinking of this one too, but my project is still in it's early days and I'm stilling figuring out a nice stack first

@jschaf jschaf closed this as completed in 29b8bc8 Mar 6, 2021
@jschaf jschaf reopened this Mar 6, 2021
@jschaf
Copy link
Owner

jschaf commented Mar 6, 2021

This is partially resolved by 29b8bc8. That commit fixes the generated code so it least compiles. It won't run however. Coaxing pgx to scan an array of composite types is difficult to do without knowing the OIDs ahead of time. If you implement the Decode methods for Blocks in the same package it will probably work.

I have a working sketch so that pggen will generate working code. It depends on a few things:

  • The composite fields consist of known postgres types. We need the OID because we're using pgtype.CompositeType instead of CompositeFields. We can't use CompositeFields because it doesn't implement all the methods needed by pgtype.Array.
  • This only works for decoding. We can't use the same logic for encoding because we don't know the OID for the composite type. We use 0 here which works because the OID is only need for decoding.
// SearchScreenshots implements Querier.SearchScreenshots.
func (q *DBQuerier) SearchScreenshots(ctx context.Context, params SearchScreenshotsParams) ([]SearchScreenshotsRow, error) {
	rows, err := q.conn.Query(ctx, searchScreenshotsSQL, params.Body, params.Limit, params.Offset)
	if rows != nil {
		defer rows.Close()
	}
	if err != nil {
		return nil, fmt.Errorf("query SearchScreenshots: %w", err)
	}
	items := []SearchScreenshotsRow{}
	blockRow, err := pgtype.NewCompositeType("blocks", []pgtype.CompositeTypeField{
		{Name: "id", OID: pgtype.Int4OID},
		{Name: "screenshot_id", OID: pgtype.Int8OID},
		{Name: "body", OID: pgtype.TextOID},
	}, q.conn.(*pgx.Conn).ConnInfo())
	if err != nil {
		return nil, fmt.Errorf("build composite type block: %w", err)
	}
	blockArray := pgtype.NewArrayType("_block", 0, func() pgtype.ValueTranscoder {
		return blockRow.NewTypeValue().(*pgtype.CompositeType)
	})
	for rows.Next() {
		var item SearchScreenshotsRow
		if err := rows.Scan(&item.ID, blockArray); err != nil {
			return nil, fmt.Errorf("scan SearchScreenshots row: %w", err)
		}
		blockArray.AssignTo(&item.Blocks)
		items = append(items, item)
	}
	if err := rows.Err(); err != nil {
		return nil, fmt.Errorf("close SearchScreenshots rows: %w", err)
	}
	return items, err
}

jschaf added a commit that referenced this issue Mar 7, 2021
@jschaf
Copy link
Owner

jschaf commented Mar 7, 2021

This should be fixed in 2dd57b9 and ad75bc3. Give it a go and re-open if you run into any issues.

There's a tested example in https://github.com/jschaf/pggen/tree/main/example/composite

This is one of the more brittle implementations that pggen supports. There's a few caveats:

  • pggen prefers to use the pgtype types like pgtype.Int8 for composite fields instead of whatever was passed on the command line via --go-flag. If we can't find an existing pgtype, we use the type in --go-flag. This type must be a pgtype.ValueTranscoder.

  • An array of composite types cannot contain a field that is also a nested composite type. This is a temporary restriction. It's possible to support arbitrarily nested composite types but it's tricky because the pgtype.NewCompositeType constructor returns an error so we can't build the struct in one go.

  • An array of composite types cannot contain field that is also an array type. Similar restriction as above.

@jschaf jschaf closed this as completed Mar 7, 2021
@sentriz
Copy link
Author

sentriz commented Mar 7, 2021

amazing! this is great news. i'll be trying it out very shortly

@sentriz
Copy link
Author

sentriz commented Mar 14, 2021

this works really well, thanks very much :)

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

2 participants