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

Skip initdb during cargo pgx init if $USER == "root" #650

Merged
merged 3 commits into from
Aug 26, 2022
Merged

Skip initdb during cargo pgx init if $USER == "root" #650

merged 3 commits into from
Aug 26, 2022

Conversation

anth0nyleung
Copy link

@anth0nyleung anth0nyleung commented Aug 25, 2022

Notes

In some use case, we would set up rust toolchain and pgx as root user, we do not necessarily need pgx to spin up running postgres instance. However during cargo pgx init, as pgx is trying to initdb for postgres, it will fail to do so since initdb cannot be executed as root user.

Changes

If current user is root user, skip doing initdb.

Testing

cargo test --features pg14 --no-default-features passed

> ls -l ~/.pgx   
total 0

> cargo pgx init --pg14 /usr/local/pgsql/bin/pg_config 
   Validating /usr/local/pgsql/bin/pg_config
 Initializing data directory at /Users/antholeu/.pgx/data-14

> ls -l ~/.pgx                                        
total 8
drwx------  24 antholeu  staff   768B Aug 25 16:36 data-14
-rw-r--r--   1 antholeu  staff    48B Aug 25 16:36 config.toml
> ls -l ~/.pgx                                             
total 0

> sudo cargo pgx init --pg14 /usr/local/pgsql/bin/pg_config
   Validating /usr/local/pgsql/bin/pg_config
   Skipping initdb as current user is root user

> ls -l ~/.pgx                                             
total 8
-rw-r--r--  1 root  staff    48B Aug 25 16:37 config.toml

Copy link
Member

@workingjubilee workingjubilee left a comment

Choose a reason for hiding this comment

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

This looks good per se! I just have some design-level questions so I can better understand why to add this, and not other solutions.

cargo-pgx/src/command/init.rs Outdated Show resolved Hide resolved
Comment on lines +181 to +183
if !datadir.exists() {
initdb(&bindir, &datadir)?;
}
Copy link
Member

@workingjubilee workingjubilee Aug 25, 2022

Choose a reason for hiding this comment

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

Currently we avoid running initdb if pg_config.data_dir() exists.

But presumably, in the case that the user has specified a given path, the database should in fact always not be initialized by us, as the user wants a specific database to be used and likely has it configured the way they like it, or will want to do so themselves?

It may be that we will not be able to execute successive commands using cargo pgx if initdb has not been run, but we will run into that issue if this flag is passed as well, so we must handle that case anyways. After all, cargo pgx cannot simply assume that the system state has remain unchanged between runtime sessions. This is especially true if cargo pgx is pointed at a specific PostgreSQL database and so is not "in control" of it.

Is this hypothesis incorrect?

Copy link
Member

@workingjubilee workingjubilee Aug 25, 2022

Choose a reason for hiding this comment

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

This hypothesis is incorrect! PGX expects a specific path for the data_dir to exist, it's not actually controlled by pg_config, see: https://github.com/tcdi/pgx/blob/1a89778046547c52c714fe6885069eab5638f965/pgx-utils/src/pg_config.rs#L199-L202

Copy link
Author

Choose a reason for hiding this comment

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

datadir is currently hardcoded to data-<major-version>, user is not allow to specify this.

@workingjubilee workingjubilee changed the title Support a flag to skip initdb during cargo pgx init Skip initdb during cargo pgx init if $USER == "root" Aug 25, 2022
@workingjubilee
Copy link
Member

Thank you!

@workingjubilee workingjubilee merged commit 35f0888 into pgcentralfoundation:develop Aug 26, 2022
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.

2 participants