Bug in virtual table BestIndex/Filter implementation

[patrickdevivo]

Hi there! First off, thank you so much for all the work that's been put into this package, it's really great and I (and I'm sure many others) really appreciate all the continued maintenance and improvements.

I heavily use the bindings to the SQLite virtual table callbacks, in particular for this project. When implementing virtual tables that have a more complicated usage of BestIndex/Filter, I noticed that the idxStr value of the sqlite3_index_info struct is getting lost between the call to BestIndex and Filter. Digging around a bit, I see this line: https://github.com/mattn/go-sqlite3/blob/master/sqlite3_opt_vtable.go#L476 which appears to free the value immediately after goBestIndex completes, making it unavailable when it makes its way back into the go Filter call here: https://github.com/mattn/go-sqlite3/blob/master/sqlite3_opt_vtable.go#L516 (I believe).

Basically, in a go vtab implementation, I'll set IdxStr on the *sqlite3.IndexResult returned by BestIndex, however it's an empty string when Filter ends up being called.

I've "fixed" this by pointing to this fork/branch: master...patrickdevivo:free-idx-str which removes the deferred free line. The idxStr param now seems to be set fine. However, I'm unsure if this possibly introduces a memory leak if that string is not properly freed.

This "fix" is good enough for me for now, however I'd love some guidance on what the "right way" might be and I'd be happy to open a PR here.

Thanks so much!

[rittneje]

Yes, your change will introduce a memory leak that will grow without bound each time goVBestIndex (and by extension, xBestIndex) is called. According to the SQLite documentation, the way to handle this situation is to use sqlite3_malloc, and set needToFreeIdxStr in the sqlite3_index_info struct.

info.idxStr = (*C.char)(C.sqlite3_malloc(len(res.IdxStr)+1))
if info.idxStr == nil {
    // C.malloc and C.CString ordinarily do this for you. See https://golang.org/cmd/cgo/
    panic("out of memory")
}
info.needToFreeIdxStr = 1

idxStr := *(*[]byte)(unsafe.Pointer(&reflect.SliceHeader{
    Data: uintptr(unsafe.Pointer(info.idxStr)),
    Len: len(res.IdxStr)+1),
    Cap: len(res.IdxStr)+1),
}))
copy(idxStr, res.IdxStr)
idxStr[len(idxStr)-1] = 0 // null-terminated string

[patrickdevivo]

@rittneje thank you for the guidance! I've opened up this PR: #898 based on your snippet