Skip to content

[Postgres] Use session role for ALL requests #61386

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

Merged
merged 1 commit into from
Apr 16, 2025

Conversation

troopa81
Copy link
Contributor

@troopa81 troopa81 commented Apr 8, 2025

Adds session_role to the connection information so:

  • It's automatically used for every request. The old system was only working for provider requests and was error prone because it was easy to forget to set the session role before the request.
  • Avoid setting/resetting role request if different request sender were sharing the same connection information to the exception of the session role.

New contribution would have to use the new QgsPostresConn::connectionInfo() method to avoid new issue. I can see no way to enforce this instead of adding session role to QgsDataSourceUri (which has been already refused before, and IMHO for good reasons)

FYI @rldhont

Fixes #58516

Funded by Les agences de l'eau - DSIUN

@troopa81 troopa81 added Bug Either a bug report, or a bug fix. Let's hope for the latter! PostGIS data provider backport queued_ltr_backports Queued Backports backport release-3_42 labels Apr 8, 2025
Copy link

github-actions bot commented Apr 8, 2025

🪟 Windows Qt6 builds

Download Windows Qt6 builds of this PR for testing.
(Built from commit c2b3c5a)

🪟 Windows builds

Download Windows builds of this PR for testing.
Debug symbols for this build are available here.
(Built from commit c2b3c5a)

@github-actions github-actions bot added this to the 3.44.0 milestone Apr 8, 2025
@troopa81 troopa81 force-pushed the fix_pg_roles branch 2 times, most recently from 7c2f8ff to 669f503 Compare April 9, 2025 12:57
Adds session_role to the connection information so:
- It's automatically used for every request. The old system was only
working for provider requests and was error prone because it was easy
to forget to set the session role before the request.
- Avoid setting/resetting role request if different request sender
were sharing the same connection information to the exception of the
session role.

Fixes qgis#58516
@troopa81 troopa81 merged commit 1a80357 into qgis:master Apr 16, 2025
32 checks passed
@qgis-bot
Copy link
Collaborator

The backport to queued_ltr_backports failed:

The process '/usr/bin/git' failed with exit code 1
stderr
error: could not apply c2b3c5a42f9... fix(PostgresRole): Use session role for ALL requests
hint: After resolving the conflicts, mark them with
hint: "git add/rm <pathspec>", then run
hint: "git cherry-pick --continue".
hint: You can instead skip this commit with "git cherry-pick --skip".
hint: To abort and get back to the state before "git cherry-pick",
hint: run "git cherry-pick --abort".
hint: Disable this message with "git config set advice.mergeConflict false"

stdout
Auto-merging src/providers/postgres/qgscolumntypethread.cpp
Auto-merging src/providers/postgres/qgspostgresconn.cpp
CONFLICT (content): Merge conflict in src/providers/postgres/qgspostgresconn.cpp
Auto-merging src/providers/postgres/qgspostgresconn.h
CONFLICT (content): Merge conflict in src/providers/postgres/qgspostgresconn.h
Auto-merging src/providers/postgres/qgspostgresdataitems.cpp
Auto-merging src/providers/postgres/qgspostgresfeatureiterator.cpp
Auto-merging src/providers/postgres/qgspostgresprovider.cpp
CONFLICT (content): Merge conflict in src/providers/postgres/qgspostgresprovider.cpp
Auto-merging src/providers/postgres/qgspostgresproviderconnection.cpp
Auto-merging src/providers/postgres/raster/qgspostgresrasterprovider.cpp
Auto-merging tests/src/python/test_qgsproviderconnection_postgres.py
CONFLICT (content): Merge conflict in tests/src/python/test_qgsproviderconnection_postgres.py

To backport manually, run these commands in your terminal:

# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add .worktrees/backport-queued_ltr_backports queued_ltr_backports
# Navigate to the new working tree
cd .worktrees/backport-queued_ltr_backports
# Create a new branch
git switch --create backport-61386-to-queued_ltr_backports
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick c2b3c5a42f997f5a5fdbc20b3f79ec48e4db3eae
# Push it to GitHub
git push --set-upstream origin backport-61386-to-queued_ltr_backports
# Go back to the original working tree
cd ../..
# Delete the working tree
git worktree remove .worktrees/backport-queued_ltr_backports

Then, create a pull request where the base branch is queued_ltr_backports and the compare/head branch is backport-61386-to-queued_ltr_backports.

@qgis-bot qgis-bot added the failed backport The automated backport attempt failed, needs a manual backport label Apr 16, 2025
@qgis-bot
Copy link
Collaborator

The backport to release-3_42 failed:

The process '/usr/bin/git' failed with exit code 1
stderr
error: could not apply c2b3c5a42f9... fix(PostgresRole): Use session role for ALL requests
hint: After resolving the conflicts, mark them with
hint: "git add/rm <pathspec>", then run
hint: "git cherry-pick --continue".
hint: You can instead skip this commit with "git cherry-pick --skip".
hint: To abort and get back to the state before "git cherry-pick",
hint: run "git cherry-pick --abort".
hint: Disable this message with "git config set advice.mergeConflict false"

stdout
Auto-merging src/providers/postgres/qgscolumntypethread.cpp
Auto-merging src/providers/postgres/qgspostgresconn.cpp
CONFLICT (content): Merge conflict in src/providers/postgres/qgspostgresconn.cpp
Auto-merging src/providers/postgres/qgspostgresconn.h
CONFLICT (content): Merge conflict in src/providers/postgres/qgspostgresconn.h
Auto-merging src/providers/postgres/qgspostgresdataitems.cpp
Auto-merging src/providers/postgres/qgspostgresprovider.cpp
CONFLICT (content): Merge conflict in src/providers/postgres/qgspostgresprovider.cpp
Auto-merging src/providers/postgres/qgspostgresproviderconnection.cpp
Auto-merging tests/src/python/test_qgsproviderconnection_postgres.py
CONFLICT (content): Merge conflict in tests/src/python/test_qgsproviderconnection_postgres.py

To backport manually, run these commands in your terminal:

# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add .worktrees/backport-release-3_42 release-3_42
# Navigate to the new working tree
cd .worktrees/backport-release-3_42
# Create a new branch
git switch --create backport-61386-to-release-3_42
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick c2b3c5a42f997f5a5fdbc20b3f79ec48e4db3eae
# Push it to GitHub
git push --set-upstream origin backport-61386-to-release-3_42
# Go back to the original working tree
cd ../..
# Delete the working tree
git worktree remove .worktrees/backport-release-3_42

Then, create a pull request where the base branch is release-3_42 and the compare/head branch is backport-61386-to-release-3_42.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport queued_ltr_backports Queued Backports backport release-3_42 Bug Either a bug report, or a bug fix. Let's hope for the latter! failed backport The automated backport attempt failed, needs a manual backport PostGIS data provider
Projects
None yet
Development

Successfully merging this pull request may close these issues.

postgres session role ignored by "Export to PostgreSQL"
3 participants