On Thu, Feb 23, 2023 at 03:10:05PM +0100, Daniel Gustafsson wrote:
> In fixing the CFBot test error in the previous version I realized through
> off-list discussion that the GUC name was badly chosen. Incorporating the
> value of another GUC in the name is a bad idea, so the attached version reverts
> to "scram_iterations=<int>". Should there ever be another SCRAM method
> standardized (which seems a slim chance to happen before the v17 freeze) we can
> make a backwards compatible change to "<method>:<iterations> | <iterations>"
> where the latter is a default for all. Internally the variable contains
> sha_256 though, that part I think is fine for readability.
Okay by me if you want to go this way. We could always have the
compatibility argument later on if it proves necessary.
Anyway, the patch does that in libpq:
@@ -1181,6 +1181,10 @@ pqSaveParameterStatus(PGconn *conn, const char *name, const char *value)
conn->in_hot_standby =
(strcmp(value, "on") == 0) ? PG_BOOL_YES : PG_BOOL_NO;
}
+ else if (strcmp(name, "scram_sha_256_iterations") == 0)
+ {
+ conn->scram_sha_256_iterations = atoi(value);
+ }
This should match on "scram_iterations", which is the name of the
GUC. Would the long-term plan be to use multiple variables in conn if
we ever get to <method>:<iterations> that would require more parsing?
This is fine by me, just asking.
Perhaps there should be a test with \password to make sure that libpq
gets the call when the GUC is updated by a SET command?
--
Michael
