Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update bundled PostgreSQL to 16.4 #7804

Merged
merged 3 commits into from
Oct 2, 2024
+20 −4
Merged

Update bundled PostgreSQL to 16.4 #7804

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
923b979
Update bundled PostgreSQL to 16.4
scotttrinh Sep 26, 2024
e657220
Add additional set_config to allowlist
scotttrinh Sep 26, 2024
7d569ef
fix
aljazerzen Sep 27, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 16 additions & 3 deletions edb/pgsql/resolver/static.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -179,9 +179,13 @@ def eval_FuncCall(
)

if fn_name == "set_config":
# HACK: allow set_config('search_path', '', false) to support pg_dump
# HACK: allow set_config('bytea_output','hex',false) to support pgadmin
# HACK: allow set_config('jit', ...) to support asyncpg
# HACK: pg_dump
# - set_config('search_path', '', false)
# - set_config(name, 'view, foreign-table', false)
# HACK: pgadmin
# - set_config('bytea_output','hex',false)
# HACK: asyncpg
# - set_config('jit', ...)
if args := eval_list(expr.args, ctx=ctx):
name, value, is_local = args
if isinstance(name, pgast.StringConstant):
Expand All @@ -205,6 +209,15 @@ def eval_FuncCall(

if name.val == "jit":
return value
if (
isinstance(value, pgast.StringConstant)
and isinstance(is_local, pgast.BooleanConstant)
):
if (
value.val == "view, foreign-table"
and not is_local.val
):
return value
Comment on lines +215 to +223
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd prefer not to do this here, because we are not checking the name of the setting being set. But to check that, we need the full query, because it is of this form:

SELECT set_config(name, ..., ...) FROM pg_setting WHERE name = '...'

I'll add a pre-resolving step that convert such SELECT set_configs into SET commands, which we do handle correctly.

Copy link
Contributor

Choose a reason for hiding this comment

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

I've tried that in #7810, but it turns out that if we compile SELECT set_config into SET, we need to inject a result set and CommandComplete message, which is hard/complex to do.

So I might abandon that effort and fix your approach instead.

Copy link
Contributor

Choose a reason for hiding this comment

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

We will not abandon that effort, but just put it off until some later time.

For now, this workaround is good enough. Let's hope no one will use "view, foreign-table" setting value anywhere...


raise errors.QueryError(
"function set_config is not supported", span=expr.span
Expand Down
2 changes: 1 addition & 1 deletion postgres
Open in desktop
Submodule postgres updated 459 files
Loading