remove dummy_diags and embed a default diag in Db for use

[jiacai2050]

There are some dummy_diags used in several API, it is called dummy because user can't access it, some example:

• https://github.com/vrischmann/zig-sqlite/blob/00ac50a1b79ea5cec2e8ed2fc344bc2f546c171f/sqlite.zig#L1143
• https://github.com/vrischmann/zig-sqlite/blob/00ac50a1b79ea5cec2e8ed2fc344bc2f546c171f/sqlite.zig#L1574

I propose another way to populate diags

Add a Diagnostics object in Db, and use it instead of create a dummy one if user don't provide it

Users don't need to create a diag object when invoke API after adopt this, and I think this make it easy for user to use.

Any ideas?

[vrischmann]

Hi,

technically it's called dummy because it doesn't serve any purpose aside from making it easier to implement internal functions which take a Diagnostics pointer. Doing this:

var dummy_diags = Diagnostics{};
var diags = options.diags orelse &dummy_diags;

means I always have a valid Diagnostics pointer and I don't have to do a if (diags) |p| { } check everytime.

But I could have just use a ?*Diagnostics everywhere and the dummy_diags wouldn't be necessary, I could just write this:

if (options.diags) |diags| {
    diags.err = getLastDetailedErrorFromDb(db.db);
}

If we added a Diagnostics field it would not be usable if you execute queries in parallel from multiple threads: they would step on each other. IMO the Diagnostics object should be allocated and freed as close as possible to the call site of the public function which takes the Diagnostics, that way you don't have to worry about concurrency. I think the current API is fine as is.

[jiacai2050]

IMO the Diagnostics object should be allocated and freed as close as possible to the call site of the public function which takes the Diagnostics,

After some thought, this make sense to as well. We should be careful with memory allocation in low-level programming.

BTW, https://github.com/Hejsil/zig-clap also use this Diagnostics pattern to populate error details.