10000 Indicate Postgres schema requirement by allanrogerr · Pull Request #1461 · minio/docs · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Indicate Postgres schema requirement #1461

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

allanrogerr
Copy link
Contributor
@allanrogerr allanrogerr commented Apr 28, 2025

https://min.io/docs/minio/linux/reference/minio-server/settings/notifications/postgresql.html#minio-server-envvar-bucket-notification-postgresql

It seems that minio previously inferred the db schema from the string passed e.g. minio.minioevents, however. In the current minio server, we need to independently specify a db schema in order to setup postgres notifications else we hit pq: no schema has been selected to create in (*pq.Error). I believe that the documentation should change to reflect this.

@docs team: Please help me communicate this appropriately. Some more documentation is needed to make this clear.

@ravind This is the commit which introduced the change. minio/minio@701da12 - this is the release https://github.com/minio/minio/releases/tag/RELEASE.2024-04-28T17-53-50Z

I understand the commit is for further sanitization of the sql inputs - however it also forces the user to perform the above command modifications.

e.g.

export MINIO_NOTIFY_POSTGRES_ENABLE_PRIMARY="on"
export MINIO_NOTIFY_POSTGRES_CONNECTION_STRING_PRIMARY="host=localhost port=5432 dbname=postgres user=postgres password=postgres sslmode=disable options='-c search_path=minio'"
export MINIO_NOTIFY_POSTGRES_TABLE_PRIMARY="minioevents"
export MINIO_NOTIFY_POSTGRES_FORMAT_PRIMARY="namespace"
export MINIO_NOTIFY_POSTGRES_MAX_OPEN_CONNECTIONS_PRIMARY="2"
export MINIO_NOTIFY_POSTGRES_COMMENT_PRIMARY="PostgreSQL Notification Event Logging for MinIO"

or

mc admin config set <alias> --insecure notify_postgres:PRIMARY \
enable="on" \
connection_string="host=localhost port=5432 dbname=postgres user=postgres password=postgres options='-c search_path=minio' sslmode=disable" \
table="minioevents" \
format="namespace" \
max_open_connections="2" \
comment="PostgreSQL Notification Event Logging for MinIO"

@ramondeklein
Copy link
Contributor

@allanrogerr The intention of my PR wasn't to remove schema detection. It only adds a check to make sure that the passed table-name is valid. It looks like it doesn't allow specifying a table-name with a schema anymore (so it's too strict). We probably should support specifying a table with a schema.

@ravindk89
Copy link
Collaborator

@ramondeklein with your latest changes is this "gone away"?

@ramondeklein
Copy link
Contributor
ramondeklein commented May 13, 2025

@ravindk89 Yes, with https://github.com/miniohq/eos/pull/766 the user can specify the schema with the table name again. There is no need to specify the "search path" anymore. You can use minio.minioevents again. When users hit this issue on a version before the PR, then we should just ask them to upgrade. Not accepting the schema in the table-name is a bug. The workaround is the search path, but there is no need to document 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.

4 participants
0