stephenafamo / bob

SQL query builder and ORM/Factory generator for Go with support for PostgreSQL, MySQL and SQLite
https://bob.stephenafamo.com
MIT License
701 stars 37 forks source link

incorrect way of loading mac address. #233

Closed d0zer11st closed 2 days ago

d0zer11st commented 3 weeks ago

Getting mac address not the same I stored before.

I think, the issue is in the Stringer.Scan() func, for macaddr the underlying type is net.HardwareAddr, simple type conversion is not the way to use it.

https://github.com/stephenafamo/bob/blob/52634026e61a207a4e4efa2959932f1913f2176f/types/marshal.go#L76-L89

how to reproduce:

  1. bob version: github.com/stephenafamo/bob v0.27.1
  2. postgres 16.2
  3. Table
    create table example
    (
    id bigint primary key generated always as identity,
    mac           macaddr not null,
    hostname text not null
    )
  4. Code example.

    macToTest := "1b:1b:63:84:45:e6"
    hwAddr, parseMacErr := net.ParseMAC(macToTest)
    if parseMacErr != nil {
        log.Fatal(parseMacErr)
    }
    log.Printf("parsed mac: %s", hwAddr)
    hostNameString := uuid.New().String()
    exampleSetter := &models.ExampleSetter{
        Mac:      omit.From(types.Stringer[net.HardwareAddr]{Val: hwAddr}),
        Hostname: omit.From(hostNameString),
    }
    
    _, err := models.Examples.InsertMany(ctx, bobDB, exampleSetter)
    if err != nil {
        log.Fatal(err)
    }
    
    selectQueryWhere := psql.Select(
        sm.From(models.TableNames.Examples),
        sm.Where(
            models.ExampleColumns.Hostname.EQ(
                psql.Arg(hostNameString),
            ),
        ),
    )
    
    all, err := bob.All(ctx, bobDB, selectQueryWhere, scan.StructMapper[*models.Example]())
    if err != nil {
        log.Fatal(err)
    }
    
    hwAddrStrConv := net.HardwareAddr(macToTest)
    
    fmt.Printf("strconv: %s\n", hwAddrStrConv.String())  // the same way it implemented in Stringer.Scan()
    fmt.Printf("     db: %s\n", all[0].Mac.Val.String()) // loaded value
    fmt.Printf(" parsed: %s\n", hwAddr.String())         // parsed
    
    if !strings.EqualFold(all[0].Mac.Val.String(), hwAddr.String()) {
        log.Fatalf("%s is not the same as %s", all[0].Mac.Val.String(), hwAddr.String())
    }

Print results:

strconv: 31:62:3a:31:62:3a:36:33:3a:38:34:3a:34:35:3a:65:36
     db: 31:62:3a:31:62:3a:36:33:3a:38:34:3a:34:35:3a:65:36
 parsed: 1b:1b:63:84:45:e6

I also tried to insert Mac with just type conversion like this:

    hostNameString := uuid.New().String()

        macToTest := "1b:1b:63:84:45:e6"
    hwAddr := net.HardwareAddr(macToTest)

    exampleSetter := &models.ExampleSetter{
        Mac:      omit.From(types.Stringer[net.HardwareAddr]{Val: hwAddr}),
        Hostname: omit.From(hostNameString),
    }

    _, err := models.Examples.InsertMany(ctx, bobDB, exampleSetter)
    if err != nil {
        log.Fatal(err)
    }

that leads to error

ERROR: invalid input syntax for type macaddr: "31:62:3a:31:62:3a:36:33:3a:38:34:3a:34:35:3a:65:36" (SQLSTATE 22P02)
stephenafamo commented 3 weeks ago

Yeah, I see the issue.

I should not have assumed that it would be able to work like this, types.Stringer as a whole probably does not work. I would probably have to create a dedicated type for it.

I wish net.HardwareAddr implemented Text or Binary Marshaler.

d0zer11st commented 3 weeks ago

yeah, as I see there is only one usage of Stringer and it is for macaddr. the dedicated type would work better.

stephenafamo commented 2 days ago

This should have been fixed in v0.28.0 which was released on 2024-06-25