On 05/05/2017 03:42 PM, Michael Paquier wrote:
> + This option is obsolete but still accepted for backwards
> + compatibility.
> Isn't that incorrect English? It seems to me that this be non-plural,
> as "for backward compatibility".
I changed most cases to "backward compatibility", except the one in
create_role.sgml, because there were other instances of "backwards
compatibility" on that page, and I didn't want this to stick out.
> The comment at the top of check_password() in passwordcheck.c does not
> mention scram, you may want to update that.
Reworded the comment, to not list all the possible values.
> + /*
> + * We never store passwords in plaintext, so
> this shouldn't
> + * happen.
> + */
> break;
> An error here is overthinking?
It's not shown in the diff's context, but an error is returned just
after the switch statement. I considered leaving out the "case
PASSWORD_TYPE_PLAINTEXT" altogether, but then you might get compiler
warnings complaining that that enum value is not handled. I also
considered putting a an explicit "default:" there, which returns an
error, but then you'd again miss out on compiler warnings, if you add a
new password hash type and forget to add a case here to handle it.
> -- consistency of password entries
> -SET password_encryption = 'plain';
> -CREATE ROLE regress_passwd1 PASSWORD 'role_pwd1';
> SET password_encryption = 'md5';
> Nit: this is skipping directly to role number 2.
Renumbered the test roles.
Committed with those little cleanups. Thanks for the review!
- Heikki
