fix: Set duckdb search_path according to postgres #1465
Conversation
| @@ -78,9 +78,6 @@ pub trait BaseFdw { | |||
| handler, | |||
| )?; | |||
|
|
|||
| // Ensure we are in the same DuckDB schema as the Postgres schema | |||
| connection::execute(format!("SET SCHEMA '{schema_name}'").as_str(), [])?; | |||
There was a problem hiding this comment.
If my understanding is correct, the code here is to set the duckdb schema for the exector hook. The code here can be replaced by the set_search_path.
|
This problem only happens in the executor hook. I couldn't find a good way to test it. |
kysshsy
changed the title
|
From big picture, this PR still has potential problem. You actually want to implicitly expand the sync scope of one kind of metas, search path, between pg and pg_lakehouse(duckdb). This may cause problem. Because two sides in pg and pg_lakehouse(duckdb) may have independent and different schemas, the sync of search path from pg to pg_lakehouse may include unwanted schemas in pg_lakehouse, and even worse results in mistakes for unwanted table including. And further intentions to implicitly expand the sync scope of any meta may face this problem. There are actually several ways to solve this problem. Silently setting the search path is simple, but, IMHO, it is not the good way considering its destructive consequences, especially for enterprise level apps. A good solution needs to be analyzed and determined from a global perspective. It is great to see you issues several PRs recently in supporting prepare stmts pushdown. My suggestion is that you can put all the problems you have observed into one issue (new or original paradedb/pg_analytics#51), and we can have discussions to see what commonalities and characteristics these problems have, which ones can be easily solved first, which ones require changes to the existing foundation, and even finally where pg_lakehouse will end up. |
kysshsy
force-pushed
the
fix_search_path
branch
from
35ddfee
to
7434969
Compare
|
Paradedb maintainers are busy with refactoring and migrating repositories, I guess. After the migration is complete, I'll send over the issues I've observed so far. However, this PR actually addresses an existing bug and isn't solely for 'prepare stmt' pushdown. |
Appreciate you working on this PR :) You're right, we're about to push version |
|
closed. move to paradedb/pg_analytics#40 |
Ticket(s) Closed
What
Why
How
set Duckdb search path according to postgres search path.
Tests