New Result Serialization #26
Conversation
|
@gropaul first and foremost: thank you for this useful contribution and welcome to quackscience! I think you might need to include |
|
@gropaul I see you're in Amsterdam - I got some stickers for you 👇 
|
|
Please don't merge this yet. I think there might be a bug in how the serializer is used. I'll take a look at it the next days |
|
|
||
| struct ReqStats { | ||
| float elapsed_sec; | ||
| uint64_t read_bytes; |
There was a problem hiding this comment.
Do we perhaps want to remove read_bytes and read_rows until they are used @lmangani ? Otherwise people will just be confused
There was a problem hiding this comment.
FYI read_bytes and read_rows are only used by the UI to display the results and emulate Clickhouse query stats
There was a problem hiding this comment.
But aren't hey always 0?
There was a problem hiding this comment.
They shouldn't be when using the JSONCompact format but I'll re-check
|
I moved the formatting configuration up to the root level mirroring duckdb/extension-template#101, so Should I submit a separate PR applying the DuckDB formatting guidelines to the repo? That way people can use the formatter without having to worry about a big unrelated git diff |
|
A this is awesome @NiclasHaderer thank you! |
|
@gropaul @NiclasHaderer thank you both for this marvelous opensource christmas present! |
|
I think it's ready. However I'm still convinced that the read bytes are set. |
Hi Dear Quackscience Team,
I was using the extension (I really love it!) and discovered that all the values are serialized as strings in the JsonCompact mode. This was sometimes a bit itchy for me, especially when I had list values or structs, etc., as I had to recursively parse the JSON to a class.
This is why I propose changing to this new parser, which handles all the (nested) types in more detail. I tested it with
SELECT * FROM test_all_types();and it did not crash (🎉). The new parser has large chunks of code from a parser that @NiclasHaderer once wrote, and I think he got inspired by the duckdb JSON extension.Also, I switched to an internal function to get the type name, as the current version often returned
string.Let me know what you think and if you have some change requests.
Best regards,
Paul