public inbox for [email protected]  

help / color / mirror / Atom feed

[PATCH v3 3/4] expand binaryheap api
16+ messages / 6 participants
[nested] [flat]

* [PATCH v3 3/4] expand binaryheap api
@ 2023-07-20 16:52 Nathan Bossart <[email protected]>
  0 siblings, 0 replies; 16+ messages in thread

From: Nathan Bossart @ 2023-07-20 16:52 UTC (permalink / raw)

---
 src/common/binaryheap.c      | 29 +++++++++++++++++++++++++++++
 src/include/lib/binaryheap.h |  3 +++
 2 files changed, 32 insertions(+)

diff --git a/src/common/binaryheap.c b/src/common/binaryheap.c
index f21838b946..fdd8c0024a 100644
--- a/src/common/binaryheap.c
+++ b/src/common/binaryheap.c
@@ -211,6 +211,35 @@ binaryheap_remove_first(binaryheap *heap)
 	return result;
 }
 
+/*
+ * binaryheap_remove_node
+ *
+ * Removes the nth node from the heap.  The caller must ensure that there are
+ * at least (n - 1) nodes in the heap.  O(log n) worst case.
+ */
+void
+binaryheap_remove_node(binaryheap *heap, int n)
+{
+	int			cmp;
+
+	Assert(!binaryheap_empty(heap) && heap->bh_has_heap_property);
+	Assert(n >= 0 && n < heap->bh_size);
+
+	/* compare last node to the one that is being removed */
+	cmp = heap->bh_compare(heap->bh_nodes[--heap->bh_size],
+						   heap->bh_nodes[n],
+						   heap->bh_arg);
+
+	/* remove the last node, placing it in the vacated entry */
+	heap->bh_nodes[n] = heap->bh_nodes[heap->bh_size];
+
+	/* sift as needed to preserve the heap property */
+	if (cmp > 0)
+		sift_up(heap, n);
+	else if (cmp < 0)
+		sift_down(heap, n);
+}
+
 /*
  * binaryheap_replace_first
  *
diff --git a/src/include/lib/binaryheap.h b/src/include/lib/binaryheap.h
index 52f7b06b25..20a7f71244 100644
--- a/src/include/lib/binaryheap.h
+++ b/src/include/lib/binaryheap.h
@@ -47,8 +47,11 @@ extern void binaryheap_build(binaryheap *heap);
 extern void binaryheap_add(binaryheap *heap, Datum d);
 extern Datum binaryheap_first(binaryheap *heap);
 extern Datum binaryheap_remove_first(binaryheap *heap);
+extern void binaryheap_remove_node(binaryheap *heap, int n);
 extern void binaryheap_replace_first(binaryheap *heap, Datum d);
 
 #define binaryheap_empty(h)			((h)->bh_size == 0)
+#define binaryheap_size(h)			((h)->bh_size)
+#define binaryheap_get_node(h, n)	((h)->bh_nodes[n])
 
 #endif							/* BINARYHEAP_H */
-- 
2.25.1


--sdtB3X0nJg68CQEu
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
	filename="v3-0004-use-priority-queue-for-pg_restore-ready_list.patch"



^ permalink  raw  reply  [nested|flat] 16+ messages in thread

* [PATCH v5 2/3] expand binaryheap api
@ 2023-07-20 16:52 Nathan Bossart <[email protected]>
  0 siblings, 0 replies; 16+ messages in thread

From: Nathan Bossart @ 2023-07-20 16:52 UTC (permalink / raw)

---
 src/common/binaryheap.c      | 29 +++++++++++++++++++++++++++++
 src/include/lib/binaryheap.h |  3 +++
 2 files changed, 32 insertions(+)

diff --git a/src/common/binaryheap.c b/src/common/binaryheap.c
index a6be4b42ae..d24a61fd2b 100644
--- a/src/common/binaryheap.c
+++ b/src/common/binaryheap.c
@@ -211,6 +211,35 @@ binaryheap_remove_first(binaryheap *heap)
 	return result;
 }
 
+/*
+ * binaryheap_remove_node
+ *
+ * Removes the nth node from the heap.  The caller must ensure that there are
+ * at least (n - 1) nodes in the heap.  O(log n) worst case.
+ */
+void
+binaryheap_remove_node(binaryheap *heap, int n)
+{
+	int			cmp;
+
+	Assert(!binaryheap_empty(heap) && heap->bh_has_heap_property);
+	Assert(n >= 0 && n < heap->bh_size);
+
+	/* compare last node to the one that is being removed */
+	cmp = heap->bh_compare(heap->bh_nodes[--heap->bh_size],
+						   heap->bh_nodes[n],
+						   heap->bh_arg);
+
+	/* remove the last node, placing it in the vacated entry */
+	heap->bh_nodes[n] = heap->bh_nodes[heap->bh_size];
+
+	/* sift as needed to preserve the heap property */
+	if (cmp > 0)
+		sift_up(heap, n);
+	else if (cmp < 0)
+		sift_down(heap, n);
+}
+
 /*
  * binaryheap_replace_first
  *
diff --git a/src/include/lib/binaryheap.h b/src/include/lib/binaryheap.h
index ae0af68f0d..84c08d7ebb 100644
--- a/src/include/lib/binaryheap.h
+++ b/src/include/lib/binaryheap.h
@@ -56,8 +56,11 @@ extern void binaryheap_build(binaryheap *heap);
 extern void binaryheap_add(binaryheap *heap, bh_elem_type d);
 extern bh_elem_type binaryheap_first(binaryheap *heap);
 extern bh_elem_type binaryheap_remove_first(binaryheap *heap);
+extern void binaryheap_remove_node(binaryheap *heap, int n);
 extern void binaryheap_replace_first(binaryheap *heap, bh_elem_type d);
 
 #define binaryheap_empty(h)			((h)->bh_size == 0)
+#define binaryheap_size(h)			((h)->bh_size)
+#define binaryheap_get_node(h, n)	((h)->bh_nodes[n])
 
 #endif							/* BINARYHEAP_H */
-- 
2.25.1


--3MwIy2ne0vdjdPXF
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
	filename="v5-0003-use-priority-queue-for-pg_restore-ready_list.patch"



^ permalink  raw  reply  [nested|flat] 16+ messages in thread

* [PATCH v2 3/4] expand binaryheap api
@ 2023-07-20 16:52 Nathan Bossart <[email protected]>
  0 siblings, 0 replies; 16+ messages in thread

From: Nathan Bossart @ 2023-07-20 16:52 UTC (permalink / raw)

---
 src/common/binaryheap.c      | 28 ++++++++++++++++++++++++++++
 src/include/lib/binaryheap.h |  3 +++
 2 files changed, 31 insertions(+)

diff --git a/src/common/binaryheap.c b/src/common/binaryheap.c
index 7a590eec4a..526e56bbd0 100644
--- a/src/common/binaryheap.c
+++ b/src/common/binaryheap.c
@@ -211,6 +211,34 @@ binaryheap_remove_first(binaryheap *heap)
 	return result;
 }
 
+/*
+ * binaryheap_remove_node
+ *
+ * Removes the given node from the heap.  O(log n) worst case.
+ */
+void
+binaryheap_remove_node(binaryheap *heap, int n)
+{
+	int			cmp;
+
+	Assert(!binaryheap_empty(heap) && heap->bh_has_heap_property);
+	Assert(n >= 0 && n < heap->bh_size);
+
+	/* compare last node to the one that is being removed */
+	cmp = heap->bh_compare(heap->bh_nodes[--heap->bh_size],
+						   heap->bh_nodes[n],
+						   heap->bh_arg);
+
+	/* remove the last node, placing it in the vacated entry */
+	heap->bh_nodes[n] = heap->bh_nodes[heap->bh_size];
+
+	/* sift as needed to preserve the heap property */
+	if (cmp > 0)
+		sift_up(heap, n);
+	else if (cmp < 0)
+		sift_down(heap, n);
+}
+
 /*
  * binaryheap_replace_first
  *
diff --git a/src/include/lib/binaryheap.h b/src/include/lib/binaryheap.h
index 06e3878cf7..9f09aff29f 100644
--- a/src/include/lib/binaryheap.h
+++ b/src/include/lib/binaryheap.h
@@ -76,8 +76,11 @@ extern void binaryheap_build(binaryheap *heap);
 extern void binaryheap_add(binaryheap *heap, Datum d);
 extern Datum binaryheap_first(binaryheap *heap);
 extern Datum binaryheap_remove_first(binaryheap *heap);
+extern void binaryheap_remove_node(binaryheap *heap, int n);
 extern void binaryheap_replace_first(binaryheap *heap, Datum d);
 
 #define binaryheap_empty(h)			((h)->bh_size == 0)
+#define binaryheap_size(h)			((h)->bh_size)
+#define binaryheap_get_node(h, n)	((h)->bh_nodes[n])
 
 #endif							/* BINARYHEAP_H */
-- 
2.25.1


--qDbXVdCdHGoSgWSk
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
	filename="v2-0004-use-priority-queue-for-pg_restore-ready_list.patch"



^ permalink  raw  reply  [nested|flat] 16+ messages in thread

* [PATCH v2 3/4] expand binaryheap api
@ 2023-07-20 16:52 Nathan Bossart <[email protected]>
  0 siblings, 0 replies; 16+ messages in thread

From: Nathan Bossart @ 2023-07-20 16:52 UTC (permalink / raw)

---
 src/common/binaryheap.c      | 28 ++++++++++++++++++++++++++++
 src/include/lib/binaryheap.h |  3 +++
 2 files changed, 31 insertions(+)

diff --git a/src/common/binaryheap.c b/src/common/binaryheap.c
index 7a590eec4a..526e56bbd0 100644
--- a/src/common/binaryheap.c
+++ b/src/common/binaryheap.c
@@ -211,6 +211,34 @@ binaryheap_remove_first(binaryheap *heap)
 	return result;
 }
 
+/*
+ * binaryheap_remove_node
+ *
+ * Removes the given node from the heap.  O(log n) worst case.
+ */
+void
+binaryheap_remove_node(binaryheap *heap, int n)
+{
+	int			cmp;
+
+	Assert(!binaryheap_empty(heap) && heap->bh_has_heap_property);
+	Assert(n >= 0 && n < heap->bh_size);
+
+	/* compare last node to the one that is being removed */
+	cmp = heap->bh_compare(heap->bh_nodes[--heap->bh_size],
+						   heap->bh_nodes[n],
+						   heap->bh_arg);
+
+	/* remove the last node, placing it in the vacated entry */
+	heap->bh_nodes[n] = heap->bh_nodes[heap->bh_size];
+
+	/* sift as needed to preserve the heap property */
+	if (cmp > 0)
+		sift_up(heap, n);
+	else if (cmp < 0)
+		sift_down(heap, n);
+}
+
 /*
  * binaryheap_replace_first
  *
diff --git a/src/include/lib/binaryheap.h b/src/include/lib/binaryheap.h
index 06e3878cf7..9f09aff29f 100644
--- a/src/include/lib/binaryheap.h
+++ b/src/include/lib/binaryheap.h
@@ -76,8 +76,11 @@ extern void binaryheap_build(binaryheap *heap);
 extern void binaryheap_add(binaryheap *heap, Datum d);
 extern Datum binaryheap_first(binaryheap *heap);
 extern Datum binaryheap_remove_first(binaryheap *heap);
+extern void binaryheap_remove_node(binaryheap *heap, int n);
 extern void binaryheap_replace_first(binaryheap *heap, Datum d);
 
 #define binaryheap_empty(h)			((h)->bh_size == 0)
+#define binaryheap_size(h)			((h)->bh_size)
+#define binaryheap_get_node(h, n)	((h)->bh_nodes[n])
 
 #endif							/* BINARYHEAP_H */
-- 
2.25.1


--qDbXVdCdHGoSgWSk
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
	filename="v2-0004-use-priority-queue-for-pg_restore-ready_list.patch"



^ permalink  raw  reply  [nested|flat] 16+ messages in thread

* [PATCH v5 2/3] expand binaryheap api
@ 2023-07-20 16:52 Nathan Bossart <[email protected]>
  0 siblings, 0 replies; 16+ messages in thread

From: Nathan Bossart @ 2023-07-20 16:52 UTC (permalink / raw)

---
 src/common/binaryheap.c      | 29 +++++++++++++++++++++++++++++
 src/include/lib/binaryheap.h |  3 +++
 2 files changed, 32 insertions(+)

diff --git a/src/common/binaryheap.c b/src/common/binaryheap.c
index a6be4b42ae..d24a61fd2b 100644
--- a/src/common/binaryheap.c
+++ b/src/common/binaryheap.c
@@ -211,6 +211,35 @@ binaryheap_remove_first(binaryheap *heap)
 	return result;
 }
 
+/*
+ * binaryheap_remove_node
+ *
+ * Removes the nth node from the heap.  The caller must ensure that there are
+ * at least (n - 1) nodes in the heap.  O(log n) worst case.
+ */
+void
+binaryheap_remove_node(binaryheap *heap, int n)
+{
+	int			cmp;
+
+	Assert(!binaryheap_empty(heap) && heap->bh_has_heap_property);
+	Assert(n >= 0 && n < heap->bh_size);
+
+	/* compare last node to the one that is being removed */
+	cmp = heap->bh_compare(heap->bh_nodes[--heap->bh_size],
+						   heap->bh_nodes[n],
+						   heap->bh_arg);
+
+	/* remove the last node, placing it in the vacated entry */
+	heap->bh_nodes[n] = heap->bh_nodes[heap->bh_size];
+
+	/* sift as needed to preserve the heap property */
+	if (cmp > 0)
+		sift_up(heap, n);
+	else if (cmp < 0)
+		sift_down(heap, n);
+}
+
 /*
  * binaryheap_replace_first
  *
diff --git a/src/include/lib/binaryheap.h b/src/include/lib/binaryheap.h
index ae0af68f0d..84c08d7ebb 100644
--- a/src/include/lib/binaryheap.h
+++ b/src/include/lib/binaryheap.h
@@ -56,8 +56,11 @@ extern void binaryheap_build(binaryheap *heap);
 extern void binaryheap_add(binaryheap *heap, bh_elem_type d);
 extern bh_elem_type binaryheap_first(binaryheap *heap);
 extern bh_elem_type binaryheap_remove_first(binaryheap *heap);
+extern void binaryheap_remove_node(binaryheap *heap, int n);
 extern void binaryheap_replace_first(binaryheap *heap, bh_elem_type d);
 
 #define binaryheap_empty(h)			((h)->bh_size == 0)
+#define binaryheap_size(h)			((h)->bh_size)
+#define binaryheap_get_node(h, n)	((h)->bh_nodes[n])
 
 #endif							/* BINARYHEAP_H */
-- 
2.25.1


--3MwIy2ne0vdjdPXF
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
	filename="v5-0003-use-priority-queue-for-pg_restore-ready_list.patch"



^ permalink  raw  reply  [nested|flat] 16+ messages in thread

* [PATCH v4 3/4] expand binaryheap api
@ 2023-07-20 16:52 Nathan Bossart <[email protected]>
  0 siblings, 0 replies; 16+ messages in thread

From: Nathan Bossart @ 2023-07-20 16:52 UTC (permalink / raw)

---
 src/common/binaryheap.c      | 29 +++++++++++++++++++++++++++++
 src/include/lib/binaryheap.h |  3 +++
 2 files changed, 32 insertions(+)

diff --git a/src/common/binaryheap.c b/src/common/binaryheap.c
index 400a730c85..aaf529d6a9 100644
--- a/src/common/binaryheap.c
+++ b/src/common/binaryheap.c
@@ -211,6 +211,35 @@ binaryheap_remove_first(binaryheap *heap)
 	return result;
 }
 
+/*
+ * binaryheap_remove_node
+ *
+ * Removes the nth node from the heap.  The caller must ensure that there are
+ * at least (n - 1) nodes in the heap.  O(log n) worst case.
+ */
+void
+binaryheap_remove_node(binaryheap *heap, int n)
+{
+	int			cmp;
+
+	Assert(!binaryheap_empty(heap) && heap->bh_has_heap_property);
+	Assert(n >= 0 && n < heap->bh_size);
+
+	/* compare last node to the one that is being removed */
+	cmp = heap->bh_compare(heap->bh_nodes[--heap->bh_size],
+						   heap->bh_nodes[n],
+						   heap->bh_arg);
+
+	/* remove the last node, placing it in the vacated entry */
+	heap->bh_nodes[n] = heap->bh_nodes[heap->bh_size];
+
+	/* sift as needed to preserve the heap property */
+	if (cmp > 0)
+		sift_up(heap, n);
+	else if (cmp < 0)
+		sift_down(heap, n);
+}
+
 /*
  * binaryheap_replace_first
  *
diff --git a/src/include/lib/binaryheap.h b/src/include/lib/binaryheap.h
index 71a25db0f4..06fe2ff213 100644
--- a/src/include/lib/binaryheap.h
+++ b/src/include/lib/binaryheap.h
@@ -47,8 +47,11 @@ extern void binaryheap_build(binaryheap *heap);
 extern void binaryheap_add(binaryheap *heap, void *d);
 extern void *binaryheap_first(binaryheap *heap);
 extern void *binaryheap_remove_first(binaryheap *heap);
+extern void binaryheap_remove_node(binaryheap *heap, int n);
 extern void binaryheap_replace_first(binaryheap *heap, void *d);
 
 #define binaryheap_empty(h)			((h)->bh_size == 0)
+#define binaryheap_size(h)			((h)->bh_size)
+#define binaryheap_get_node(h, n)	((h)->bh_nodes[n])
 
 #endif							/* BINARYHEAP_H */
-- 
2.25.1


--qMm9M+Fa2AknHoGS
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
	filename="v4-0004-use-priority-queue-for-pg_restore-ready_list.patch"



^ permalink  raw  reply  [nested|flat] 16+ messages in thread

* [PATCH v3 3/4] expand binaryheap api
@ 2023-07-20 16:52 Nathan Bossart <[email protected]>
  0 siblings, 0 replies; 16+ messages in thread

From: Nathan Bossart @ 2023-07-20 16:52 UTC (permalink / raw)

---
 src/common/binaryheap.c      | 29 +++++++++++++++++++++++++++++
 src/include/lib/binaryheap.h |  3 +++
 2 files changed, 32 insertions(+)

diff --git a/src/common/binaryheap.c b/src/common/binaryheap.c
index f21838b946..fdd8c0024a 100644
--- a/src/common/binaryheap.c
+++ b/src/common/binaryheap.c
@@ -211,6 +211,35 @@ binaryheap_remove_first(binaryheap *heap)
 	return result;
 }
 
+/*
+ * binaryheap_remove_node
+ *
+ * Removes the nth node from the heap.  The caller must ensure that there are
+ * at least (n - 1) nodes in the heap.  O(log n) worst case.
+ */
+void
+binaryheap_remove_node(binaryheap *heap, int n)
+{
+	int			cmp;
+
+	Assert(!binaryheap_empty(heap) && heap->bh_has_heap_property);
+	Assert(n >= 0 && n < heap->bh_size);
+
+	/* compare last node to the one that is being removed */
+	cmp = heap->bh_compare(heap->bh_nodes[--heap->bh_size],
+						   heap->bh_nodes[n],
+						   heap->bh_arg);
+
+	/* remove the last node, placing it in the vacated entry */
+	heap->bh_nodes[n] = heap->bh_nodes[heap->bh_size];
+
+	/* sift as needed to preserve the heap property */
+	if (cmp > 0)
+		sift_up(heap, n);
+	else if (cmp < 0)
+		sift_down(heap, n);
+}
+
 /*
  * binaryheap_replace_first
  *
diff --git a/src/include/lib/binaryheap.h b/src/include/lib/binaryheap.h
index 52f7b06b25..20a7f71244 100644
--- a/src/include/lib/binaryheap.h
+++ b/src/include/lib/binaryheap.h
@@ -47,8 +47,11 @@ extern void binaryheap_build(binaryheap *heap);
 extern void binaryheap_add(binaryheap *heap, Datum d);
 extern Datum binaryheap_first(binaryheap *heap);
 extern Datum binaryheap_remove_first(binaryheap *heap);
+extern void binaryheap_remove_node(binaryheap *heap, int n);
 extern void binaryheap_replace_first(binaryheap *heap, Datum d);
 
 #define binaryheap_empty(h)			((h)->bh_size == 0)
+#define binaryheap_size(h)			((h)->bh_size)
+#define binaryheap_get_node(h, n)	((h)->bh_nodes[n])
 
 #endif							/* BINARYHEAP_H */
-- 
2.25.1


--sdtB3X0nJg68CQEu
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
	filename="v3-0004-use-priority-queue-for-pg_restore-ready_list.patch"



^ permalink  raw  reply  [nested|flat] 16+ 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
@ 2026-01-29 16:23 Tom Lane <[email protected]>
  2026-02-02 05:50 ` 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]>
  2026-02-02 10:54 ` 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]>
  0 siblings, 2 replies; 16+ messages in thread

From: Tom Lane @ 2026-01-29 16:23 UTC (permalink / raw)
  To: Andrew Dunstan <[email protected]>; +Cc: Mahendra Singh Thalor <[email protected]>; Nathan Bossart <[email protected]>; Álvaro Herrera <[email protected]>; Srinath Reddy <[email protected]>; [email protected]

Andrew Dunstan <[email protected]> writes:
> These patches need a little copy editing (e.g. 
> "check_database_role_names_in_old_cluser" seems to be missing a "t") and 
> the error messages and comments need some tidying, but I think they are 
> basically sound.
> Is there any objection to them in principle?

+1 in principle.  As you say, there's some tidying needed.
A couple of points I noted:

1. check_lfcr_in_objname is about as unmusical a name as I can readily
imagine.  I was thinking about proposing "reject_newline_in_name"
instead, but really I would drop that subroutine altogether and just
code the checks in-line, because:

2. I don't think this approach to constructing the error message
meets our translatability guidelines.  Better to just write out
"role name \"%s\" contains ..." or "database name \"%s\" contains
...".  We do use the other approach in some cases where it saves
dozens of repetitive messages, but when there are only ever going
to be two I'd rather err on the side of translatability.

3. I do not like the tests added to 040_createuser.pl, as they
do not verify that the command fails for the expected reason.

4. There's no point in running check_database_role_names_in_old_cluser
against a v19 or later source server.

			regards, tom lane






^ permalink  raw  reply  [nested|flat] 16+ 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
  2026-01-29 16:23 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]>
@ 2026-02-02 05:50 ` Mahendra Singh Thalor <[email protected]>
  2026-02-02 11:16   ` 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]>
  1 sibling, 1 reply; 16+ messages in thread

From: Mahendra Singh Thalor @ 2026-02-02 05:50 UTC (permalink / raw)
  To: Tom Lane <[email protected]>; +Cc: Andrew Dunstan <[email protected]>; Nathan Bossart <[email protected]>; Álvaro Herrera <[email protected]>; Srinath Reddy <[email protected]>; [email protected]

Thanks Tom for the review.

On Thu, 29 Jan 2026 at 21:54, Tom Lane <[email protected]> wrote:
>
> Andrew Dunstan <[email protected]> writes:
> > These patches need a little copy editing (e.g.
> > "check_database_role_names_in_old_cluser" seems to be missing a "t") and
> > the error messages and comments need some tidying, but I think they are
> > basically sound.

Thanks Andrew for the review.
Fixed this typo.

> > Is there any objection to them in principle?
>
> +1 in principle.  As you say, there's some tidying needed.
> A couple of points I noted:
>
> 1. check_lfcr_in_objname is about as unmusical a name as I can readily
> imagine.  I was thinking about proposing "reject_newline_in_name"
> instead, but really I would drop that subroutine altogether and just
> code the checks in-line, because:

Fixed. As of now, I renamed it to reject_newline_in_name and changed
the error message as per suggestion but I kept this function as
previously suggested by some reviewers but I can remove this if this
looks odd.

>
> 2. I don't think this approach to constructing the error message
> meets our translatability guidelines.  Better to just write out
> "role name \"%s\" contains ..." or "database name \"%s\" contains
> ...".  We do use the other approach in some cases where it saves
> dozens of repetitive messages, but when there are only ever going
> to be two I'd rather err on the side of translatability.
>

Fixed.

> 3. I do not like the tests added to 040_createuser.pl, as they
> do not verify that the command fails for the expected reason.
>

Fixed. Added test case in .sql file to verify invalid names.

> 4. There's no point in running check_database_role_names_in_old_cluser
> against a v19 or later source server.

Fixed.

>
>                         regards, tom lane

Here, I am attaching updated patches for the review.

-- 
Thanks and Regards
Mahendra Singh Thalor
EnterpriseDB: http://www.enterprisedb.com


Attachments:

  [application/octet-stream] v08_0001_don-t-allow-newline-or-carriage-return-in-db-user-role-names.patch (8.5K, ../../CAKYtNAr0TcS4NWgduwdqTcApTs_RBaiRY+C+WAay3-xXjkDC_w@mail.gmail.com/2-v08_0001_don-t-allow-newline-or-carriage-return-in-db-user-role-names.patch)
  download | inline diff:
From b5091279568339213978adc42d98b1053c17b23d Mon Sep 17 00:00:00 2001
From: Mahendra Singh Thalor <[email protected]>
Date: Mon, 2 Feb 2026 10:52:29 +0530
Subject: [PATCH 1/2] don't allow newline or carriage return character in name 
 for  database/user/role

While creating database, if database name has any newline or carriage
return character in name, then through error becuase these special
character are not allowed in dbname when dump command is executed.

do same for "CREATE ROLE", "CREATE USER" also.

This will add check for:
"CREATE DATABASE", "CREATE ROLE", "CREATE USER",
"RENAME DATABASE/USER/ROLE"

-------------------------
As we will not allow these \n\r in names, then we will never get these
names in dump also.

If we are dumping from older branch, then we will fail with same old error.
(dump will fail in older branches so no need to add extra handling for dump.)

Also remove comment added by 142c24c23447f212e642a0ffac9af878b93f490d commit.

Remove one test of 8b845520fb0aa50fea7aae44a45cee1b6d87845d commit.
---
 src/backend/commands/dbcommands.c               |  4 ++++
 src/backend/commands/define.c                   | 17 +++++++++++++++++
 src/backend/commands/user.c                     |  4 ++++
 src/bin/pg_dump/t/010_dump_connstr.pl           | 14 --------------
 src/bin/scripts/t/020_createdb.pl               | 12 ++++++++++++
 src/fe_utils/string_utils.c                     |  6 ------
 src/include/commands/defrem.h                   |  1 +
 .../modules/unsafe_tests/expected/rolenames.out |  4 ++++
 src/test/modules/unsafe_tests/sql/rolenames.sql |  2 ++
 9 files changed, 44 insertions(+), 20 deletions(-)
 mode change 100644 => 100755 src/bin/pg_dump/t/010_dump_connstr.pl
 mode change 100644 => 100755 src/bin/scripts/t/020_createdb.pl

diff --git a/src/backend/commands/dbcommands.c b/src/backend/commands/dbcommands.c
index 87949054f26..a89604c3d52 100644
--- a/src/backend/commands/dbcommands.c
+++ b/src/backend/commands/dbcommands.c
@@ -742,6 +742,8 @@ createdb(ParseState *pstate, const CreatedbStmt *stmt)
 	CreateDBStrategy dbstrategy = CREATEDB_WAL_LOG;
 	createdb_failure_params fparms;
 
+	reject_newline_in_name(dbname, "database");
+
 	/* Extract options from the statement node tree */
 	foreach(option, stmt->options)
 	{
@@ -1910,6 +1912,8 @@ RenameDatabase(const char *oldname, const char *newname)
 	int			npreparedxacts;
 	ObjectAddress address;
 
+	reject_newline_in_name(newname, "database");
+
 	/*
 	 * Look up the target database's OID, and get exclusive lock on it. We
 	 * need this for the same reasons as DROP DATABASE.
diff --git a/src/backend/commands/define.c b/src/backend/commands/define.c
index 4172cc9bacb..bbd06ffd190 100644
--- a/src/backend/commands/define.c
+++ b/src/backend/commands/define.c
@@ -374,3 +374,20 @@ errorConflictingDefElem(DefElem *defel, ParseState *pstate)
 			errmsg("conflicting or redundant options"),
 			parser_errposition(pstate, defel->location));
 }
+
+/*
+ * reject_newline_in_name
+ *
+ * If name has a newline or carriage return character then error will be reported
+ * as these special characters are not allowed in names due to shell command error
+ * in dump.
+ */
+void
+reject_newline_in_name(const char *objname, const char *objtype)
+{
+	/* Report error if name has \n or \r character. */
+	if (strpbrk(objname, "\n\r"))
+		ereport(ERROR,
+				(errcode(ERRCODE_INVALID_PARAMETER_VALUE)),
+				errmsg("\"%s\" name \"%s\" contains a newline or carriage return character",objtype, objname));
+}
diff --git a/src/backend/commands/user.c b/src/backend/commands/user.c
index 8fb9ea25db0..877c8236fab 100644
--- a/src/backend/commands/user.c
+++ b/src/backend/commands/user.c
@@ -171,6 +171,8 @@ CreateRole(ParseState *pstate, CreateRoleStmt *stmt)
 	DefElem    *dbypassRLS = NULL;
 	GrantRoleOptions popt;
 
+	reject_newline_in_name(stmt->role, "role");
+
 	/* The defaults can vary depending on the original statement type */
 	switch (stmt->stmt_type)
 	{
@@ -1348,6 +1350,8 @@ RenameRole(const char *oldname, const char *newname)
 	ObjectAddress address;
 	Form_pg_authid authform;
 
+	reject_newline_in_name(newname, "role");
+
 	rel = table_open(AuthIdRelationId, RowExclusiveLock);
 	dsc = RelationGetDescr(rel);
 
diff --git a/src/bin/pg_dump/t/010_dump_connstr.pl b/src/bin/pg_dump/t/010_dump_connstr.pl
old mode 100644
new mode 100755
index dc7a33658db..bf2c3b6d00b
--- a/src/bin/pg_dump/t/010_dump_connstr.pl
+++ b/src/bin/pg_dump/t/010_dump_connstr.pl
@@ -153,20 +153,6 @@ $node->command_ok(
 	],
 	'pg_dumpall --dbname accepts connection string');
 
-$node->run_log(
-	[ 'createdb', '--username' => $src_bootstrap_super, "foo\n\rbar" ]);
-
-# not sufficient to use --roles-only here
-$node->command_fails(
-	[
-		'pg_dumpall', '--no-sync',
-		'--username' => $src_bootstrap_super,
-		'--file' => $discard,
-	],
-	'pg_dumpall with \n\r in database name');
-$node->run_log(
-	[ 'dropdb', '--username' => $src_bootstrap_super, "foo\n\rbar" ]);
-
 
 # make a table, so the parallel worker has something to dump
 $node->safe_psql(
diff --git a/src/bin/scripts/t/020_createdb.pl b/src/bin/scripts/t/020_createdb.pl
old mode 100644
new mode 100755
index 83b0077383a..18ff0254d59
--- a/src/bin/scripts/t/020_createdb.pl
+++ b/src/bin/scripts/t/020_createdb.pl
@@ -241,6 +241,18 @@ $node->command_fails(
 	],
 	'fails for invalid locale provider');
 
+$node->command_fails_like(
+    [ 'createdb', "invalid \n dbname" ],
+    qr(contains a newline or carriage return character),
+    'fails if database name containing newline character in name'
+);
+
+$node->command_fails_like(
+    [ 'createdb', "invalid \r dbname" ],
+    qr(contains a newline or carriage return character),,
+    'fails if database name containing carriage return character in name'
+);
+
 # Check use of templates with shared dependencies copied from the template.
 my ($ret, $stdout, $stderr) = $node->psql(
 	'foobar2',
diff --git a/src/fe_utils/string_utils.c b/src/fe_utils/string_utils.c
index 89ce396ed4f..38fffbd036b 100644
--- a/src/fe_utils/string_utils.c
+++ b/src/fe_utils/string_utils.c
@@ -568,12 +568,6 @@ appendByteaLiteral(PQExpBuffer buf, const unsigned char *str, size_t length,
  * Append the given string to the shell command being built in the buffer,
  * with shell-style quoting as needed to create exactly one argument.
  *
- * Forbid LF or CR characters, which have scant practical use beyond designing
- * security breaches.  The Windows command shell is unusable as a conduit for
- * arguments containing LF or CR characters.  A future major release should
- * reject those characters in CREATE ROLE and CREATE DATABASE, because use
- * there eventually leads to errors here.
- *
  * appendShellString() simply prints an error and dies if LF or CR appears.
  * appendShellStringNoError() omits those characters from the result, and
  * returns false if there were any.
diff --git a/src/include/commands/defrem.h b/src/include/commands/defrem.h
index 8f4a2d9bbc1..bd930b8905d 100644
--- a/src/include/commands/defrem.h
+++ b/src/include/commands/defrem.h
@@ -162,5 +162,6 @@ extern TypeName *defGetTypeName(DefElem *def);
 extern int	defGetTypeLength(DefElem *def);
 extern List *defGetStringList(DefElem *def);
 pg_noreturn extern void errorConflictingDefElem(DefElem *defel, ParseState *pstate);
+extern void reject_newline_in_name(const char *objname, const char *objtype);
 
 #endif							/* DEFREM_H */
diff --git a/src/test/modules/unsafe_tests/expected/rolenames.out b/src/test/modules/unsafe_tests/expected/rolenames.out
index 61396b2a805..71c1d935091 100644
--- a/src/test/modules/unsafe_tests/expected/rolenames.out
+++ b/src/test/modules/unsafe_tests/expected/rolenames.out
@@ -102,6 +102,10 @@ DETAIL:  Role names starting with "pg_" are reserved.
 CREATE ROLE "pg_abcdef"; -- error
 ERROR:  role name "pg_abcdef" is reserved
 DETAIL:  Role names starting with "pg_" are reserved.
+CREATE ROLE "invalid
+rolename"; -- error
+ERROR:  "role" name "invalid
+rolename" contains a newline or carriage return character
 CREATE ROLE regress_testrol0 SUPERUSER LOGIN;
 CREATE ROLE regress_testrolx SUPERUSER LOGIN;
 CREATE ROLE regress_testrol2 SUPERUSER;
diff --git a/src/test/modules/unsafe_tests/sql/rolenames.sql b/src/test/modules/unsafe_tests/sql/rolenames.sql
index adac36536db..70b342abeb6 100644
--- a/src/test/modules/unsafe_tests/sql/rolenames.sql
+++ b/src/test/modules/unsafe_tests/sql/rolenames.sql
@@ -75,6 +75,8 @@ CREATE ROLE pg_abc; -- error
 CREATE ROLE "pg_abc"; -- error
 CREATE ROLE pg_abcdef; -- error
 CREATE ROLE "pg_abcdef"; -- error
+CREATE ROLE "invalid
+rolename"; -- error
 
 CREATE ROLE regress_testrol0 SUPERUSER LOGIN;
 CREATE ROLE regress_testrolx SUPERUSER LOGIN;
-- 
2.52.0



  [application/octet-stream] v08_0002-add-handling-to-pg_upgrade-to-report-alert-for-invalid-names.patch (3.7K, ../../CAKYtNAr0TcS4NWgduwdqTcApTs_RBaiRY+C+WAay3-xXjkDC_w@mail.gmail.com/3-v08_0002-add-handling-to-pg_upgrade-to-report-alert-for-invalid-names.patch)
  download | inline diff:
From ccd70705f6468b9e726ab34b415442c15707b02c Mon Sep 17 00:00:00 2001
From: Mahendra Singh Thalor <[email protected]>
Date: Mon, 2 Feb 2026 11:00:29 +0530
Subject: [PATCH 2/2] add handling to pg_upgrade to report alert for invalid 
 database, user and role names.

If database/role/user name has any newline or carraige return character
in name, then pg_upgrade will report ALERT for these.
---
---
 src/bin/pg_upgrade/check.c | 78 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 78 insertions(+)

diff --git a/src/bin/pg_upgrade/check.c b/src/bin/pg_upgrade/check.c
index a8d20a92a98..ecfe45a1b67 100644
--- a/src/bin/pg_upgrade/check.c
+++ b/src/bin/pg_upgrade/check.c
@@ -34,6 +34,7 @@ static void check_new_cluster_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_role_names_in_old_cluster(ClusterInfo *cluster);
 
 /*
  * DataTypesUsageChecks - definitions of data type checks for the old cluster
@@ -600,6 +601,14 @@ check_and_dump_old_cluster(void)
 	 */
 	check_for_connection_status(&old_cluster);
 
+	/*
+	 * Validate database, user and role names from old cluser.  No need to
+	 * check in 19 or newver version as newline and carriage return are not
+	 * allowed at the creation time of object.
+	 */
+	if (GET_MAJOR_VERSION(old_cluster.major_version) < 1900)
+		check_database_role_names_in_old_cluster(&old_cluster);
+
 	/*
 	 * Extract a list of databases, tables, and logical replication slots from
 	 * the old cluster.
@@ -2500,3 +2509,72 @@ check_old_cluster_subscription_state(void)
 	else
 		check_ok();
 }
+
+/*
+ * check_database_role_names_in_old_cluster()
+ *
+ * If any database, user or role name has newline or carriage return character
+ * in name, then this will report those as these special characters are not
+ * allowed in these names from v19.
+ */
+static void
+check_database_role_names_in_old_cluster(ClusterInfo *cluster)
+{
+	int			i;
+	PGconn		*conn_template1;
+	PGresult	*res;
+	int			ntups;
+	FILE		*script = NULL;
+	char		output_path[MAXPGPATH];
+	int			count = 0;
+
+	prep_status("Checking names of databases and roles");
+
+	snprintf(output_path, sizeof(output_path), "%s/%s",
+			log_opts.basedir,
+			"db_role_invalid_names.txt");
+
+	conn_template1 = connectToServer(cluster, "template1");
+
+	/*
+	 * Get database, user/role names from cluster.  Can't use
+	 * pg_authid because only superusers can view it.
+	 */
+	res = executeQueryOrDie(conn_template1,
+			"SELECT datname AS objname, 'database' AS objtype "
+			"FROM pg_catalog.pg_database UNION ALL "
+			"SELECT rolname AS objname, 'role' AS objtype "
+			"FROM pg_catalog.pg_roles ORDER BY 2 ");
+
+	ntups = PQntuples(res);
+	for (i = 0; i < ntups; i++)
+	{
+		char	*objname = PQgetvalue(res, i, 0);
+		char	*objtype = PQgetvalue(res, i, 1);
+
+		/* If name has \n or \r, then report it. */
+		if (strpbrk(objname, "\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 : %s name = \"%s\"\n", ++count, objtype, objname);
+		}
+	}
+
+	PQclear(res);
+	PQfinish(conn_template1);
+
+	if (script)
+	{
+		fclose(script);
+		pg_log(PG_REPORT, "fatal");
+		pg_fatal("All the database and role names should have only valid characters. A newline or \n"
+				"carriage return character is not allowed in these object names.  To fix this, please \n"
+				"rename these names with valid names. \n"
+				"To see all %d invalid object names, refer db_role_invalid_names.txt file. \n"
+				"    %s", count, output_path);
+	}
+	else
+		check_ok();
+}
-- 
2.52.0



^ permalink  raw  reply  [nested|flat] 16+ 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
  2026-01-29 16:23 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]>
  2026-02-02 05:50 ` 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]>
@ 2026-02-02 11:16   ` Álvaro Herrera <[email protected]>
  2026-02-03 19:14     ` 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]>
  0 siblings, 1 reply; 16+ messages in thread

From: Álvaro Herrera @ 2026-02-02 11:16 UTC (permalink / raw)
  To: Mahendra Singh Thalor <[email protected]>; +Cc: Tom Lane <[email protected]>; Andrew Dunstan <[email protected]>; Nathan Bossart <[email protected]>; Srinath Reddy <[email protected]>; [email protected]


> +void
> +reject_newline_in_name(const char *objname, const char *objtype)
> +{
> +	/* Report error if name has \n or \r character. */
> +	if (strpbrk(objname, "\n\r"))
> +		ereport(ERROR,
> +				(errcode(ERRCODE_INVALID_PARAMETER_VALUE)),
> +				errmsg("\"%s\" name \"%s\" contains a newline or carriage return character",objtype, objname));
> +}

I think this error message doesn't work very well.  Having the
"database" word be an untranslatable piece of the message makes no sense
to me.  I would rather have the ereport() in each place where this is
needed so that we can have a complete phrase to translate, and avoid
this wrinkle.  Alternatively you could pass the error message from the
caller (to abstract away the strpbrk() call) but I'm not sure that's
really all that useful.

BTW, looking at generate_db in src/bin/pg_upgrade/t/002_pg_upgrade.pl I
wonder why don't we reject BEL here also.  (Or actually, maybe not, but
I think commit 322becb6085c was wrong to lose the part of the comment
that explained the reason.)

> --- a/src/bin/scripts/t/020_createdb.pl
> +++ b/src/bin/scripts/t/020_createdb.pl
> @@ -241,6 +241,18 @@ $node->command_fails(
>  	],
>  	'fails for invalid locale provider');
>  
> +$node->command_fails_like(
> +    [ 'createdb', "invalid \n dbname" ],
> +    qr(contains a newline or carriage return character),
> +    'fails if database name containing newline character in name'
> +);
> +
> +$node->command_fails_like(
> +    [ 'createdb', "invalid \r dbname" ],
> +    qr(contains a newline or carriage return character),,
> +    'fails if database name containing carriage return character in name'
> +);

Note there are two commas the the qr() line in the second stanza.  Seems
to be innocuous, because the test log shows

# Running: createdb invalid 
 dbname
[11:57:00.942](0.012s) ok 34 - fails if database name containing newline character in name: exit code not 0
[11:57:00.942](0.000s) ok 35 - fails if database name containing newline character in name: matches
# Running: createdb invalid ^M dbname
[11:57:00.953](0.011s) ok 36 - fails if database name containing carriage return character in name: exit code not 0
[11:57:00.954](0.000s) ok 37 - fails if database name containing carriage return character in name: matches

but it'd look nicer without those commas.  Also, the "fails if ...
containing" test names sound ungrammatical to me.

-- 
Álvaro Herrera        Breisgau, Deutschland  —  https://www.EnterpriseDB.com/
"Always assume the user will do much worse than the stupidest thing
you can imagine."                                (Julien PUYDT)






^ permalink  raw  reply  [nested|flat] 16+ 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
  2026-01-29 16:23 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]>
  2026-02-02 05:50 ` 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]>
  2026-02-02 11:16   ` 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]>
@ 2026-02-03 19:14     ` Mahendra Singh Thalor <[email protected]>
  2026-02-21 18:16       ` 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]>
  0 siblings, 1 reply; 16+ messages in thread

From: Mahendra Singh Thalor @ 2026-02-03 19:14 UTC (permalink / raw)
  To: Álvaro Herrera <[email protected]>; +Cc: Tom Lane <[email protected]>; Andrew Dunstan <[email protected]>; Nathan Bossart <[email protected]>; Srinath Reddy <[email protected]>; [email protected]

Thanks Álvaro, Tom and Nathan for the review.

On Mon, 2 Feb 2026 at 16:46, Álvaro Herrera <[email protected]> wrote:
>
>
> > +void
> > +reject_newline_in_name(const char *objname, const char *objtype)
> > +{
> > +     /* Report error if name has \n or \r character. */
> > +     if (strpbrk(objname, "\n\r"))
> > +             ereport(ERROR,
> > +                             (errcode(ERRCODE_INVALID_PARAMETER_VALUE)),
> > +                             errmsg("\"%s\" name \"%s\" contains a newline or carriage return character",objtype, objname));
> > +}
>
> I think this error message doesn't work very well.  Having the
> "database" word be an untranslatable piece of the message makes no sense
> to me.  I would rather have the ereport() in each place where this is
> needed so that we can have a complete phrase to translate, and avoid
> this wrinkle.  Alternatively you could pass the error message from the
> caller (to abstract away the strpbrk() call) but I'm not sure that's
> really all that useful.

Fixed.

>
> BTW, looking at generate_db in src/bin/pg_upgrade/t/002_pg_upgrade.pl I
> wonder why don't we reject BEL here also.  (Or actually, maybe not, but
> I think commit 322becb6085c was wrong to lose the part of the comment
> that explained the reason.)
>
> > --- a/src/bin/scripts/t/020_createdb.pl
> > +++ b/src/bin/scripts/t/020_createdb.pl
> > @@ -241,6 +241,18 @@ $node->command_fails(
> >       ],
> >       'fails for invalid locale provider');
> >
> > +$node->command_fails_like(
> > +    [ 'createdb', "invalid \n dbname" ],
> > +    qr(contains a newline or carriage return character),
> > +    'fails if database name containing newline character in name'
> > +);
> > +
> > +$node->command_fails_like(
> > +    [ 'createdb', "invalid \r dbname" ],
> > +    qr(contains a newline or carriage return character),,
> > +    'fails if database name containing carriage return character in name'
> > +);
>
> Note there are two commas the the qr() line in the second stanza.  Seems
> to be innocuous, because the test log shows

Fixed.

>
> # Running: createdb invalid
>  dbname
> [11:57:00.942](0.012s) ok 34 - fails if database name containing newline character in name: exit code not 0
> [11:57:00.942](0.000s) ok 35 - fails if database name containing newline character in name: matches
> # Running: createdb invalid ^M dbname
> [11:57:00.953](0.011s) ok 36 - fails if database name containing carriage return character in name: exit code not 0
> [11:57:00.954](0.000s) ok 37 - fails if database name containing carriage return character in name: matches
>
> but it'd look nicer without those commas.  Also, the "fails if ...
> containing" test names sound ungrammatical to me.

Fixed.

>
> --
> Álvaro Herrera        Breisgau, Deutschland  —  https://www.EnterpriseDB.com/
> "Always assume the user will do much worse than the stupidest thing
> you can imagine."                                (Julien PUYDT)

Based on suggestions, I removed the new added function and changed the
error message and added a check for tablespace also.

Here, I am attaching updated patches.

-- 
Thanks and Regards
Mahendra Singh Thalor
EnterpriseDB: http://www.enterprisedb.com


Attachments:

  [text/x-patch] v09_0001_don-t-allow-newline-or-carriage-return-in-db-user-role-names.patch (12.9K, ../../CAKYtNApHc3PMoU3ZkUYmEt_tqKAx+e+5n=Gzk6c=SwcYdqRsAw@mail.gmail.com/2-v09_0001_don-t-allow-newline-or-carriage-return-in-db-user-role-names.patch)
  download | inline diff:
From 29c2da33a521094d9969d2cacd3ef4152f5da933 Mon Sep 17 00:00:00 2001
From: Mahendra Singh Thalor <[email protected]>
Date: Wed, 4 Feb 2026 00:12:48 +0530
Subject: [PATCH 1/2] don't allow newline or carriage return character in name 
 for  database/user/role/tablespace

While creating database, if database name has any newline or carriage
return character in name, then through error becuase these special
character are not allowed in dbname when dump command is executed.

do same for "CREATE ROLE", "CREATE USER" also.

This will add check for:
"CREATE DATABASE", "CREATE ROLE", "CREATE USER", "CREATE TABLESPACE"
"RENAME DATABASE/USER/ROLE/TABLESPACE"

-------------------------
As we will not allow these \n\r in names, then we will never get these
names in dump also.

If we are dumping from older branch, then we will fail with same old error.
(dump will fail in older branches so no need to add extra handling for dump.)

Also remove comment added by 142c24c23447f212e642a0ffac9af878b93f490d commit.

Remove one test of 8b845520fb0aa50fea7aae44a45cee1b6d87845d commit.
---
 src/backend/commands/dbcommands.c                  | 12 ++++++++++++
 src/backend/commands/tablespace.c                  | 12 ++++++++++++
 src/backend/commands/user.c                        | 12 ++++++++++++
 src/bin/pg_dump/t/010_dump_connstr.pl              | 14 --------------
 src/bin/scripts/t/020_createdb.pl                  | 12 ++++++++++++
 src/fe_utils/string_utils.c                        |  6 ------
 src/include/commands/defrem.h                      |  1 +
 .../unsafe_tests/expected/alter_system_table.out   |  5 +++++
 .../modules/unsafe_tests/expected/rolenames.out    |  4 ++++
 .../unsafe_tests/sql/alter_system_table.sql        |  4 ++++
 src/test/modules/unsafe_tests/sql/rolenames.sql    |  2 ++
 src/test/regress/expected/tablespace.out           |  5 +++++
 src/test/regress/sql/tablespace.sql                |  4 ++++
 13 files changed, 73 insertions(+), 20 deletions(-)
 mode change 100644 => 100755 src/bin/pg_dump/t/010_dump_connstr.pl
 mode change 100644 => 100755 src/bin/scripts/t/020_createdb.pl

diff --git a/src/backend/commands/dbcommands.c b/src/backend/commands/dbcommands.c
index 87949054f26..6396d3e7caf 100644
--- a/src/backend/commands/dbcommands.c
+++ b/src/backend/commands/dbcommands.c
@@ -742,6 +742,12 @@ createdb(ParseState *pstate, const CreatedbStmt *stmt)
 	CreateDBStrategy dbstrategy = CREATEDB_WAL_LOG;
 	createdb_failure_params fparms;
 
+	/* Report error if name has \n or \r character. */
+	if (strpbrk(dbname, "\n\r"))
+		ereport(ERROR,
+				(errcode(ERRCODE_INVALID_PARAMETER_VALUE)),
+				errmsg("database name \"%s\" contains a newline or carriage return character",dbname));
+
 	/* Extract options from the statement node tree */
 	foreach(option, stmt->options)
 	{
@@ -1910,6 +1916,12 @@ RenameDatabase(const char *oldname, const char *newname)
 	int			npreparedxacts;
 	ObjectAddress address;
 
+	/* Report error if name has \n or \r character. */
+	if (strpbrk(newname, "\n\r"))
+		ereport(ERROR,
+				(errcode(ERRCODE_INVALID_PARAMETER_VALUE)),
+				errmsg("database name \"%s\" contains a newline or carriage return character",newname));
+
 	/*
 	 * Look up the target database's OID, and get exclusive lock on it. We
 	 * need this for the same reasons as DROP DATABASE.
diff --git a/src/backend/commands/tablespace.c b/src/backend/commands/tablespace.c
index 0b064891932..f6dba30da55 100644
--- a/src/backend/commands/tablespace.c
+++ b/src/backend/commands/tablespace.c
@@ -241,6 +241,12 @@ CreateTableSpace(CreateTableSpaceStmt *stmt)
 				(errcode(ERRCODE_INVALID_NAME),
 				 errmsg("tablespace location cannot contain single quotes")));
 
+	/* Report error if name has \n or \r character. */
+	if (strpbrk(stmt->tablespacename, "\n\r"))
+		ereport(ERROR,
+				(errcode(ERRCODE_INVALID_PARAMETER_VALUE)),
+				errmsg("tablespace name \"%s\" contains a newline or carriage return character",stmt->tablespacename));
+
 	in_place = allow_in_place_tablespaces && strlen(location) == 0;
 
 	/*
@@ -970,6 +976,12 @@ RenameTableSpace(const char *oldname, const char *newname)
 				 errmsg("unacceptable tablespace name \"%s\"", newname),
 				 errdetail("The prefix \"pg_\" is reserved for system tablespaces.")));
 
+	/* Report error if name has \n or \r character. */
+	if (strpbrk(newname, "\n\r"))
+		ereport(ERROR,
+				(errcode(ERRCODE_INVALID_PARAMETER_VALUE)),
+				errmsg("tablespace name \"%s\" contains a newline or carriage return character",newname));
+
 	/*
 	 * If built with appropriate switch, whine when regression-testing
 	 * conventions for tablespace names are violated.
diff --git a/src/backend/commands/user.c b/src/backend/commands/user.c
index 8fb9ea25db0..5b9d6634e87 100644
--- a/src/backend/commands/user.c
+++ b/src/backend/commands/user.c
@@ -171,6 +171,12 @@ CreateRole(ParseState *pstate, CreateRoleStmt *stmt)
 	DefElem    *dbypassRLS = NULL;
 	GrantRoleOptions popt;
 
+	/* Report error if name has \n or \r character. */
+	if (strpbrk(stmt->role, "\n\r"))
+		ereport(ERROR,
+				(errcode(ERRCODE_INVALID_PARAMETER_VALUE)),
+				errmsg("role name \"%s\" contains a newline or carriage return character",stmt->role));
+
 	/* The defaults can vary depending on the original statement type */
 	switch (stmt->stmt_type)
 	{
@@ -1348,6 +1354,12 @@ RenameRole(const char *oldname, const char *newname)
 	ObjectAddress address;
 	Form_pg_authid authform;
 
+	/* Report error if name has \n or \r character. */
+	if (strpbrk(newname, "\n\r"))
+		ereport(ERROR,
+				(errcode(ERRCODE_INVALID_PARAMETER_VALUE)),
+				errmsg("role name \"%s\" contains a newline or carriage return character",newname));
+
 	rel = table_open(AuthIdRelationId, RowExclusiveLock);
 	dsc = RelationGetDescr(rel);
 
diff --git a/src/bin/pg_dump/t/010_dump_connstr.pl b/src/bin/pg_dump/t/010_dump_connstr.pl
old mode 100644
new mode 100755
index dc7a33658db..bf2c3b6d00b
--- a/src/bin/pg_dump/t/010_dump_connstr.pl
+++ b/src/bin/pg_dump/t/010_dump_connstr.pl
@@ -153,20 +153,6 @@ $node->command_ok(
 	],
 	'pg_dumpall --dbname accepts connection string');
 
-$node->run_log(
-	[ 'createdb', '--username' => $src_bootstrap_super, "foo\n\rbar" ]);
-
-# not sufficient to use --roles-only here
-$node->command_fails(
-	[
-		'pg_dumpall', '--no-sync',
-		'--username' => $src_bootstrap_super,
-		'--file' => $discard,
-	],
-	'pg_dumpall with \n\r in database name');
-$node->run_log(
-	[ 'dropdb', '--username' => $src_bootstrap_super, "foo\n\rbar" ]);
-
 
 # make a table, so the parallel worker has something to dump
 $node->safe_psql(
diff --git a/src/bin/scripts/t/020_createdb.pl b/src/bin/scripts/t/020_createdb.pl
old mode 100644
new mode 100755
index 83b0077383a..a0995868363
--- a/src/bin/scripts/t/020_createdb.pl
+++ b/src/bin/scripts/t/020_createdb.pl
@@ -241,6 +241,18 @@ $node->command_fails(
 	],
 	'fails for invalid locale provider');
 
+$node->command_fails_like(
+    [ 'createdb', "invalid \n dbname" ],
+    qr(contains a newline or carriage return character),
+    'fails if database name contains a newline character in name'
+);
+
+$node->command_fails_like(
+    [ 'createdb', "invalid \r dbname" ],
+    qr(contains a newline or carriage return character),
+    'fails if database name contains a carriage return character in name'
+);
+
 # Check use of templates with shared dependencies copied from the template.
 my ($ret, $stdout, $stderr) = $node->psql(
 	'foobar2',
diff --git a/src/fe_utils/string_utils.c b/src/fe_utils/string_utils.c
index 89ce396ed4f..38fffbd036b 100644
--- a/src/fe_utils/string_utils.c
+++ b/src/fe_utils/string_utils.c
@@ -568,12 +568,6 @@ appendByteaLiteral(PQExpBuffer buf, const unsigned char *str, size_t length,
  * Append the given string to the shell command being built in the buffer,
  * with shell-style quoting as needed to create exactly one argument.
  *
- * Forbid LF or CR characters, which have scant practical use beyond designing
- * security breaches.  The Windows command shell is unusable as a conduit for
- * arguments containing LF or CR characters.  A future major release should
- * reject those characters in CREATE ROLE and CREATE DATABASE, because use
- * there eventually leads to errors here.
- *
  * appendShellString() simply prints an error and dies if LF or CR appears.
  * appendShellStringNoError() omits those characters from the result, and
  * returns false if there were any.
diff --git a/src/include/commands/defrem.h b/src/include/commands/defrem.h
index 8f4a2d9bbc1..bd930b8905d 100644
--- a/src/include/commands/defrem.h
+++ b/src/include/commands/defrem.h
@@ -162,5 +162,6 @@ extern TypeName *defGetTypeName(DefElem *def);
 extern int	defGetTypeLength(DefElem *def);
 extern List *defGetStringList(DefElem *def);
 pg_noreturn extern void errorConflictingDefElem(DefElem *defel, ParseState *pstate);
+extern void reject_newline_in_name(const char *objname, const char *objtype);
 
 #endif							/* DEFREM_H */
diff --git a/src/test/modules/unsafe_tests/expected/alter_system_table.out b/src/test/modules/unsafe_tests/expected/alter_system_table.out
index b73b9442b8d..39c90ce30a7 100644
--- a/src/test/modules/unsafe_tests/expected/alter_system_table.out
+++ b/src/test/modules/unsafe_tests/expected/alter_system_table.out
@@ -64,6 +64,11 @@ ERROR:  permission denied: "pg_description" is a system catalog
 CREATE TABLESPACE pg_foo LOCATION '/no/such/location';
 ERROR:  unacceptable tablespace name "pg_foo"
 DETAIL:  The prefix "pg_" is reserved for system tablespaces.
+-- contains \n\r tablespace name
+CREATE TABLESPACE "inavlid
+name" LOCATION '/no/such/location';
+ERROR:  tablespace name "inavlid
+name" contains a newline or carriage return character
 -- triggers
 CREATE FUNCTION tf1() RETURNS trigger
 LANGUAGE plpgsql
diff --git a/src/test/modules/unsafe_tests/expected/rolenames.out b/src/test/modules/unsafe_tests/expected/rolenames.out
index 61396b2a805..9dfb8475e16 100644
--- a/src/test/modules/unsafe_tests/expected/rolenames.out
+++ b/src/test/modules/unsafe_tests/expected/rolenames.out
@@ -102,6 +102,10 @@ DETAIL:  Role names starting with "pg_" are reserved.
 CREATE ROLE "pg_abcdef"; -- error
 ERROR:  role name "pg_abcdef" is reserved
 DETAIL:  Role names starting with "pg_" are reserved.
+CREATE ROLE "invalid
+rolename"; -- error
+ERROR:  role name "invalid
+rolename" contains a newline or carriage return character
 CREATE ROLE regress_testrol0 SUPERUSER LOGIN;
 CREATE ROLE regress_testrolx SUPERUSER LOGIN;
 CREATE ROLE regress_testrol2 SUPERUSER;
diff --git a/src/test/modules/unsafe_tests/sql/alter_system_table.sql b/src/test/modules/unsafe_tests/sql/alter_system_table.sql
index c1515100845..9812115a6f0 100644
--- a/src/test/modules/unsafe_tests/sql/alter_system_table.sql
+++ b/src/test/modules/unsafe_tests/sql/alter_system_table.sql
@@ -64,6 +64,10 @@ ALTER TABLE pg_description SET SCHEMA public;
 -- reserved tablespace name
 CREATE TABLESPACE pg_foo LOCATION '/no/such/location';
 
+-- contains \n\r tablespace name
+CREATE TABLESPACE "inavlid
+name" LOCATION '/no/such/location';
+
 -- triggers
 CREATE FUNCTION tf1() RETURNS trigger
 LANGUAGE plpgsql
diff --git a/src/test/modules/unsafe_tests/sql/rolenames.sql b/src/test/modules/unsafe_tests/sql/rolenames.sql
index adac36536db..70b342abeb6 100644
--- a/src/test/modules/unsafe_tests/sql/rolenames.sql
+++ b/src/test/modules/unsafe_tests/sql/rolenames.sql
@@ -75,6 +75,8 @@ CREATE ROLE pg_abc; -- error
 CREATE ROLE "pg_abc"; -- error
 CREATE ROLE pg_abcdef; -- error
 CREATE ROLE "pg_abcdef"; -- error
+CREATE ROLE "invalid
+rolename"; -- error
 
 CREATE ROLE regress_testrol0 SUPERUSER LOGIN;
 CREATE ROLE regress_testrolx SUPERUSER LOGIN;
diff --git a/src/test/regress/expected/tablespace.out b/src/test/regress/expected/tablespace.out
index a90e39e5738..185880a3217 100644
--- a/src/test/regress/expected/tablespace.out
+++ b/src/test/regress/expected/tablespace.out
@@ -958,6 +958,11 @@ ALTER TABLE ALL IN TABLESPACE regress_tblspace_renamed SET TABLESPACE pg_default
 NOTICE:  no matching relations in tablespace "regress_tblspace_renamed" found
 ALTER MATERIALIZED VIEW ALL IN TABLESPACE regress_tblspace_renamed SET TABLESPACE pg_default;
 NOTICE:  no matching relations in tablespace "regress_tblspace_renamed" found
+-- Should fail, contains \n in name
+ALTER TABLESPACE regress_tblspace_renamed RENAME TO "invalid
+name";
+ERROR:  tablespace name "invalid
+name" contains a newline or carriage return character
 -- Should succeed
 DROP TABLESPACE regress_tblspace_renamed;
 DROP SCHEMA testschema CASCADE;
diff --git a/src/test/regress/sql/tablespace.sql b/src/test/regress/sql/tablespace.sql
index dfe3db096e2..c43a59e5957 100644
--- a/src/test/regress/sql/tablespace.sql
+++ b/src/test/regress/sql/tablespace.sql
@@ -430,6 +430,10 @@ ALTER MATERIALIZED VIEW ALL IN TABLESPACE regress_tblspace_renamed SET TABLESPAC
 ALTER TABLE ALL IN TABLESPACE regress_tblspace_renamed SET TABLESPACE pg_default;
 ALTER MATERIALIZED VIEW ALL IN TABLESPACE regress_tblspace_renamed SET TABLESPACE pg_default;
 
+-- Should fail, contains \n in name
+ALTER TABLESPACE regress_tblspace_renamed RENAME TO "invalid
+name";
+
 -- Should succeed
 DROP TABLESPACE regress_tblspace_renamed;
 
-- 
2.52.0



  [text/x-patch] v09_0002-add-handling-to-pg_upgrade-to-report-alert-for-invalid-names.patch (3.9K, ../../CAKYtNApHc3PMoU3ZkUYmEt_tqKAx+e+5n=Gzk6c=SwcYdqRsAw@mail.gmail.com/3-v09_0002-add-handling-to-pg_upgrade-to-report-alert-for-invalid-names.patch)
  download | inline diff:
From bb9c366c2cbba4e3916366619d83ed74e01c9a6a Mon Sep 17 00:00:00 2001
From: Mahendra Singh Thalor <[email protected]>
Date: Wed, 4 Feb 2026 00:36:58 +0530
Subject: [PATCH 2/2] add handling to pg_upgrade to report alert for invalid 
 database, user, role and tablespace names.

If database/role/user/tablespace name has any newline or carraige return character
in name, then pg_upgrade will report ALERT for these.
---
---
---
 src/bin/pg_upgrade/check.c | 80 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 80 insertions(+)

diff --git a/src/bin/pg_upgrade/check.c b/src/bin/pg_upgrade/check.c
index a8d20a92a98..be3e574459c 100644
--- a/src/bin/pg_upgrade/check.c
+++ b/src/bin/pg_upgrade/check.c
@@ -34,6 +34,7 @@ static void check_new_cluster_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_db_role_tablespace_names_in_old_cluster(ClusterInfo *cluster);
 
 /*
  * DataTypesUsageChecks - definitions of data type checks for the old cluster
@@ -600,6 +601,14 @@ check_and_dump_old_cluster(void)
 	 */
 	check_for_connection_status(&old_cluster);
 
+	/*
+	 * Validate database, user, role and tablespace names from old cluser.
+	 * No need to check in 19 or newver version as newline and carriage
+	 * return are not allowed at the creation time of object.
+	 */
+	if (GET_MAJOR_VERSION(old_cluster.major_version) < 1900)
+		check_db_role_tablespace_names_in_old_cluster(&old_cluster);
+
 	/*
 	 * Extract a list of databases, tables, and logical replication slots from
 	 * the old cluster.
@@ -2500,3 +2509,74 @@ check_old_cluster_subscription_state(void)
 	else
 		check_ok();
 }
+
+/*
+ * check_db_role_tablespace_names_in_old_cluster()
+ *
+ * If any database, user, role or tablespace name has newline or carriage return
+ * character in name, then this will report those as these special characters
+ * are not allowed in these names from v19.
+ */
+static void
+check_db_role_tablespace_names_in_old_cluster(ClusterInfo *cluster)
+{
+	int			i;
+	PGconn		*conn_template1;
+	PGresult	*res;
+	int			ntups;
+	FILE		*script = NULL;
+	char		output_path[MAXPGPATH];
+	int			count = 0;
+
+	prep_status("Checking names of databases, roles and tablespace");
+
+	snprintf(output_path, sizeof(output_path), "%s/%s",
+			log_opts.basedir,
+			"db_role_tablespace_invalid_names.txt");
+
+	conn_template1 = connectToServer(cluster, "template1");
+
+	/*
+	 * Get database, user/role and tablespacenames from cluster.  Can't use
+	 * pg_authid because only superusers can view it.
+	 */
+	res = executeQueryOrDie(conn_template1,
+			"SELECT datname AS objname, 'database' AS objtype "
+			"FROM pg_catalog.pg_database UNION ALL "
+			"SELECT rolname AS objname, 'role' AS objtype "
+			"FROM pg_catalog.pg_roles UNION ALL "
+			"SELECT spcname AS objname, 'tablespace' AS objtype "
+			"FROM pg_catalog.pg_tablespace ORDER BY 2 ");
+
+	ntups = PQntuples(res);
+	for (i = 0; i < ntups; i++)
+	{
+		char	*objname = PQgetvalue(res, i, 0);
+		char	*objtype = PQgetvalue(res, i, 1);
+
+		/* If name has \n or \r, then report it. */
+		if (strpbrk(objname, "\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 : %s name = \"%s\"\n", ++count, objtype, objname);
+		}
+	}
+
+	PQclear(res);
+	PQfinish(conn_template1);
+
+	if (script)
+	{
+		fclose(script);
+		pg_log(PG_REPORT, "fatal");
+		pg_fatal("All the database, role and tablespace names should have only valid characters. A newline or \n"
+				"carriage return character is not allowed in these object names.  To fix this, please \n"
+				"rename these names with valid names. \n"
+				"To see all %d invalid object names, refer db_role_tablespace_invalid_names.txt file. \n"
+				"    %s", count, output_path);
+	}
+	else
+		check_ok();
+}
-- 
2.52.0



^ permalink  raw  reply  [nested|flat] 16+ 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
  2026-01-29 16:23 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]>
  2026-02-02 05:50 ` 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]>
  2026-02-02 11:16   ` 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]>
  2026-02-03 19:14     ` 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]>
@ 2026-02-21 18:16       ` Andrew Dunstan <[email protected]>
  2026-02-23 21:03         ` 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]>
  0 siblings, 1 reply; 16+ messages in thread

From: Andrew Dunstan @ 2026-02-21 18:16 UTC (permalink / raw)
  To: Mahendra Singh Thalor <[email protected]>; Álvaro Herrera <[email protected]>; +Cc: Tom Lane <[email protected]>; Nathan Bossart <[email protected]>; Srinath Reddy <[email protected]>; [email protected]


On 2026-02-03 Tu 2:14 PM, Mahendra Singh Thalor wrote:
> Thanks Álvaro, Tom and Nathan for the review.
>
> On Mon, 2 Feb 2026 at 16:46, Álvaro Herrera <[email protected]> wrote:
>>
>>> +void
>>> +reject_newline_in_name(const char *objname, const char *objtype)
>>> +{
>>> +     /* Report error if name has \n or \r character. */
>>> +     if (strpbrk(objname, "\n\r"))
>>> +             ereport(ERROR,
>>> +                             (errcode(ERRCODE_INVALID_PARAMETER_VALUE)),
>>> +                             errmsg("\"%s\" name \"%s\" contains a newline or carriage return character",objtype, objname));
>>> +}
>> I think this error message doesn't work very well.  Having the
>> "database" word be an untranslatable piece of the message makes no sense
>> to me.  I would rather have the ereport() in each place where this is
>> needed so that we can have a complete phrase to translate, and avoid
>> this wrinkle.  Alternatively you could pass the error message from the
>> caller (to abstract away the strpbrk() call) but I'm not sure that's
>> really all that useful.
> Fixed.
>
>> BTW, looking at generate_db in src/bin/pg_upgrade/t/002_pg_upgrade.pl I
>> wonder why don't we reject BEL here also.  (Or actually, maybe not, but
>> I think commit 322becb6085c was wrong to lose the part of the comment
>> that explained the reason.)
>>
>>> --- a/src/bin/scripts/t/020_createdb.pl
>>> +++ b/src/bin/scripts/t/020_createdb.pl
>>> @@ -241,6 +241,18 @@ $node->command_fails(
>>>        ],
>>>        'fails for invalid locale provider');
>>>
>>> +$node->command_fails_like(
>>> +    [ 'createdb', "invalid \n dbname" ],
>>> +    qr(contains a newline or carriage return character),
>>> +    'fails if database name containing newline character in name'
>>> +);
>>> +
>>> +$node->command_fails_like(
>>> +    [ 'createdb', "invalid \r dbname" ],
>>> +    qr(contains a newline or carriage return character),,
>>> +    'fails if database name containing carriage return character in name'
>>> +);
>> Note there are two commas the the qr() line in the second stanza.  Seems
>> to be innocuous, because the test log shows
> Fixed.
>
>> # Running: createdb invalid
>>   dbname
>> [11:57:00.942](0.012s) ok 34 - fails if database name containing newline character in name: exit code not 0
>> [11:57:00.942](0.000s) ok 35 - fails if database name containing newline character in name: matches
>> # Running: createdb invalid ^M dbname
>> [11:57:00.953](0.011s) ok 36 - fails if database name containing carriage return character in name: exit code not 0
>> [11:57:00.954](0.000s) ok 37 - fails if database name containing carriage return character in name: matches
>>
>> but it'd look nicer without those commas.  Also, the "fails if ...
>> containing" test names sound ungrammatical to me.
> Fixed.
>
>> --
>> Álvaro Herrera        Breisgau, Deutschland  —  https://www.EnterpriseDB.com/
>> "Always assume the user will do much worse than the stupidest thing
>> you can imagine."                                (Julien PUYDT)
> Based on suggestions, I removed the new added function and changed the
> error message and added a check for tablespace also.
>
> Here, I am attaching updated patches.


OK, I have squashed these two together and tweaked them a bit. I propose 
to commit this patch fairly soon.


cheers


andrew


--
Andrew Dunstan
EDB: https://www.enterprisedb.com


Attachments:

  [text/x-patch] v10-ban-crlf-in-global-names.patch (15.4K, ../../[email protected]/2-v10-ban-crlf-in-global-names.patch)
  download | inline diff:
diff --git a/src/backend/commands/dbcommands.c b/src/backend/commands/dbcommands.c
index 33311760df7..740c526e92d 100644
--- a/src/backend/commands/dbcommands.c
+++ b/src/backend/commands/dbcommands.c
@@ -743,6 +743,12 @@ createdb(ParseState *pstate, const CreatedbStmt *stmt)
 	CreateDBStrategy dbstrategy = CREATEDB_WAL_LOG;
 	createdb_failure_params fparms;
 
+	/* Report error if name has \n or \r character. */
+	if (strpbrk(dbname, "\n\r"))
+		ereport(ERROR,
+				(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+				 errmsg("database name \"%s\" contains a newline or carriage return character", dbname)));
+
 	/* Extract options from the statement node tree */
 	foreach(option, stmt->options)
 	{
@@ -1911,6 +1917,12 @@ RenameDatabase(const char *oldname, const char *newname)
 	int			npreparedxacts;
 	ObjectAddress address;
 
+	/* Report error if name has \n or \r character. */
+	if (strpbrk(newname, "\n\r"))
+		ereport(ERROR,
+				(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+				 errmsg("database name \"%s\" contains a newline or carriage return character", newname)));
+
 	/*
 	 * Look up the target database's OID, and get exclusive lock on it. We
 	 * need this for the same reasons as DROP DATABASE.
diff --git a/src/backend/commands/tablespace.c b/src/backend/commands/tablespace.c
index 3511a4ec0fd..ed2a93a09db 100644
--- a/src/backend/commands/tablespace.c
+++ b/src/backend/commands/tablespace.c
@@ -242,6 +242,12 @@ CreateTableSpace(CreateTableSpaceStmt *stmt)
 				(errcode(ERRCODE_INVALID_NAME),
 				 errmsg("tablespace location cannot contain single quotes")));
 
+	/* Report error if name has \n or \r character. */
+	if (strpbrk(stmt->tablespacename, "\n\r"))
+		ereport(ERROR,
+				(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+				 errmsg("tablespace name \"%s\" contains a newline or carriage return character", stmt->tablespacename)));
+
 	in_place = allow_in_place_tablespaces && strlen(location) == 0;
 
 	/*
@@ -971,6 +977,12 @@ RenameTableSpace(const char *oldname, const char *newname)
 				 errmsg("unacceptable tablespace name \"%s\"", newname),
 				 errdetail("The prefix \"pg_\" is reserved for system tablespaces.")));
 
+	/* Report error if name has \n or \r character. */
+	if (strpbrk(newname, "\n\r"))
+		ereport(ERROR,
+				(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+				 errmsg("tablespace name \"%s\" contains a newline or carriage return character", newname)));
+
 	/*
 	 * If built with appropriate switch, whine when regression-testing
 	 * conventions for tablespace names are violated.
diff --git a/src/backend/commands/user.c b/src/backend/commands/user.c
index 8fb9ea25db0..be11c49f919 100644
--- a/src/backend/commands/user.c
+++ b/src/backend/commands/user.c
@@ -171,6 +171,12 @@ CreateRole(ParseState *pstate, CreateRoleStmt *stmt)
 	DefElem    *dbypassRLS = NULL;
 	GrantRoleOptions popt;
 
+	/* Report error if name has \n or \r character. */
+	if (strpbrk(stmt->role, "\n\r"))
+		ereport(ERROR,
+				(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+				 errmsg("role name \"%s\" contains a newline or carriage return character", stmt->role)));
+
 	/* The defaults can vary depending on the original statement type */
 	switch (stmt->stmt_type)
 	{
@@ -1348,6 +1354,12 @@ RenameRole(const char *oldname, const char *newname)
 	ObjectAddress address;
 	Form_pg_authid authform;
 
+	/* Report error if name has \n or \r character. */
+	if (strpbrk(newname, "\n\r"))
+		ereport(ERROR,
+				(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+				 errmsg("role name \"%s\" contains a newline or carriage return character", newname)));
+
 	rel = table_open(AuthIdRelationId, RowExclusiveLock);
 	dsc = RelationGetDescr(rel);
 
diff --git a/src/bin/pg_dump/t/002_pg_dump.pl b/src/bin/pg_dump/t/002_pg_dump.pl
index a8dcc2b5c75..bc7a082f57a 100644
--- a/src/bin/pg_dump/t/002_pg_dump.pl
+++ b/src/bin/pg_dump/t/002_pg_dump.pl
@@ -2048,13 +2048,8 @@ my %tests = (
 		},
 	},
 
-	'newline of role or table name in comment' => {
-		create_sql => qq{CREATE ROLE regress_newline;
-						 ALTER ROLE regress_newline SET enable_seqscan = off;
-						 ALTER ROLE regress_newline
-							RENAME TO "regress_newline\nattack";
-
-						 -- meet getPartitioningInfo() "unsafe" condition
+	'newline of table name in comment' => {
+		create_sql => qq{-- meet getPartitioningInfo() "unsafe" condition
 						 CREATE TYPE pp_colors AS
 							ENUM ('green', 'blue', 'black');
 						 CREATE TABLE pp_enumpart (a pp_colors)
diff --git a/src/bin/pg_dump/t/003_pg_dump_with_server.pl b/src/bin/pg_dump/t/003_pg_dump_with_server.pl
index 70f830e10dc..349add67d50 100644
--- a/src/bin/pg_dump/t/003_pg_dump_with_server.pl
+++ b/src/bin/pg_dump/t/003_pg_dump_with_server.pl
@@ -16,22 +16,6 @@ my $port = $node->port;
 $node->init;
 $node->start;
 
-#########################################
-# pg_dumpall: newline in database name
-
-$node->safe_psql('postgres', qq{CREATE DATABASE "regress_\nattack"});
-
-my (@cmd, $stdout, $stderr);
-@cmd = ("pg_dumpall", '--port' => $port, '--exclude-database=postgres');
-print("# Running: " . join(" ", @cmd) . "\n");
-my $result = IPC::Run::run \@cmd, '>' => \$stdout, '2>' => \$stderr;
-ok(!$result, "newline in dbname: exit code not 0");
-like(
-	$stderr,
-	qr/shell command argument contains a newline/,
-	"newline in dbname: stderr matches");
-unlike($stdout, qr/^attack/m, "newline in dbname: no comment escape");
-
 #########################################
 # Verify that dumping foreign data includes only foreign tables of
 # matching servers
diff --git a/src/bin/pg_dump/t/010_dump_connstr.pl b/src/bin/pg_dump/t/010_dump_connstr.pl
old mode 100644
new mode 100755
index dc7a33658db..bf2c3b6d00b
--- a/src/bin/pg_dump/t/010_dump_connstr.pl
+++ b/src/bin/pg_dump/t/010_dump_connstr.pl
@@ -153,20 +153,6 @@ $node->command_ok(
 	],
 	'pg_dumpall --dbname accepts connection string');
 
-$node->run_log(
-	[ 'createdb', '--username' => $src_bootstrap_super, "foo\n\rbar" ]);
-
-# not sufficient to use --roles-only here
-$node->command_fails(
-	[
-		'pg_dumpall', '--no-sync',
-		'--username' => $src_bootstrap_super,
-		'--file' => $discard,
-	],
-	'pg_dumpall with \n\r in database name');
-$node->run_log(
-	[ 'dropdb', '--username' => $src_bootstrap_super, "foo\n\rbar" ]);
-
 
 # make a table, so the parallel worker has something to dump
 $node->safe_psql(
diff --git a/src/bin/pg_upgrade/check.c b/src/bin/pg_upgrade/check.c
index 5c73773bf0e..c0e7ec35168 100644
--- a/src/bin/pg_upgrade/check.c
+++ b/src/bin/pg_upgrade/check.c
@@ -34,6 +34,7 @@ static void check_new_cluster_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_old_cluster_global_names(ClusterInfo *cluster);
 
 /*
  * DataTypesUsageChecks - definitions of data type checks for the old cluster
@@ -600,6 +601,14 @@ check_and_dump_old_cluster(void)
 	 */
 	check_for_connection_status(&old_cluster);
 
+	/*
+	 * Validate database, user, role and tablespace names from the old cluster.
+	 * No need to check in 19 or newer as newline and carriage return are
+	 * not allowed at the creation time of the object.
+	 */
+	if (GET_MAJOR_VERSION(old_cluster.major_version) < 1900)
+		check_old_cluster_global_names(&old_cluster);
+
 	/*
 	 * Extract a list of databases, tables, and logical replication slots from
 	 * the old cluster.
@@ -2500,3 +2509,73 @@ check_old_cluster_subscription_state(void)
 	else
 		check_ok();
 }
+
+/*
+ * check_old_cluster_global_names()
+ *
+ * Raise an error if any database, role, or tablespace name contains a newline
+ * or carriage return character.  Such names are not allowed in v19 and later.
+ */
+static void
+check_old_cluster_global_names(ClusterInfo *cluster)
+{
+	int			i;
+	PGconn		*conn_template1;
+	PGresult	*res;
+	int			ntups;
+	FILE		*script = NULL;
+	char		output_path[MAXPGPATH];
+	int			count = 0;
+
+	prep_status("Checking names of databases, roles and tablespaces");
+
+	snprintf(output_path, sizeof(output_path), "%s/%s",
+			log_opts.basedir,
+			"db_role_tablespace_invalid_names.txt");
+
+	conn_template1 = connectToServer(cluster, "template1");
+
+	/*
+	 * Get database, user/role and tablespacenames from cluster.  Can't use
+	 * pg_authid because only superusers can view it.
+	 */
+	res = executeQueryOrDie(conn_template1,
+			"SELECT datname AS objname, 'database' AS objtype "
+			"FROM pg_catalog.pg_database UNION ALL "
+			"SELECT rolname AS objname, 'role' AS objtype "
+			"FROM pg_catalog.pg_roles UNION ALL "
+			"SELECT spcname AS objname, 'tablespace' AS objtype "
+			"FROM pg_catalog.pg_tablespace ORDER BY 2 ");
+
+	ntups = PQntuples(res);
+	for (i = 0; i < ntups; i++)
+	{
+		char	*objname = PQgetvalue(res, i, 0);
+		char	*objtype = PQgetvalue(res, i, 1);
+
+		/* If name has \n or \r, then report it. */
+		if (strpbrk(objname, "\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 : %s name = \"%s\"\n", ++count, objtype, objname);
+		}
+	}
+
+	PQclear(res);
+	PQfinish(conn_template1);
+
+	if (script)
+	{
+		fclose(script);
+		pg_log(PG_REPORT, "fatal");
+		pg_fatal("All the database, role and tablespace names should have only valid characters. A newline or \n"
+				"carriage return character is not allowed in these object names.  To fix this, please \n"
+				"rename these names with valid names. \n"
+				"To see all %d invalid object names, refer db_role_tablespace_invalid_names.txt file. \n"
+				"    %s", count, output_path);
+	}
+	else
+		check_ok();
+}
diff --git a/src/bin/scripts/t/020_createdb.pl b/src/bin/scripts/t/020_createdb.pl
old mode 100644
new mode 100755
index 83b0077383a..a0995868363
--- a/src/bin/scripts/t/020_createdb.pl
+++ b/src/bin/scripts/t/020_createdb.pl
@@ -241,6 +241,18 @@ $node->command_fails(
 	],
 	'fails for invalid locale provider');
 
+$node->command_fails_like(
+    [ 'createdb', "invalid \n dbname" ],
+    qr(contains a newline or carriage return character),
+    'fails if database name contains a newline character in name'
+);
+
+$node->command_fails_like(
+    [ 'createdb', "invalid \r dbname" ],
+    qr(contains a newline or carriage return character),
+    'fails if database name contains a carriage return character in name'
+);
+
 # Check use of templates with shared dependencies copied from the template.
 my ($ret, $stdout, $stderr) = $node->psql(
 	'foobar2',
diff --git a/src/fe_utils/string_utils.c b/src/fe_utils/string_utils.c
index 89ce396ed4f..38fffbd036b 100644
--- a/src/fe_utils/string_utils.c
+++ b/src/fe_utils/string_utils.c
@@ -568,12 +568,6 @@ appendByteaLiteral(PQExpBuffer buf, const unsigned char *str, size_t length,
  * Append the given string to the shell command being built in the buffer,
  * with shell-style quoting as needed to create exactly one argument.
  *
- * Forbid LF or CR characters, which have scant practical use beyond designing
- * security breaches.  The Windows command shell is unusable as a conduit for
- * arguments containing LF or CR characters.  A future major release should
- * reject those characters in CREATE ROLE and CREATE DATABASE, because use
- * there eventually leads to errors here.
- *
  * appendShellString() simply prints an error and dies if LF or CR appears.
  * appendShellStringNoError() omits those characters from the result, and
  * returns false if there were any.
diff --git a/src/test/modules/unsafe_tests/expected/alter_system_table.out b/src/test/modules/unsafe_tests/expected/alter_system_table.out
index b73b9442b8d..9ea6061f4ae 100644
--- a/src/test/modules/unsafe_tests/expected/alter_system_table.out
+++ b/src/test/modules/unsafe_tests/expected/alter_system_table.out
@@ -64,6 +64,11 @@ ERROR:  permission denied: "pg_description" is a system catalog
 CREATE TABLESPACE pg_foo LOCATION '/no/such/location';
 ERROR:  unacceptable tablespace name "pg_foo"
 DETAIL:  The prefix "pg_" is reserved for system tablespaces.
+-- contains \n\r tablespace name
+CREATE TABLESPACE "invalid
+name" LOCATION '/no/such/location';
+ERROR:  tablespace name "invalid
+name" contains a newline or carriage return character
 -- triggers
 CREATE FUNCTION tf1() RETURNS trigger
 LANGUAGE plpgsql
diff --git a/src/test/modules/unsafe_tests/expected/rolenames.out b/src/test/modules/unsafe_tests/expected/rolenames.out
index 61396b2a805..9dfb8475e16 100644
--- a/src/test/modules/unsafe_tests/expected/rolenames.out
+++ b/src/test/modules/unsafe_tests/expected/rolenames.out
@@ -102,6 +102,10 @@ DETAIL:  Role names starting with "pg_" are reserved.
 CREATE ROLE "pg_abcdef"; -- error
 ERROR:  role name "pg_abcdef" is reserved
 DETAIL:  Role names starting with "pg_" are reserved.
+CREATE ROLE "invalid
+rolename"; -- error
+ERROR:  role name "invalid
+rolename" contains a newline or carriage return character
 CREATE ROLE regress_testrol0 SUPERUSER LOGIN;
 CREATE ROLE regress_testrolx SUPERUSER LOGIN;
 CREATE ROLE regress_testrol2 SUPERUSER;
diff --git a/src/test/modules/unsafe_tests/sql/alter_system_table.sql b/src/test/modules/unsafe_tests/sql/alter_system_table.sql
index c1515100845..1b0eb727eaa 100644
--- a/src/test/modules/unsafe_tests/sql/alter_system_table.sql
+++ b/src/test/modules/unsafe_tests/sql/alter_system_table.sql
@@ -64,6 +64,10 @@ ALTER TABLE pg_description SET SCHEMA public;
 -- reserved tablespace name
 CREATE TABLESPACE pg_foo LOCATION '/no/such/location';
 
+-- contains \n\r tablespace name
+CREATE TABLESPACE "invalid
+name" LOCATION '/no/such/location';
+
 -- triggers
 CREATE FUNCTION tf1() RETURNS trigger
 LANGUAGE plpgsql
diff --git a/src/test/modules/unsafe_tests/sql/rolenames.sql b/src/test/modules/unsafe_tests/sql/rolenames.sql
index adac36536db..70b342abeb6 100644
--- a/src/test/modules/unsafe_tests/sql/rolenames.sql
+++ b/src/test/modules/unsafe_tests/sql/rolenames.sql
@@ -75,6 +75,8 @@ CREATE ROLE pg_abc; -- error
 CREATE ROLE "pg_abc"; -- error
 CREATE ROLE pg_abcdef; -- error
 CREATE ROLE "pg_abcdef"; -- error
+CREATE ROLE "invalid
+rolename"; -- error
 
 CREATE ROLE regress_testrol0 SUPERUSER LOGIN;
 CREATE ROLE regress_testrolx SUPERUSER LOGIN;
diff --git a/src/test/regress/expected/tablespace.out b/src/test/regress/expected/tablespace.out
index a90e39e5738..185880a3217 100644
--- a/src/test/regress/expected/tablespace.out
+++ b/src/test/regress/expected/tablespace.out
@@ -958,6 +958,11 @@ ALTER TABLE ALL IN TABLESPACE regress_tblspace_renamed SET TABLESPACE pg_default
 NOTICE:  no matching relations in tablespace "regress_tblspace_renamed" found
 ALTER MATERIALIZED VIEW ALL IN TABLESPACE regress_tblspace_renamed SET TABLESPACE pg_default;
 NOTICE:  no matching relations in tablespace "regress_tblspace_renamed" found
+-- Should fail, contains \n in name
+ALTER TABLESPACE regress_tblspace_renamed RENAME TO "invalid
+name";
+ERROR:  tablespace name "invalid
+name" contains a newline or carriage return character
 -- Should succeed
 DROP TABLESPACE regress_tblspace_renamed;
 DROP SCHEMA testschema CASCADE;
diff --git a/src/test/regress/sql/tablespace.sql b/src/test/regress/sql/tablespace.sql
index dfe3db096e2..c43a59e5957 100644
--- a/src/test/regress/sql/tablespace.sql
+++ b/src/test/regress/sql/tablespace.sql
@@ -430,6 +430,10 @@ ALTER MATERIALIZED VIEW ALL IN TABLESPACE regress_tblspace_renamed SET TABLESPAC
 ALTER TABLE ALL IN TABLESPACE regress_tblspace_renamed SET TABLESPACE pg_default;
 ALTER MATERIALIZED VIEW ALL IN TABLESPACE regress_tblspace_renamed SET TABLESPACE pg_default;
 
+-- Should fail, contains \n in name
+ALTER TABLESPACE regress_tblspace_renamed RENAME TO "invalid
+name";
+
 -- Should succeed
 DROP TABLESPACE regress_tblspace_renamed;
 


^ permalink  raw  reply  [nested|flat] 16+ 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
  2026-01-29 16:23 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]>
  2026-02-02 05:50 ` 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]>
  2026-02-02 11:16   ` 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]>
  2026-02-03 19:14     ` 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]>
  2026-02-21 18:16       ` 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]>
@ 2026-02-23 21:03         ` Nathan Bossart <[email protected]>
  2026-02-24 15:58           ` 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]>
  0 siblings, 1 reply; 16+ messages in thread

From: Nathan Bossart @ 2026-02-23 21:03 UTC (permalink / raw)
  To: Andrew Dunstan <[email protected]>; +Cc: Mahendra Singh Thalor <[email protected]>; Álvaro Herrera <[email protected]>; Tom Lane <[email protected]>; Srinath Reddy <[email protected]>; [email protected]

On Sat, Feb 21, 2026 at 01:16:05PM -0500, Andrew Dunstan wrote:
> OK, I have squashed these two together and tweaked them a bit. I propose to
> commit this patch fairly soon.

koel seems to be unhappy with this one:

	https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=koel&dt=2026-02-23%2020%3A19%3A04

-- 
nathan






^ permalink  raw  reply  [nested|flat] 16+ 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
  2026-01-29 16:23 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]>
  2026-02-02 05:50 ` 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]>
  2026-02-02 11:16   ` 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]>
  2026-02-03 19:14     ` 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]>
  2026-02-21 18:16       ` 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]>
  2026-02-23 21:03         ` 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]>
@ 2026-02-24 15:58           ` Andrew Dunstan <[email protected]>
  0 siblings, 0 replies; 16+ messages in thread

From: Andrew Dunstan @ 2026-02-24 15:58 UTC (permalink / raw)
  To: Nathan Bossart <[email protected]>; +Cc: Mahendra Singh Thalor <[email protected]>; Álvaro Herrera <[email protected]>; Tom Lane <[email protected]>; Srinath Reddy <[email protected]>; [email protected]


On 2026-02-23 Mo 4:03 PM, Nathan Bossart wrote:
> On Sat, Feb 21, 2026 at 01:16:05PM -0500, Andrew Dunstan wrote:
>> OK, I have squashed these two together and tweaked them a bit. I propose to
>> commit this patch fairly soon.
> koel seems to be unhappy with this one:
>
> 	https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=koel&dt=2026-02-23%2020%3A19%3A04
>

Thanks, it's been fixed. (I need to work out why my git hook didn't find 
this.)


cheers


andrew

--
Andrew Dunstan
EDB: https://www.enterprisedb.com







^ permalink  raw  reply  [nested|flat] 16+ 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
  2026-01-29 16:23 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]>
@ 2026-02-02 10:54 ` Álvaro Herrera <[email protected]>
  2026-02-02 14:14   ` 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]>
  1 sibling, 1 reply; 16+ messages in thread

From: Álvaro Herrera @ 2026-02-02 10:54 UTC (permalink / raw)
  To: Tom Lane <[email protected]>; +Cc: Andrew Dunstan <[email protected]>; Mahendra Singh Thalor <[email protected]>; Nathan Bossart <[email protected]>; Srinath Reddy <[email protected]>; [email protected]

Hello,

This is not an objection to this patch, but I don't see why we should
allow newlines in tablespace names if we're going to restrict them in
names of roles and databases.  Is it only because pg_dumpall happens not
to dump a comment with the tablespace name on it, like we do for dbs and
roles?  Sounds unprincipled.

I think keeping these things under the same rules makes the most sense.

-- 
Álvaro Herrera        Breisgau, Deutschland  —  https://www.EnterpriseDB.com/
"The problem with the facetime model is not just that it's demoralizing, but
that the people pretending to work interrupt the ones actually working."
                  -- Paul Graham, http://www.paulgraham.com/opensource.html






^ permalink  raw  reply  [nested|flat] 16+ 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
  2026-01-29 16:23 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]>
  2026-02-02 10:54 ` 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]>
@ 2026-02-02 14:14   ` Tom Lane <[email protected]>
  0 siblings, 0 replies; 16+ messages in thread

From: Tom Lane @ 2026-02-02 14:14 UTC (permalink / raw)
  To: Álvaro Herrera <[email protected]>; +Cc: Andrew Dunstan <[email protected]>; Mahendra Singh Thalor <[email protected]>; Nathan Bossart <[email protected]>; Srinath Reddy <[email protected]>; [email protected]

=?utf-8?Q?=C3=81lvaro?= Herrera <[email protected]> writes:
> This is not an objection to this patch, but I don't see why we should
> allow newlines in tablespace names if we're going to restrict them in
> names of roles and databases.  Is it only because pg_dumpall happens not
> to dump a comment with the tablespace name on it, like we do for dbs and
> roles?  Sounds unprincipled.
> I think keeping these things under the same rules makes the most sense.

Yeah, I was thinking the same.

			regards, tom lane






^ permalink  raw  reply  [nested|flat] 16+ messages in thread

end of thread, other threads:[~2026-02-24 15:58 UTC | newest]

Thread overview: 16+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2023-07-20 16:52 [PATCH v3 3/4] expand binaryheap api Nathan Bossart <[email protected]>
2023-07-20 16:52 [PATCH v5 2/3] expand binaryheap api Nathan Bossart <[email protected]>
2023-07-20 16:52 [PATCH v2 3/4] expand binaryheap api Nathan Bossart <[email protected]>
2023-07-20 16:52 [PATCH v2 3/4] expand binaryheap api Nathan Bossart <[email protected]>
2023-07-20 16:52 [PATCH v5 2/3] expand binaryheap api Nathan Bossart <[email protected]>
2023-07-20 16:52 [PATCH v4 3/4] expand binaryheap api Nathan Bossart <[email protected]>
2023-07-20 16:52 [PATCH v3 3/4] expand binaryheap api Nathan Bossart <[email protected]>
2026-01-29 16:23 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]>
2026-02-02 05:50 ` 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]>
2026-02-02 11:16   ` 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]>
2026-02-03 19:14     ` 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]>
2026-02-21 18:16       ` 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]>
2026-02-23 21:03         ` 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]>
2026-02-24 15:58           ` 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]>
2026-02-02 10:54 ` 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]>
2026-02-02 14:14   ` 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]>

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox