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

Zero value of slices #76

Closed
FcoManueel opened this issue Oct 11, 2015 · 6 comments
Closed

Zero value of slices #76

FcoManueel opened this issue Oct 11, 2015 · 6 comments

Comments

@FcoManueel
Copy link

short version: It would be nice if null::text[] gets decoded as []string(nil) instead of []string{}

A column of array type, let say text[], can be mapped to a slice of strings []string. In such column we can store both a null array (null::text[]) or an empty array ('{}'::text[]).
Go has equivalents for both of them, a null array would be []string(nil) and an empty array would be []string{}.

Currently the library makes no difference between null and empty arrays, and maps both to []string{}. The issue with this is that the zero value of a slice is a nil slice, not an empty one, so if I have an structure like the following:

type Object struct{
    id               string
    stringSlice []string
}

and then I create a new Object in the following way:

a := Object{id: "1"}

then a.stringSlice will have a value of []string(nil), but if I save a to the db and then retrieve it, the stringSlice property gets decoded as []string{}. Therefore, if I compare my original struct and my retrieved struct, they are different.

Here's a playground with the mentioned example: http://play.golang.org/p/N4rf_47sL-

Same thing applies to the other supported slice types.

Thanks for such a great package!

@vmihailenco
Copy link
Member

Hi,

Unfortunately I can't reproduce the issue:

package main

import (
    "fmt"

    "gopkg.in/pg.v3"
)

type Person struct {
    Emails []string
}

func main() {
    db := pg.Connect(&pg.Options{
        User: "postgres",
    })

    var person Person
    _, err := db.QueryOne(&person, `SELECT null::text[] AS emails`)
    if err != nil {
        panic(err)
    }
    fmt.Printf("%#v\n", person)

    var emails []string
    _, err = db.QueryOne(pg.LoadInto(&emails), `SELECT null::text[] AS emails`)
    if err != nil {
        panic(err)
    }
    fmt.Printf("%#v\n", emails)
}
main.Person{Emails:[]string(nil)}
[]string(nil)

And there is also a test for decoding nil slice: https://github.com/go-pg/pg/blob/master/types_test.go#L189

Can it be that you use old version of the package?

@FcoManueel
Copy link
Author

I see. I assumed that what I was seeing was happening while decoding the slice, but now I see its when inserting the value.

Inside the method appendStringSlice there's this part:

if len(v) == 0 {
    return append(dst, `{}`...)
}

and, since both empty and nil slices have length 0, we will append {} to the query for both.

I confirmed this by adding at line 80 the following check:

if reflect.DeepEqual(src, []string(nil)) {
    return appendNull(dst)
}

... of course there must be better ways to do it 😄specially since the same is happening for the other types of slices.

After adding that code, my original and retrieved structs were the same.

@FcoManueel
Copy link
Author

I'll paste here the code I used to reproduce my original use case, in case it's of any use:

First I created a table inside the test database

CREATE TABLE persons (
id text NOT NULL,
emails text[]
);

And this is my test code:

package main

import (
    "fmt"
    "gopkg.in/pg.v3"
)

type Person struct {
    ID     string
    Emails []string
}

type Persons[]*Person
func (contents *Persons) NewRecord() interface{} {
    content := &Person{}
    *contents = append(*contents, content)
    return content
}

func main() {
    db := pg.Connect(&pg.Options{
        User: "postgres",
        Database: "test",
    })
    db.Exec(`TRUNCATE TABLE persons;`)

    // Create and insert a person without emails
    var person1 = Person{ID: "1"}
    fmt.Printf("%#v\n", person1.Emails)
    _, err := db.ExecOne(`INSERT INTO persons(id, emails) VALUES (?id, ?emails);`, person1)

    // Get person from database
    var allPersons Persons
    _, err = db.Query(&allPersons, `SELECT id, emails FROM persons;`)
    if err != nil {
        panic(err)
    }

    if len(allPersons) != 1 {
        fmt.Println("len(allPersons) :", len(allPersons))
        panic("Expected only 1 person")
    }
    personFromDb := allPersons[0]
    fmt.Printf("%#v\n", personFromDb.Emails)
}

The obtained output was the following:

[]string(nil)
[]string{}

vmihailenco added a commit that referenced this issue Oct 13, 2015
Properly encode nil slices and maps. Fixes #76.
@vmihailenco
Copy link
Member

Thanks for the report. Should be fixed in v3.3.14.

@FcoManueel
Copy link
Author

Awesome, it works.
Thanks!

@vmihailenco
Copy link
Member

You are welcome!

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