pg.ddx.io pgsql-committers@postgresql.org mailing list archivehelp / color / mirror / Atom feed
pgsql: Allow adjusting session_authorization and role in parallel worke 7+ messages / 1 participants [nested] [flat]
* pgsql: Allow adjusting session_authorization and role in parallel worke @ 2024-08-10 19:52 Tom Lane <tgl@sss.pgh.pa.us> 0 siblings, 0 replies; 7+ messages in thread From: Tom Lane @ 2024-08-10 19:52 UTC (permalink / raw) To: pgsql-committers@lists.postgresql.org Allow adjusting session_authorization and role in parallel workers. The code intends to allow GUCs to be set within parallel workers via function SET clauses, but not otherwise. However, doing so fails for "session_authorization" and "role", because the assign hooks for those attempt to set the subsidiary "is_superuser" GUC, and that call falls foul of the "not otherwise" prohibition. We can't switch to using GUC_ACTION_SAVE for this, so instead add a new GUC variable flag GUC_ALLOW_IN_PARALLEL to mark is_superuser as being safe to set anyway. (This is okay because is_superuser has context PGC_INTERNAL and thus only hard-wired calls can change it. We'd need more thought before applying the flag to other GUCs; but maybe there are other use-cases.) This isn't the prettiest fix perhaps, but other alternatives we thought of would be much more invasive. While here, correct a thinko in commit 059de3ca4: when rejecting a GUC setting within a parallel worker, we should return 0 not -1 if the ereport doesn't longjmp. (This seems to have no consequences right now because no caller cares, but it's inconsistent.) Improve the comments to try to forestall future confusion of the same kind. Despite the lack of field complaints, this seems worth back-patching. Thanks to Nathan Bossart for the idea to invent a new flag, and for review. Discussion: https://postgr.es/m/2833457.1723229039@sss.pgh.pa.us Branch ------ REL_15_STABLE Details ------- https://git.postgresql.org/pg/commitdiff/2f4e895be76044fc09bdf96617043c94a12079e7 Modified Files -------------- src/backend/utils/misc/guc.c | 28 ++++++++++++++--------- src/include/utils/guc.h | 1 + src/test/regress/expected/select_parallel.out | 33 +++++++++++++++++++++++++++ src/test/regress/sql/select_parallel.sql | 23 +++++++++++++++++++ 4 files changed, 74 insertions(+), 11 deletions(-) ^ permalink raw reply [nested|flat] 7+ messages in thread
* pgsql: Allow adjusting session_authorization and role in parallel worke @ 2024-08-10 19:52 Tom Lane <tgl@sss.pgh.pa.us> 0 siblings, 0 replies; 7+ messages in thread From: Tom Lane @ 2024-08-10 19:52 UTC (permalink / raw) To: pgsql-committers@lists.postgresql.org Allow adjusting session_authorization and role in parallel workers. The code intends to allow GUCs to be set within parallel workers via function SET clauses, but not otherwise. However, doing so fails for "session_authorization" and "role", because the assign hooks for those attempt to set the subsidiary "is_superuser" GUC, and that call falls foul of the "not otherwise" prohibition. We can't switch to using GUC_ACTION_SAVE for this, so instead add a new GUC variable flag GUC_ALLOW_IN_PARALLEL to mark is_superuser as being safe to set anyway. (This is okay because is_superuser has context PGC_INTERNAL and thus only hard-wired calls can change it. We'd need more thought before applying the flag to other GUCs; but maybe there are other use-cases.) This isn't the prettiest fix perhaps, but other alternatives we thought of would be much more invasive. While here, correct a thinko in commit 059de3ca4: when rejecting a GUC setting within a parallel worker, we should return 0 not -1 if the ereport doesn't longjmp. (This seems to have no consequences right now because no caller cares, but it's inconsistent.) Improve the comments to try to forestall future confusion of the same kind. Despite the lack of field complaints, this seems worth back-patching. Thanks to Nathan Bossart for the idea to invent a new flag, and for review. Discussion: https://postgr.es/m/2833457.1723229039@sss.pgh.pa.us Branch ------ master Details ------- https://git.postgresql.org/pg/commitdiff/364de74cff281e7363c7ca8de4fbf04c6e16f8ed Modified Files -------------- src/backend/utils/misc/guc.c | 45 ++++++++++++++++----------- src/backend/utils/misc/guc_tables.c | 2 +- src/include/utils/guc.h | 1 + src/test/regress/expected/select_parallel.out | 33 ++++++++++++++++++++ src/test/regress/sql/select_parallel.sql | 23 ++++++++++++++ 5 files changed, 85 insertions(+), 19 deletions(-) ^ permalink raw reply [nested|flat] 7+ messages in thread
* pgsql: Allow adjusting session_authorization and role in parallel worke @ 2024-08-10 19:52 Tom Lane <tgl@sss.pgh.pa.us> 0 siblings, 0 replies; 7+ messages in thread From: Tom Lane @ 2024-08-10 19:52 UTC (permalink / raw) To: pgsql-committers@lists.postgresql.org Allow adjusting session_authorization and role in parallel workers. The code intends to allow GUCs to be set within parallel workers via function SET clauses, but not otherwise. However, doing so fails for "session_authorization" and "role", because the assign hooks for those attempt to set the subsidiary "is_superuser" GUC, and that call falls foul of the "not otherwise" prohibition. We can't switch to using GUC_ACTION_SAVE for this, so instead add a new GUC variable flag GUC_ALLOW_IN_PARALLEL to mark is_superuser as being safe to set anyway. (This is okay because is_superuser has context PGC_INTERNAL and thus only hard-wired calls can change it. We'd need more thought before applying the flag to other GUCs; but maybe there are other use-cases.) This isn't the prettiest fix perhaps, but other alternatives we thought of would be much more invasive. While here, correct a thinko in commit 059de3ca4: when rejecting a GUC setting within a parallel worker, we should return 0 not -1 if the ereport doesn't longjmp. (This seems to have no consequences right now because no caller cares, but it's inconsistent.) Improve the comments to try to forestall future confusion of the same kind. Despite the lack of field complaints, this seems worth back-patching. Thanks to Nathan Bossart for the idea to invent a new flag, and for review. Discussion: https://postgr.es/m/2833457.1723229039@sss.pgh.pa.us Branch ------ REL_14_STABLE Details ------- https://git.postgresql.org/pg/commitdiff/546a26b3d8df771624163e7338f281ff8313a8b9 Modified Files -------------- src/backend/utils/misc/guc.c | 28 ++++++++++++++--------- src/include/utils/guc.h | 1 + src/test/regress/expected/select_parallel.out | 33 +++++++++++++++++++++++++++ src/test/regress/sql/select_parallel.sql | 23 +++++++++++++++++++ 4 files changed, 74 insertions(+), 11 deletions(-) ^ permalink raw reply [nested|flat] 7+ messages in thread
* pgsql: Allow adjusting session_authorization and role in parallel worke @ 2024-08-10 19:52 Tom Lane <tgl@sss.pgh.pa.us> 0 siblings, 0 replies; 7+ messages in thread From: Tom Lane @ 2024-08-10 19:52 UTC (permalink / raw) To: pgsql-committers@lists.postgresql.org Allow adjusting session_authorization and role in parallel workers. The code intends to allow GUCs to be set within parallel workers via function SET clauses, but not otherwise. However, doing so fails for "session_authorization" and "role", because the assign hooks for those attempt to set the subsidiary "is_superuser" GUC, and that call falls foul of the "not otherwise" prohibition. We can't switch to using GUC_ACTION_SAVE for this, so instead add a new GUC variable flag GUC_ALLOW_IN_PARALLEL to mark is_superuser as being safe to set anyway. (This is okay because is_superuser has context PGC_INTERNAL and thus only hard-wired calls can change it. We'd need more thought before applying the flag to other GUCs; but maybe there are other use-cases.) This isn't the prettiest fix perhaps, but other alternatives we thought of would be much more invasive. While here, correct a thinko in commit 059de3ca4: when rejecting a GUC setting within a parallel worker, we should return 0 not -1 if the ereport doesn't longjmp. (This seems to have no consequences right now because no caller cares, but it's inconsistent.) Improve the comments to try to forestall future confusion of the same kind. Despite the lack of field complaints, this seems worth back-patching. Thanks to Nathan Bossart for the idea to invent a new flag, and for review. Discussion: https://postgr.es/m/2833457.1723229039@sss.pgh.pa.us Branch ------ REL_13_STABLE Details ------- https://git.postgresql.org/pg/commitdiff/adf9808fa966460f822cb8ccea424cef43f27e2b Modified Files -------------- src/backend/utils/misc/guc.c | 36 ++++++++++++++++----------- src/include/utils/guc.h | 1 + src/test/regress/expected/select_parallel.out | 33 ++++++++++++++++++++++++ src/test/regress/sql/select_parallel.sql | 23 +++++++++++++++++ 4 files changed, 78 insertions(+), 15 deletions(-) ^ permalink raw reply [nested|flat] 7+ messages in thread
* pgsql: Allow adjusting session_authorization and role in parallel worke @ 2024-08-10 19:52 Tom Lane <tgl@sss.pgh.pa.us> 0 siblings, 0 replies; 7+ messages in thread From: Tom Lane @ 2024-08-10 19:52 UTC (permalink / raw) To: pgsql-committers@lists.postgresql.org Allow adjusting session_authorization and role in parallel workers. The code intends to allow GUCs to be set within parallel workers via function SET clauses, but not otherwise. However, doing so fails for "session_authorization" and "role", because the assign hooks for those attempt to set the subsidiary "is_superuser" GUC, and that call falls foul of the "not otherwise" prohibition. We can't switch to using GUC_ACTION_SAVE for this, so instead add a new GUC variable flag GUC_ALLOW_IN_PARALLEL to mark is_superuser as being safe to set anyway. (This is okay because is_superuser has context PGC_INTERNAL and thus only hard-wired calls can change it. We'd need more thought before applying the flag to other GUCs; but maybe there are other use-cases.) This isn't the prettiest fix perhaps, but other alternatives we thought of would be much more invasive. While here, correct a thinko in commit 059de3ca4: when rejecting a GUC setting within a parallel worker, we should return 0 not -1 if the ereport doesn't longjmp. (This seems to have no consequences right now because no caller cares, but it's inconsistent.) Improve the comments to try to forestall future confusion of the same kind. Despite the lack of field complaints, this seems worth back-patching. Thanks to Nathan Bossart for the idea to invent a new flag, and for review. Discussion: https://postgr.es/m/2833457.1723229039@sss.pgh.pa.us Branch ------ REL_17_STABLE Details ------- https://git.postgresql.org/pg/commitdiff/2b8d33f66c134b5f1b0a94a7e40af2fbb6a086b7 Modified Files -------------- src/backend/utils/misc/guc.c | 45 ++++++++++++++++----------- src/backend/utils/misc/guc_tables.c | 2 +- src/include/utils/guc.h | 1 + src/test/regress/expected/select_parallel.out | 33 ++++++++++++++++++++ src/test/regress/sql/select_parallel.sql | 23 ++++++++++++++ 5 files changed, 85 insertions(+), 19 deletions(-) ^ permalink raw reply [nested|flat] 7+ messages in thread
* pgsql: Allow adjusting session_authorization and role in parallel worke @ 2024-08-10 19:52 Tom Lane <tgl@sss.pgh.pa.us> 0 siblings, 0 replies; 7+ messages in thread From: Tom Lane @ 2024-08-10 19:52 UTC (permalink / raw) To: pgsql-committers@lists.postgresql.org Allow adjusting session_authorization and role in parallel workers. The code intends to allow GUCs to be set within parallel workers via function SET clauses, but not otherwise. However, doing so fails for "session_authorization" and "role", because the assign hooks for those attempt to set the subsidiary "is_superuser" GUC, and that call falls foul of the "not otherwise" prohibition. We can't switch to using GUC_ACTION_SAVE for this, so instead add a new GUC variable flag GUC_ALLOW_IN_PARALLEL to mark is_superuser as being safe to set anyway. (This is okay because is_superuser has context PGC_INTERNAL and thus only hard-wired calls can change it. We'd need more thought before applying the flag to other GUCs; but maybe there are other use-cases.) This isn't the prettiest fix perhaps, but other alternatives we thought of would be much more invasive. While here, correct a thinko in commit 059de3ca4: when rejecting a GUC setting within a parallel worker, we should return 0 not -1 if the ereport doesn't longjmp. (This seems to have no consequences right now because no caller cares, but it's inconsistent.) Improve the comments to try to forestall future confusion of the same kind. Despite the lack of field complaints, this seems worth back-patching. Thanks to Nathan Bossart for the idea to invent a new flag, and for review. Discussion: https://postgr.es/m/2833457.1723229039@sss.pgh.pa.us Branch ------ REL_16_STABLE Details ------- https://git.postgresql.org/pg/commitdiff/f3ab5d3a2d19aa9f7243cee6ecd7fec6a36c63c1 Modified Files -------------- src/backend/utils/misc/guc.c | 26 +++++++++++++-------- src/backend/utils/misc/guc_tables.c | 2 +- src/include/utils/guc.h | 1 + src/test/regress/expected/select_parallel.out | 33 +++++++++++++++++++++++++++ src/test/regress/sql/select_parallel.sql | 23 +++++++++++++++++++ 5 files changed, 74 insertions(+), 11 deletions(-) ^ permalink raw reply [nested|flat] 7+ messages in thread
* pgsql: Allow adjusting session_authorization and role in parallel worke @ 2024-08-10 19:52 Tom Lane <tgl@sss.pgh.pa.us> 0 siblings, 0 replies; 7+ messages in thread From: Tom Lane @ 2024-08-10 19:52 UTC (permalink / raw) To: pgsql-committers@lists.postgresql.org Allow adjusting session_authorization and role in parallel workers. The code intends to allow GUCs to be set within parallel workers via function SET clauses, but not otherwise. However, doing so fails for "session_authorization" and "role", because the assign hooks for those attempt to set the subsidiary "is_superuser" GUC, and that call falls foul of the "not otherwise" prohibition. We can't switch to using GUC_ACTION_SAVE for this, so instead add a new GUC variable flag GUC_ALLOW_IN_PARALLEL to mark is_superuser as being safe to set anyway. (This is okay because is_superuser has context PGC_INTERNAL and thus only hard-wired calls can change it. We'd need more thought before applying the flag to other GUCs; but maybe there are other use-cases.) This isn't the prettiest fix perhaps, but other alternatives we thought of would be much more invasive. While here, correct a thinko in commit 059de3ca4: when rejecting a GUC setting within a parallel worker, we should return 0 not -1 if the ereport doesn't longjmp. (This seems to have no consequences right now because no caller cares, but it's inconsistent.) Improve the comments to try to forestall future confusion of the same kind. Despite the lack of field complaints, this seems worth back-patching. Thanks to Nathan Bossart for the idea to invent a new flag, and for review. Discussion: https://postgr.es/m/2833457.1723229039@sss.pgh.pa.us Branch ------ REL_12_STABLE Details ------- https://git.postgresql.org/pg/commitdiff/adc28d01e98bdfaf9720d363447124d22565620a Modified Files -------------- src/backend/utils/misc/guc.c | 36 ++++++++++++++++----------- src/include/utils/guc.h | 1 + src/test/regress/expected/select_parallel.out | 33 ++++++++++++++++++++++++ src/test/regress/sql/select_parallel.sql | 23 +++++++++++++++++ 4 files changed, 78 insertions(+), 15 deletions(-) ^ permalink raw reply [nested|flat] 7+ messages in thread
end of thread, other threads:[~2024-08-10 19:52 UTC | newest] Thread overview: 7+ messages (download: mbox mbox.gz follow: Atom feed) -- links below jump to the message on this page -- 2024-08-10 19:52 pgsql: Allow adjusting session_authorization and role in parallel worke Tom Lane <tgl@sss.pgh.pa.us> 2024-08-10 19:52 pgsql: Allow adjusting session_authorization and role in parallel worke Tom Lane <tgl@sss.pgh.pa.us> 2024-08-10 19:52 pgsql: Allow adjusting session_authorization and role in parallel worke Tom Lane <tgl@sss.pgh.pa.us> 2024-08-10 19:52 pgsql: Allow adjusting session_authorization and role in parallel worke Tom Lane <tgl@sss.pgh.pa.us> 2024-08-10 19:52 pgsql: Allow adjusting session_authorization and role in parallel worke Tom Lane <tgl@sss.pgh.pa.us> 2024-08-10 19:52 pgsql: Allow adjusting session_authorization and role in parallel worke Tom Lane <tgl@sss.pgh.pa.us> 2024-08-10 19:52 pgsql: Allow adjusting session_authorization and role in parallel worke Tom Lane <tgl@sss.pgh.pa.us>
This inbox is served by DDX for PostgreSQL; see mirroring instructions for how to clone and mirror all data and code used for this inbox