go-jet / jet

Type safe SQL builder with code generation and automatic query result data mapping
Apache License 2.0
2.52k stars 118 forks source link

Additional select field changes record count. #277

Closed CmpHDL closed 11 months ago

CmpHDL commented 12 months ago

Describe the bug When adding a table.Records.AllColumns as a field in the select, instead of multiple records only one is returned. I double checked the stmt.DebugSQL() and ran the query as is and it returned multiple records in the console.

Environment

Code snippet

type CStruct struct {
    model.Cs
}

type R struct {
    PField  decimal.Decimal `json:"p" alias:"p"`
    Class  string       `json:"class" alias:"class"`
    Org CStruct  `json:"-"`
}

Not working: (works but only 1 record is returned.)

func (r R) Get(eid int64) ([]R, error) {
    var rs []R

    db := database.GetDB()
    stmt := table.Aes.
        SELECT(postgres.
            MIN(table.Aes.PField).AS("R.p"),
            table.Aes.Class.AS("R.class"),
            table.Cs.AllColumns).
        FROM(table.Aes.
            INNER_JOIN(table.Bs, table.Aes.BID.EQ(table.Bs.ID)).
            INNER_JOIN(table.Cs, table.Bs.CID.EQ(table.Cs.ID)).
            INNER_JOIN(table.Ds, table.Aes.DID.EQ(table.Ds.ID)).
            INNER_JOIN(table.Es, table.Ds.ID.EQ(table.Es.DID))).
        WHERE(table.Es.ID.EQ(postgres.Int(eid)).
                GROUP_BY(table.Aes.Class, table.Cs.ID).
        ORDER_BY(postgres.MIN(table.Aes.PField))
    if err := stmt.Query(db, &rs); err != nil {
        return nil, err
    }
}

Working: (returns multiple records but obviously does not fill in Org which I need filled in) Only line that was removed was table.Cs.AllColumns

func (r R) Get(eid int64) ([]R, error) {
    var rs []R

    db := database.GetDB()
    stmt := table.Aes.
        SELECT(postgres.
            MIN(table.Aes.PField).AS("R.p"),
            table.Aes.Class.AS("R.class")).
        FROM(table.Aes.
            INNER_JOIN(table.Bs, table.Aes.BID.EQ(table.Bs.ID)).
            INNER_JOIN(table.Cs, table.Bs.CID.EQ(table.Cs.ID)).
            INNER_JOIN(table.Ds, table.Aes.DID.EQ(table.Ds.ID)).
            INNER_JOIN(table.Es, table.Ds.ID.EQ(table.Es.DID))).
        WHERE(table.Es.ID.EQ(postgres.Int(eid)).
                GROUP_BY(table.Aes.Class, table.Cs.ID).
        ORDER_BY(postgres.MIN(table.Aes.PField))
    if err := stmt.Query(db, &rs); err != nil {
        return nil, err
    }
}

I tried a bunch of things with aliasing, direct dest, etc. Something odd is going on with adding the select. Sorry in advanced for the letter tables.

CmpHDL commented 12 months ago

I downgraded to 2.7.0 and it still does not seem to be working. I'm 100% positive it was working before though. So strange.

CmpHDL commented 12 months ago

It was working before not because of a different version but because I didn't need/have the Org before so the table.Cs.AllColumns wasn't there. After I added it, it only appeared to have been working because one record would return fine and there's usually only 1-2 but it was truly always returning 1 record even if there were two.

go-jet commented 12 months ago

Since CStruct embeds model.Cs(and I assume it has a field with a primary key tag), all the query rows are now grouped around model.Cs primary key. You'll always get a single' R' result if all the rows have the same Cs primary key. To fix it, you can change the destination, for instance:

type R struct {
        Org CStruct  `json:"-"`
        List []struct {
        PField  decimal.Decimal `json:"p" alias:"p"`
        Class  string       `json:"class" alias:"class"`
        }
}

You'll also need to update query aliases accordingly. Or you can introduce another primary key field so the results are not grouped only over Cs primary key.

type R struct {
        SomeID int `sql:"primary_key"`
    PField  decimal.Decimal `json:"p" alias:"p"`
    Class  string       `json:"class" alias:"class"`
    Org CStruct  `json:"-"`
}
CmpHDL commented 11 months ago

Sweet and thank you @go-jet that makes sense, I'm trying to implement this now. This will kinda nerf any min/max or similar grouped queries that also have any structs since you can't GROUP_BY the MIN's ID. Is there a way we can rows.Next() or similiar to bypass this? Your first suggestion will probably work fine, just brainstorming.

I can see why it does this but not sure I agree that it should work this way. Is it using a map under the hood which uses the first primary key it finds?

CmpHDL commented 11 months ago

Since CStruct embeds model.Cs(and I assume it has a field with a primary key tag), all the query rows are now grouped around model.Cs primary key. You'll always get a single' R' result if all the rows have the same Cs primary key. To fix it, you can change the destination, for instance:

type R struct {
        Org CStruct  `json:"-"`
        List []struct {
      PField  decimal.Decimal `json:"p" alias:"p"`
      Class  string       `json:"class" alias:"class"`
        }
}

This worked, thank you! I just made an anonymous dest and looped over over it to recreate the slice. Still curious about situations where looping through again would be less ideal.

go-jet commented 11 months ago

Is there a way we can rows.Next() or similiar to bypass this?

Yes, there is a Rows method.

I can see why it does this but not sure I agree that it should work this way. Is it using a map under the hood which uses the first primary key it finds?

Yes, there is a map with an entry for each composite primary key on each level - wiki.