go-ozzo / ozzo-dbx

A Go (golang) package that enhances the standard database/sql package by providing powerful data retrieval methods as well as DB-agnostic query building capabilities.
MIT License
636 stars 90 forks source link

Column 'Id' in where clause is ambiguous when InnerJoin used #47

Open kolkov opened 7 years ago

kolkov commented 7 years ago

Hi! Is it possible to get main table Id column only by default?

func (dao *DeviceDAO) Get(rs app.RequestScope, id int) (*models.Device, error) {
    var analysis models.Device
    err := rs.Tx().Select().
        InnerJoin("brand", dbx.NewExp("`device`.`brand_id` = `brand`.`id`")).
        Model(id, &analysis)
    return &analysis, err
}

Query SQL: SELECT * FROM `device` INNER JOIN `brand` ON `device`.`brand_id` = `brand`.`id` WHERE `Id`=5"

msg="Error 1052: Column 'Id' in where clause is ambiguous" UserId=1

lawzava commented 7 years ago

I think it already goes to SQL problems. Just try explicitly choosing what you need in Select. e.g. device.id as Id

kolkov commented 7 years ago

I am sure that we can make the necessary changes to this method.

func (s *SelectQuery) Model(pk, model interface{}) error {
    t := reflect.TypeOf(model)
    if t.Kind() == reflect.Ptr {
        t = t.Elem()
    }
    if t.Kind() != reflect.Struct {
        return VarTypeError("must be a pointer to a struct")
    }
    si := getStructInfo(t, s.FieldMapper)
    if len(si.pkNames) == 1 {
        return s.AndWhere(HashExp{si.nameMap[si.pkNames[0]].dbName: pk}).One(model)
    }

    if len(si.pkNames) == 0 {
        return MissingPKError
    }
    return CompositePKError
}

And use s.join to detect if second table used in this sql query. image

kPshi commented 6 years ago

In order to also prevent collisions when joining on a table twice one should better give every table an alias in every query and use this alias for every query. Like

SELECT * FROM `device` AS `device1` INNER JOIN `device` AS `device2` ON `device1`.`brand_id` = `device2`.`brand_id` WHERE `device1`.`Id`=5

A bit easier to implement would be to call tables t1 to t<n> but that might make statements way less readable when diagnosing problems. Personally I would not even try to leave out the alias or name it mytable when using mytable only once. That would save a few bytes only but make the code unnecessarily more complex.

The example above would then look like

SELECT * FROM `device` as `device1` INNER JOIN `brand` as `brand1` ON `device1`.`brand_id` = `brand1`.`id` WHERE `device1`.`Id`=5"