zig-sqlite icon indicating copy to clipboard operation
zig-sqlite copied to clipboard

Allow Db.exec_multi to take query with spaces

Open greenfork opened this issue 3 years ago • 6 comments

This allows to keep SQL queries in a separate file where spaces and newlines are very common.

greenfork avatar Aug 28 '22 17:08 greenfork

I think this already works, it's tested here. i tested locally with spaces too and it works fine, sqlite already ignores whitespaces and comments.

Do you have a test case that doesn't work with the current code ?

vrischmann avatar Aug 28 '22 17:08 vrischmann

Right, thanks. The problem is the final newline. I have a separate SQL file and I do @embedFile, so the trailing newline is in the variable as well. What do you think the correct approach would be? One option is to make callers strip final whitespace. Mine option is a bit too much since we only need it at the end, at the same I'm not sure I know a better place to strip the final whitespace.

test "sqlite: exec multi newline" {
    var db = try getTestDb();
    defer db.deinit();
    try db.execMulti("create table a(b int);\n", .{});
    const val = try db.one(i32, "select max(c) from b", .{}, .{});
    try testing.expectEqual(@as(?i32, 0), val);
}

greenfork avatar Aug 29 '22 02:08 greenfork

By the way, fixing it on the caller's site is more involved than just query[0 .. query.len - 1] since in DynamicStatement.prepare it takes query.ptr, so we need to insert a null at the end of the query before the final whitespace.

greenfork avatar Aug 29 '22 12:08 greenfork

I'm still not sure I understand the problem. Your test case fails for another reason, you're selecting from the table b which doesn't exist.

This works fine:

test "sqlite: exec multi newline" {
    var db = try getTestDb();
    defer db.deinit();
    try db.execMulti("create table a(b int);\n", .{});
    const val = try db.one(i32, "select max(b) from a", .{}, .{});
    try testing.expectEqual(@as(?i32, 0), val);
}

vrischmann avatar Aug 29 '22 21:08 vrischmann

Running this test on master

test "sqlite: exec multi newline" {
    var diags = Diagnostics{};
    var db = try getTestDb();
    defer db.deinit();
    db.execMulti("create table a(b int);\n", .{
        .diags = &diags,
    }) catch |err| {
        std.log.err("Got error {}. Diagnostics: {s}", .{ err, diags });
        return err;
    };
}

with zig built test produces this output:

Test [3/50] test "native-Debug-multi sqlite: exec multi newline"... [default] (err): Got error error.SQLiteError. Diagnostics: {code: 0, near: -1, message: the input query is not valid SQL (empty string or a comment)}
Test [3/50] test "native-Debug-multi sqlite: exec multi newline"... FAIL (SQLiteError)
/home/grfork/reps/kisa/deps/zig-sqlite/sqlite.zig:1599:17: 0x24d826 in DynamicStatement.prepare (test)
                return error.SQLiteError;
                ^
/home/grfork/reps/kisa/deps/zig-sqlite/sqlite.zig:486:16: 0x244044 in Db.prepareDynamicWithDiags (test)
        return try DynamicStatement.prepare(self, query, options, 0);
               ^
/home/grfork/reps/kisa/deps/zig-sqlite/sqlite.zig:861:24: 0x2381db in Db.execMulti (test)
                stmt = try self.prepareDynamicWithDiags(new_query, new_options);
                       ^
/home/grfork/reps/kisa/deps/zig-sqlite/sqlite.zig:2360:9: 0x21f4f1 in test "native-Debug-multi sqlite: exec multi newline" (test)
        return err;
        ^
49 passed; 0 skipped; 1 failed.
1 errors were logged.
error: the following test command failed with exit code 1:
/home/grfork/reps/kisa/deps/zig-sqlite/zig-cache/o/17d07fb7d4d2aa8d347213801e4f8ea7/test /home/grfork/zig-linux-x86_64-0.10.0-dev.3685+dae7aeb33/zig
error: test...
error: The following command exited with error code 1:
/home/grfork/zig-linux-x86_64-0.10.0-dev.3685+dae7aeb33/zig test -fstage1 /home/grfork/reps/kisa/deps/zig-sqlite/sqlite.zig -lc -lsqlite3 --test-name-prefix native-Debug-multi  --cache-dir /home/grfork/reps/kisa/deps/zig-sqlite/zig-cache --global-cache-dir /home/grfork/.cache/zig --name test -fno-single-threaded --pkg-begin build_options /home/grfork/reps/kisa/deps/zig-sqlite/zig-cache/options/JWYz7ByIA0maQkPaEkvjasUVcgwDdWXrViXUDmcywjW9syJoPrPfTtJcntMCWnYa --pkg-end --enable-cache
error: the following build command failed with exit code 1:
/home/grfork/reps/kisa/deps/zig-sqlite/zig-cache/o/e8c7e47bf7278113a988e32bda21a73b/build /home/grfork/zig-linux-x86_64-0.10.0-dev.3685+dae7aeb33/zig /home/grfork/reps/kisa/deps/zig-sqlite /home/grfork/reps/kisa/deps/zig-sqlite/zig-cache /home/grfork/.cache/zig test

When I run your example test, I get this output:

Test [3/50] test "native-Debug-multi sqlite: exec multi newline"... FAIL (SQLiteError)
/home/grfork/reps/kisa/deps/zig-sqlite/sqlite.zig:1599:17: 0x24d866 in DynamicStatement.prepare (test)
                return error.SQLiteError;
                ^
/home/grfork/reps/kisa/deps/zig-sqlite/sqlite.zig:486:16: 0x244084 in Db.prepareDynamicWithDiags (test)
        return try DynamicStatement.prepare(self, query, options, 0);
               ^
/home/grfork/reps/kisa/deps/zig-sqlite/sqlite.zig:861:24: 0x2380cb in Db.execMulti (test)
                stmt = try self.prepareDynamicWithDiags(new_query, new_options);
                       ^
/home/grfork/reps/kisa/deps/zig-sqlite/sqlite.zig:2355:5: 0x21f334 in test "native-Debug-multi sqlite: exec multi newline" (test)
    try db.execMulti("create table a(b int);\n", .{});
    ^
49 passed; 0 skipped; 1 failed.
error: the following test command failed with exit code 1:
/home/grfork/reps/kisa/deps/zig-sqlite/zig-cache/o/499c5766f7fba74d265a6478b34ad25f/test /home/grfork/zig-linux-x86_64-0.10.0-dev.3685+dae7aeb33/zig
error: test...
error: The following command exited with error code 1:
/home/grfork/zig-linux-x86_64-0.10.0-dev.3685+dae7aeb33/zig test -fstage1 /home/grfork/reps/kisa/deps/zig-sqlite/sqlite.zig -lc -lsqlite3 --test-name-prefix native-Debug-multi  --cache-dir /home/grfork/reps/kisa/deps/zig-sqlite/zig-cache --global-cache-dir /home/grfork/.cache/zig --name test -fno-single-threaded --pkg-begin build_options /home/grfork/reps/kisa/deps/zig-sqlite/zig-cache/options/JWYz7ByIA0maQkPaEkvjasUVcgwDdWXrViXUDmcywjW9syJoPrPfTtJcntMCWnYa --pkg-end --enable-cache
error: the following build command failed with exit code 1:
/home/grfork/reps/kisa/deps/zig-sqlite/zig-cache/o/e8c7e47bf7278113a988e32bda21a73b/build /home/grfork/zig-linux-x86_64-0.10.0-dev.3685+dae7aeb33/zig /home/grfork/reps/kisa/deps/zig-sqlite /home/grfork/reps/kisa/deps/zig-sqlite/zig-cache /home/grfork/.cache/zig test

Could it be caused by different Zig versions? I use 3685+dae7aeb33

greenfork avatar Aug 30 '22 02:08 greenfork

test "sqlite: exec multi newline" { var db = try getTestDb(); defer db.deinit(); try db.execMulti("create table a(b int);\n", .{}); const val = try db.one(i32, "select max(b) from a", .{}, .{}); try testing.expectEqual(@as(?i32, 0), val); }

This also fails in my environment.

Test [2/49] test "native-Debug-multi sqlite: exec multi"... [default] (err): diags is {code: 0, near: -1, message: the input query is not valid SQL (empty string or a comment)}

Zig version: "0.10.0-dev.3071+8e75ba653"

jiacai2050 avatar Aug 30 '22 04:08 jiacai2050

Hi @greenfork. It's been a while, do you still have this issue ?

vrischmann avatar Jun 03 '23 17:06 vrischmann

I don't use the updated versions right now, I think we can close it and I will re-open if it comes up again.

greenfork avatar Jun 04 '23 02:06 greenfork