Skip to content

Conversation

JelteF
Copy link
Collaborator

@JelteF JelteF commented May 7, 2025

pg_duckdb changes result types of queries at the planning stage. This is needed for pg_duckdb to function correctly. However, Postgres code is sometimes not written with this in mind. One such case is sql functions, as reported in #749. There it causes a crash, because Postgres expects a different type to be returned by the function than what is actually returned.

As far as I can tell there is no easy way of fixing this, apart from changing Postgres to learn how to deal with this. I'll open issue/patch to Postgres for this at some point in the near future.

For now, we simply start disallowing all DuckDB execution inside functions. That is quite a shame because using DuckDB execution in functions works fine in many other cases. Sadly, I cannot think of a way to only disallow the crashing cases. Since a very important part of pg_duckdb 1.0 is stability, it seems better to err on the side of caution and disable this feature for now and stabilize it in a future release.

Fixes #749

@JelteF JelteF force-pushed the disable-non-top-level-duckdb branch 2 times, most recently from d2a82b7 to ee02b78 Compare May 7, 2025 10:42
@JelteF JelteF requested a review from Y-- May 7, 2025 10:46
pg_duckdb changes result types of queries at the planning stage. This is
needed for pg_duckdb to function correctly. However, Postgres code is
sometimes not written with this in mind. One such case is sql functions,
as reported in #749. There it causes a crash, because Postgres expects
a different type to be returned by the function than what is actually
returned.

As far as I can tell there is no easy way of fixing this, apart from
changing Postgres to learn how to deal with this. I'll open issue/patch
to Postgres for this at some point in the near future.

For now, we simply start disallowing all DuckDB execution inside
functions. That is quite a shame because using DuckDB execution in
functions works fine in many other cases. Sadly, I cannot think of a way
to only disallow the crashing cases. Since a very important part of
pg_duckdb 1.0 is stability, it seems better to err on the side of
caution and disable this feature for now and stabilize it in a future
release.
@JelteF JelteF force-pushed the disable-non-top-level-duckdb branch from ee02b78 to 9c21ce5 Compare May 7, 2025 10:53
Copy link
Collaborator

@Y-- Y-- left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, should we add a GUC that allow users to bypass it if they know what they are doing (and the risk they take)?

JelteF and others added 2 commits May 7, 2025 14:02
Co-authored-by: Y. <y.lemaout@gmail.com>
@JelteF JelteF requested a review from Y-- May 7, 2025 12:03
@JelteF JelteF enabled auto-merge (squash) May 7, 2025 12:03
@JelteF JelteF merged commit 6bd05ec into main May 7, 2025
6 checks passed
@JelteF JelteF deleted the disable-non-top-level-duckdb branch May 7, 2025 12:36
JelteF pushed a commit that referenced this pull request Aug 15, 2025
DuckDB execution inside functions was disallowed in
#764 for good reason
However, some users depend on running analytics with pg_duckdb inside
functions

Given that (1) the problematic cases are rare, and (2) pg_duckdb is
mostly used for read queries, failures are limited to crashing the
current session and can be recovered by restarting the session without
data loss. This change introduces an unsafe GUC to restore this
capability for users who need it
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Server crashes executing "select * from usersview"
2 participants