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.

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

Support SET <name> TO DEFAULT and RESET <name> #8851

Open
BugenZhao opened this issue Mar 29, 2023 · 5 comments
Open

Support SET <name> TO DEFAULT and RESET <name> #8851

BugenZhao opened this issue Mar 29, 2023 · 5 comments
Labels

Comments

@BugenZhao
Copy link
Member

Is your feature request related to a problem? Please describe.

According to the documentation of PostgreSQL:

RESET restores run-time parameters to their default values. RESET is an alternative spelling for

SET configuration_parameter TO DEFAULT

Refer to SET for details.

The default value is defined as the value that the parameter would have had, if no SET had ever been issued for it in the current session. The actual source of this value might be a compiled-in default, the configuration file, command-line options, or per-database or per-user default settings.

Currently, we treat DEFAULT literally as a string, which behaves incorrectly.

Describe the solution you'd like

Support this syntax along with #8850. There're two columns named boot_val and reset_val for each variable and parameter in the pg_catalog.pg_settings. Should we make this the source of the truth?

Describe alternatives you've considered

No response

Additional context

No response

@BugenZhao BugenZhao added type/feature component/frontend Protocol, parsing, binder. labels Mar 29, 2023
@github-actions github-actions bot added this to the release-0.19 milestone Mar 29, 2023
@fuyufjh
Copy link
Member

fuyufjh commented Apr 4, 2023

@Gun9niR Can you please take a look?

@Gun9niR
Copy link
Contributor

Gun9niR commented Apr 4, 2023

the value that the parameter would have had, if no SET had ever been issued for it in the current session

IIUC, this value can only be the compiled-in default in our case, since we don't provide any way to change their initial values of session parameters.

@xiangjinwu
Copy link
Contributor

the value that the parameter would have had, if no SET had ever been issued for it in the current session

IIUC, this value can only be the compiled-in default in our case, since we don't provide any way to change their initial values of session parameters.

Just FYI the pgwire protocol can specify reset_val in the startup message, but they are currently ignored by our implementation. We have not heard any complaints about lacking this (except for pg regress test #8386), and some clients (psycopg2 #3253 sqlancer #3406) use standalone set statements on startup rather than putting them in the protocol message.

fn process_startup_msg(&mut self, msg: FeStartupMessage) -> PsqlResult<()> {
let db_name = msg
.config
.get("database")
.cloned()
.unwrap_or_else(|| "dev".to_string());
let user_name = msg
.config
.get("user")
.cloned()
.unwrap_or_else(|| "root".to_string());

To see it in effect, add print msg.config.get("timezone") above and start psql with environment variable PGTZ:

PGTZ=PST8PDT psql -h localhost -p 4566 -d dev -U root

@Gun9niR
Copy link
Contributor

Gun9niR commented Apr 6, 2023

Then for SET we should reset the value to reset_val (after supporting it at FE).

For system parameters, according to PostgreSQL's doc about ALTER SYSTEM SET:

ALTER SYSTEM writes the given parameter setting to the postgresql.auto.conf file, which is read in addition to postgresql.conf. Setting a parameter to DEFAULT, or using the RESET variant, removes that configuration entry from the postgresql.auto.conf file. Use RESET ALL to remove all such configuration entries.

So I guess ALTER SYSTEM RESET should set the value to the one specified when the cluster is started for the first time 🤔 ?

Copy link
Contributor

github-actions bot commented Jul 3, 2024

This issue has been open for 60 days with no activity.

If you think it is still relevant today, and needs to be done in the near future, you can comment to update the status, or just manually remove the no-issue-activity label.

You can also confidently close this issue as not planned to keep our backlog clean.
Don't worry if you think the issue is still valuable to continue in the future.
It's searchable and can be reopened when it's time. 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

6 participants