Factorize simple DuckDB queries #272
Conversation
| @@ -15,13 +15,24 @@ class DuckDBManager { | |||
| static inline DuckDBManager & | |||
| Get() { | |||
| static DuckDBManager instance; | |||
| if (!instance.database) { | |||
| instance.Initialize(); | |||
There was a problem hiding this comment.
Perform initialization out of constructor so we don't throw during object creation.
Y--
force-pushed
the
yl/factorize-simple-ddb-queries
branch
from
e1a4a43
to
6341cf0
Compare
| duckdb::unique_ptr<duckdb::QueryResult> DuckDBQueryOrThrow(duckdb::ClientContext &context, const std::string &query); | ||
|
|
||
| duckdb::unique_ptr<duckdb::QueryResult> DuckDBQueryOrThrow(duckdb::Connection &connection, const std::string &query); | ||
|
|
||
| duckdb::unique_ptr<duckdb::QueryResult> DuckDBQueryOrThrow(const std::string &query); | ||
|
|
There was a problem hiding this comment.
I think it'd be good to have const char* versions of this too that use an elog(ERROR. In many cases where we want to do a DuckDB query, we'll be in a Postgres C context.
That could be a separate PR though.
There was a problem hiding this comment.
Yeah I'm coming with a dedicated PR that rationalize this.
There was a problem hiding this comment.
I've actually made so that they always throw PG errors for now (which was the case already)
| elog(ERROR, "(PGDuckDB/duckdb_create_table_trigger) Could not create table: %s", result->GetError().c_str()); | ||
| } | ||
|
|
||
| pgduckdb::DuckDBQueryOrThrow(pgduckdb_get_tabledef(relid)); |
There was a problem hiding this comment.
It seems incorrect to go from elog to throw here. This will cause a Postgres crash currently if this fails.
There was a problem hiding this comment.
Ah yes - I extracted it from my PR that wraps all functions with try/catch but didn't pay attention here. Will review other places too.
There was a problem hiding this comment.
I've made these function "throw" PG errors.
DuckDBManager::GetConnection()member into a staticCreateConnection()DuckDBQueryOrThrowto simplify recurring pattern