public inbox for [email protected]help / color / mirror / Atom feed
[PATCH v6 07/10] is vs are plural 8+ messages / 6 participants [nested] [flat]
* [PATCH v6 07/10] is vs are plural @ 2019-04-04 23:57 Justin Pryzby <[email protected]> 0 siblings, 0 replies; 8+ messages in thread From: Justin Pryzby @ 2019-04-04 23:57 UTC (permalink / raw) Should backpatch to v12 --- doc/src/sgml/ref/alter_table.sgml | 2 +- doc/src/sgml/sources.sgml | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/doc/src/sgml/ref/alter_table.sgml b/doc/src/sgml/ref/alter_table.sgml index b2eb7097a9..fa848e0bdf 100644 --- a/doc/src/sgml/ref/alter_table.sgml +++ b/doc/src/sgml/ref/alter_table.sgml @@ -889,7 +889,7 @@ WITH ( MODULUS <replaceable class="parameter">numeric_literal</replaceable>, REM from the parent table will be created in the partition, if they don't already exist. If any of the <literal>CHECK</literal> constraints of the table being - attached is marked <literal>NO INHERIT</literal>, the command will fail; + attached are marked <literal>NO INHERIT</literal>, the command will fail; such constraints must be recreated without the <literal>NO INHERIT</literal> clause. </para> diff --git a/doc/src/sgml/sources.sgml b/doc/src/sgml/sources.sgml index 283c3e0357..12704c6fdf 100644 --- a/doc/src/sgml/sources.sgml +++ b/doc/src/sgml/sources.sgml @@ -466,8 +466,8 @@ Hint: the addendum enough for error messages. Detail and hint messages can be relegated to a verbose mode, or perhaps a pop-up error-details window. Also, details and hints would normally be suppressed from the server log to save - space. Reference to implementation details is best avoided since users - aren't expected to know the details. + space. References to implementation details are best avoided since users + aren't expected to know them. </para> </simplesect> -- 2.17.0 --C+ts3FVlLX8+P6JN Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v6-0008-doc-backup-manifests.patch" ^ permalink raw reply [nested|flat] 8+ messages in thread
* Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote @ 2025-04-06 17:21 Tom Lane <[email protected]> 0 siblings, 1 reply; 8+ messages in thread From: Tom Lane @ 2025-04-06 17:21 UTC (permalink / raw) To: Álvaro Herrera <[email protected]>; +Cc: Andrew Dunstan <[email protected]>; Nathan Bossart <[email protected]>; Mahendra Singh Thalor <[email protected]>; Srinath Reddy <[email protected]>; [email protected] =?utf-8?Q?=C3=81lvaro?= Herrera <[email protected]> writes: > Grumble. I'd rather introduce a partial restriction now only for names > that affect the tools failing outright (pg_dumpall in particular), than > do nothing for this release. If we feel the need to extend the > restriction later, that's easy to do and bothers nobody. We've wanted > these characters to be forbidden on database names for a long time, but > nobody has cared as much for their use on other types of names. I'm not > even sure we'd support the idea of forbidding them on all names (though > the SQL standard doesn't allow control chars in identifiers.) I'd be 100% behind forbidding all ASCII control characters in all identifiers. I can't see any situation in which that's a good thing, and I can think of plenty where it's a mistake (eg your editor decided to change space to tab) or done with underhanded intent. If we can cite the SQL standard then it's an entirely defensible restriction. Having said that, I'm not quite sure where we ought to implement the restriction, and it's possible that there are multiple places that would need to check. I concur that the day before feature freeze is not a good time to be designing this. Let's defer. regards, tom lane ^ permalink raw reply [nested|flat] 8+ messages in thread
* Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote @ 2025-04-06 17:34 Álvaro Herrera <[email protected]> parent: Tom Lane <[email protected]> 0 siblings, 1 reply; 8+ messages in thread From: Álvaro Herrera @ 2025-04-06 17:34 UTC (permalink / raw) To: Tom Lane <[email protected]>; +Cc: Andrew Dunstan <[email protected]>; Nathan Bossart <[email protected]>; Mahendra Singh Thalor <[email protected]>; Srinath Reddy <[email protected]>; [email protected] On 2025-Apr-06, Tom Lane wrote: > I'd be 100% behind forbidding all ASCII control characters in all > identifiers. I can't see any situation in which that's a good thing, > and I can think of plenty where it's a mistake (eg your editor > decided to change space to tab) or done with underhanded intent. Right. > If we can cite the SQL standard then it's an entirely defensible > restriction. We can. It says (in 5.2 <token> and <separator>) <regular identifier> ::= <identifier body> <identifier body> ::= <identifier start> [ <identifier part>... ] <identifier part> ::= <identifier start> | <identifier extend> <identifier start> ::= !! See the Syntax Rules. <identifier extend> ::= !! See the Syntax Rules. Syntax Rules 1) An <identifier start> is any character in the Unicode General Category classes “Lu”, “Ll”, “Lt”, “Lm”, “Lo”, or “Nl”. NOTE 112 — The Unicode General Category classes “Lu”, “Ll”, “Lt”, “Lm”, “Lo”, and “Nl” are assigned to Unicode characters that are, respectively, upper-case letters, lower-case letters, title-case letters, modifier letters, other letters, and letter numbers. 2) An <identifier extend> is U+00B7, “Middle Dot”, or any character in the Unicode General Category classes “Mn”, “Mc”, “Nd”, or “Pc”. NOTE 113 — The Unicode General Category classes “Mn”, “Mc”, “Nd”, and “Pc”, are assigned to Unicode characters that are, respectively, non-spacing marks, spacing combining marks, decimal numbers, and connector punctuations. The class for control characters is "C", so there are allowed nowhere. https://www.unicode.org/charts/script/ > Having said that, I'm not quite sure where we ought to implement > the restriction, and it's possible that there are multiple places > that would need to check. Yeah, a general ban on control characters for all identifiers is harder to implement than a restricted ban, because it probably involves the lexer, and I'm not sure the resulting "syntax error" type of rejections are going to be nice enough to users. A C-function based rejection seems more convenient at this stage. > I concur that the day before feature freeze is not a good time to be > designing this. Let's defer. Augh. -- Álvaro Herrera PostgreSQL Developer — https://www.EnterpriseDB.com/ "In fact, the basic problem with Perl 5's subroutines is that they're not crufty enough, so the cruft leaks out into user-defined code instead, by the Conservation of Cruft Principle." (Larry Wall, Apocalypse 6) ^ permalink raw reply [nested|flat] 8+ messages in thread
* Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote @ 2025-04-06 17:51 Tom Lane <[email protected]> parent: Álvaro Herrera <[email protected]> 0 siblings, 1 reply; 8+ messages in thread From: Tom Lane @ 2025-04-06 17:51 UTC (permalink / raw) To: Álvaro Herrera <[email protected]>; +Cc: Andrew Dunstan <[email protected]>; Nathan Bossart <[email protected]>; Mahendra Singh Thalor <[email protected]>; Srinath Reddy <[email protected]>; [email protected] =?utf-8?Q?=C3=81lvaro?= Herrera <[email protected]> writes: > On 2025-Apr-06, Tom Lane wrote: >> If we can cite the SQL standard then it's an entirely defensible >> restriction. > We can. It says (in 5.2 <token> and <separator>) > <regular identifier> ::= <identifier body> > <identifier body> ::= <identifier start> [ <identifier part>... ] > <identifier part> ::= <identifier start> | <identifier extend> > <identifier start> ::= !! See the Syntax Rules. > <identifier extend> ::= !! See the Syntax Rules. Hmm, but that's about non-quoted identifiers, so of course their character set is pretty restricted. What's of concern here is what's allowed in double-quoted identifiers. AFAICS there's not much restriction: it can be any <nondoublequote character>, and SR 7 says 7) A <nondoublequote character> is any character of the source language character set other than a <double quote>. NOTE 115 — “source language character set” is defined in Subclause 4.10.1, “Host languages”, in ISO/IEC 9075-1. The referenced bit of 9075-1 is pretty vague too: No matter what binding style is chosen, SQL-statements are written in an implementation-defined character set, known as the source language character set. The source language character set is not required to be the same as the character set of any character string appearing in SQL-data. So I'm not really seeing anything there that justifies forbidding any characters. However, I still think that if we're going to forbid CR or LF, we might as well go the whole way and forbid all the ASCII control characters; none of them are any saner to use in identifiers than those two. (I'd be for banning and similar as well, on the same usability grounds as banning tabs, except that putting an encoding dependency into this rule will not end well.) regards, tom lane ^ permalink raw reply [nested|flat] 8+ messages in thread
* Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote @ 2025-04-06 21:10 Andrew Dunstan <[email protected]> parent: Tom Lane <[email protected]> 0 siblings, 1 reply; 8+ messages in thread From: Andrew Dunstan @ 2025-04-06 21:10 UTC (permalink / raw) To: Tom Lane <[email protected]>; Álvaro Herrera <[email protected]>; +Cc: Nathan Bossart <[email protected]>; Mahendra Singh Thalor <[email protected]>; Srinath Reddy <[email protected]>; [email protected] On 2025-04-06 Su 1:51 PM, Tom Lane wrote: > =?utf-8?Q?=C3=81lvaro?= Herrera <[email protected]> writes: >> On 2025-Apr-06, Tom Lane wrote: >>> If we can cite the SQL standard then it's an entirely defensible >>> restriction. >> We can. It says (in 5.2 <token> and <separator>) >> <regular identifier> ::= <identifier body> >> <identifier body> ::= <identifier start> [ <identifier part>... ] >> <identifier part> ::= <identifier start> | <identifier extend> >> <identifier start> ::= !! See the Syntax Rules. >> <identifier extend> ::= !! See the Syntax Rules. > Hmm, but that's about non-quoted identifiers, so of course their > character set is pretty restricted. What's of concern here is > what's allowed in double-quoted identifiers. AFAICS there's > not much restriction: it can be any <nondoublequote character>, > and SR 7 says > > 7) A <nondoublequote character> is any character of the source > language character set other than a <double quote>. > > NOTE 115 — “source language character set” is defined in > Subclause 4.10.1, “Host languages”, in ISO/IEC 9075-1. > > The referenced bit of 9075-1 is pretty vague too: > > No matter what binding style is chosen, SQL-statements are written > in an implementation-defined character set, known as the source > language character set. The source language character set is not > required to be the same as the character set of any character > string appearing in SQL-data. > > So I'm not really seeing anything there that justifies forbidding any > characters. However, I still think that if we're going to forbid CR > or LF, we might as well go the whole way and forbid all the ASCII > control characters; none of them are any saner to use in identifiers > than those two. (I'd be for banning and similar as well, on > the same usability grounds as banning tabs, except that putting an > encoding dependency into this rule will not end well.) Sound like we have some work to do, and that's not going to happen in 24 hours. cheers andrew -- Andrew Dunstan EDB: https://www.enterprisedb.com ^ permalink raw reply [nested|flat] 8+ messages in thread
* Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote @ 2025-04-10 18:28 Mahendra Singh Thalor <[email protected]> parent: Andrew Dunstan <[email protected]> 0 siblings, 1 reply; 8+ messages in thread From: Mahendra Singh Thalor @ 2025-04-10 18:28 UTC (permalink / raw) To: Andrew Dunstan <[email protected]>; +Cc: Tom Lane <[email protected]>; Álvaro Herrera <[email protected]>; Nathan Bossart <[email protected]>; Srinath Reddy <[email protected]>; [email protected] On Mon, 7 Apr 2025 at 02:40, Andrew Dunstan <[email protected]> wrote: > > > On 2025-04-06 Su 1:51 PM, Tom Lane wrote: > > =?utf-8?Q?=C3=81lvaro?= Herrera <[email protected]> writes: > >> On 2025-Apr-06, Tom Lane wrote: > >>> If we can cite the SQL standard then it's an entirely defensible > >>> restriction. > >> We can. It says (in 5.2 <token> and <separator>) > >> <regular identifier> ::= <identifier body> > >> <identifier body> ::= <identifier start> [ <identifier part>... ] > >> <identifier part> ::= <identifier start> | <identifier extend> > >> <identifier start> ::= !! See the Syntax Rules. > >> <identifier extend> ::= !! See the Syntax Rules. > > Hmm, but that's about non-quoted identifiers, so of course their > > character set is pretty restricted. What's of concern here is > > what's allowed in double-quoted identifiers. AFAICS there's > > not much restriction: it can be any <nondoublequote character>, > > and SR 7 says > > > > 7) A <nondoublequote character> is any character of the source > > language character set other than a <double quote>. > > > > NOTE 115 — “source language character set” is defined in > > Subclause 4.10.1, “Host languages”, in ISO/IEC 9075-1. > > > > The referenced bit of 9075-1 is pretty vague too: > > > > No matter what binding style is chosen, SQL-statements are written > > in an implementation-defined character set, known as the source > > language character set. The source language character set is not > > required to be the same as the character set of any character > > string appearing in SQL-data. > > > > So I'm not really seeing anything there that justifies forbidding any > > characters. However, I still think that if we're going to forbid CR > > or LF, we might as well go the whole way and forbid all the ASCII > > control characters; none of them are any saner to use in identifiers > > than those two. (I'd be for banning and similar as well, on > > the same usability grounds as banning tabs, except that putting an > > encoding dependency into this rule will not end well.) > > > Sound like we have some work to do, and that's not going to happen in 24 > hours. > > > cheers > > > andrew > > > -- > Andrew Dunstan > EDB: https://www.enterprisedb.com > Hi, As per above discussions, for v18, we will not do any change to server side to fix the issue of \n\r in database names. But as a cleanup patch, we can give an alert to the user by "pg_upgrade --check". As per current code, pg_dump and pg_upgrade will fail with "shell command" error but in the attached patch, we will give some extra info to the user by "pg_upgrade --check" so that they can fix database names before trying to upgrade. Here, I am attaching a patch which will give a list of invalid database names in "pg_upgrade --check". We can consider this as a cleanup patch. -- Thanks and Regards Mahendra Singh Thalor EnterpriseDB: http://www.enterprisedb.com Attachments: [application/octet-stream] 0001-pg_upgrade-add-alert-for-invalid-database-names.patch (3.1K, ../../CAKYtNArodApzf7OkF49VS4eO-u_Qwy0Pup3FMTw_QoLi5xeczQ@mail.gmail.com/2-0001-pg_upgrade-add-alert-for-invalid-database-names.patch) download | inline diff: From 14db9fc7157b0b7d935dfd025a37d9a0839aa9c7 Mon Sep 17 00:00:00 2001 From: Mahendra Singh Thalor <[email protected]> Date: Thu, 10 Apr 2025 23:47:45 +0530 Subject: [PATCH] pg_upgrade : add alert for invalid database names When any database has \n or \r in database name, then dump will report shell command error. so better, we will give alert to user to fix it. --- src/bin/pg_upgrade/check.c | 67 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 67 insertions(+) diff --git a/src/bin/pg_upgrade/check.c b/src/bin/pg_upgrade/check.c index 18c2d652bb6..be5db5ddae5 100644 --- a/src/bin/pg_upgrade/check.c +++ b/src/bin/pg_upgrade/check.c @@ -31,6 +31,7 @@ static void check_new_cluster_logical_replication_slots(void); static void check_new_cluster_subscription_configuration(void); static void check_old_cluster_for_valid_slots(void); static void check_old_cluster_subscription_state(void); +static void check_database_names_in_old_cluser(ClusterInfo *cluster); /* * DataTypesUsageChecks - definitions of data type checks for the old cluster @@ -598,6 +599,9 @@ check_and_dump_old_cluster(void) */ check_for_connection_status(&old_cluster); + /* Validate database names from old cluser. */ + check_database_names_in_old_cluser(&old_cluster); + /* * Extract a list of databases, tables, and logical replication slots from * the old cluster. @@ -2269,3 +2273,66 @@ check_old_cluster_subscription_state(void) else check_ok(); } + +/* + * check_database_names_in_old_cluser() + * + * If any database name has newline or carriage return character in name, + * then this will report those as these special characters are not allowed + * for database names in dump. See appendShellStringNoError + */ +static void +check_database_names_in_old_cluser(ClusterInfo *cluster) +{ + int i; + PGconn *conn_template1; + PGresult *res; + int ntups; + FILE *script = NULL; + char output_path[MAXPGPATH]; + int count = 0; + + prep_status("Checking database names"); + + snprintf(output_path, sizeof(output_path), "%s/%s", + log_opts.basedir, + "invalid_db_names.txt"); + + conn_template1 = connectToServer(cluster, "template1"); + + /* Get database names from cluster. */ + res = executeQueryOrDie(conn_template1, + "SELECT datname " + "FROM pg_catalog.pg_database"); + + ntups = PQntuples(res); + for (i = 0; i < ntups; i++) + { + char *dbname = PQgetvalue(res, i, 0); + + /* If name has \n or \r, then report it. */ + if (strpbrk(dbname, "\n\r")) + { + if (script == NULL && (script = fopen_priv(output_path, "w")) == NULL) + pg_fatal("could not open file \"%s\": %m", output_path); + + fprintf(script, "%d : database name = \"%s\"\n", ++count, dbname); + } + } + + PQclear(res); + PQfinish(conn_template1); + + if (script) + { + fclose(script); + pg_log(PG_REPORT, "fatal"); + pg_fatal("All the database names should have only valid characters. A newline or a carriage\n" + "return character is not allowed in these database names. To fix this, please rename\n" + "these names with valid names. \n" + "To see all %d invalid database names, refer invalid_db_names.txt file. \n" + " %s", count, output_path); + } + else + check_ok(); +} -- 2.39.3 ^ permalink raw reply [nested|flat] 8+ messages in thread
* Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote @ 2025-04-11 14:47 Nathan Bossart <[email protected]> parent: Mahendra Singh Thalor <[email protected]> 0 siblings, 1 reply; 8+ messages in thread From: Nathan Bossart @ 2025-04-11 14:47 UTC (permalink / raw) To: Mahendra Singh Thalor <[email protected]>; +Cc: Andrew Dunstan <[email protected]>; Tom Lane <[email protected]>; Álvaro Herrera <[email protected]>; Srinath Reddy <[email protected]>; [email protected] On Thu, Apr 10, 2025 at 11:58:41PM +0530, Mahendra Singh Thalor wrote: > As per above discussions, for v18, we will not do any change to server > side to fix the issue of \n\r in database names. But as a cleanup > patch, we can give an alert to the user by "pg_upgrade --check". As > per current code, pg_dump and pg_upgrade will fail with "shell > command" error but in the attached patch, we will give some extra info > to the user by "pg_upgrade --check" so that they can fix database > names before trying to upgrade. > > Here, I am attaching a patch which will give a list of invalid > database names in "pg_upgrade --check". We can consider this as a > cleanup patch. Are you proposing this for v18? I think this is all v19 material at this point. Perhaps we could argue this is a bug fix that should be back-patched, but IMHO that's a bit of a stretch. I don't sense a tremendous amount of urgency, either. -- nathan ^ permalink raw reply [nested|flat] 8+ messages in thread
* Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote @ 2025-06-24 06:07 Mahendra Singh Thalor <[email protected]> parent: Nathan Bossart <[email protected]> 0 siblings, 0 replies; 8+ messages in thread From: Mahendra Singh Thalor @ 2025-06-24 06:07 UTC (permalink / raw) To: Nathan Bossart <[email protected]>; +Cc: Andrew Dunstan <[email protected]>; Tom Lane <[email protected]>; Álvaro Herrera <[email protected]>; Srinath Reddy <[email protected]>; [email protected] On Fri, 11 Apr 2025 at 20:17, Nathan Bossart <[email protected]> wrote: > > On Thu, Apr 10, 2025 at 11:58:41PM +0530, Mahendra Singh Thalor wrote: > > As per above discussions, for v18, we will not do any change to server > > side to fix the issue of \n\r in database names. But as a cleanup > > patch, we can give an alert to the user by "pg_upgrade --check". As > > per current code, pg_dump and pg_upgrade will fail with "shell > > command" error but in the attached patch, we will give some extra info > > to the user by "pg_upgrade --check" so that they can fix database > > names before trying to upgrade. > > > > Here, I am attaching a patch which will give a list of invalid > > database names in "pg_upgrade --check". We can consider this as a > > cleanup patch. > > Are you proposing this for v18? I think this is all v19 material at this > point. Perhaps we could argue this is a bug fix that should be > back-patched, but IMHO that's a bit of a stretch. I don't sense a > tremendous amount of urgency, either. > > -- > nathan Thanks Nathan for the review. I want to re-start this thread for v19. I posted v06* patches in my previous updates[1]. Please someone review it and let me know feedback. v06 patches: v06_patches <https://www.postgresql.org/message-id/CAKYtNAqC5pkjmh8UgvbNLtMyEVeKUtDF3_%2B9dvG9zb8YrhTJQQ%40mail.g...; [1] : https://www.postgresql.org/message-id/CAKYtNAqC5pkjmh8UgvbNLtMyEVeKUtDF3_%2B9dvG9zb8YrhTJQQ%40mail.g... -- Thanks and Regards Mahendra Singh Thalor EnterpriseDB: http://www.enterprisedb.com ^ permalink raw reply [nested|flat] 8+ messages in thread
end of thread, other threads:[~2025-06-24 06:07 UTC | newest] Thread overview: 8+ messages (download: mbox mbox.gz follow: Atom feed) -- links below jump to the message on this page -- 2019-04-04 23:57 [PATCH v6 07/10] is vs are plural Justin Pryzby <[email protected]> 2025-04-06 17:21 Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Tom Lane <[email protected]> 2025-04-06 17:34 ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Álvaro Herrera <[email protected]> 2025-04-06 17:51 ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Tom Lane <[email protected]> 2025-04-06 21:10 ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Andrew Dunstan <[email protected]> 2025-04-10 18:28 ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Mahendra Singh Thalor <[email protected]> 2025-04-11 14:47 ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Nathan Bossart <[email protected]> 2025-06-24 06:07 ` Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote Mahendra Singh Thalor <[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