attic-labs / noms

The versioned, forkable, syncable database
Apache License 2.0
7.44k stars 266 forks source link

Adding structs with empty values fails #3831

Closed lak closed 5 years ago

lak commented 5 years ago

Hard to discover when dealing with large datasets, but easy to reproduce:

noms struct new --name Song /tmp/testdb attribute ""
lak commented 5 years ago

Here's the stacktrace:

        Error Trace:    try.go:99
                    try.go:51
                    noms_set.go:106
                    noms_struct.go:112
                    noms_struct.go:52
                    noms_struct.go:38
                    noms.go:100
                    proc.go:201
                    asm_amd64.s:1333
    Error:          Expected true

goroutine 1 [running]:
github.com/attic-labs/noms/go/d.PanicIfTrue(0xc000200a01)
    /Users/luke/src/github.com/attic-labs/noms/go/d/try.go:51 +0xaa
main.argumentToValue(0x7ffeefbffa1a, 0x0, 0x1b25c40, 0xc000200ba0, 0x0, 0x0, 0x0, 0x0)
    /Users/luke/src/github.com/attic-labs/noms/cmd/noms/noms_set.go:106 +0x46
main.applyStructEdits(0x183e6f0, 0x3, 0x7ffeefbffa04, 0xb, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, ...)
    /Users/luke/src/github.com/attic-labs/noms/cmd/noms/noms_struct.go:112 +0x1d4
main.nomsStructNew(0x7ffeefbffa04, 0xb, 0x7ffeefbff9ff, 0x4, 0xc000200a40, 0x2, 0x2, 0x6)
    /Users/luke/src/github.com/attic-labs/noms/cmd/noms/noms_struct.go:52 +0x203
main.nomsStruct.func1(0xc00009fa80, 0xa, 0xc00009fa80)
    /Users/luke/src/github.com/attic-labs/noms/cmd/noms/noms_struct.go:38 +0x296
main.main()
    /Users/luke/src/github.com/attic-labs/noms/cmd/noms/noms.go:100 +0xd78
lak commented 5 years ago

Note that I could see this not being considered a bug, but at least in that case the error message should be improved. IMO, though, empty strings should be allowed.

aboodman commented 5 years ago

eep, yeah, looks like I for some reason thought it was impossible to end up with an empty string here. That's silly.

On Fri, Feb 1, 2019 at 6:34 PM Luke Kanies notifications@github.com wrote:

Note that I could see this not being considered a bug, but at least in that case the error message should be improved. IMO, though, empty strings should be allowed.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/attic-labs/noms/issues/3831#issuecomment-459927641, or mute the thread https://github.com/notifications/unsubscribe-auth/AAE6BJWA3moolhg-yn3tnTpiYeW4Ximaks5vJPlKgaJpZM4afX0O .