vrischmann / zig-sqlite

zig-sqlite is a small wrapper around sqlite's C API, making it easier to use with Zig.
MIT License
367 stars 49 forks source link

Error when using statement iterator #90

Closed johanfforsberg closed 2 years ago

johanfforsberg commented 2 years ago

Hi!

This library looks very neat! However I ran into a problem.

I have created a statement and want to iterate over the results, e.g:

var iter = try stmt.iterator(MyItem, .{});
while(try iter.next(.{}))` |item| {
...

This raises a compile error:

./libs/zig-sqlite/sqlite.zig:1355:17: error: options passed to readPointer must have an allocator field
                @compileError("options passed to readPointer must have an allocator field");
                ^
./libs/zig-sqlite/sqlite.zig:1470:53: note: called from here
                    .Pointer => try self.readPointer(FieldType, options, i),
                                                    ^
./libs/zig-sqlite/sqlite.zig:1436:47: note: called from here
                const ret = try self.readField(field.field_type, options, i);
                                              ^
./libs/zig-sqlite/sqlite.zig:1118:47: note: called from here
                    return try self.readStruct(options);
                                              ^
./libs/zig-sqlite/sqlite.zig:1067:64: note: called from here
        pub fn next(self: *Self, options: QueryOptions) !?Type {

However the QueryOptions taken by next() does not have such a field.

I worked around it by adding an allocator field to QueryOptions but it's unclear to me if it's really necessary or if the check is in error.

I am using your master branch, and zig 0.10.0-dev.2431+0e6285c8f.

vrischmann commented 2 years ago

If allocation is needed you should instead use nextAlloc.

The error is actually really misleading if you don't know the internals, I'll have to work on that.

The explanation is that we call readStruct and readField in both cases (using next or nextAlloc) and because their options argument is anytype we can provide an anonymous struct with an allocator or not. See the code. To make the error clear next should just do its own verification that the type doesn't require allocation.

Hope that helps.

johanfforsberg commented 2 years ago

Oh that was quick :)

So, the actual error is that I need allocation but am trying to use the non allocating version. I should have read the code more carefully, I am still a little confused over what needs allocation in this case. Anyway, I got it to work with iterAlloc() now, thanks!

(The readme needs a small update though, the example uses &area.allocator which should be arena.allocator() in zig 0.10.)

vrischmann commented 2 years ago

So, the actual error is that I need allocation but am trying to use the non allocating version. I should have read the code more carefully, I am still a little confused over what needs allocation in this case. Anyway, I got it to work with iterAlloc() now, thanks!

I'll try to explain.

Basically any pointer types (single element, slice) requires an allocator because they aren't copyable. If you copy a slice for example, you're not copying its memory, only the pointer and length. Primitive types and arrays are copyable and thus don't require an allocator.

It probably would have been possible to design the iterator in such a way that it uses a single allocator for calls to next but I think taking an allocator per iteration is more flexible (for example you can easily use an arena per row and then you could give ownership of this "row data" to some other code).

So, the actual error is that I need allocation but am trying to use the non allocating version. I should have read the code more carefully, I am still a little confused over what needs allocation in this case. Anyway, I got it to work with iterAlloc() now, thanks!

Thanks, I'll fix it.