cldellow / datasette-parquet

Add DuckDB, Parquet, CSV and JSON lines support to Datasette
Apache License 2.0
47 stars 7 forks source link

`Row` class is not JSON serializable #13

Closed Mause closed 1 year ago

Mause commented 1 year ago

You can see this by loading this link: https://dux.fly.dev/parquet.json?sql=select+version%28%29

Which returns:

{
  "ok": false,
  "error": "Object of type Row is not JSON serializable",
  "status": 500, 
  "title": null
}
Mause commented 1 year ago

Probably just needs a special case here? https://github.com/cldellow/datasette-parquet/blob/main/datasette_parquet/patches.py

cldellow commented 1 year ago

Thanks for the report + repro case! True, we don't support serializing Row -- I think that'll be needed to support DuckDB's composite types (list, struct, map).

But poking around briefly, I think version() probably just returns a vanilla string, so something else must be going on that's causing grief.

For example, the non JSON endpoint at https://dux.fly.dev/parquet?sql=select+version%28%29 renders the way I'd expect.

In order to interop with Datasette, this plugin gins up something that sniffs a lot like a sqlite3.Row object. My suspicion is that it doesn't sniff quite right to some part of Datasette, and so Datasette does the wrong thing in some part of the JSON processing pipeline. I'll try to dig in this weekend.

cldellow commented 1 year ago

Oh, duh, this is what you were trying to say, and I just missed the plot entirely.

Yes, we need what you propose, in order to match the explicit isinstance checks at https://github.com/simonw/datasette/blob/f0fadc28ddb9f82e5cc1ecaa51e8a342eb6dc528/datasette/utils/__init__.py#L165-L168

cldellow commented 1 year ago

Released in 0.5, and https://dux.fly.dev/parquet.json?sql=select+version%28%29 works as expected now.