JSONB support #513
JSONB support #513
Conversation
There was a problem hiding this comment.
Thanks! These tests cover some nice ground around JSON support. Sadly half of them result in warnings at the moment, but I think it'd be good to add them already anyway. At least to make sure that we don't crash/error, but fall back gracefully to Postgres execution.
| -- JSON | ||
| set duckdb.force_execution to true; | ||
| -- -> int operator | ||
| select '[{"a":"foo"},{"b":"bar"},{"c":"baz"}]'::json->2; |
There was a problem hiding this comment.
These queries need to have a FROM clause. Queries that select only constants are not being executed by duckdb currently.
| -- Test Case 1: Access JSON Object Field (->) | ||
| SELECT id, data->'a' AS a_value | ||
| FROM test_json; | ||
| WARNING: (PGDuckDB/CreatePlan) Prepared query returned an error: 'Binder Error: No function matches the given name and argument types 'json_extract("UnsupportedPostgresType (Oid=3802)", VARCHAR)'. You might need to add explicit type casts. |
There was a problem hiding this comment.
It would be really good to start supporting JSONB.
| -- Create table for JSON testing | ||
| CREATE TABLE test_json ( | ||
| id SERIAL PRIMARY KEY, | ||
| data JSONB |
There was a problem hiding this comment.
Initially you had some tests for the JSON type too. I think these tests would be more useful if they used the json type. Since jsonb is currently not supported at all, all the tests return a warning.
| data JSONB | |
| data JSON |
To be clear I think the jsonb tests are good to have, but they should probably be added together with some basic jsonb support. Having them all throw a warning in the same way is not super useful.
In duckdb#496 we started using the newest clang-format to format our files, but CI was still installing the old version. This meant that we were not catching some unformatted files correctly. An example of this being: duckdb#511 (comment) This starts using the correct clang-format version in CI too and formats any incorrectly formatted files.
…b#521) To construct output string use`AddStringOrBlob` which doesn't require that input is valid UTF8.
It was [pointed out][1] by @dpxcc that one of our tests was not testing what it was describing to test. This fixes that. To be clear the functionality was already working. [1]: duckdb#241 (comment)
Idea is to reconstruct query based on duckdb filtering information, for each table and use that information to plan postgres execution. This plan will, potentially exeucute with parallel workers. If no workers are available we will scan this local to thread. This approach has advantage that it also will support all other scan nodes that are available (and postgresql thinks are best - index/index only/bitmap scan, also partitioned tables should be possible) Fixes duckdb#243 --------- Co-authored-by: Yves <[email protected]> Co-authored-by: Jelte Fennema-Nio <[email protected]>
I noticed two issues with the new `approx_count_distinct` implementation: 1. If no FROM clause was used it was not possible to use it 2. It would not be detected correctly as duckdb-only without `duckdb.force_execution = true` (or some other mechanism). This fixes both of those issues. Related to duckdb#499
| @@ -896,6 +897,7 @@ ConvertPostgresToBaseDuckColumnType(Form_pg_attribute &attribute) { | |||
| return duckdb::LogicalTypeId::UUID; | |||
| case JSONOID: | |||
| case JSONARRAYOID: | |||
| case JSONBOID: | |||
There was a problem hiding this comment.
We should also support jsonb insida a postgres array:
| case JSONBOID: | |
| case JSONBOID: | |
| case JSONBARRAYOID: |
This would need some tests in array_type_support.sql
|
@JelteF your requested changes were made |
There was a problem hiding this comment.
Thanks for this work. This looks good now. Next steps in my order of priority would be:
- Start supporting some of duckdb its JSON functions
- Introduce some of the JSONB operators that postgres supports to DuckDB.
- Introduce a PG compatibility mode in duckdb to make existing operators behave exactly the same.
Adding support for JSONB and JSONB[]
Fixes #516