agora inbox for [email protected]help / color / mirror / Atom feed
[PATCH 2/2] review 5+ messages / 2 participants [nested] [flat]
* [PATCH 2/2] review @ 2021-03-09 20:09 Tomas Vondra <[email protected]> 0 siblings, 0 replies; 5+ messages in thread From: Tomas Vondra @ 2021-03-09 20:09 UTC (permalink / raw) --- src/backend/optimizer/path/costsize.c | 77 ++++++++++++++++++++------- src/backend/optimizer/path/pathkeys.c | 14 ++++- 2 files changed, 69 insertions(+), 22 deletions(-) diff --git a/src/backend/optimizer/path/costsize.c b/src/backend/optimizer/path/costsize.c index 1639258aaf..f55a4f20e5 100644 --- a/src/backend/optimizer/path/costsize.c +++ b/src/backend/optimizer/path/costsize.c @@ -1755,6 +1755,8 @@ cost_recursive_union(Path *runion, Path *nrterm, Path *rterm) * is_fake_var * Workaround for generate_append_tlist() which generates fake Vars with * varno == 0, that will cause a fail of estimate_num_group() call + * + * XXX Ummm, why would estimate_num_group fail with this? */ static bool is_fake_var(Expr *expr) @@ -1828,27 +1830,53 @@ get_width_cost_multiplier(PlannerInfo *root, Expr *expr) * compute_cpu_sort_cost * compute CPU cost of sort (i.e. in-memory) * + * The main thing we need to calculate to estimate sort CPU costs is the number + * of calls to the comparator functions. The difficulty is that for multi-column + * sorts there may be different data types involved (for some of which the calls + * may be much more expensive). Furthermore, the columns may have very different + * number of distinct values - the higher the number, the fewer comparisons will + * be needed for the following columns. + * + * The algoritm is incremental - we add pathkeys one by one, and at each step we + * estimate the number of necessary comparisons (based on the number of distinct + * groups in the current pathkey prefix and the new pathkey), and the comparison + * costs (which is data type specific). + * + * Estimation of the number of comparisons is based on ideas from two sources: + * + * 1) "Algorithms" (course), Robert Sedgewick, Kevin Wayne [https://algs4.cs.princeton.edu/home/] + * + * 2) "Quicksort Is Optimal For Many Equal Keys" (paper), Sebastian Wild, + * arXiv:1608.04906v4 [cs.DS] 1 Nov 2017. [https://arxiv.org/abs/1608.04906] + * + * In term of that paper, let N - number of tuples, Xi - number of tuples with + * key Ki, then the estimate of number of comparisons is: + * + * log(N! / (X1! * X2! * ..)) ~ sum(Xi * log(N/Xi)) + * + * In our case all Xi are the same because now we don't have any estimation of + * group sizes, we have only know the estimate of number of groups (distinct + * values). In that case, formula becomes: + * + * N * log(NumberOfGroups) + * + * For multi-column sorts we need to estimate the number of comparisons for + * each individual column - for example with columns (c1, c2, ..., ck) we + * can estimate that number of comparions on ck is roughly + * + * ncomparisons(c1, c2, ..., ck) / ncomparisons(c1, c2, ..., c(k-1)) + * + * Let k be a column number, Gk - number of groups defined by k columns, and Fk + * the cost of the comparison is + * + * N * sum( Fk * log(Gk) ) + * + * Note: We also consider column witdth, not just the comparator cost. + * * NOTE: some callers currently pass NIL for pathkeys because they * can't conveniently supply the sort keys. In this case, it will fallback to * simple comparison cost estimate. - * - * Estimation algorithm is based on ideas from course Algorithms, - * Robert Sedgewick, Kevin Wayne, https://algs4.cs.princeton.edu/home/ and paper - * "Quicksort Is Optimal For Many Equal Keys", Sebastian Wild, - * arXiv:1608.04906v4 [cs.DS] 1 Nov 2017. - * - * In term of that papers, let N - number of tuples, Xi - number of tuples with - * key Ki, then estimation is: - * log(N! / (X1! * X2! * ..)) ~ sum(Xi * log(N/Xi)) - * In our case all Xi are the same because now we don't have an estimation of - * group sizes, we have only estimation of number of groups. In this case, - * formula becomes: N * log(NumberOfGroups). Next, to support correct estimation - * of multi-column sort we need separately compute each column, so, let k is a - * column number, Gk - number of groups defined by k columns: - * N * sum( Fk * log(Gk) ) - * Fk is a function costs (including width) for k columns. */ - static Cost compute_cpu_sort_cost(PlannerInfo *root, List *pathkeys, int nPresortedKeys, Cost comparison_cost, double tuples, double output_tuples, @@ -1862,7 +1890,7 @@ compute_cpu_sort_cost(PlannerInfo *root, List *pathkeys, int nPresortedKeys, bool has_fake_var = false; int i = 0; Oid prev_datatype = InvalidOid; - Cost funcCost = 0.; + Cost funcCost = 0.0; List *cache_varinfos = NIL; /* fallback if pathkeys is unknown */ @@ -1873,6 +1901,10 @@ compute_cpu_sort_cost(PlannerInfo *root, List *pathkeys, int nPresortedKeys, * a total number of tuple comparisons of N log2 K; but the constant * factor is a bit higher than for quicksort. Tweak it so that the * cost curve is continuous at the crossover point. + * + * XXX I suppose the "quicksort factor" references to 1.5 at the end + * of this function, but I'm not sure. I suggest we introduce some simple + * constants for that, instead of magic values. */ output_tuples = (heapSort) ? 2.0 * output_tuples : tuples; per_tuple_cost += 2.0 * cpu_operator_cost * LOG2(output_tuples); @@ -1888,7 +1920,6 @@ compute_cpu_sort_cost(PlannerInfo *root, List *pathkeys, int nPresortedKeys, * - per column comparison function cost * - we try to compute needed number of comparison per column */ - foreach(lc, pathkeys) { PathKey *pathkey = (PathKey*)lfirst(lc); @@ -1952,6 +1983,11 @@ compute_cpu_sort_cost(PlannerInfo *root, List *pathkeys, int nPresortedKeys, * Don't use DEFAULT_NUM_DISTINCT because it used for only one * column while here we try to estimate number of groups over * set of columns. + * + * XXX Perhaps this should use DEFAULT_NUM_DISTINCT at least to + * limit the calculated values, somehow? + * + * XXX What's the logic of the following formula? */ nGroups = ceil(2.0 + sqrt(tuples) * list_length(pathkeyExprs) / list_length(pathkeys)); @@ -1968,6 +2004,7 @@ compute_cpu_sort_cost(PlannerInfo *root, List *pathkeys, int nPresortedKeys, { if (tuplesPerPrevGroup < output_tuples) /* comparing only inside output_tuples */ + /* XXX why not to use the same multiplier (1.5)? */ correctedNGroups = ceil(2.0 * output_tuples / (tuplesPerPrevGroup / nGroups)); else @@ -1993,7 +2030,7 @@ compute_cpu_sort_cost(PlannerInfo *root, List *pathkeys, int nPresortedKeys, tuplesPerPrevGroup = ceil(1.5 * tuplesPerPrevGroup / nGroups); /* - * We could skip all followed columns for cost estimation, because we + * We could skip all following columns for cost estimation, because we * believe that tuples are unique by set ot previous columns */ if (tuplesPerPrevGroup <= 1.0) diff --git a/src/backend/optimizer/path/pathkeys.c b/src/backend/optimizer/path/pathkeys.c index 7beb32488a..b092c3e055 100644 --- a/src/backend/optimizer/path/pathkeys.c +++ b/src/backend/optimizer/path/pathkeys.c @@ -515,10 +515,19 @@ pathkey_sort_cost_comparator(const void *_a, const void *_b) return 0; return 1; } + /* * Order tail of list of group pathkeys by uniqueness descendetly. It allows to * speedup sorting. Returns newly allocated lists, old ones stay untouched. * n_preordered defines a head of list which order should be prevented. + * + * XXX But we're not generating this only based on uniqueness (that's a bad + * term anyway, because we're using ndistinct estimates, not uniqueness). + * We're also using the comparator cost to calculate the expected sort cost, + * and optimize that. + * + * XXX This should explain how we generate the values - all permutations for + * up to 4 values, etc. */ void get_cheapest_group_keys_order(PlannerInfo *root, double nrows, @@ -597,8 +606,9 @@ get_cheapest_group_keys_order(PlannerInfo *root, double nrows, else { /* - * Since v13 list_free() can clean list elements so for original list not to be modified it should be copied to - * a new one which can then be cleaned safely if needed. + * Since v13 list_free() can clean list elements so for original list + * not to be modified it should be copied to a new one which can then + * be cleaned safely if needed. */ new_group_pathkeys = list_copy(*group_pathkeys); nToPermute = nFreeKeys; -- 2.29.2 --------------A955BFDEDE369DCD57A45C5A-- ^ permalink raw reply [nested|flat] 5+ messages in thread
* [PATCH v3 1/1] remove db_user_namespace @ 2023-06-30 19:46 Nathan Bossart <[email protected]> 0 siblings, 0 replies; 5+ messages in thread From: Nathan Bossart @ 2023-06-30 19:46 UTC (permalink / raw) --- doc/src/sgml/client-auth.sgml | 5 -- doc/src/sgml/config.sgml | 52 ------------------- src/backend/libpq/auth.c | 5 -- src/backend/libpq/hba.c | 12 ----- src/backend/postmaster/postmaster.c | 19 ------- src/backend/utils/misc/guc_tables.c | 9 ---- src/backend/utils/misc/postgresql.conf.sample | 1 - src/include/libpq/pqcomm.h | 2 - 8 files changed, 105 deletions(-) diff --git a/doc/src/sgml/client-auth.sgml b/doc/src/sgml/client-auth.sgml index 204d09df67..6c95f0df1e 100644 --- a/doc/src/sgml/client-auth.sgml +++ b/doc/src/sgml/client-auth.sgml @@ -1253,11 +1253,6 @@ omicron bryanh guest1 attacks. </para> - <para> - The <literal>md5</literal> method cannot be used with - the <xref linkend="guc-db-user-namespace"/> feature. - </para> - <para> To ease transition from the <literal>md5</literal> method to the newer SCRAM method, if <literal>md5</literal> is specified as a method diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml index 6262cb7bb2..e6cea8ddfc 100644 --- a/doc/src/sgml/config.sgml +++ b/doc/src/sgml/config.sgml @@ -1188,58 +1188,6 @@ include_dir 'conf.d' </para> </listitem> </varlistentry> - - <varlistentry id="guc-db-user-namespace" xreflabel="db_user_namespace"> - <term><varname>db_user_namespace</varname> (<type>boolean</type>) - <indexterm> - <primary><varname>db_user_namespace</varname> configuration parameter</primary> - </indexterm> - </term> - <listitem> - <para> - This parameter enables per-database user names. It is off by default. - This parameter can only be set in the <filename>postgresql.conf</filename> - file or on the server command line. - </para> - - <para> - If this is on, you should create users as <replaceable>username@dbname</replaceable>. - When <replaceable>username</replaceable> is passed by a connecting client, - <literal>@</literal> and the database name are appended to the user - name and that database-specific user name is looked up by the - server. Note that when you create users with names containing - <literal>@</literal> within the SQL environment, you will need to - quote the user name. - </para> - - <para> - With this parameter enabled, you can still create ordinary global - users. Simply append <literal>@</literal> when specifying the user - name in the client, e.g., <literal>joe@</literal>. The <literal>@</literal> - will be stripped off before the user name is looked up by the - server. - </para> - - <para> - <varname>db_user_namespace</varname> causes the client's and - server's user name representation to differ. - Authentication checks are always done with the server's user name - so authentication methods must be configured for the - server's user name, not the client's. Because - <literal>md5</literal> uses the user name as salt on both the - client and server, <literal>md5</literal> cannot be used with - <varname>db_user_namespace</varname>. - </para> - - <note> - <para> - This feature is intended as a temporary measure until a - complete solution is found. At that time, this option will - be removed. - </para> - </note> - </listitem> - </varlistentry> </variablelist> </sect2> diff --git a/src/backend/libpq/auth.c b/src/backend/libpq/auth.c index a98b934a8e..65d452f099 100644 --- a/src/backend/libpq/auth.c +++ b/src/backend/libpq/auth.c @@ -873,11 +873,6 @@ CheckMD5Auth(Port *port, char *shadow_pass, const char **logdetail) char *passwd; int result; - if (Db_user_namespace) - ereport(FATAL, - (errcode(ERRCODE_INVALID_AUTHORIZATION_SPECIFICATION), - errmsg("MD5 authentication is not supported when \"db_user_namespace\" is enabled"))); - /* include the salt to use for computing the response */ if (!pg_strong_random(md5Salt, 4)) { diff --git a/src/backend/libpq/hba.c b/src/backend/libpq/hba.c index f89f138f3c..5d4ddbb04d 100644 --- a/src/backend/libpq/hba.c +++ b/src/backend/libpq/hba.c @@ -1741,19 +1741,7 @@ parse_hba_line(TokenizedAuthLine *tok_line, int elevel) else if (strcmp(token->string, "reject") == 0) parsedline->auth_method = uaReject; else if (strcmp(token->string, "md5") == 0) - { - if (Db_user_namespace) - { - ereport(elevel, - (errcode(ERRCODE_CONFIG_FILE_ERROR), - errmsg("MD5 authentication is not supported when \"db_user_namespace\" is enabled"), - errcontext("line %d of configuration file \"%s\"", - line_num, file_name))); - *err_msg = "MD5 authentication is not supported when \"db_user_namespace\" is enabled"; - return NULL; - } parsedline->auth_method = uaMD5; - } else if (strcmp(token->string, "scram-sha-256") == 0) parsedline->auth_method = uaSCRAM; else if (strcmp(token->string, "pam") == 0) diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c index 0b1de9efb2..9c8ec779f9 100644 --- a/src/backend/postmaster/postmaster.c +++ b/src/backend/postmaster/postmaster.c @@ -236,7 +236,6 @@ int AuthenticationTimeout = 60; bool log_hostname; /* for ps display and logging */ bool Log_connections = false; -bool Db_user_namespace = false; bool enable_bonjour = false; char *bonjour_name; @@ -2272,24 +2271,6 @@ retry1: if (port->database_name == NULL || port->database_name[0] == '\0') port->database_name = pstrdup(port->user_name); - if (Db_user_namespace) - { - /* - * If user@, it is a global user, remove '@'. We only want to do this - * if there is an '@' at the end and no earlier in the user string or - * they may fake as a local user of another database attaching to this - * database. - */ - if (strchr(port->user_name, '@') == - port->user_name + strlen(port->user_name) - 1) - *strchr(port->user_name, '@') = '\0'; - else - { - /* Append '@' and dbname */ - port->user_name = psprintf("%s@%s", port->user_name, port->database_name); - } - } - if (am_walsender) MyBackendType = B_WAL_SENDER; else diff --git a/src/backend/utils/misc/guc_tables.c b/src/backend/utils/misc/guc_tables.c index f8ef87d26d..0c38af3f69 100644 --- a/src/backend/utils/misc/guc_tables.c +++ b/src/backend/utils/misc/guc_tables.c @@ -1534,15 +1534,6 @@ struct config_bool ConfigureNamesBool[] = false, NULL, NULL, NULL }, - { - {"db_user_namespace", PGC_SIGHUP, CONN_AUTH_AUTH, - gettext_noop("Enables per-database user names."), - NULL - }, - &Db_user_namespace, - false, - NULL, NULL, NULL - }, { {"default_transaction_read_only", PGC_USERSET, CLIENT_CONN_STATEMENT, gettext_noop("Sets the default read-only status of new transactions."), diff --git a/src/backend/utils/misc/postgresql.conf.sample b/src/backend/utils/misc/postgresql.conf.sample index e4c0269fa3..c768af9a73 100644 --- a/src/backend/utils/misc/postgresql.conf.sample +++ b/src/backend/utils/misc/postgresql.conf.sample @@ -96,7 +96,6 @@ #authentication_timeout = 1min # 1s-600s #password_encryption = scram-sha-256 # scram-sha-256 or md5 #scram_iterations = 4096 -#db_user_namespace = off # GSSAPI using Kerberos #krb_server_keyfile = 'FILE:${sysconfdir}/krb5.keytab' diff --git a/src/include/libpq/pqcomm.h b/src/include/libpq/pqcomm.h index c85090259d..3da00f7983 100644 --- a/src/include/libpq/pqcomm.h +++ b/src/include/libpq/pqcomm.h @@ -103,8 +103,6 @@ typedef ProtocolVersion MsgType; typedef uint32 PacketLen; -extern PGDLLIMPORT bool Db_user_namespace; - /* * In protocol 3.0 and later, the startup packet length is not fixed, but * we set an arbitrary limit on it anyway. This is just to prevent simple -- 2.25.1 --CE+1k2dSO48ffgeK-- ^ permalink raw reply [nested|flat] 5+ messages in thread
* [PATCH v4 1/1] remove db_user_namespace @ 2023-06-30 19:46 Nathan Bossart <[email protected]> 0 siblings, 0 replies; 5+ messages in thread From: Nathan Bossart @ 2023-06-30 19:46 UTC (permalink / raw) --- doc/src/sgml/client-auth.sgml | 5 -- doc/src/sgml/config.sgml | 52 ------------------- src/backend/commands/variable.c | 15 ++++++ src/backend/libpq/auth.c | 5 -- src/backend/libpq/hba.c | 12 ----- src/backend/postmaster/postmaster.c | 19 ------- src/backend/utils/misc/guc_tables.c | 16 ++++-- src/backend/utils/misc/postgresql.conf.sample | 1 - src/include/libpq/pqcomm.h | 2 - src/include/utils/guc_hooks.h | 1 + .../unsafe_tests/expected/guc_privs.out | 4 ++ .../modules/unsafe_tests/sql/guc_privs.sql | 3 ++ 12 files changed, 35 insertions(+), 100 deletions(-) diff --git a/doc/src/sgml/client-auth.sgml b/doc/src/sgml/client-auth.sgml index 204d09df67..6c95f0df1e 100644 --- a/doc/src/sgml/client-auth.sgml +++ b/doc/src/sgml/client-auth.sgml @@ -1253,11 +1253,6 @@ omicron bryanh guest1 attacks. </para> - <para> - The <literal>md5</literal> method cannot be used with - the <xref linkend="guc-db-user-namespace"/> feature. - </para> - <para> To ease transition from the <literal>md5</literal> method to the newer SCRAM method, if <literal>md5</literal> is specified as a method diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml index 6262cb7bb2..e6cea8ddfc 100644 --- a/doc/src/sgml/config.sgml +++ b/doc/src/sgml/config.sgml @@ -1188,58 +1188,6 @@ include_dir 'conf.d' </para> </listitem> </varlistentry> - - <varlistentry id="guc-db-user-namespace" xreflabel="db_user_namespace"> - <term><varname>db_user_namespace</varname> (<type>boolean</type>) - <indexterm> - <primary><varname>db_user_namespace</varname> configuration parameter</primary> - </indexterm> - </term> - <listitem> - <para> - This parameter enables per-database user names. It is off by default. - This parameter can only be set in the <filename>postgresql.conf</filename> - file or on the server command line. - </para> - - <para> - If this is on, you should create users as <replaceable>username@dbname</replaceable>. - When <replaceable>username</replaceable> is passed by a connecting client, - <literal>@</literal> and the database name are appended to the user - name and that database-specific user name is looked up by the - server. Note that when you create users with names containing - <literal>@</literal> within the SQL environment, you will need to - quote the user name. - </para> - - <para> - With this parameter enabled, you can still create ordinary global - users. Simply append <literal>@</literal> when specifying the user - name in the client, e.g., <literal>joe@</literal>. The <literal>@</literal> - will be stripped off before the user name is looked up by the - server. - </para> - - <para> - <varname>db_user_namespace</varname> causes the client's and - server's user name representation to differ. - Authentication checks are always done with the server's user name - so authentication methods must be configured for the - server's user name, not the client's. Because - <literal>md5</literal> uses the user name as salt on both the - client and server, <literal>md5</literal> cannot be used with - <varname>db_user_namespace</varname>. - </para> - - <note> - <para> - This feature is intended as a temporary measure until a - complete solution is found. At that time, this option will - be removed. - </para> - </note> - </listitem> - </varlistentry> </variablelist> </sect2> diff --git a/src/backend/commands/variable.c b/src/backend/commands/variable.c index f0f2e07655..b6a2fa2512 100644 --- a/src/backend/commands/variable.c +++ b/src/backend/commands/variable.c @@ -1157,6 +1157,21 @@ check_bonjour(bool *newval, void **extra, GucSource source) return true; } +bool +check_db_user_namespace(bool *newval, void **extra, GucSource source) +{ + if (*newval) + { + /* check the GUC's definition for an explanation */ + GUC_check_errcode(ERRCODE_FEATURE_NOT_SUPPORTED); + GUC_check_errmsg("db_user_namespace is not supported"); + + return false; + } + + return true; +} + bool check_default_with_oids(bool *newval, void **extra, GucSource source) { diff --git a/src/backend/libpq/auth.c b/src/backend/libpq/auth.c index a98b934a8e..65d452f099 100644 --- a/src/backend/libpq/auth.c +++ b/src/backend/libpq/auth.c @@ -873,11 +873,6 @@ CheckMD5Auth(Port *port, char *shadow_pass, const char **logdetail) char *passwd; int result; - if (Db_user_namespace) - ereport(FATAL, - (errcode(ERRCODE_INVALID_AUTHORIZATION_SPECIFICATION), - errmsg("MD5 authentication is not supported when \"db_user_namespace\" is enabled"))); - /* include the salt to use for computing the response */ if (!pg_strong_random(md5Salt, 4)) { diff --git a/src/backend/libpq/hba.c b/src/backend/libpq/hba.c index f89f138f3c..5d4ddbb04d 100644 --- a/src/backend/libpq/hba.c +++ b/src/backend/libpq/hba.c @@ -1741,19 +1741,7 @@ parse_hba_line(TokenizedAuthLine *tok_line, int elevel) else if (strcmp(token->string, "reject") == 0) parsedline->auth_method = uaReject; else if (strcmp(token->string, "md5") == 0) - { - if (Db_user_namespace) - { - ereport(elevel, - (errcode(ERRCODE_CONFIG_FILE_ERROR), - errmsg("MD5 authentication is not supported when \"db_user_namespace\" is enabled"), - errcontext("line %d of configuration file \"%s\"", - line_num, file_name))); - *err_msg = "MD5 authentication is not supported when \"db_user_namespace\" is enabled"; - return NULL; - } parsedline->auth_method = uaMD5; - } else if (strcmp(token->string, "scram-sha-256") == 0) parsedline->auth_method = uaSCRAM; else if (strcmp(token->string, "pam") == 0) diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c index 0b1de9efb2..9c8ec779f9 100644 --- a/src/backend/postmaster/postmaster.c +++ b/src/backend/postmaster/postmaster.c @@ -236,7 +236,6 @@ int AuthenticationTimeout = 60; bool log_hostname; /* for ps display and logging */ bool Log_connections = false; -bool Db_user_namespace = false; bool enable_bonjour = false; char *bonjour_name; @@ -2272,24 +2271,6 @@ retry1: if (port->database_name == NULL || port->database_name[0] == '\0') port->database_name = pstrdup(port->user_name); - if (Db_user_namespace) - { - /* - * If user@, it is a global user, remove '@'. We only want to do this - * if there is an '@' at the end and no earlier in the user string or - * they may fake as a local user of another database attaching to this - * database. - */ - if (strchr(port->user_name, '@') == - port->user_name + strlen(port->user_name) - 1) - *strchr(port->user_name, '@') = '\0'; - else - { - /* Append '@' and dbname */ - port->user_name = psprintf("%s@%s", port->user_name, port->database_name); - } - } - if (am_walsender) MyBackendType = B_WAL_SENDER; else diff --git a/src/backend/utils/misc/guc_tables.c b/src/backend/utils/misc/guc_tables.c index f8ef87d26d..94e87b7bd4 100644 --- a/src/backend/utils/misc/guc_tables.c +++ b/src/backend/utils/misc/guc_tables.c @@ -505,6 +505,7 @@ bool check_function_bodies = true; */ bool default_with_oids = false; bool session_auth_is_superuser; +bool Db_user_namespace = false; int log_min_error_statement = ERROR; int log_min_messages = WARNING; @@ -1534,14 +1535,21 @@ struct config_bool ConfigureNamesBool[] = false, NULL, NULL, NULL }, + + /* + * db_user_namespace was removed in PostgreSQL 17, but we tolerate the + * parameter being set to false to avoid unnecessarily breaking older dump + * files. + */ { - {"db_user_namespace", PGC_SIGHUP, CONN_AUTH_AUTH, - gettext_noop("Enables per-database user names."), - NULL + {"db_user_namespace", PGC_SIGHUP, COMPAT_OPTIONS_PREVIOUS, + gettext_noop("db_user_namespace is no longer supported; this can only be false."), + NULL, + GUC_NO_SHOW_ALL | GUC_NOT_IN_SAMPLE }, &Db_user_namespace, false, - NULL, NULL, NULL + check_db_user_namespace, NULL, NULL }, { {"default_transaction_read_only", PGC_USERSET, CLIENT_CONN_STATEMENT, diff --git a/src/backend/utils/misc/postgresql.conf.sample b/src/backend/utils/misc/postgresql.conf.sample index e4c0269fa3..c768af9a73 100644 --- a/src/backend/utils/misc/postgresql.conf.sample +++ b/src/backend/utils/misc/postgresql.conf.sample @@ -96,7 +96,6 @@ #authentication_timeout = 1min # 1s-600s #password_encryption = scram-sha-256 # scram-sha-256 or md5 #scram_iterations = 4096 -#db_user_namespace = off # GSSAPI using Kerberos #krb_server_keyfile = 'FILE:${sysconfdir}/krb5.keytab' diff --git a/src/include/libpq/pqcomm.h b/src/include/libpq/pqcomm.h index c85090259d..3da00f7983 100644 --- a/src/include/libpq/pqcomm.h +++ b/src/include/libpq/pqcomm.h @@ -103,8 +103,6 @@ typedef ProtocolVersion MsgType; typedef uint32 PacketLen; -extern PGDLLIMPORT bool Db_user_namespace; - /* * In protocol 3.0 and later, the startup packet length is not fixed, but * we set an arbitrary limit on it anyway. This is just to prevent simple diff --git a/src/include/utils/guc_hooks.h b/src/include/utils/guc_hooks.h index 2ecb9fc086..c51d44ec15 100644 --- a/src/include/utils/guc_hooks.h +++ b/src/include/utils/guc_hooks.h @@ -49,6 +49,7 @@ extern bool check_cluster_name(char **newval, void **extra, GucSource source); extern const char *show_data_directory_mode(void); extern bool check_datestyle(char **newval, void **extra, GucSource source); extern void assign_datestyle(const char *newval, void *extra); +extern bool check_db_user_namespace(bool *newval, void **extra, GucSource source); extern bool check_default_table_access_method(char **newval, void **extra, GucSource source); extern bool check_default_tablespace(char **newval, void **extra, diff --git a/src/test/modules/unsafe_tests/expected/guc_privs.out b/src/test/modules/unsafe_tests/expected/guc_privs.out index f43a1da214..17f7a0c980 100644 --- a/src/test/modules/unsafe_tests/expected/guc_privs.out +++ b/src/test/modules/unsafe_tests/expected/guc_privs.out @@ -40,6 +40,10 @@ RESET autovacuum; -- fail, requires reload ERROR: parameter "autovacuum" cannot be changed now ALTER SYSTEM SET autovacuum = OFF; -- ok ALTER SYSTEM RESET autovacuum; -- ok +ALTER SYSTEM SET db_user_namespace = OFF; -- ok +ALTER SYSTEM SET db_user_namespace = ON; -- fail, cannot be changed +ERROR: db_user_namespace is not supported +ALTER SYSTEM RESET db_user_namespace; -- ok -- PGC_SUSET SET lc_messages = 'C'; -- ok RESET lc_messages; -- ok diff --git a/src/test/modules/unsafe_tests/sql/guc_privs.sql b/src/test/modules/unsafe_tests/sql/guc_privs.sql index 7a4fb24b9d..233ce1a5ac 100644 --- a/src/test/modules/unsafe_tests/sql/guc_privs.sql +++ b/src/test/modules/unsafe_tests/sql/guc_privs.sql @@ -31,6 +31,9 @@ SET autovacuum = OFF; -- fail, requires reload RESET autovacuum; -- fail, requires reload ALTER SYSTEM SET autovacuum = OFF; -- ok ALTER SYSTEM RESET autovacuum; -- ok +ALTER SYSTEM SET db_user_namespace = OFF; -- ok +ALTER SYSTEM SET db_user_namespace = ON; -- fail, cannot be changed +ALTER SYSTEM RESET db_user_namespace; -- ok -- PGC_SUSET SET lc_messages = 'C'; -- ok RESET lc_messages; -- ok -- 2.25.1 --jRHKVT23PllUwdXP-- ^ permalink raw reply [nested|flat] 5+ messages in thread
* [PATCH v1 1/1] remove db_user_namespace @ 2023-06-30 19:46 Nathan Bossart <[email protected]> 0 siblings, 0 replies; 5+ messages in thread From: Nathan Bossart @ 2023-06-30 19:46 UTC (permalink / raw) --- doc/src/sgml/config.sgml | 52 ------------------- src/backend/libpq/auth.c | 5 -- src/backend/libpq/hba.c | 12 ----- src/backend/postmaster/postmaster.c | 19 ------- src/backend/utils/misc/guc_tables.c | 9 ---- src/backend/utils/misc/postgresql.conf.sample | 1 - src/include/libpq/pqcomm.h | 2 - 7 files changed, 100 deletions(-) diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml index 6262cb7bb2..e6cea8ddfc 100644 --- a/doc/src/sgml/config.sgml +++ b/doc/src/sgml/config.sgml @@ -1188,58 +1188,6 @@ include_dir 'conf.d' </para> </listitem> </varlistentry> - - <varlistentry id="guc-db-user-namespace" xreflabel="db_user_namespace"> - <term><varname>db_user_namespace</varname> (<type>boolean</type>) - <indexterm> - <primary><varname>db_user_namespace</varname> configuration parameter</primary> - </indexterm> - </term> - <listitem> - <para> - This parameter enables per-database user names. It is off by default. - This parameter can only be set in the <filename>postgresql.conf</filename> - file or on the server command line. - </para> - - <para> - If this is on, you should create users as <replaceable>username@dbname</replaceable>. - When <replaceable>username</replaceable> is passed by a connecting client, - <literal>@</literal> and the database name are appended to the user - name and that database-specific user name is looked up by the - server. Note that when you create users with names containing - <literal>@</literal> within the SQL environment, you will need to - quote the user name. - </para> - - <para> - With this parameter enabled, you can still create ordinary global - users. Simply append <literal>@</literal> when specifying the user - name in the client, e.g., <literal>joe@</literal>. The <literal>@</literal> - will be stripped off before the user name is looked up by the - server. - </para> - - <para> - <varname>db_user_namespace</varname> causes the client's and - server's user name representation to differ. - Authentication checks are always done with the server's user name - so authentication methods must be configured for the - server's user name, not the client's. Because - <literal>md5</literal> uses the user name as salt on both the - client and server, <literal>md5</literal> cannot be used with - <varname>db_user_namespace</varname>. - </para> - - <note> - <para> - This feature is intended as a temporary measure until a - complete solution is found. At that time, this option will - be removed. - </para> - </note> - </listitem> - </varlistentry> </variablelist> </sect2> diff --git a/src/backend/libpq/auth.c b/src/backend/libpq/auth.c index a98b934a8e..65d452f099 100644 --- a/src/backend/libpq/auth.c +++ b/src/backend/libpq/auth.c @@ -873,11 +873,6 @@ CheckMD5Auth(Port *port, char *shadow_pass, const char **logdetail) char *passwd; int result; - if (Db_user_namespace) - ereport(FATAL, - (errcode(ERRCODE_INVALID_AUTHORIZATION_SPECIFICATION), - errmsg("MD5 authentication is not supported when \"db_user_namespace\" is enabled"))); - /* include the salt to use for computing the response */ if (!pg_strong_random(md5Salt, 4)) { diff --git a/src/backend/libpq/hba.c b/src/backend/libpq/hba.c index f89f138f3c..5d4ddbb04d 100644 --- a/src/backend/libpq/hba.c +++ b/src/backend/libpq/hba.c @@ -1741,19 +1741,7 @@ parse_hba_line(TokenizedAuthLine *tok_line, int elevel) else if (strcmp(token->string, "reject") == 0) parsedline->auth_method = uaReject; else if (strcmp(token->string, "md5") == 0) - { - if (Db_user_namespace) - { - ereport(elevel, - (errcode(ERRCODE_CONFIG_FILE_ERROR), - errmsg("MD5 authentication is not supported when \"db_user_namespace\" is enabled"), - errcontext("line %d of configuration file \"%s\"", - line_num, file_name))); - *err_msg = "MD5 authentication is not supported when \"db_user_namespace\" is enabled"; - return NULL; - } parsedline->auth_method = uaMD5; - } else if (strcmp(token->string, "scram-sha-256") == 0) parsedline->auth_method = uaSCRAM; else if (strcmp(token->string, "pam") == 0) diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c index 4c49393fc5..33a13fdf32 100644 --- a/src/backend/postmaster/postmaster.c +++ b/src/backend/postmaster/postmaster.c @@ -236,7 +236,6 @@ int AuthenticationTimeout = 60; bool log_hostname; /* for ps display and logging */ bool Log_connections = false; -bool Db_user_namespace = false; bool enable_bonjour = false; char *bonjour_name; @@ -2272,24 +2271,6 @@ retry1: if (port->database_name == NULL || port->database_name[0] == '\0') port->database_name = pstrdup(port->user_name); - if (Db_user_namespace) - { - /* - * If user@, it is a global user, remove '@'. We only want to do this - * if there is an '@' at the end and no earlier in the user string or - * they may fake as a local user of another database attaching to this - * database. - */ - if (strchr(port->user_name, '@') == - port->user_name + strlen(port->user_name) - 1) - *strchr(port->user_name, '@') = '\0'; - else - { - /* Append '@' and dbname */ - port->user_name = psprintf("%s@%s", port->user_name, port->database_name); - } - } - /* * Truncate given database and user names to length of a Postgres name. * This avoids lookup failures when overlength names are given. diff --git a/src/backend/utils/misc/guc_tables.c b/src/backend/utils/misc/guc_tables.c index 71e27f8eb0..25d9008bb6 100644 --- a/src/backend/utils/misc/guc_tables.c +++ b/src/backend/utils/misc/guc_tables.c @@ -1534,15 +1534,6 @@ struct config_bool ConfigureNamesBool[] = false, NULL, NULL, NULL }, - { - {"db_user_namespace", PGC_SIGHUP, CONN_AUTH_AUTH, - gettext_noop("Enables per-database user names."), - NULL - }, - &Db_user_namespace, - false, - NULL, NULL, NULL - }, { {"default_transaction_read_only", PGC_USERSET, CLIENT_CONN_STATEMENT, gettext_noop("Sets the default read-only status of new transactions."), diff --git a/src/backend/utils/misc/postgresql.conf.sample b/src/backend/utils/misc/postgresql.conf.sample index e4c0269fa3..c768af9a73 100644 --- a/src/backend/utils/misc/postgresql.conf.sample +++ b/src/backend/utils/misc/postgresql.conf.sample @@ -96,7 +96,6 @@ #authentication_timeout = 1min # 1s-600s #password_encryption = scram-sha-256 # scram-sha-256 or md5 #scram_iterations = 4096 -#db_user_namespace = off # GSSAPI using Kerberos #krb_server_keyfile = 'FILE:${sysconfdir}/krb5.keytab' diff --git a/src/include/libpq/pqcomm.h b/src/include/libpq/pqcomm.h index c85090259d..3da00f7983 100644 --- a/src/include/libpq/pqcomm.h +++ b/src/include/libpq/pqcomm.h @@ -103,8 +103,6 @@ typedef ProtocolVersion MsgType; typedef uint32 PacketLen; -extern PGDLLIMPORT bool Db_user_namespace; - /* * In protocol 3.0 and later, the startup packet length is not fixed, but * we set an arbitrary limit on it anyway. This is just to prevent simple -- 2.25.1 --Kj7319i9nmIyA2yE-- ^ permalink raw reply [nested|flat] 5+ messages in thread
* [PATCH v2 1/1] remove db_user_namespace @ 2023-06-30 19:46 Nathan Bossart <[email protected]> 0 siblings, 0 replies; 5+ messages in thread From: Nathan Bossart @ 2023-06-30 19:46 UTC (permalink / raw) --- doc/src/sgml/client-auth.sgml | 5 -- doc/src/sgml/config.sgml | 52 ------------------- src/backend/libpq/auth.c | 5 -- src/backend/libpq/hba.c | 12 ----- src/backend/postmaster/postmaster.c | 19 ------- src/backend/utils/misc/guc_tables.c | 9 ---- src/backend/utils/misc/postgresql.conf.sample | 1 - src/include/libpq/pqcomm.h | 2 - 8 files changed, 105 deletions(-) diff --git a/doc/src/sgml/client-auth.sgml b/doc/src/sgml/client-auth.sgml index 204d09df67..6c95f0df1e 100644 --- a/doc/src/sgml/client-auth.sgml +++ b/doc/src/sgml/client-auth.sgml @@ -1253,11 +1253,6 @@ omicron bryanh guest1 attacks. </para> - <para> - The <literal>md5</literal> method cannot be used with - the <xref linkend="guc-db-user-namespace"/> feature. - </para> - <para> To ease transition from the <literal>md5</literal> method to the newer SCRAM method, if <literal>md5</literal> is specified as a method diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml index 6262cb7bb2..e6cea8ddfc 100644 --- a/doc/src/sgml/config.sgml +++ b/doc/src/sgml/config.sgml @@ -1188,58 +1188,6 @@ include_dir 'conf.d' </para> </listitem> </varlistentry> - - <varlistentry id="guc-db-user-namespace" xreflabel="db_user_namespace"> - <term><varname>db_user_namespace</varname> (<type>boolean</type>) - <indexterm> - <primary><varname>db_user_namespace</varname> configuration parameter</primary> - </indexterm> - </term> - <listitem> - <para> - This parameter enables per-database user names. It is off by default. - This parameter can only be set in the <filename>postgresql.conf</filename> - file or on the server command line. - </para> - - <para> - If this is on, you should create users as <replaceable>username@dbname</replaceable>. - When <replaceable>username</replaceable> is passed by a connecting client, - <literal>@</literal> and the database name are appended to the user - name and that database-specific user name is looked up by the - server. Note that when you create users with names containing - <literal>@</literal> within the SQL environment, you will need to - quote the user name. - </para> - - <para> - With this parameter enabled, you can still create ordinary global - users. Simply append <literal>@</literal> when specifying the user - name in the client, e.g., <literal>joe@</literal>. The <literal>@</literal> - will be stripped off before the user name is looked up by the - server. - </para> - - <para> - <varname>db_user_namespace</varname> causes the client's and - server's user name representation to differ. - Authentication checks are always done with the server's user name - so authentication methods must be configured for the - server's user name, not the client's. Because - <literal>md5</literal> uses the user name as salt on both the - client and server, <literal>md5</literal> cannot be used with - <varname>db_user_namespace</varname>. - </para> - - <note> - <para> - This feature is intended as a temporary measure until a - complete solution is found. At that time, this option will - be removed. - </para> - </note> - </listitem> - </varlistentry> </variablelist> </sect2> diff --git a/src/backend/libpq/auth.c b/src/backend/libpq/auth.c index a98b934a8e..65d452f099 100644 --- a/src/backend/libpq/auth.c +++ b/src/backend/libpq/auth.c @@ -873,11 +873,6 @@ CheckMD5Auth(Port *port, char *shadow_pass, const char **logdetail) char *passwd; int result; - if (Db_user_namespace) - ereport(FATAL, - (errcode(ERRCODE_INVALID_AUTHORIZATION_SPECIFICATION), - errmsg("MD5 authentication is not supported when \"db_user_namespace\" is enabled"))); - /* include the salt to use for computing the response */ if (!pg_strong_random(md5Salt, 4)) { diff --git a/src/backend/libpq/hba.c b/src/backend/libpq/hba.c index f89f138f3c..5d4ddbb04d 100644 --- a/src/backend/libpq/hba.c +++ b/src/backend/libpq/hba.c @@ -1741,19 +1741,7 @@ parse_hba_line(TokenizedAuthLine *tok_line, int elevel) else if (strcmp(token->string, "reject") == 0) parsedline->auth_method = uaReject; else if (strcmp(token->string, "md5") == 0) - { - if (Db_user_namespace) - { - ereport(elevel, - (errcode(ERRCODE_CONFIG_FILE_ERROR), - errmsg("MD5 authentication is not supported when \"db_user_namespace\" is enabled"), - errcontext("line %d of configuration file \"%s\"", - line_num, file_name))); - *err_msg = "MD5 authentication is not supported when \"db_user_namespace\" is enabled"; - return NULL; - } parsedline->auth_method = uaMD5; - } else if (strcmp(token->string, "scram-sha-256") == 0) parsedline->auth_method = uaSCRAM; else if (strcmp(token->string, "pam") == 0) diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c index 4c49393fc5..33a13fdf32 100644 --- a/src/backend/postmaster/postmaster.c +++ b/src/backend/postmaster/postmaster.c @@ -236,7 +236,6 @@ int AuthenticationTimeout = 60; bool log_hostname; /* for ps display and logging */ bool Log_connections = false; -bool Db_user_namespace = false; bool enable_bonjour = false; char *bonjour_name; @@ -2272,24 +2271,6 @@ retry1: if (port->database_name == NULL || port->database_name[0] == '\0') port->database_name = pstrdup(port->user_name); - if (Db_user_namespace) - { - /* - * If user@, it is a global user, remove '@'. We only want to do this - * if there is an '@' at the end and no earlier in the user string or - * they may fake as a local user of another database attaching to this - * database. - */ - if (strchr(port->user_name, '@') == - port->user_name + strlen(port->user_name) - 1) - *strchr(port->user_name, '@') = '\0'; - else - { - /* Append '@' and dbname */ - port->user_name = psprintf("%s@%s", port->user_name, port->database_name); - } - } - /* * Truncate given database and user names to length of a Postgres name. * This avoids lookup failures when overlength names are given. diff --git a/src/backend/utils/misc/guc_tables.c b/src/backend/utils/misc/guc_tables.c index 71e27f8eb0..25d9008bb6 100644 --- a/src/backend/utils/misc/guc_tables.c +++ b/src/backend/utils/misc/guc_tables.c @@ -1534,15 +1534,6 @@ struct config_bool ConfigureNamesBool[] = false, NULL, NULL, NULL }, - { - {"db_user_namespace", PGC_SIGHUP, CONN_AUTH_AUTH, - gettext_noop("Enables per-database user names."), - NULL - }, - &Db_user_namespace, - false, - NULL, NULL, NULL - }, { {"default_transaction_read_only", PGC_USERSET, CLIENT_CONN_STATEMENT, gettext_noop("Sets the default read-only status of new transactions."), diff --git a/src/backend/utils/misc/postgresql.conf.sample b/src/backend/utils/misc/postgresql.conf.sample index e4c0269fa3..c768af9a73 100644 --- a/src/backend/utils/misc/postgresql.conf.sample +++ b/src/backend/utils/misc/postgresql.conf.sample @@ -96,7 +96,6 @@ #authentication_timeout = 1min # 1s-600s #password_encryption = scram-sha-256 # scram-sha-256 or md5 #scram_iterations = 4096 -#db_user_namespace = off # GSSAPI using Kerberos #krb_server_keyfile = 'FILE:${sysconfdir}/krb5.keytab' diff --git a/src/include/libpq/pqcomm.h b/src/include/libpq/pqcomm.h index c85090259d..3da00f7983 100644 --- a/src/include/libpq/pqcomm.h +++ b/src/include/libpq/pqcomm.h @@ -103,8 +103,6 @@ typedef ProtocolVersion MsgType; typedef uint32 PacketLen; -extern PGDLLIMPORT bool Db_user_namespace; - /* * In protocol 3.0 and later, the startup packet length is not fixed, but * we set an arbitrary limit on it anyway. This is just to prevent simple -- 2.25.1 --X1bOJ3K7DJ5YkBrT-- ^ permalink raw reply [nested|flat] 5+ messages in thread
end of thread, other threads:[~2023-06-30 19:46 UTC | newest] Thread overview: 5+ messages (download: mbox mbox.gz follow: Atom feed) -- links below jump to the message on this page -- 2021-03-09 20:09 [PATCH 2/2] review Tomas Vondra <[email protected]> 2023-06-30 19:46 [PATCH v1 1/1] remove db_user_namespace Nathan Bossart <[email protected]> 2023-06-30 19:46 [PATCH v2 1/1] remove db_user_namespace Nathan Bossart <[email protected]> 2023-06-30 19:46 [PATCH v3 1/1] remove db_user_namespace Nathan Bossart <[email protected]> 2023-06-30 19:46 [PATCH v4 1/1] remove db_user_namespace Nathan Bossart <[email protected]>
This inbox is served by agora; see mirroring instructions for how to clone and mirror all data and code used for this inbox