public inbox for [email protected]  

help / color / mirror / Atom feed

[PATCH 1/1] Fix a corner-case in COPY FROM backslash processing.
54+ messages / 10 participants
[nested] [flat]

* [PATCH 1/1] Fix a corner-case in COPY FROM backslash processing.
@ 2021-02-03 12:00  Heikki Linnakangas <[email protected]>
  0 siblings, 0 replies; 54+ messages in thread

From: Heikki Linnakangas @ 2021-02-03 12:00 UTC (permalink / raw)

If a multi-byte character is escaped with a backslash in TEXT mode input,
we didn't always treat the escape correctly. If:

- a multi-byte character is escaped with a backslash, and
- the second byte of the character is 0x5C, i.e. the ASCII code of a
  backslash (\), and
- the next character is a dot (.), then

copyreadline we would incorrectly interpret the sequence as an end-of-copy
marker (\.). this can only happen in encodings that can "embed" ascii
characters as the second byte. one example of such sequence is
'\x5ca45c2e666f6f' in Big5 encoding. If you put that in a file, and
load it with COPY FROM, you'd incorrectly get an "end-of-copy marker
corrupt" error.
---
 src/backend/commands/copyfromparse.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/src/backend/commands/copyfromparse.c b/src/backend/commands/copyfromparse.c
index 798e18e013..c20ec482db 100644
--- a/src/backend/commands/copyfromparse.c
+++ b/src/backend/commands/copyfromparse.c
@@ -1084,7 +1084,7 @@ CopyReadLineText(CopyFromState cstate)
 				break;
 			}
 			else if (!cstate->opts.csv_mode)
-
+			{
 				/*
 				 * If we are here, it means we found a backslash followed by
 				 * something other than a period.  In non-CSV mode, anything
@@ -1095,8 +1095,17 @@ CopyReadLineText(CopyFromState cstate)
 				 * backslashes are not special, so we want to process the
 				 * character after the backslash just like a normal character,
 				 * so we don't increment in those cases.
+				 *
+				 * In principle, we would need to use pg_encoding_mblen() to
+				 * skip over the character in some encodings, like at the end
+				 * of the loop.  But if it's a multi-byte character, it cannot
+				 * have any special meaning and skipping isn't necessary for
+				 * correctness.  We can fall through to process it normally
+				 * instead.
 				 */
-				raw_buf_ptr++;
+				if (!IS_HIGHBIT_SET(c2))
+					raw_buf_ptr++;
+			}
 		}
 
 		/*
-- 
2.30.0


--------------97F3138F3612F1A4F9200D93--





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

* [PATCH 1/1] Fix a corner-case in COPY FROM backslash processing.
@ 2021-02-03 12:00  Heikki Linnakangas <[email protected]>
  0 siblings, 0 replies; 54+ messages in thread

From: Heikki Linnakangas @ 2021-02-03 12:00 UTC (permalink / raw)

If a multi-byte character is escaped with a backslash in TEXT mode input,
we didn't always treat the escape correctly. If:

- a multi-byte character is escaped with a backslash, and
- the second byte of the character is 0x5C, i.e. the ASCII code of a
  backslash (\), and
- the next character is a dot (.), then

copyreadline we would incorrectly interpret the sequence as an end-of-copy
marker (\.). this can only happen in encodings that can "embed" ascii
characters as the second byte. one example of such sequence is
'\x5ca45c2e666f6f' in Big5 encoding. If you put that in a file, and
load it with COPY FROM, you'd incorrectly get an "end-of-copy marker
corrupt" error.
---
 src/backend/commands/copyfromparse.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/src/backend/commands/copyfromparse.c b/src/backend/commands/copyfromparse.c
index 798e18e013..c20ec482db 100644
--- a/src/backend/commands/copyfromparse.c
+++ b/src/backend/commands/copyfromparse.c
@@ -1084,7 +1084,7 @@ CopyReadLineText(CopyFromState cstate)
 				break;
 			}
 			else if (!cstate->opts.csv_mode)
-
+			{
 				/*
 				 * If we are here, it means we found a backslash followed by
 				 * something other than a period.  In non-CSV mode, anything
@@ -1095,8 +1095,17 @@ CopyReadLineText(CopyFromState cstate)
 				 * backslashes are not special, so we want to process the
 				 * character after the backslash just like a normal character,
 				 * so we don't increment in those cases.
+				 *
+				 * In principle, we would need to use pg_encoding_mblen() to
+				 * skip over the character in some encodings, like at the end
+				 * of the loop.  But if it's a multi-byte character, it cannot
+				 * have any special meaning and skipping isn't necessary for
+				 * correctness.  We can fall through to process it normally
+				 * instead.
 				 */
-				raw_buf_ptr++;
+				if (!IS_HIGHBIT_SET(c2))
+					raw_buf_ptr++;
+			}
 		}
 
 		/*
-- 
2.30.0


--------------97F3138F3612F1A4F9200D93--





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

* [PATCH 1/1] Fix a corner-case in COPY FROM backslash processing.
@ 2021-02-03 12:00  Heikki Linnakangas <[email protected]>
  0 siblings, 0 replies; 54+ messages in thread

From: Heikki Linnakangas @ 2021-02-03 12:00 UTC (permalink / raw)

If a multi-byte character is escaped with a backslash in TEXT mode input,
we didn't always treat the escape correctly. If:

- a multi-byte character is escaped with a backslash, and
- the second byte of the character is 0x5C, i.e. the ASCII code of a
  backslash (\), and
- the next character is a dot (.), then

copyreadline we would incorrectly interpret the sequence as an end-of-copy
marker (\.). this can only happen in encodings that can "embed" ascii
characters as the second byte. one example of such sequence is
'\x5ca45c2e666f6f' in Big5 encoding. If you put that in a file, and
load it with COPY FROM, you'd incorrectly get an "end-of-copy marker
corrupt" error.
---
 src/backend/commands/copyfromparse.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/src/backend/commands/copyfromparse.c b/src/backend/commands/copyfromparse.c
index 798e18e013..c20ec482db 100644
--- a/src/backend/commands/copyfromparse.c
+++ b/src/backend/commands/copyfromparse.c
@@ -1084,7 +1084,7 @@ CopyReadLineText(CopyFromState cstate)
 				break;
 			}
 			else if (!cstate->opts.csv_mode)
-
+			{
 				/*
 				 * If we are here, it means we found a backslash followed by
 				 * something other than a period.  In non-CSV mode, anything
@@ -1095,8 +1095,17 @@ CopyReadLineText(CopyFromState cstate)
 				 * backslashes are not special, so we want to process the
 				 * character after the backslash just like a normal character,
 				 * so we don't increment in those cases.
+				 *
+				 * In principle, we would need to use pg_encoding_mblen() to
+				 * skip over the character in some encodings, like at the end
+				 * of the loop.  But if it's a multi-byte character, it cannot
+				 * have any special meaning and skipping isn't necessary for
+				 * correctness.  We can fall through to process it normally
+				 * instead.
 				 */
-				raw_buf_ptr++;
+				if (!IS_HIGHBIT_SET(c2))
+					raw_buf_ptr++;
+			}
 		}
 
 		/*
-- 
2.30.0


--------------97F3138F3612F1A4F9200D93--





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

* [PATCH 1/1] Fix a corner-case in COPY FROM backslash processing.
@ 2021-02-03 12:00  Heikki Linnakangas <[email protected]>
  0 siblings, 0 replies; 54+ messages in thread

From: Heikki Linnakangas @ 2021-02-03 12:00 UTC (permalink / raw)

If a multi-byte character is escaped with a backslash in TEXT mode input,
we didn't always treat the escape correctly. If:

- a multi-byte character is escaped with a backslash, and
- the second byte of the character is 0x5C, i.e. the ASCII code of a
  backslash (\), and
- the next character is a dot (.), then

copyreadline we would incorrectly interpret the sequence as an end-of-copy
marker (\.). this can only happen in encodings that can "embed" ascii
characters as the second byte. one example of such sequence is
'\x5ca45c2e666f6f' in Big5 encoding. If you put that in a file, and
load it with COPY FROM, you'd incorrectly get an "end-of-copy marker
corrupt" error.
---
 src/backend/commands/copyfromparse.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/src/backend/commands/copyfromparse.c b/src/backend/commands/copyfromparse.c
index 798e18e013..c20ec482db 100644
--- a/src/backend/commands/copyfromparse.c
+++ b/src/backend/commands/copyfromparse.c
@@ -1084,7 +1084,7 @@ CopyReadLineText(CopyFromState cstate)
 				break;
 			}
 			else if (!cstate->opts.csv_mode)
-
+			{
 				/*
 				 * If we are here, it means we found a backslash followed by
 				 * something other than a period.  In non-CSV mode, anything
@@ -1095,8 +1095,17 @@ CopyReadLineText(CopyFromState cstate)
 				 * backslashes are not special, so we want to process the
 				 * character after the backslash just like a normal character,
 				 * so we don't increment in those cases.
+				 *
+				 * In principle, we would need to use pg_encoding_mblen() to
+				 * skip over the character in some encodings, like at the end
+				 * of the loop.  But if it's a multi-byte character, it cannot
+				 * have any special meaning and skipping isn't necessary for
+				 * correctness.  We can fall through to process it normally
+				 * instead.
 				 */
-				raw_buf_ptr++;
+				if (!IS_HIGHBIT_SET(c2))
+					raw_buf_ptr++;
+			}
 		}
 
 		/*
-- 
2.30.0


--------------97F3138F3612F1A4F9200D93--





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

* [PATCH 1/1] Fix a corner-case in COPY FROM backslash processing.
@ 2021-02-03 12:00  Heikki Linnakangas <[email protected]>
  0 siblings, 0 replies; 54+ messages in thread

From: Heikki Linnakangas @ 2021-02-03 12:00 UTC (permalink / raw)

If a multi-byte character is escaped with a backslash in TEXT mode input,
we didn't always treat the escape correctly. If:

- a multi-byte character is escaped with a backslash, and
- the second byte of the character is 0x5C, i.e. the ASCII code of a
  backslash (\), and
- the next character is a dot (.), then

copyreadline we would incorrectly interpret the sequence as an end-of-copy
marker (\.). this can only happen in encodings that can "embed" ascii
characters as the second byte. one example of such sequence is
'\x5ca45c2e666f6f' in Big5 encoding. If you put that in a file, and
load it with COPY FROM, you'd incorrectly get an "end-of-copy marker
corrupt" error.
---
 src/backend/commands/copyfromparse.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/src/backend/commands/copyfromparse.c b/src/backend/commands/copyfromparse.c
index 798e18e013..c20ec482db 100644
--- a/src/backend/commands/copyfromparse.c
+++ b/src/backend/commands/copyfromparse.c
@@ -1084,7 +1084,7 @@ CopyReadLineText(CopyFromState cstate)
 				break;
 			}
 			else if (!cstate->opts.csv_mode)
-
+			{
 				/*
 				 * If we are here, it means we found a backslash followed by
 				 * something other than a period.  In non-CSV mode, anything
@@ -1095,8 +1095,17 @@ CopyReadLineText(CopyFromState cstate)
 				 * backslashes are not special, so we want to process the
 				 * character after the backslash just like a normal character,
 				 * so we don't increment in those cases.
+				 *
+				 * In principle, we would need to use pg_encoding_mblen() to
+				 * skip over the character in some encodings, like at the end
+				 * of the loop.  But if it's a multi-byte character, it cannot
+				 * have any special meaning and skipping isn't necessary for
+				 * correctness.  We can fall through to process it normally
+				 * instead.
 				 */
-				raw_buf_ptr++;
+				if (!IS_HIGHBIT_SET(c2))
+					raw_buf_ptr++;
+			}
 		}
 
 		/*
-- 
2.30.0


--------------97F3138F3612F1A4F9200D93--





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

* [PATCH 1/1] Fix a corner-case in COPY FROM backslash processing.
@ 2021-02-03 12:00  Heikki Linnakangas <[email protected]>
  0 siblings, 0 replies; 54+ messages in thread

From: Heikki Linnakangas @ 2021-02-03 12:00 UTC (permalink / raw)

If a multi-byte character is escaped with a backslash in TEXT mode input,
we didn't always treat the escape correctly. If:

- a multi-byte character is escaped with a backslash, and
- the second byte of the character is 0x5C, i.e. the ASCII code of a
  backslash (\), and
- the next character is a dot (.), then

copyreadline we would incorrectly interpret the sequence as an end-of-copy
marker (\.). this can only happen in encodings that can "embed" ascii
characters as the second byte. one example of such sequence is
'\x5ca45c2e666f6f' in Big5 encoding. If you put that in a file, and
load it with COPY FROM, you'd incorrectly get an "end-of-copy marker
corrupt" error.
---
 src/backend/commands/copyfromparse.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/src/backend/commands/copyfromparse.c b/src/backend/commands/copyfromparse.c
index 798e18e013..c20ec482db 100644
--- a/src/backend/commands/copyfromparse.c
+++ b/src/backend/commands/copyfromparse.c
@@ -1084,7 +1084,7 @@ CopyReadLineText(CopyFromState cstate)
 				break;
 			}
 			else if (!cstate->opts.csv_mode)
-
+			{
 				/*
 				 * If we are here, it means we found a backslash followed by
 				 * something other than a period.  In non-CSV mode, anything
@@ -1095,8 +1095,17 @@ CopyReadLineText(CopyFromState cstate)
 				 * backslashes are not special, so we want to process the
 				 * character after the backslash just like a normal character,
 				 * so we don't increment in those cases.
+				 *
+				 * In principle, we would need to use pg_encoding_mblen() to
+				 * skip over the character in some encodings, like at the end
+				 * of the loop.  But if it's a multi-byte character, it cannot
+				 * have any special meaning and skipping isn't necessary for
+				 * correctness.  We can fall through to process it normally
+				 * instead.
 				 */
-				raw_buf_ptr++;
+				if (!IS_HIGHBIT_SET(c2))
+					raw_buf_ptr++;
+			}
 		}
 
 		/*
-- 
2.30.0


--------------97F3138F3612F1A4F9200D93--





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

* [PATCH 1/1] Fix a corner-case in COPY FROM backslash processing.
@ 2021-02-03 12:00  Heikki Linnakangas <[email protected]>
  0 siblings, 0 replies; 54+ messages in thread

From: Heikki Linnakangas @ 2021-02-03 12:00 UTC (permalink / raw)

If a multi-byte character is escaped with a backslash in TEXT mode input,
we didn't always treat the escape correctly. If:

- a multi-byte character is escaped with a backslash, and
- the second byte of the character is 0x5C, i.e. the ASCII code of a
  backslash (\), and
- the next character is a dot (.), then

copyreadline we would incorrectly interpret the sequence as an end-of-copy
marker (\.). this can only happen in encodings that can "embed" ascii
characters as the second byte. one example of such sequence is
'\x5ca45c2e666f6f' in Big5 encoding. If you put that in a file, and
load it with COPY FROM, you'd incorrectly get an "end-of-copy marker
corrupt" error.
---
 src/backend/commands/copyfromparse.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/src/backend/commands/copyfromparse.c b/src/backend/commands/copyfromparse.c
index 798e18e013..c20ec482db 100644
--- a/src/backend/commands/copyfromparse.c
+++ b/src/backend/commands/copyfromparse.c
@@ -1084,7 +1084,7 @@ CopyReadLineText(CopyFromState cstate)
 				break;
 			}
 			else if (!cstate->opts.csv_mode)
-
+			{
 				/*
 				 * If we are here, it means we found a backslash followed by
 				 * something other than a period.  In non-CSV mode, anything
@@ -1095,8 +1095,17 @@ CopyReadLineText(CopyFromState cstate)
 				 * backslashes are not special, so we want to process the
 				 * character after the backslash just like a normal character,
 				 * so we don't increment in those cases.
+				 *
+				 * In principle, we would need to use pg_encoding_mblen() to
+				 * skip over the character in some encodings, like at the end
+				 * of the loop.  But if it's a multi-byte character, it cannot
+				 * have any special meaning and skipping isn't necessary for
+				 * correctness.  We can fall through to process it normally
+				 * instead.
 				 */
-				raw_buf_ptr++;
+				if (!IS_HIGHBIT_SET(c2))
+					raw_buf_ptr++;
+			}
 		}
 
 		/*
-- 
2.30.0


--------------97F3138F3612F1A4F9200D93--





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

* [PATCH 1/1] Fix a corner-case in COPY FROM backslash processing.
@ 2021-02-03 12:00  Heikki Linnakangas <[email protected]>
  0 siblings, 0 replies; 54+ messages in thread

From: Heikki Linnakangas @ 2021-02-03 12:00 UTC (permalink / raw)

If a multi-byte character is escaped with a backslash in TEXT mode input,
we didn't always treat the escape correctly. If:

- a multi-byte character is escaped with a backslash, and
- the second byte of the character is 0x5C, i.e. the ASCII code of a
  backslash (\), and
- the next character is a dot (.), then

copyreadline we would incorrectly interpret the sequence as an end-of-copy
marker (\.). this can only happen in encodings that can "embed" ascii
characters as the second byte. one example of such sequence is
'\x5ca45c2e666f6f' in Big5 encoding. If you put that in a file, and
load it with COPY FROM, you'd incorrectly get an "end-of-copy marker
corrupt" error.
---
 src/backend/commands/copyfromparse.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/src/backend/commands/copyfromparse.c b/src/backend/commands/copyfromparse.c
index 798e18e013..c20ec482db 100644
--- a/src/backend/commands/copyfromparse.c
+++ b/src/backend/commands/copyfromparse.c
@@ -1084,7 +1084,7 @@ CopyReadLineText(CopyFromState cstate)
 				break;
 			}
 			else if (!cstate->opts.csv_mode)
-
+			{
 				/*
 				 * If we are here, it means we found a backslash followed by
 				 * something other than a period.  In non-CSV mode, anything
@@ -1095,8 +1095,17 @@ CopyReadLineText(CopyFromState cstate)
 				 * backslashes are not special, so we want to process the
 				 * character after the backslash just like a normal character,
 				 * so we don't increment in those cases.
+				 *
+				 * In principle, we would need to use pg_encoding_mblen() to
+				 * skip over the character in some encodings, like at the end
+				 * of the loop.  But if it's a multi-byte character, it cannot
+				 * have any special meaning and skipping isn't necessary for
+				 * correctness.  We can fall through to process it normally
+				 * instead.
 				 */
-				raw_buf_ptr++;
+				if (!IS_HIGHBIT_SET(c2))
+					raw_buf_ptr++;
+			}
 		}
 
 		/*
-- 
2.30.0


--------------97F3138F3612F1A4F9200D93--





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

* [PATCH 1/1] Fix a corner-case in COPY FROM backslash processing.
@ 2021-02-03 12:00  Heikki Linnakangas <[email protected]>
  0 siblings, 0 replies; 54+ messages in thread

From: Heikki Linnakangas @ 2021-02-03 12:00 UTC (permalink / raw)

If a multi-byte character is escaped with a backslash in TEXT mode input,
we didn't always treat the escape correctly. If:

- a multi-byte character is escaped with a backslash, and
- the second byte of the character is 0x5C, i.e. the ASCII code of a
  backslash (\), and
- the next character is a dot (.), then

copyreadline we would incorrectly interpret the sequence as an end-of-copy
marker (\.). this can only happen in encodings that can "embed" ascii
characters as the second byte. one example of such sequence is
'\x5ca45c2e666f6f' in Big5 encoding. If you put that in a file, and
load it with COPY FROM, you'd incorrectly get an "end-of-copy marker
corrupt" error.
---
 src/backend/commands/copyfromparse.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/src/backend/commands/copyfromparse.c b/src/backend/commands/copyfromparse.c
index 798e18e013..c20ec482db 100644
--- a/src/backend/commands/copyfromparse.c
+++ b/src/backend/commands/copyfromparse.c
@@ -1084,7 +1084,7 @@ CopyReadLineText(CopyFromState cstate)
 				break;
 			}
 			else if (!cstate->opts.csv_mode)
-
+			{
 				/*
 				 * If we are here, it means we found a backslash followed by
 				 * something other than a period.  In non-CSV mode, anything
@@ -1095,8 +1095,17 @@ CopyReadLineText(CopyFromState cstate)
 				 * backslashes are not special, so we want to process the
 				 * character after the backslash just like a normal character,
 				 * so we don't increment in those cases.
+				 *
+				 * In principle, we would need to use pg_encoding_mblen() to
+				 * skip over the character in some encodings, like at the end
+				 * of the loop.  But if it's a multi-byte character, it cannot
+				 * have any special meaning and skipping isn't necessary for
+				 * correctness.  We can fall through to process it normally
+				 * instead.
 				 */
-				raw_buf_ptr++;
+				if (!IS_HIGHBIT_SET(c2))
+					raw_buf_ptr++;
+			}
 		}
 
 		/*
-- 
2.30.0


--------------97F3138F3612F1A4F9200D93--





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

* [PATCH 1/1] Fix a corner-case in COPY FROM backslash processing.
@ 2021-02-03 12:00  Heikki Linnakangas <[email protected]>
  0 siblings, 0 replies; 54+ messages in thread

From: Heikki Linnakangas @ 2021-02-03 12:00 UTC (permalink / raw)

If a multi-byte character is escaped with a backslash in TEXT mode input,
we didn't always treat the escape correctly. If:

- a multi-byte character is escaped with a backslash, and
- the second byte of the character is 0x5C, i.e. the ASCII code of a
  backslash (\), and
- the next character is a dot (.), then

copyreadline we would incorrectly interpret the sequence as an end-of-copy
marker (\.). this can only happen in encodings that can "embed" ascii
characters as the second byte. one example of such sequence is
'\x5ca45c2e666f6f' in Big5 encoding. If you put that in a file, and
load it with COPY FROM, you'd incorrectly get an "end-of-copy marker
corrupt" error.
---
 src/backend/commands/copyfromparse.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/src/backend/commands/copyfromparse.c b/src/backend/commands/copyfromparse.c
index 798e18e013..c20ec482db 100644
--- a/src/backend/commands/copyfromparse.c
+++ b/src/backend/commands/copyfromparse.c
@@ -1084,7 +1084,7 @@ CopyReadLineText(CopyFromState cstate)
 				break;
 			}
 			else if (!cstate->opts.csv_mode)
-
+			{
 				/*
 				 * If we are here, it means we found a backslash followed by
 				 * something other than a period.  In non-CSV mode, anything
@@ -1095,8 +1095,17 @@ CopyReadLineText(CopyFromState cstate)
 				 * backslashes are not special, so we want to process the
 				 * character after the backslash just like a normal character,
 				 * so we don't increment in those cases.
+				 *
+				 * In principle, we would need to use pg_encoding_mblen() to
+				 * skip over the character in some encodings, like at the end
+				 * of the loop.  But if it's a multi-byte character, it cannot
+				 * have any special meaning and skipping isn't necessary for
+				 * correctness.  We can fall through to process it normally
+				 * instead.
 				 */
-				raw_buf_ptr++;
+				if (!IS_HIGHBIT_SET(c2))
+					raw_buf_ptr++;
+			}
 		}
 
 		/*
-- 
2.30.0


--------------97F3138F3612F1A4F9200D93--





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

* [PATCH 1/1] Fix a corner-case in COPY FROM backslash processing.
@ 2021-02-03 12:00  Heikki Linnakangas <[email protected]>
  0 siblings, 0 replies; 54+ messages in thread

From: Heikki Linnakangas @ 2021-02-03 12:00 UTC (permalink / raw)

If a multi-byte character is escaped with a backslash in TEXT mode input,
we didn't always treat the escape correctly. If:

- a multi-byte character is escaped with a backslash, and
- the second byte of the character is 0x5C, i.e. the ASCII code of a
  backslash (\), and
- the next character is a dot (.), then

copyreadline we would incorrectly interpret the sequence as an end-of-copy
marker (\.). this can only happen in encodings that can "embed" ascii
characters as the second byte. one example of such sequence is
'\x5ca45c2e666f6f' in Big5 encoding. If you put that in a file, and
load it with COPY FROM, you'd incorrectly get an "end-of-copy marker
corrupt" error.
---
 src/backend/commands/copyfromparse.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/src/backend/commands/copyfromparse.c b/src/backend/commands/copyfromparse.c
index 798e18e013..c20ec482db 100644
--- a/src/backend/commands/copyfromparse.c
+++ b/src/backend/commands/copyfromparse.c
@@ -1084,7 +1084,7 @@ CopyReadLineText(CopyFromState cstate)
 				break;
 			}
 			else if (!cstate->opts.csv_mode)
-
+			{
 				/*
 				 * If we are here, it means we found a backslash followed by
 				 * something other than a period.  In non-CSV mode, anything
@@ -1095,8 +1095,17 @@ CopyReadLineText(CopyFromState cstate)
 				 * backslashes are not special, so we want to process the
 				 * character after the backslash just like a normal character,
 				 * so we don't increment in those cases.
+				 *
+				 * In principle, we would need to use pg_encoding_mblen() to
+				 * skip over the character in some encodings, like at the end
+				 * of the loop.  But if it's a multi-byte character, it cannot
+				 * have any special meaning and skipping isn't necessary for
+				 * correctness.  We can fall through to process it normally
+				 * instead.
 				 */
-				raw_buf_ptr++;
+				if (!IS_HIGHBIT_SET(c2))
+					raw_buf_ptr++;
+			}
 		}
 
 		/*
-- 
2.30.0


--------------97F3138F3612F1A4F9200D93--





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

* [PATCH 1/1] Fix a corner-case in COPY FROM backslash processing.
@ 2021-02-03 12:00  Heikki Linnakangas <[email protected]>
  0 siblings, 0 replies; 54+ messages in thread

From: Heikki Linnakangas @ 2021-02-03 12:00 UTC (permalink / raw)

If a multi-byte character is escaped with a backslash in TEXT mode input,
we didn't always treat the escape correctly. If:

- a multi-byte character is escaped with a backslash, and
- the second byte of the character is 0x5C, i.e. the ASCII code of a
  backslash (\), and
- the next character is a dot (.), then

copyreadline we would incorrectly interpret the sequence as an end-of-copy
marker (\.). this can only happen in encodings that can "embed" ascii
characters as the second byte. one example of such sequence is
'\x5ca45c2e666f6f' in Big5 encoding. If you put that in a file, and
load it with COPY FROM, you'd incorrectly get an "end-of-copy marker
corrupt" error.
---
 src/backend/commands/copyfromparse.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/src/backend/commands/copyfromparse.c b/src/backend/commands/copyfromparse.c
index 798e18e013..c20ec482db 100644
--- a/src/backend/commands/copyfromparse.c
+++ b/src/backend/commands/copyfromparse.c
@@ -1084,7 +1084,7 @@ CopyReadLineText(CopyFromState cstate)
 				break;
 			}
 			else if (!cstate->opts.csv_mode)
-
+			{
 				/*
 				 * If we are here, it means we found a backslash followed by
 				 * something other than a period.  In non-CSV mode, anything
@@ -1095,8 +1095,17 @@ CopyReadLineText(CopyFromState cstate)
 				 * backslashes are not special, so we want to process the
 				 * character after the backslash just like a normal character,
 				 * so we don't increment in those cases.
+				 *
+				 * In principle, we would need to use pg_encoding_mblen() to
+				 * skip over the character in some encodings, like at the end
+				 * of the loop.  But if it's a multi-byte character, it cannot
+				 * have any special meaning and skipping isn't necessary for
+				 * correctness.  We can fall through to process it normally
+				 * instead.
 				 */
-				raw_buf_ptr++;
+				if (!IS_HIGHBIT_SET(c2))
+					raw_buf_ptr++;
+			}
 		}
 
 		/*
-- 
2.30.0


--------------97F3138F3612F1A4F9200D93--





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

* [PATCH 1/1] Fix a corner-case in COPY FROM backslash processing.
@ 2021-02-03 12:00  Heikki Linnakangas <[email protected]>
  0 siblings, 0 replies; 54+ messages in thread

From: Heikki Linnakangas @ 2021-02-03 12:00 UTC (permalink / raw)

If a multi-byte character is escaped with a backslash in TEXT mode input,
we didn't always treat the escape correctly. If:

- a multi-byte character is escaped with a backslash, and
- the second byte of the character is 0x5C, i.e. the ASCII code of a
  backslash (\), and
- the next character is a dot (.), then

copyreadline we would incorrectly interpret the sequence as an end-of-copy
marker (\.). this can only happen in encodings that can "embed" ascii
characters as the second byte. one example of such sequence is
'\x5ca45c2e666f6f' in Big5 encoding. If you put that in a file, and
load it with COPY FROM, you'd incorrectly get an "end-of-copy marker
corrupt" error.
---
 src/backend/commands/copyfromparse.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/src/backend/commands/copyfromparse.c b/src/backend/commands/copyfromparse.c
index 798e18e013..c20ec482db 100644
--- a/src/backend/commands/copyfromparse.c
+++ b/src/backend/commands/copyfromparse.c
@@ -1084,7 +1084,7 @@ CopyReadLineText(CopyFromState cstate)
 				break;
 			}
 			else if (!cstate->opts.csv_mode)
-
+			{
 				/*
 				 * If we are here, it means we found a backslash followed by
 				 * something other than a period.  In non-CSV mode, anything
@@ -1095,8 +1095,17 @@ CopyReadLineText(CopyFromState cstate)
 				 * backslashes are not special, so we want to process the
 				 * character after the backslash just like a normal character,
 				 * so we don't increment in those cases.
+				 *
+				 * In principle, we would need to use pg_encoding_mblen() to
+				 * skip over the character in some encodings, like at the end
+				 * of the loop.  But if it's a multi-byte character, it cannot
+				 * have any special meaning and skipping isn't necessary for
+				 * correctness.  We can fall through to process it normally
+				 * instead.
 				 */
-				raw_buf_ptr++;
+				if (!IS_HIGHBIT_SET(c2))
+					raw_buf_ptr++;
+			}
 		}
 
 		/*
-- 
2.30.0


--------------97F3138F3612F1A4F9200D93--





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

* [PATCH 1/1] Fix a corner-case in COPY FROM backslash processing.
@ 2021-02-03 12:00  Heikki Linnakangas <[email protected]>
  0 siblings, 0 replies; 54+ messages in thread

From: Heikki Linnakangas @ 2021-02-03 12:00 UTC (permalink / raw)

If a multi-byte character is escaped with a backslash in TEXT mode input,
we didn't always treat the escape correctly. If:

- a multi-byte character is escaped with a backslash, and
- the second byte of the character is 0x5C, i.e. the ASCII code of a
  backslash (\), and
- the next character is a dot (.), then

copyreadline we would incorrectly interpret the sequence as an end-of-copy
marker (\.). this can only happen in encodings that can "embed" ascii
characters as the second byte. one example of such sequence is
'\x5ca45c2e666f6f' in Big5 encoding. If you put that in a file, and
load it with COPY FROM, you'd incorrectly get an "end-of-copy marker
corrupt" error.
---
 src/backend/commands/copyfromparse.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/src/backend/commands/copyfromparse.c b/src/backend/commands/copyfromparse.c
index 798e18e013..c20ec482db 100644
--- a/src/backend/commands/copyfromparse.c
+++ b/src/backend/commands/copyfromparse.c
@@ -1084,7 +1084,7 @@ CopyReadLineText(CopyFromState cstate)
 				break;
 			}
 			else if (!cstate->opts.csv_mode)
-
+			{
 				/*
 				 * If we are here, it means we found a backslash followed by
 				 * something other than a period.  In non-CSV mode, anything
@@ -1095,8 +1095,17 @@ CopyReadLineText(CopyFromState cstate)
 				 * backslashes are not special, so we want to process the
 				 * character after the backslash just like a normal character,
 				 * so we don't increment in those cases.
+				 *
+				 * In principle, we would need to use pg_encoding_mblen() to
+				 * skip over the character in some encodings, like at the end
+				 * of the loop.  But if it's a multi-byte character, it cannot
+				 * have any special meaning and skipping isn't necessary for
+				 * correctness.  We can fall through to process it normally
+				 * instead.
 				 */
-				raw_buf_ptr++;
+				if (!IS_HIGHBIT_SET(c2))
+					raw_buf_ptr++;
+			}
 		}
 
 		/*
-- 
2.30.0


--------------97F3138F3612F1A4F9200D93--





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

* [PATCH 1/1] Fix a corner-case in COPY FROM backslash processing.
@ 2021-02-03 12:00  Heikki Linnakangas <[email protected]>
  0 siblings, 0 replies; 54+ messages in thread

From: Heikki Linnakangas @ 2021-02-03 12:00 UTC (permalink / raw)

If a multi-byte character is escaped with a backslash in TEXT mode input,
we didn't always treat the escape correctly. If:

- a multi-byte character is escaped with a backslash, and
- the second byte of the character is 0x5C, i.e. the ASCII code of a
  backslash (\), and
- the next character is a dot (.), then

copyreadline we would incorrectly interpret the sequence as an end-of-copy
marker (\.). this can only happen in encodings that can "embed" ascii
characters as the second byte. one example of such sequence is
'\x5ca45c2e666f6f' in Big5 encoding. If you put that in a file, and
load it with COPY FROM, you'd incorrectly get an "end-of-copy marker
corrupt" error.
---
 src/backend/commands/copyfromparse.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/src/backend/commands/copyfromparse.c b/src/backend/commands/copyfromparse.c
index 798e18e013..c20ec482db 100644
--- a/src/backend/commands/copyfromparse.c
+++ b/src/backend/commands/copyfromparse.c
@@ -1084,7 +1084,7 @@ CopyReadLineText(CopyFromState cstate)
 				break;
 			}
 			else if (!cstate->opts.csv_mode)
-
+			{
 				/*
 				 * If we are here, it means we found a backslash followed by
 				 * something other than a period.  In non-CSV mode, anything
@@ -1095,8 +1095,17 @@ CopyReadLineText(CopyFromState cstate)
 				 * backslashes are not special, so we want to process the
 				 * character after the backslash just like a normal character,
 				 * so we don't increment in those cases.
+				 *
+				 * In principle, we would need to use pg_encoding_mblen() to
+				 * skip over the character in some encodings, like at the end
+				 * of the loop.  But if it's a multi-byte character, it cannot
+				 * have any special meaning and skipping isn't necessary for
+				 * correctness.  We can fall through to process it normally
+				 * instead.
 				 */
-				raw_buf_ptr++;
+				if (!IS_HIGHBIT_SET(c2))
+					raw_buf_ptr++;
+			}
 		}
 
 		/*
-- 
2.30.0


--------------97F3138F3612F1A4F9200D93--





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

* [PATCH 1/1] Fix a corner-case in COPY FROM backslash processing.
@ 2021-02-03 12:00  Heikki Linnakangas <[email protected]>
  0 siblings, 0 replies; 54+ messages in thread

From: Heikki Linnakangas @ 2021-02-03 12:00 UTC (permalink / raw)

If a multi-byte character is escaped with a backslash in TEXT mode input,
we didn't always treat the escape correctly. If:

- a multi-byte character is escaped with a backslash, and
- the second byte of the character is 0x5C, i.e. the ASCII code of a
  backslash (\), and
- the next character is a dot (.), then

copyreadline we would incorrectly interpret the sequence as an end-of-copy
marker (\.). this can only happen in encodings that can "embed" ascii
characters as the second byte. one example of such sequence is
'\x5ca45c2e666f6f' in Big5 encoding. If you put that in a file, and
load it with COPY FROM, you'd incorrectly get an "end-of-copy marker
corrupt" error.
---
 src/backend/commands/copyfromparse.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/src/backend/commands/copyfromparse.c b/src/backend/commands/copyfromparse.c
index 798e18e013..c20ec482db 100644
--- a/src/backend/commands/copyfromparse.c
+++ b/src/backend/commands/copyfromparse.c
@@ -1084,7 +1084,7 @@ CopyReadLineText(CopyFromState cstate)
 				break;
 			}
 			else if (!cstate->opts.csv_mode)
-
+			{
 				/*
 				 * If we are here, it means we found a backslash followed by
 				 * something other than a period.  In non-CSV mode, anything
@@ -1095,8 +1095,17 @@ CopyReadLineText(CopyFromState cstate)
 				 * backslashes are not special, so we want to process the
 				 * character after the backslash just like a normal character,
 				 * so we don't increment in those cases.
+				 *
+				 * In principle, we would need to use pg_encoding_mblen() to
+				 * skip over the character in some encodings, like at the end
+				 * of the loop.  But if it's a multi-byte character, it cannot
+				 * have any special meaning and skipping isn't necessary for
+				 * correctness.  We can fall through to process it normally
+				 * instead.
 				 */
-				raw_buf_ptr++;
+				if (!IS_HIGHBIT_SET(c2))
+					raw_buf_ptr++;
+			}
 		}
 
 		/*
-- 
2.30.0


--------------97F3138F3612F1A4F9200D93--





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

* [PATCH 1/1] Fix a corner-case in COPY FROM backslash processing.
@ 2021-02-03 12:00  Heikki Linnakangas <[email protected]>
  0 siblings, 0 replies; 54+ messages in thread

From: Heikki Linnakangas @ 2021-02-03 12:00 UTC (permalink / raw)

If a multi-byte character is escaped with a backslash in TEXT mode input,
we didn't always treat the escape correctly. If:

- a multi-byte character is escaped with a backslash, and
- the second byte of the character is 0x5C, i.e. the ASCII code of a
  backslash (\), and
- the next character is a dot (.), then

copyreadline we would incorrectly interpret the sequence as an end-of-copy
marker (\.). this can only happen in encodings that can "embed" ascii
characters as the second byte. one example of such sequence is
'\x5ca45c2e666f6f' in Big5 encoding. If you put that in a file, and
load it with COPY FROM, you'd incorrectly get an "end-of-copy marker
corrupt" error.
---
 src/backend/commands/copyfromparse.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/src/backend/commands/copyfromparse.c b/src/backend/commands/copyfromparse.c
index 798e18e013..c20ec482db 100644
--- a/src/backend/commands/copyfromparse.c
+++ b/src/backend/commands/copyfromparse.c
@@ -1084,7 +1084,7 @@ CopyReadLineText(CopyFromState cstate)
 				break;
 			}
 			else if (!cstate->opts.csv_mode)
-
+			{
 				/*
 				 * If we are here, it means we found a backslash followed by
 				 * something other than a period.  In non-CSV mode, anything
@@ -1095,8 +1095,17 @@ CopyReadLineText(CopyFromState cstate)
 				 * backslashes are not special, so we want to process the
 				 * character after the backslash just like a normal character,
 				 * so we don't increment in those cases.
+				 *
+				 * In principle, we would need to use pg_encoding_mblen() to
+				 * skip over the character in some encodings, like at the end
+				 * of the loop.  But if it's a multi-byte character, it cannot
+				 * have any special meaning and skipping isn't necessary for
+				 * correctness.  We can fall through to process it normally
+				 * instead.
 				 */
-				raw_buf_ptr++;
+				if (!IS_HIGHBIT_SET(c2))
+					raw_buf_ptr++;
+			}
 		}
 
 		/*
-- 
2.30.0


--------------97F3138F3612F1A4F9200D93--





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

* [PATCH 1/1] Fix a corner-case in COPY FROM backslash processing.
@ 2021-02-03 12:00  Heikki Linnakangas <[email protected]>
  0 siblings, 0 replies; 54+ messages in thread

From: Heikki Linnakangas @ 2021-02-03 12:00 UTC (permalink / raw)

If a multi-byte character is escaped with a backslash in TEXT mode input,
we didn't always treat the escape correctly. If:

- a multi-byte character is escaped with a backslash, and
- the second byte of the character is 0x5C, i.e. the ASCII code of a
  backslash (\), and
- the next character is a dot (.), then

copyreadline we would incorrectly interpret the sequence as an end-of-copy
marker (\.). this can only happen in encodings that can "embed" ascii
characters as the second byte. one example of such sequence is
'\x5ca45c2e666f6f' in Big5 encoding. If you put that in a file, and
load it with COPY FROM, you'd incorrectly get an "end-of-copy marker
corrupt" error.
---
 src/backend/commands/copyfromparse.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/src/backend/commands/copyfromparse.c b/src/backend/commands/copyfromparse.c
index 798e18e013..c20ec482db 100644
--- a/src/backend/commands/copyfromparse.c
+++ b/src/backend/commands/copyfromparse.c
@@ -1084,7 +1084,7 @@ CopyReadLineText(CopyFromState cstate)
 				break;
 			}
 			else if (!cstate->opts.csv_mode)
-
+			{
 				/*
 				 * If we are here, it means we found a backslash followed by
 				 * something other than a period.  In non-CSV mode, anything
@@ -1095,8 +1095,17 @@ CopyReadLineText(CopyFromState cstate)
 				 * backslashes are not special, so we want to process the
 				 * character after the backslash just like a normal character,
 				 * so we don't increment in those cases.
+				 *
+				 * In principle, we would need to use pg_encoding_mblen() to
+				 * skip over the character in some encodings, like at the end
+				 * of the loop.  But if it's a multi-byte character, it cannot
+				 * have any special meaning and skipping isn't necessary for
+				 * correctness.  We can fall through to process it normally
+				 * instead.
 				 */
-				raw_buf_ptr++;
+				if (!IS_HIGHBIT_SET(c2))
+					raw_buf_ptr++;
+			}
 		}
 
 		/*
-- 
2.30.0


--------------97F3138F3612F1A4F9200D93--





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

* [PATCH 1/1] Fix a corner-case in COPY FROM backslash processing.
@ 2021-02-03 12:00  Heikki Linnakangas <[email protected]>
  0 siblings, 0 replies; 54+ messages in thread

From: Heikki Linnakangas @ 2021-02-03 12:00 UTC (permalink / raw)

If a multi-byte character is escaped with a backslash in TEXT mode input,
we didn't always treat the escape correctly. If:

- a multi-byte character is escaped with a backslash, and
- the second byte of the character is 0x5C, i.e. the ASCII code of a
  backslash (\), and
- the next character is a dot (.), then

copyreadline we would incorrectly interpret the sequence as an end-of-copy
marker (\.). this can only happen in encodings that can "embed" ascii
characters as the second byte. one example of such sequence is
'\x5ca45c2e666f6f' in Big5 encoding. If you put that in a file, and
load it with COPY FROM, you'd incorrectly get an "end-of-copy marker
corrupt" error.
---
 src/backend/commands/copyfromparse.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/src/backend/commands/copyfromparse.c b/src/backend/commands/copyfromparse.c
index 798e18e013..c20ec482db 100644
--- a/src/backend/commands/copyfromparse.c
+++ b/src/backend/commands/copyfromparse.c
@@ -1084,7 +1084,7 @@ CopyReadLineText(CopyFromState cstate)
 				break;
 			}
 			else if (!cstate->opts.csv_mode)
-
+			{
 				/*
 				 * If we are here, it means we found a backslash followed by
 				 * something other than a period.  In non-CSV mode, anything
@@ -1095,8 +1095,17 @@ CopyReadLineText(CopyFromState cstate)
 				 * backslashes are not special, so we want to process the
 				 * character after the backslash just like a normal character,
 				 * so we don't increment in those cases.
+				 *
+				 * In principle, we would need to use pg_encoding_mblen() to
+				 * skip over the character in some encodings, like at the end
+				 * of the loop.  But if it's a multi-byte character, it cannot
+				 * have any special meaning and skipping isn't necessary for
+				 * correctness.  We can fall through to process it normally
+				 * instead.
 				 */
-				raw_buf_ptr++;
+				if (!IS_HIGHBIT_SET(c2))
+					raw_buf_ptr++;
+			}
 		}
 
 		/*
-- 
2.30.0


--------------97F3138F3612F1A4F9200D93--





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

* [PATCH 1/1] Fix a corner-case in COPY FROM backslash processing.
@ 2021-02-03 12:00  Heikki Linnakangas <[email protected]>
  0 siblings, 0 replies; 54+ messages in thread

From: Heikki Linnakangas @ 2021-02-03 12:00 UTC (permalink / raw)

If a multi-byte character is escaped with a backslash in TEXT mode input,
we didn't always treat the escape correctly. If:

- a multi-byte character is escaped with a backslash, and
- the second byte of the character is 0x5C, i.e. the ASCII code of a
  backslash (\), and
- the next character is a dot (.), then

copyreadline we would incorrectly interpret the sequence as an end-of-copy
marker (\.). this can only happen in encodings that can "embed" ascii
characters as the second byte. one example of such sequence is
'\x5ca45c2e666f6f' in Big5 encoding. If you put that in a file, and
load it with COPY FROM, you'd incorrectly get an "end-of-copy marker
corrupt" error.
---
 src/backend/commands/copyfromparse.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/src/backend/commands/copyfromparse.c b/src/backend/commands/copyfromparse.c
index 798e18e013..c20ec482db 100644
--- a/src/backend/commands/copyfromparse.c
+++ b/src/backend/commands/copyfromparse.c
@@ -1084,7 +1084,7 @@ CopyReadLineText(CopyFromState cstate)
 				break;
 			}
 			else if (!cstate->opts.csv_mode)
-
+			{
 				/*
 				 * If we are here, it means we found a backslash followed by
 				 * something other than a period.  In non-CSV mode, anything
@@ -1095,8 +1095,17 @@ CopyReadLineText(CopyFromState cstate)
 				 * backslashes are not special, so we want to process the
 				 * character after the backslash just like a normal character,
 				 * so we don't increment in those cases.
+				 *
+				 * In principle, we would need to use pg_encoding_mblen() to
+				 * skip over the character in some encodings, like at the end
+				 * of the loop.  But if it's a multi-byte character, it cannot
+				 * have any special meaning and skipping isn't necessary for
+				 * correctness.  We can fall through to process it normally
+				 * instead.
 				 */
-				raw_buf_ptr++;
+				if (!IS_HIGHBIT_SET(c2))
+					raw_buf_ptr++;
+			}
 		}
 
 		/*
-- 
2.30.0


--------------97F3138F3612F1A4F9200D93--





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

* [PATCH 1/1] Fix a corner-case in COPY FROM backslash processing.
@ 2021-02-03 12:00  Heikki Linnakangas <[email protected]>
  0 siblings, 0 replies; 54+ messages in thread

From: Heikki Linnakangas @ 2021-02-03 12:00 UTC (permalink / raw)

If a multi-byte character is escaped with a backslash in TEXT mode input,
we didn't always treat the escape correctly. If:

- a multi-byte character is escaped with a backslash, and
- the second byte of the character is 0x5C, i.e. the ASCII code of a
  backslash (\), and
- the next character is a dot (.), then

copyreadline we would incorrectly interpret the sequence as an end-of-copy
marker (\.). this can only happen in encodings that can "embed" ascii
characters as the second byte. one example of such sequence is
'\x5ca45c2e666f6f' in Big5 encoding. If you put that in a file, and
load it with COPY FROM, you'd incorrectly get an "end-of-copy marker
corrupt" error.
---
 src/backend/commands/copyfromparse.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/src/backend/commands/copyfromparse.c b/src/backend/commands/copyfromparse.c
index 798e18e013..c20ec482db 100644
--- a/src/backend/commands/copyfromparse.c
+++ b/src/backend/commands/copyfromparse.c
@@ -1084,7 +1084,7 @@ CopyReadLineText(CopyFromState cstate)
 				break;
 			}
 			else if (!cstate->opts.csv_mode)
-
+			{
 				/*
 				 * If we are here, it means we found a backslash followed by
 				 * something other than a period.  In non-CSV mode, anything
@@ -1095,8 +1095,17 @@ CopyReadLineText(CopyFromState cstate)
 				 * backslashes are not special, so we want to process the
 				 * character after the backslash just like a normal character,
 				 * so we don't increment in those cases.
+				 *
+				 * In principle, we would need to use pg_encoding_mblen() to
+				 * skip over the character in some encodings, like at the end
+				 * of the loop.  But if it's a multi-byte character, it cannot
+				 * have any special meaning and skipping isn't necessary for
+				 * correctness.  We can fall through to process it normally
+				 * instead.
 				 */
-				raw_buf_ptr++;
+				if (!IS_HIGHBIT_SET(c2))
+					raw_buf_ptr++;
+			}
 		}
 
 		/*
-- 
2.30.0


--------------97F3138F3612F1A4F9200D93--





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

* [PATCH 1/1] Fix a corner-case in COPY FROM backslash processing.
@ 2021-02-03 12:00  Heikki Linnakangas <[email protected]>
  0 siblings, 0 replies; 54+ messages in thread

From: Heikki Linnakangas @ 2021-02-03 12:00 UTC (permalink / raw)

If a multi-byte character is escaped with a backslash in TEXT mode input,
we didn't always treat the escape correctly. If:

- a multi-byte character is escaped with a backslash, and
- the second byte of the character is 0x5C, i.e. the ASCII code of a
  backslash (\), and
- the next character is a dot (.), then

copyreadline we would incorrectly interpret the sequence as an end-of-copy
marker (\.). this can only happen in encodings that can "embed" ascii
characters as the second byte. one example of such sequence is
'\x5ca45c2e666f6f' in Big5 encoding. If you put that in a file, and
load it with COPY FROM, you'd incorrectly get an "end-of-copy marker
corrupt" error.
---
 src/backend/commands/copyfromparse.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/src/backend/commands/copyfromparse.c b/src/backend/commands/copyfromparse.c
index 798e18e013..c20ec482db 100644
--- a/src/backend/commands/copyfromparse.c
+++ b/src/backend/commands/copyfromparse.c
@@ -1084,7 +1084,7 @@ CopyReadLineText(CopyFromState cstate)
 				break;
 			}
 			else if (!cstate->opts.csv_mode)
-
+			{
 				/*
 				 * If we are here, it means we found a backslash followed by
 				 * something other than a period.  In non-CSV mode, anything
@@ -1095,8 +1095,17 @@ CopyReadLineText(CopyFromState cstate)
 				 * backslashes are not special, so we want to process the
 				 * character after the backslash just like a normal character,
 				 * so we don't increment in those cases.
+				 *
+				 * In principle, we would need to use pg_encoding_mblen() to
+				 * skip over the character in some encodings, like at the end
+				 * of the loop.  But if it's a multi-byte character, it cannot
+				 * have any special meaning and skipping isn't necessary for
+				 * correctness.  We can fall through to process it normally
+				 * instead.
 				 */
-				raw_buf_ptr++;
+				if (!IS_HIGHBIT_SET(c2))
+					raw_buf_ptr++;
+			}
 		}
 
 		/*
-- 
2.30.0


--------------97F3138F3612F1A4F9200D93--





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

* [PATCH v2 1/1] Fix a corner-case in COPY FROM backslash processing.
@ 2021-02-04 19:36  Heikki Linnakangas <[email protected]>
  0 siblings, 0 replies; 54+ messages in thread

From: Heikki Linnakangas @ 2021-02-04 19:36 UTC (permalink / raw)

If a multi-byte character is escaped with a backslash in TEXT mode input,
we didn't always treat the escape correctly. If:

- a multi-byte character is escaped with a backslash, and
- the second byte of the character is 0x5C, i.e. the ASCII code of a
  backslash (\), and
- the next character is a dot (.), then

CopyReadLineText function would incorrectly interpret the sequence as an
end-of-copy marker (\.). This can only happen in encodings that can
"embed" ascii characters as the second byte. One example of such sequence
is '\x5ca45c2e666f6f' in Big5 encoding. If you put that in a file, and
load it with COPY FROM, you'd incorrectly get an "end-of-copy marker
corrupt" error.

Backpatch to all supported versions.

Reviewed-by: John Naylor, Kyotaro Horiguchi
Discussion: https://www.postgresql.org/message-id/a897f84f-8dca-8798-3139-07da5bb38728%40iki.fi
---
 src/backend/commands/copyfromparse.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/src/backend/commands/copyfromparse.c b/src/backend/commands/copyfromparse.c
index b843d315b1..315b16fd7a 100644
--- a/src/backend/commands/copyfromparse.c
+++ b/src/backend/commands/copyfromparse.c
@@ -1084,7 +1084,7 @@ CopyReadLineText(CopyFromState cstate)
 				break;
 			}
 			else if (!cstate->opts.csv_mode)
-
+			{
 				/*
 				 * If we are here, it means we found a backslash followed by
 				 * something other than a period.  In non-CSV mode, anything
@@ -1095,8 +1095,16 @@ CopyReadLineText(CopyFromState cstate)
 				 * backslashes are not special, so we want to process the
 				 * character after the backslash just like a normal character,
 				 * so we don't increment in those cases.
+				 *
+				 * Set 'c' to skip whole character correctly in multi-byte
+				 * encodings.  If we don't have the whole character in the
+				 * buffer yet, we might loop back to process it, after all,
+				 * but that's OK because multi-byte characters cannot have any
+				 * special meaning.
 				 */
 				raw_buf_ptr++;
+				c = c2;
+			}
 		}
 
 		/*
-- 
2.30.0


--------------CFC80B40DFDD66DF067F288D--





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

* [PATCH v2 1/1] Fix a corner-case in COPY FROM backslash processing.
@ 2021-02-04 19:36  Heikki Linnakangas <[email protected]>
  0 siblings, 0 replies; 54+ messages in thread

From: Heikki Linnakangas @ 2021-02-04 19:36 UTC (permalink / raw)

If a multi-byte character is escaped with a backslash in TEXT mode input,
we didn't always treat the escape correctly. If:

- a multi-byte character is escaped with a backslash, and
- the second byte of the character is 0x5C, i.e. the ASCII code of a
  backslash (\), and
- the next character is a dot (.), then

CopyReadLineText function would incorrectly interpret the sequence as an
end-of-copy marker (\.). This can only happen in encodings that can
"embed" ascii characters as the second byte. One example of such sequence
is '\x5ca45c2e666f6f' in Big5 encoding. If you put that in a file, and
load it with COPY FROM, you'd incorrectly get an "end-of-copy marker
corrupt" error.

Backpatch to all supported versions.

Reviewed-by: John Naylor, Kyotaro Horiguchi
Discussion: https://www.postgresql.org/message-id/a897f84f-8dca-8798-3139-07da5bb38728%40iki.fi
---
 src/backend/commands/copyfromparse.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/src/backend/commands/copyfromparse.c b/src/backend/commands/copyfromparse.c
index b843d315b1..315b16fd7a 100644
--- a/src/backend/commands/copyfromparse.c
+++ b/src/backend/commands/copyfromparse.c
@@ -1084,7 +1084,7 @@ CopyReadLineText(CopyFromState cstate)
 				break;
 			}
 			else if (!cstate->opts.csv_mode)
-
+			{
 				/*
 				 * If we are here, it means we found a backslash followed by
 				 * something other than a period.  In non-CSV mode, anything
@@ -1095,8 +1095,16 @@ CopyReadLineText(CopyFromState cstate)
 				 * backslashes are not special, so we want to process the
 				 * character after the backslash just like a normal character,
 				 * so we don't increment in those cases.
+				 *
+				 * Set 'c' to skip whole character correctly in multi-byte
+				 * encodings.  If we don't have the whole character in the
+				 * buffer yet, we might loop back to process it, after all,
+				 * but that's OK because multi-byte characters cannot have any
+				 * special meaning.
 				 */
 				raw_buf_ptr++;
+				c = c2;
+			}
 		}
 
 		/*
-- 
2.30.0


--------------CFC80B40DFDD66DF067F288D--





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

* [PATCH v2 1/1] Fix a corner-case in COPY FROM backslash processing.
@ 2021-02-04 19:36  Heikki Linnakangas <[email protected]>
  0 siblings, 0 replies; 54+ messages in thread

From: Heikki Linnakangas @ 2021-02-04 19:36 UTC (permalink / raw)

If a multi-byte character is escaped with a backslash in TEXT mode input,
we didn't always treat the escape correctly. If:

- a multi-byte character is escaped with a backslash, and
- the second byte of the character is 0x5C, i.e. the ASCII code of a
  backslash (\), and
- the next character is a dot (.), then

CopyReadLineText function would incorrectly interpret the sequence as an
end-of-copy marker (\.). This can only happen in encodings that can
"embed" ascii characters as the second byte. One example of such sequence
is '\x5ca45c2e666f6f' in Big5 encoding. If you put that in a file, and
load it with COPY FROM, you'd incorrectly get an "end-of-copy marker
corrupt" error.

Backpatch to all supported versions.

Reviewed-by: John Naylor, Kyotaro Horiguchi
Discussion: https://www.postgresql.org/message-id/a897f84f-8dca-8798-3139-07da5bb38728%40iki.fi
---
 src/backend/commands/copyfromparse.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/src/backend/commands/copyfromparse.c b/src/backend/commands/copyfromparse.c
index b843d315b1..315b16fd7a 100644
--- a/src/backend/commands/copyfromparse.c
+++ b/src/backend/commands/copyfromparse.c
@@ -1084,7 +1084,7 @@ CopyReadLineText(CopyFromState cstate)
 				break;
 			}
 			else if (!cstate->opts.csv_mode)
-
+			{
 				/*
 				 * If we are here, it means we found a backslash followed by
 				 * something other than a period.  In non-CSV mode, anything
@@ -1095,8 +1095,16 @@ CopyReadLineText(CopyFromState cstate)
 				 * backslashes are not special, so we want to process the
 				 * character after the backslash just like a normal character,
 				 * so we don't increment in those cases.
+				 *
+				 * Set 'c' to skip whole character correctly in multi-byte
+				 * encodings.  If we don't have the whole character in the
+				 * buffer yet, we might loop back to process it, after all,
+				 * but that's OK because multi-byte characters cannot have any
+				 * special meaning.
 				 */
 				raw_buf_ptr++;
+				c = c2;
+			}
 		}
 
 		/*
-- 
2.30.0


--------------CFC80B40DFDD66DF067F288D--





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

* [PATCH v2 1/1] Fix a corner-case in COPY FROM backslash processing.
@ 2021-02-04 19:36  Heikki Linnakangas <[email protected]>
  0 siblings, 0 replies; 54+ messages in thread

From: Heikki Linnakangas @ 2021-02-04 19:36 UTC (permalink / raw)

If a multi-byte character is escaped with a backslash in TEXT mode input,
we didn't always treat the escape correctly. If:

- a multi-byte character is escaped with a backslash, and
- the second byte of the character is 0x5C, i.e. the ASCII code of a
  backslash (\), and
- the next character is a dot (.), then

CopyReadLineText function would incorrectly interpret the sequence as an
end-of-copy marker (\.). This can only happen in encodings that can
"embed" ascii characters as the second byte. One example of such sequence
is '\x5ca45c2e666f6f' in Big5 encoding. If you put that in a file, and
load it with COPY FROM, you'd incorrectly get an "end-of-copy marker
corrupt" error.

Backpatch to all supported versions.

Reviewed-by: John Naylor, Kyotaro Horiguchi
Discussion: https://www.postgresql.org/message-id/a897f84f-8dca-8798-3139-07da5bb38728%40iki.fi
---
 src/backend/commands/copyfromparse.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/src/backend/commands/copyfromparse.c b/src/backend/commands/copyfromparse.c
index b843d315b1..315b16fd7a 100644
--- a/src/backend/commands/copyfromparse.c
+++ b/src/backend/commands/copyfromparse.c
@@ -1084,7 +1084,7 @@ CopyReadLineText(CopyFromState cstate)
 				break;
 			}
 			else if (!cstate->opts.csv_mode)
-
+			{
 				/*
 				 * If we are here, it means we found a backslash followed by
 				 * something other than a period.  In non-CSV mode, anything
@@ -1095,8 +1095,16 @@ CopyReadLineText(CopyFromState cstate)
 				 * backslashes are not special, so we want to process the
 				 * character after the backslash just like a normal character,
 				 * so we don't increment in those cases.
+				 *
+				 * Set 'c' to skip whole character correctly in multi-byte
+				 * encodings.  If we don't have the whole character in the
+				 * buffer yet, we might loop back to process it, after all,
+				 * but that's OK because multi-byte characters cannot have any
+				 * special meaning.
 				 */
 				raw_buf_ptr++;
+				c = c2;
+			}
 		}
 
 		/*
-- 
2.30.0


--------------CFC80B40DFDD66DF067F288D--





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

* [PATCH v2 1/1] Fix a corner-case in COPY FROM backslash processing.
@ 2021-02-04 19:36  Heikki Linnakangas <[email protected]>
  0 siblings, 0 replies; 54+ messages in thread

From: Heikki Linnakangas @ 2021-02-04 19:36 UTC (permalink / raw)

If a multi-byte character is escaped with a backslash in TEXT mode input,
we didn't always treat the escape correctly. If:

- a multi-byte character is escaped with a backslash, and
- the second byte of the character is 0x5C, i.e. the ASCII code of a
  backslash (\), and
- the next character is a dot (.), then

CopyReadLineText function would incorrectly interpret the sequence as an
end-of-copy marker (\.). This can only happen in encodings that can
"embed" ascii characters as the second byte. One example of such sequence
is '\x5ca45c2e666f6f' in Big5 encoding. If you put that in a file, and
load it with COPY FROM, you'd incorrectly get an "end-of-copy marker
corrupt" error.

Backpatch to all supported versions.

Reviewed-by: John Naylor, Kyotaro Horiguchi
Discussion: https://www.postgresql.org/message-id/a897f84f-8dca-8798-3139-07da5bb38728%40iki.fi
---
 src/backend/commands/copyfromparse.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/src/backend/commands/copyfromparse.c b/src/backend/commands/copyfromparse.c
index b843d315b1..315b16fd7a 100644
--- a/src/backend/commands/copyfromparse.c
+++ b/src/backend/commands/copyfromparse.c
@@ -1084,7 +1084,7 @@ CopyReadLineText(CopyFromState cstate)
 				break;
 			}
 			else if (!cstate->opts.csv_mode)
-
+			{
 				/*
 				 * If we are here, it means we found a backslash followed by
 				 * something other than a period.  In non-CSV mode, anything
@@ -1095,8 +1095,16 @@ CopyReadLineText(CopyFromState cstate)
 				 * backslashes are not special, so we want to process the
 				 * character after the backslash just like a normal character,
 				 * so we don't increment in those cases.
+				 *
+				 * Set 'c' to skip whole character correctly in multi-byte
+				 * encodings.  If we don't have the whole character in the
+				 * buffer yet, we might loop back to process it, after all,
+				 * but that's OK because multi-byte characters cannot have any
+				 * special meaning.
 				 */
 				raw_buf_ptr++;
+				c = c2;
+			}
 		}
 
 		/*
-- 
2.30.0


--------------CFC80B40DFDD66DF067F288D--





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

* [PATCH v2 1/1] Fix a corner-case in COPY FROM backslash processing.
@ 2021-02-04 19:36  Heikki Linnakangas <[email protected]>
  0 siblings, 0 replies; 54+ messages in thread

From: Heikki Linnakangas @ 2021-02-04 19:36 UTC (permalink / raw)

If a multi-byte character is escaped with a backslash in TEXT mode input,
we didn't always treat the escape correctly. If:

- a multi-byte character is escaped with a backslash, and
- the second byte of the character is 0x5C, i.e. the ASCII code of a
  backslash (\), and
- the next character is a dot (.), then

CopyReadLineText function would incorrectly interpret the sequence as an
end-of-copy marker (\.). This can only happen in encodings that can
"embed" ascii characters as the second byte. One example of such sequence
is '\x5ca45c2e666f6f' in Big5 encoding. If you put that in a file, and
load it with COPY FROM, you'd incorrectly get an "end-of-copy marker
corrupt" error.

Backpatch to all supported versions.

Reviewed-by: John Naylor, Kyotaro Horiguchi
Discussion: https://www.postgresql.org/message-id/a897f84f-8dca-8798-3139-07da5bb38728%40iki.fi
---
 src/backend/commands/copyfromparse.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/src/backend/commands/copyfromparse.c b/src/backend/commands/copyfromparse.c
index b843d315b1..315b16fd7a 100644
--- a/src/backend/commands/copyfromparse.c
+++ b/src/backend/commands/copyfromparse.c
@@ -1084,7 +1084,7 @@ CopyReadLineText(CopyFromState cstate)
 				break;
 			}
 			else if (!cstate->opts.csv_mode)
-
+			{
 				/*
 				 * If we are here, it means we found a backslash followed by
 				 * something other than a period.  In non-CSV mode, anything
@@ -1095,8 +1095,16 @@ CopyReadLineText(CopyFromState cstate)
 				 * backslashes are not special, so we want to process the
 				 * character after the backslash just like a normal character,
 				 * so we don't increment in those cases.
+				 *
+				 * Set 'c' to skip whole character correctly in multi-byte
+				 * encodings.  If we don't have the whole character in the
+				 * buffer yet, we might loop back to process it, after all,
+				 * but that's OK because multi-byte characters cannot have any
+				 * special meaning.
 				 */
 				raw_buf_ptr++;
+				c = c2;
+			}
 		}
 
 		/*
-- 
2.30.0


--------------CFC80B40DFDD66DF067F288D--





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

* [PATCH v2 1/1] Fix a corner-case in COPY FROM backslash processing.
@ 2021-02-04 19:36  Heikki Linnakangas <[email protected]>
  0 siblings, 0 replies; 54+ messages in thread

From: Heikki Linnakangas @ 2021-02-04 19:36 UTC (permalink / raw)

If a multi-byte character is escaped with a backslash in TEXT mode input,
we didn't always treat the escape correctly. If:

- a multi-byte character is escaped with a backslash, and
- the second byte of the character is 0x5C, i.e. the ASCII code of a
  backslash (\), and
- the next character is a dot (.), then

CopyReadLineText function would incorrectly interpret the sequence as an
end-of-copy marker (\.). This can only happen in encodings that can
"embed" ascii characters as the second byte. One example of such sequence
is '\x5ca45c2e666f6f' in Big5 encoding. If you put that in a file, and
load it with COPY FROM, you'd incorrectly get an "end-of-copy marker
corrupt" error.

Backpatch to all supported versions.

Reviewed-by: John Naylor, Kyotaro Horiguchi
Discussion: https://www.postgresql.org/message-id/a897f84f-8dca-8798-3139-07da5bb38728%40iki.fi
---
 src/backend/commands/copyfromparse.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/src/backend/commands/copyfromparse.c b/src/backend/commands/copyfromparse.c
index b843d315b1..315b16fd7a 100644
--- a/src/backend/commands/copyfromparse.c
+++ b/src/backend/commands/copyfromparse.c
@@ -1084,7 +1084,7 @@ CopyReadLineText(CopyFromState cstate)
 				break;
 			}
 			else if (!cstate->opts.csv_mode)
-
+			{
 				/*
 				 * If we are here, it means we found a backslash followed by
 				 * something other than a period.  In non-CSV mode, anything
@@ -1095,8 +1095,16 @@ CopyReadLineText(CopyFromState cstate)
 				 * backslashes are not special, so we want to process the
 				 * character after the backslash just like a normal character,
 				 * so we don't increment in those cases.
+				 *
+				 * Set 'c' to skip whole character correctly in multi-byte
+				 * encodings.  If we don't have the whole character in the
+				 * buffer yet, we might loop back to process it, after all,
+				 * but that's OK because multi-byte characters cannot have any
+				 * special meaning.
 				 */
 				raw_buf_ptr++;
+				c = c2;
+			}
 		}
 
 		/*
-- 
2.30.0


--------------CFC80B40DFDD66DF067F288D--





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

* [PATCH v2 1/1] Fix a corner-case in COPY FROM backslash processing.
@ 2021-02-04 19:36  Heikki Linnakangas <[email protected]>
  0 siblings, 0 replies; 54+ messages in thread

From: Heikki Linnakangas @ 2021-02-04 19:36 UTC (permalink / raw)

If a multi-byte character is escaped with a backslash in TEXT mode input,
we didn't always treat the escape correctly. If:

- a multi-byte character is escaped with a backslash, and
- the second byte of the character is 0x5C, i.e. the ASCII code of a
  backslash (\), and
- the next character is a dot (.), then

CopyReadLineText function would incorrectly interpret the sequence as an
end-of-copy marker (\.). This can only happen in encodings that can
"embed" ascii characters as the second byte. One example of such sequence
is '\x5ca45c2e666f6f' in Big5 encoding. If you put that in a file, and
load it with COPY FROM, you'd incorrectly get an "end-of-copy marker
corrupt" error.

Backpatch to all supported versions.

Reviewed-by: John Naylor, Kyotaro Horiguchi
Discussion: https://www.postgresql.org/message-id/a897f84f-8dca-8798-3139-07da5bb38728%40iki.fi
---
 src/backend/commands/copyfromparse.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/src/backend/commands/copyfromparse.c b/src/backend/commands/copyfromparse.c
index b843d315b1..315b16fd7a 100644
--- a/src/backend/commands/copyfromparse.c
+++ b/src/backend/commands/copyfromparse.c
@@ -1084,7 +1084,7 @@ CopyReadLineText(CopyFromState cstate)
 				break;
 			}
 			else if (!cstate->opts.csv_mode)
-
+			{
 				/*
 				 * If we are here, it means we found a backslash followed by
 				 * something other than a period.  In non-CSV mode, anything
@@ -1095,8 +1095,16 @@ CopyReadLineText(CopyFromState cstate)
 				 * backslashes are not special, so we want to process the
 				 * character after the backslash just like a normal character,
 				 * so we don't increment in those cases.
+				 *
+				 * Set 'c' to skip whole character correctly in multi-byte
+				 * encodings.  If we don't have the whole character in the
+				 * buffer yet, we might loop back to process it, after all,
+				 * but that's OK because multi-byte characters cannot have any
+				 * special meaning.
 				 */
 				raw_buf_ptr++;
+				c = c2;
+			}
 		}
 
 		/*
-- 
2.30.0


--------------CFC80B40DFDD66DF067F288D--





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

* [PATCH v2 1/1] Fix a corner-case in COPY FROM backslash processing.
@ 2021-02-04 19:36  Heikki Linnakangas <[email protected]>
  0 siblings, 0 replies; 54+ messages in thread

From: Heikki Linnakangas @ 2021-02-04 19:36 UTC (permalink / raw)

If a multi-byte character is escaped with a backslash in TEXT mode input,
we didn't always treat the escape correctly. If:

- a multi-byte character is escaped with a backslash, and
- the second byte of the character is 0x5C, i.e. the ASCII code of a
  backslash (\), and
- the next character is a dot (.), then

CopyReadLineText function would incorrectly interpret the sequence as an
end-of-copy marker (\.). This can only happen in encodings that can
"embed" ascii characters as the second byte. One example of such sequence
is '\x5ca45c2e666f6f' in Big5 encoding. If you put that in a file, and
load it with COPY FROM, you'd incorrectly get an "end-of-copy marker
corrupt" error.

Backpatch to all supported versions.

Reviewed-by: John Naylor, Kyotaro Horiguchi
Discussion: https://www.postgresql.org/message-id/a897f84f-8dca-8798-3139-07da5bb38728%40iki.fi
---
 src/backend/commands/copyfromparse.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/src/backend/commands/copyfromparse.c b/src/backend/commands/copyfromparse.c
index b843d315b1..315b16fd7a 100644
--- a/src/backend/commands/copyfromparse.c
+++ b/src/backend/commands/copyfromparse.c
@@ -1084,7 +1084,7 @@ CopyReadLineText(CopyFromState cstate)
 				break;
 			}
 			else if (!cstate->opts.csv_mode)
-
+			{
 				/*
 				 * If we are here, it means we found a backslash followed by
 				 * something other than a period.  In non-CSV mode, anything
@@ -1095,8 +1095,16 @@ CopyReadLineText(CopyFromState cstate)
 				 * backslashes are not special, so we want to process the
 				 * character after the backslash just like a normal character,
 				 * so we don't increment in those cases.
+				 *
+				 * Set 'c' to skip whole character correctly in multi-byte
+				 * encodings.  If we don't have the whole character in the
+				 * buffer yet, we might loop back to process it, after all,
+				 * but that's OK because multi-byte characters cannot have any
+				 * special meaning.
 				 */
 				raw_buf_ptr++;
+				c = c2;
+			}
 		}
 
 		/*
-- 
2.30.0


--------------CFC80B40DFDD66DF067F288D--





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

* [PATCH v2 1/1] Fix a corner-case in COPY FROM backslash processing.
@ 2021-02-04 19:36  Heikki Linnakangas <[email protected]>
  0 siblings, 0 replies; 54+ messages in thread

From: Heikki Linnakangas @ 2021-02-04 19:36 UTC (permalink / raw)

If a multi-byte character is escaped with a backslash in TEXT mode input,
we didn't always treat the escape correctly. If:

- a multi-byte character is escaped with a backslash, and
- the second byte of the character is 0x5C, i.e. the ASCII code of a
  backslash (\), and
- the next character is a dot (.), then

CopyReadLineText function would incorrectly interpret the sequence as an
end-of-copy marker (\.). This can only happen in encodings that can
"embed" ascii characters as the second byte. One example of such sequence
is '\x5ca45c2e666f6f' in Big5 encoding. If you put that in a file, and
load it with COPY FROM, you'd incorrectly get an "end-of-copy marker
corrupt" error.

Backpatch to all supported versions.

Reviewed-by: John Naylor, Kyotaro Horiguchi
Discussion: https://www.postgresql.org/message-id/a897f84f-8dca-8798-3139-07da5bb38728%40iki.fi
---
 src/backend/commands/copyfromparse.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/src/backend/commands/copyfromparse.c b/src/backend/commands/copyfromparse.c
index b843d315b1..315b16fd7a 100644
--- a/src/backend/commands/copyfromparse.c
+++ b/src/backend/commands/copyfromparse.c
@@ -1084,7 +1084,7 @@ CopyReadLineText(CopyFromState cstate)
 				break;
 			}
 			else if (!cstate->opts.csv_mode)
-
+			{
 				/*
 				 * If we are here, it means we found a backslash followed by
 				 * something other than a period.  In non-CSV mode, anything
@@ -1095,8 +1095,16 @@ CopyReadLineText(CopyFromState cstate)
 				 * backslashes are not special, so we want to process the
 				 * character after the backslash just like a normal character,
 				 * so we don't increment in those cases.
+				 *
+				 * Set 'c' to skip whole character correctly in multi-byte
+				 * encodings.  If we don't have the whole character in the
+				 * buffer yet, we might loop back to process it, after all,
+				 * but that's OK because multi-byte characters cannot have any
+				 * special meaning.
 				 */
 				raw_buf_ptr++;
+				c = c2;
+			}
 		}
 
 		/*
-- 
2.30.0


--------------CFC80B40DFDD66DF067F288D--





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

* [PATCH v2 1/1] Fix a corner-case in COPY FROM backslash processing.
@ 2021-02-04 19:36  Heikki Linnakangas <[email protected]>
  0 siblings, 0 replies; 54+ messages in thread

From: Heikki Linnakangas @ 2021-02-04 19:36 UTC (permalink / raw)

If a multi-byte character is escaped with a backslash in TEXT mode input,
we didn't always treat the escape correctly. If:

- a multi-byte character is escaped with a backslash, and
- the second byte of the character is 0x5C, i.e. the ASCII code of a
  backslash (\), and
- the next character is a dot (.), then

CopyReadLineText function would incorrectly interpret the sequence as an
end-of-copy marker (\.). This can only happen in encodings that can
"embed" ascii characters as the second byte. One example of such sequence
is '\x5ca45c2e666f6f' in Big5 encoding. If you put that in a file, and
load it with COPY FROM, you'd incorrectly get an "end-of-copy marker
corrupt" error.

Backpatch to all supported versions.

Reviewed-by: John Naylor, Kyotaro Horiguchi
Discussion: https://www.postgresql.org/message-id/a897f84f-8dca-8798-3139-07da5bb38728%40iki.fi
---
 src/backend/commands/copyfromparse.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/src/backend/commands/copyfromparse.c b/src/backend/commands/copyfromparse.c
index b843d315b1..315b16fd7a 100644
--- a/src/backend/commands/copyfromparse.c
+++ b/src/backend/commands/copyfromparse.c
@@ -1084,7 +1084,7 @@ CopyReadLineText(CopyFromState cstate)
 				break;
 			}
 			else if (!cstate->opts.csv_mode)
-
+			{
 				/*
 				 * If we are here, it means we found a backslash followed by
 				 * something other than a period.  In non-CSV mode, anything
@@ -1095,8 +1095,16 @@ CopyReadLineText(CopyFromState cstate)
 				 * backslashes are not special, so we want to process the
 				 * character after the backslash just like a normal character,
 				 * so we don't increment in those cases.
+				 *
+				 * Set 'c' to skip whole character correctly in multi-byte
+				 * encodings.  If we don't have the whole character in the
+				 * buffer yet, we might loop back to process it, after all,
+				 * but that's OK because multi-byte characters cannot have any
+				 * special meaning.
 				 */
 				raw_buf_ptr++;
+				c = c2;
+			}
 		}
 
 		/*
-- 
2.30.0


--------------CFC80B40DFDD66DF067F288D--





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

* [PATCH v2 1/1] Fix a corner-case in COPY FROM backslash processing.
@ 2021-02-04 19:36  Heikki Linnakangas <[email protected]>
  0 siblings, 0 replies; 54+ messages in thread

From: Heikki Linnakangas @ 2021-02-04 19:36 UTC (permalink / raw)

If a multi-byte character is escaped with a backslash in TEXT mode input,
we didn't always treat the escape correctly. If:

- a multi-byte character is escaped with a backslash, and
- the second byte of the character is 0x5C, i.e. the ASCII code of a
  backslash (\), and
- the next character is a dot (.), then

CopyReadLineText function would incorrectly interpret the sequence as an
end-of-copy marker (\.). This can only happen in encodings that can
"embed" ascii characters as the second byte. One example of such sequence
is '\x5ca45c2e666f6f' in Big5 encoding. If you put that in a file, and
load it with COPY FROM, you'd incorrectly get an "end-of-copy marker
corrupt" error.

Backpatch to all supported versions.

Reviewed-by: John Naylor, Kyotaro Horiguchi
Discussion: https://www.postgresql.org/message-id/a897f84f-8dca-8798-3139-07da5bb38728%40iki.fi
---
 src/backend/commands/copyfromparse.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/src/backend/commands/copyfromparse.c b/src/backend/commands/copyfromparse.c
index b843d315b1..315b16fd7a 100644
--- a/src/backend/commands/copyfromparse.c
+++ b/src/backend/commands/copyfromparse.c
@@ -1084,7 +1084,7 @@ CopyReadLineText(CopyFromState cstate)
 				break;
 			}
 			else if (!cstate->opts.csv_mode)
-
+			{
 				/*
 				 * If we are here, it means we found a backslash followed by
 				 * something other than a period.  In non-CSV mode, anything
@@ -1095,8 +1095,16 @@ CopyReadLineText(CopyFromState cstate)
 				 * backslashes are not special, so we want to process the
 				 * character after the backslash just like a normal character,
 				 * so we don't increment in those cases.
+				 *
+				 * Set 'c' to skip whole character correctly in multi-byte
+				 * encodings.  If we don't have the whole character in the
+				 * buffer yet, we might loop back to process it, after all,
+				 * but that's OK because multi-byte characters cannot have any
+				 * special meaning.
 				 */
 				raw_buf_ptr++;
+				c = c2;
+			}
 		}
 
 		/*
-- 
2.30.0


--------------CFC80B40DFDD66DF067F288D--





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

* [PATCH v2 1/1] Fix a corner-case in COPY FROM backslash processing.
@ 2021-02-04 19:36  Heikki Linnakangas <[email protected]>
  0 siblings, 0 replies; 54+ messages in thread

From: Heikki Linnakangas @ 2021-02-04 19:36 UTC (permalink / raw)

If a multi-byte character is escaped with a backslash in TEXT mode input,
we didn't always treat the escape correctly. If:

- a multi-byte character is escaped with a backslash, and
- the second byte of the character is 0x5C, i.e. the ASCII code of a
  backslash (\), and
- the next character is a dot (.), then

CopyReadLineText function would incorrectly interpret the sequence as an
end-of-copy marker (\.). This can only happen in encodings that can
"embed" ascii characters as the second byte. One example of such sequence
is '\x5ca45c2e666f6f' in Big5 encoding. If you put that in a file, and
load it with COPY FROM, you'd incorrectly get an "end-of-copy marker
corrupt" error.

Backpatch to all supported versions.

Reviewed-by: John Naylor, Kyotaro Horiguchi
Discussion: https://www.postgresql.org/message-id/a897f84f-8dca-8798-3139-07da5bb38728%40iki.fi
---
 src/backend/commands/copyfromparse.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/src/backend/commands/copyfromparse.c b/src/backend/commands/copyfromparse.c
index b843d315b1..315b16fd7a 100644
--- a/src/backend/commands/copyfromparse.c
+++ b/src/backend/commands/copyfromparse.c
@@ -1084,7 +1084,7 @@ CopyReadLineText(CopyFromState cstate)
 				break;
 			}
 			else if (!cstate->opts.csv_mode)
-
+			{
 				/*
 				 * If we are here, it means we found a backslash followed by
 				 * something other than a period.  In non-CSV mode, anything
@@ -1095,8 +1095,16 @@ CopyReadLineText(CopyFromState cstate)
 				 * backslashes are not special, so we want to process the
 				 * character after the backslash just like a normal character,
 				 * so we don't increment in those cases.
+				 *
+				 * Set 'c' to skip whole character correctly in multi-byte
+				 * encodings.  If we don't have the whole character in the
+				 * buffer yet, we might loop back to process it, after all,
+				 * but that's OK because multi-byte characters cannot have any
+				 * special meaning.
 				 */
 				raw_buf_ptr++;
+				c = c2;
+			}
 		}
 
 		/*
-- 
2.30.0


--------------CFC80B40DFDD66DF067F288D--





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

* [PATCH v2 1/1] Fix a corner-case in COPY FROM backslash processing.
@ 2021-02-04 19:36  Heikki Linnakangas <[email protected]>
  0 siblings, 0 replies; 54+ messages in thread

From: Heikki Linnakangas @ 2021-02-04 19:36 UTC (permalink / raw)

If a multi-byte character is escaped with a backslash in TEXT mode input,
we didn't always treat the escape correctly. If:

- a multi-byte character is escaped with a backslash, and
- the second byte of the character is 0x5C, i.e. the ASCII code of a
  backslash (\), and
- the next character is a dot (.), then

CopyReadLineText function would incorrectly interpret the sequence as an
end-of-copy marker (\.). This can only happen in encodings that can
"embed" ascii characters as the second byte. One example of such sequence
is '\x5ca45c2e666f6f' in Big5 encoding. If you put that in a file, and
load it with COPY FROM, you'd incorrectly get an "end-of-copy marker
corrupt" error.

Backpatch to all supported versions.

Reviewed-by: John Naylor, Kyotaro Horiguchi
Discussion: https://www.postgresql.org/message-id/a897f84f-8dca-8798-3139-07da5bb38728%40iki.fi
---
 src/backend/commands/copyfromparse.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/src/backend/commands/copyfromparse.c b/src/backend/commands/copyfromparse.c
index b843d315b1..315b16fd7a 100644
--- a/src/backend/commands/copyfromparse.c
+++ b/src/backend/commands/copyfromparse.c
@@ -1084,7 +1084,7 @@ CopyReadLineText(CopyFromState cstate)
 				break;
 			}
 			else if (!cstate->opts.csv_mode)
-
+			{
 				/*
 				 * If we are here, it means we found a backslash followed by
 				 * something other than a period.  In non-CSV mode, anything
@@ -1095,8 +1095,16 @@ CopyReadLineText(CopyFromState cstate)
 				 * backslashes are not special, so we want to process the
 				 * character after the backslash just like a normal character,
 				 * so we don't increment in those cases.
+				 *
+				 * Set 'c' to skip whole character correctly in multi-byte
+				 * encodings.  If we don't have the whole character in the
+				 * buffer yet, we might loop back to process it, after all,
+				 * but that's OK because multi-byte characters cannot have any
+				 * special meaning.
 				 */
 				raw_buf_ptr++;
+				c = c2;
+			}
 		}
 
 		/*
-- 
2.30.0


--------------CFC80B40DFDD66DF067F288D--





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

* [PATCH v2 1/1] Fix a corner-case in COPY FROM backslash processing.
@ 2021-02-04 19:36  Heikki Linnakangas <[email protected]>
  0 siblings, 0 replies; 54+ messages in thread

From: Heikki Linnakangas @ 2021-02-04 19:36 UTC (permalink / raw)

If a multi-byte character is escaped with a backslash in TEXT mode input,
we didn't always treat the escape correctly. If:

- a multi-byte character is escaped with a backslash, and
- the second byte of the character is 0x5C, i.e. the ASCII code of a
  backslash (\), and
- the next character is a dot (.), then

CopyReadLineText function would incorrectly interpret the sequence as an
end-of-copy marker (\.). This can only happen in encodings that can
"embed" ascii characters as the second byte. One example of such sequence
is '\x5ca45c2e666f6f' in Big5 encoding. If you put that in a file, and
load it with COPY FROM, you'd incorrectly get an "end-of-copy marker
corrupt" error.

Backpatch to all supported versions.

Reviewed-by: John Naylor, Kyotaro Horiguchi
Discussion: https://www.postgresql.org/message-id/a897f84f-8dca-8798-3139-07da5bb38728%40iki.fi
---
 src/backend/commands/copyfromparse.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/src/backend/commands/copyfromparse.c b/src/backend/commands/copyfromparse.c
index b843d315b1..315b16fd7a 100644
--- a/src/backend/commands/copyfromparse.c
+++ b/src/backend/commands/copyfromparse.c
@@ -1084,7 +1084,7 @@ CopyReadLineText(CopyFromState cstate)
 				break;
 			}
 			else if (!cstate->opts.csv_mode)
-
+			{
 				/*
 				 * If we are here, it means we found a backslash followed by
 				 * something other than a period.  In non-CSV mode, anything
@@ -1095,8 +1095,16 @@ CopyReadLineText(CopyFromState cstate)
 				 * backslashes are not special, so we want to process the
 				 * character after the backslash just like a normal character,
 				 * so we don't increment in those cases.
+				 *
+				 * Set 'c' to skip whole character correctly in multi-byte
+				 * encodings.  If we don't have the whole character in the
+				 * buffer yet, we might loop back to process it, after all,
+				 * but that's OK because multi-byte characters cannot have any
+				 * special meaning.
 				 */
 				raw_buf_ptr++;
+				c = c2;
+			}
 		}
 
 		/*
-- 
2.30.0


--------------CFC80B40DFDD66DF067F288D--





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

* [PATCH v2 1/1] Fix a corner-case in COPY FROM backslash processing.
@ 2021-02-04 19:36  Heikki Linnakangas <[email protected]>
  0 siblings, 0 replies; 54+ messages in thread

From: Heikki Linnakangas @ 2021-02-04 19:36 UTC (permalink / raw)

If a multi-byte character is escaped with a backslash in TEXT mode input,
we didn't always treat the escape correctly. If:

- a multi-byte character is escaped with a backslash, and
- the second byte of the character is 0x5C, i.e. the ASCII code of a
  backslash (\), and
- the next character is a dot (.), then

CopyReadLineText function would incorrectly interpret the sequence as an
end-of-copy marker (\.). This can only happen in encodings that can
"embed" ascii characters as the second byte. One example of such sequence
is '\x5ca45c2e666f6f' in Big5 encoding. If you put that in a file, and
load it with COPY FROM, you'd incorrectly get an "end-of-copy marker
corrupt" error.

Backpatch to all supported versions.

Reviewed-by: John Naylor, Kyotaro Horiguchi
Discussion: https://www.postgresql.org/message-id/a897f84f-8dca-8798-3139-07da5bb38728%40iki.fi
---
 src/backend/commands/copyfromparse.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/src/backend/commands/copyfromparse.c b/src/backend/commands/copyfromparse.c
index b843d315b1..315b16fd7a 100644
--- a/src/backend/commands/copyfromparse.c
+++ b/src/backend/commands/copyfromparse.c
@@ -1084,7 +1084,7 @@ CopyReadLineText(CopyFromState cstate)
 				break;
 			}
 			else if (!cstate->opts.csv_mode)
-
+			{
 				/*
 				 * If we are here, it means we found a backslash followed by
 				 * something other than a period.  In non-CSV mode, anything
@@ -1095,8 +1095,16 @@ CopyReadLineText(CopyFromState cstate)
 				 * backslashes are not special, so we want to process the
 				 * character after the backslash just like a normal character,
 				 * so we don't increment in those cases.
+				 *
+				 * Set 'c' to skip whole character correctly in multi-byte
+				 * encodings.  If we don't have the whole character in the
+				 * buffer yet, we might loop back to process it, after all,
+				 * but that's OK because multi-byte characters cannot have any
+				 * special meaning.
 				 */
 				raw_buf_ptr++;
+				c = c2;
+			}
 		}
 
 		/*
-- 
2.30.0


--------------CFC80B40DFDD66DF067F288D--





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

* [PATCH v2 1/1] Fix a corner-case in COPY FROM backslash processing.
@ 2021-02-04 19:36  Heikki Linnakangas <[email protected]>
  0 siblings, 0 replies; 54+ messages in thread

From: Heikki Linnakangas @ 2021-02-04 19:36 UTC (permalink / raw)

If a multi-byte character is escaped with a backslash in TEXT mode input,
we didn't always treat the escape correctly. If:

- a multi-byte character is escaped with a backslash, and
- the second byte of the character is 0x5C, i.e. the ASCII code of a
  backslash (\), and
- the next character is a dot (.), then

CopyReadLineText function would incorrectly interpret the sequence as an
end-of-copy marker (\.). This can only happen in encodings that can
"embed" ascii characters as the second byte. One example of such sequence
is '\x5ca45c2e666f6f' in Big5 encoding. If you put that in a file, and
load it with COPY FROM, you'd incorrectly get an "end-of-copy marker
corrupt" error.

Backpatch to all supported versions.

Reviewed-by: John Naylor, Kyotaro Horiguchi
Discussion: https://www.postgresql.org/message-id/a897f84f-8dca-8798-3139-07da5bb38728%40iki.fi
---
 src/backend/commands/copyfromparse.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/src/backend/commands/copyfromparse.c b/src/backend/commands/copyfromparse.c
index b843d315b1..315b16fd7a 100644
--- a/src/backend/commands/copyfromparse.c
+++ b/src/backend/commands/copyfromparse.c
@@ -1084,7 +1084,7 @@ CopyReadLineText(CopyFromState cstate)
 				break;
 			}
 			else if (!cstate->opts.csv_mode)
-
+			{
 				/*
 				 * If we are here, it means we found a backslash followed by
 				 * something other than a period.  In non-CSV mode, anything
@@ -1095,8 +1095,16 @@ CopyReadLineText(CopyFromState cstate)
 				 * backslashes are not special, so we want to process the
 				 * character after the backslash just like a normal character,
 				 * so we don't increment in those cases.
+				 *
+				 * Set 'c' to skip whole character correctly in multi-byte
+				 * encodings.  If we don't have the whole character in the
+				 * buffer yet, we might loop back to process it, after all,
+				 * but that's OK because multi-byte characters cannot have any
+				 * special meaning.
 				 */
 				raw_buf_ptr++;
+				c = c2;
+			}
 		}
 
 		/*
-- 
2.30.0


--------------CFC80B40DFDD66DF067F288D--





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

* [PATCH v2 1/1] Fix a corner-case in COPY FROM backslash processing.
@ 2021-02-04 19:36  Heikki Linnakangas <[email protected]>
  0 siblings, 0 replies; 54+ messages in thread

From: Heikki Linnakangas @ 2021-02-04 19:36 UTC (permalink / raw)

If a multi-byte character is escaped with a backslash in TEXT mode input,
we didn't always treat the escape correctly. If:

- a multi-byte character is escaped with a backslash, and
- the second byte of the character is 0x5C, i.e. the ASCII code of a
  backslash (\), and
- the next character is a dot (.), then

CopyReadLineText function would incorrectly interpret the sequence as an
end-of-copy marker (\.). This can only happen in encodings that can
"embed" ascii characters as the second byte. One example of such sequence
is '\x5ca45c2e666f6f' in Big5 encoding. If you put that in a file, and
load it with COPY FROM, you'd incorrectly get an "end-of-copy marker
corrupt" error.

Backpatch to all supported versions.

Reviewed-by: John Naylor, Kyotaro Horiguchi
Discussion: https://www.postgresql.org/message-id/a897f84f-8dca-8798-3139-07da5bb38728%40iki.fi
---
 src/backend/commands/copyfromparse.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/src/backend/commands/copyfromparse.c b/src/backend/commands/copyfromparse.c
index b843d315b1..315b16fd7a 100644
--- a/src/backend/commands/copyfromparse.c
+++ b/src/backend/commands/copyfromparse.c
@@ -1084,7 +1084,7 @@ CopyReadLineText(CopyFromState cstate)
 				break;
 			}
 			else if (!cstate->opts.csv_mode)
-
+			{
 				/*
 				 * If we are here, it means we found a backslash followed by
 				 * something other than a period.  In non-CSV mode, anything
@@ -1095,8 +1095,16 @@ CopyReadLineText(CopyFromState cstate)
 				 * backslashes are not special, so we want to process the
 				 * character after the backslash just like a normal character,
 				 * so we don't increment in those cases.
+				 *
+				 * Set 'c' to skip whole character correctly in multi-byte
+				 * encodings.  If we don't have the whole character in the
+				 * buffer yet, we might loop back to process it, after all,
+				 * but that's OK because multi-byte characters cannot have any
+				 * special meaning.
 				 */
 				raw_buf_ptr++;
+				c = c2;
+			}
 		}
 
 		/*
-- 
2.30.0


--------------CFC80B40DFDD66DF067F288D--





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

* [PATCH v2 1/1] Fix a corner-case in COPY FROM backslash processing.
@ 2021-02-04 19:36  Heikki Linnakangas <[email protected]>
  0 siblings, 0 replies; 54+ messages in thread

From: Heikki Linnakangas @ 2021-02-04 19:36 UTC (permalink / raw)

If a multi-byte character is escaped with a backslash in TEXT mode input,
we didn't always treat the escape correctly. If:

- a multi-byte character is escaped with a backslash, and
- the second byte of the character is 0x5C, i.e. the ASCII code of a
  backslash (\), and
- the next character is a dot (.), then

CopyReadLineText function would incorrectly interpret the sequence as an
end-of-copy marker (\.). This can only happen in encodings that can
"embed" ascii characters as the second byte. One example of such sequence
is '\x5ca45c2e666f6f' in Big5 encoding. If you put that in a file, and
load it with COPY FROM, you'd incorrectly get an "end-of-copy marker
corrupt" error.

Backpatch to all supported versions.

Reviewed-by: John Naylor, Kyotaro Horiguchi
Discussion: https://www.postgresql.org/message-id/a897f84f-8dca-8798-3139-07da5bb38728%40iki.fi
---
 src/backend/commands/copyfromparse.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/src/backend/commands/copyfromparse.c b/src/backend/commands/copyfromparse.c
index b843d315b1..315b16fd7a 100644
--- a/src/backend/commands/copyfromparse.c
+++ b/src/backend/commands/copyfromparse.c
@@ -1084,7 +1084,7 @@ CopyReadLineText(CopyFromState cstate)
 				break;
 			}
 			else if (!cstate->opts.csv_mode)
-
+			{
 				/*
 				 * If we are here, it means we found a backslash followed by
 				 * something other than a period.  In non-CSV mode, anything
@@ -1095,8 +1095,16 @@ CopyReadLineText(CopyFromState cstate)
 				 * backslashes are not special, so we want to process the
 				 * character after the backslash just like a normal character,
 				 * so we don't increment in those cases.
+				 *
+				 * Set 'c' to skip whole character correctly in multi-byte
+				 * encodings.  If we don't have the whole character in the
+				 * buffer yet, we might loop back to process it, after all,
+				 * but that's OK because multi-byte characters cannot have any
+				 * special meaning.
 				 */
 				raw_buf_ptr++;
+				c = c2;
+			}
 		}
 
 		/*
-- 
2.30.0


--------------CFC80B40DFDD66DF067F288D--





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

* [PATCH v2 1/1] Fix a corner-case in COPY FROM backslash processing.
@ 2021-02-04 19:36  Heikki Linnakangas <[email protected]>
  0 siblings, 0 replies; 54+ messages in thread

From: Heikki Linnakangas @ 2021-02-04 19:36 UTC (permalink / raw)

If a multi-byte character is escaped with a backslash in TEXT mode input,
we didn't always treat the escape correctly. If:

- a multi-byte character is escaped with a backslash, and
- the second byte of the character is 0x5C, i.e. the ASCII code of a
  backslash (\), and
- the next character is a dot (.), then

CopyReadLineText function would incorrectly interpret the sequence as an
end-of-copy marker (\.). This can only happen in encodings that can
"embed" ascii characters as the second byte. One example of such sequence
is '\x5ca45c2e666f6f' in Big5 encoding. If you put that in a file, and
load it with COPY FROM, you'd incorrectly get an "end-of-copy marker
corrupt" error.

Backpatch to all supported versions.

Reviewed-by: John Naylor, Kyotaro Horiguchi
Discussion: https://www.postgresql.org/message-id/a897f84f-8dca-8798-3139-07da5bb38728%40iki.fi
---
 src/backend/commands/copyfromparse.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/src/backend/commands/copyfromparse.c b/src/backend/commands/copyfromparse.c
index b843d315b1..315b16fd7a 100644
--- a/src/backend/commands/copyfromparse.c
+++ b/src/backend/commands/copyfromparse.c
@@ -1084,7 +1084,7 @@ CopyReadLineText(CopyFromState cstate)
 				break;
 			}
 			else if (!cstate->opts.csv_mode)
-
+			{
 				/*
 				 * If we are here, it means we found a backslash followed by
 				 * something other than a period.  In non-CSV mode, anything
@@ -1095,8 +1095,16 @@ CopyReadLineText(CopyFromState cstate)
 				 * backslashes are not special, so we want to process the
 				 * character after the backslash just like a normal character,
 				 * so we don't increment in those cases.
+				 *
+				 * Set 'c' to skip whole character correctly in multi-byte
+				 * encodings.  If we don't have the whole character in the
+				 * buffer yet, we might loop back to process it, after all,
+				 * but that's OK because multi-byte characters cannot have any
+				 * special meaning.
 				 */
 				raw_buf_ptr++;
+				c = c2;
+			}
 		}
 
 		/*
-- 
2.30.0


--------------CFC80B40DFDD66DF067F288D--





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

* [PATCH v2 1/1] Fix a corner-case in COPY FROM backslash processing.
@ 2021-02-04 19:36  Heikki Linnakangas <[email protected]>
  0 siblings, 0 replies; 54+ messages in thread

From: Heikki Linnakangas @ 2021-02-04 19:36 UTC (permalink / raw)

If a multi-byte character is escaped with a backslash in TEXT mode input,
we didn't always treat the escape correctly. If:

- a multi-byte character is escaped with a backslash, and
- the second byte of the character is 0x5C, i.e. the ASCII code of a
  backslash (\), and
- the next character is a dot (.), then

CopyReadLineText function would incorrectly interpret the sequence as an
end-of-copy marker (\.). This can only happen in encodings that can
"embed" ascii characters as the second byte. One example of such sequence
is '\x5ca45c2e666f6f' in Big5 encoding. If you put that in a file, and
load it with COPY FROM, you'd incorrectly get an "end-of-copy marker
corrupt" error.

Backpatch to all supported versions.

Reviewed-by: John Naylor, Kyotaro Horiguchi
Discussion: https://www.postgresql.org/message-id/a897f84f-8dca-8798-3139-07da5bb38728%40iki.fi
---
 src/backend/commands/copyfromparse.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/src/backend/commands/copyfromparse.c b/src/backend/commands/copyfromparse.c
index b843d315b1..315b16fd7a 100644
--- a/src/backend/commands/copyfromparse.c
+++ b/src/backend/commands/copyfromparse.c
@@ -1084,7 +1084,7 @@ CopyReadLineText(CopyFromState cstate)
 				break;
 			}
 			else if (!cstate->opts.csv_mode)
-
+			{
 				/*
 				 * If we are here, it means we found a backslash followed by
 				 * something other than a period.  In non-CSV mode, anything
@@ -1095,8 +1095,16 @@ CopyReadLineText(CopyFromState cstate)
 				 * backslashes are not special, so we want to process the
 				 * character after the backslash just like a normal character,
 				 * so we don't increment in those cases.
+				 *
+				 * Set 'c' to skip whole character correctly in multi-byte
+				 * encodings.  If we don't have the whole character in the
+				 * buffer yet, we might loop back to process it, after all,
+				 * but that's OK because multi-byte characters cannot have any
+				 * special meaning.
 				 */
 				raw_buf_ptr++;
+				c = c2;
+			}
 		}
 
 		/*
-- 
2.30.0


--------------CFC80B40DFDD66DF067F288D--





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

* [PATCH v2 1/1] Fix a corner-case in COPY FROM backslash processing.
@ 2021-02-04 19:36  Heikki Linnakangas <[email protected]>
  0 siblings, 0 replies; 54+ messages in thread

From: Heikki Linnakangas @ 2021-02-04 19:36 UTC (permalink / raw)

If a multi-byte character is escaped with a backslash in TEXT mode input,
we didn't always treat the escape correctly. If:

- a multi-byte character is escaped with a backslash, and
- the second byte of the character is 0x5C, i.e. the ASCII code of a
  backslash (\), and
- the next character is a dot (.), then

CopyReadLineText function would incorrectly interpret the sequence as an
end-of-copy marker (\.). This can only happen in encodings that can
"embed" ascii characters as the second byte. One example of such sequence
is '\x5ca45c2e666f6f' in Big5 encoding. If you put that in a file, and
load it with COPY FROM, you'd incorrectly get an "end-of-copy marker
corrupt" error.

Backpatch to all supported versions.

Reviewed-by: John Naylor, Kyotaro Horiguchi
Discussion: https://www.postgresql.org/message-id/a897f84f-8dca-8798-3139-07da5bb38728%40iki.fi
---
 src/backend/commands/copyfromparse.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/src/backend/commands/copyfromparse.c b/src/backend/commands/copyfromparse.c
index b843d315b1..315b16fd7a 100644
--- a/src/backend/commands/copyfromparse.c
+++ b/src/backend/commands/copyfromparse.c
@@ -1084,7 +1084,7 @@ CopyReadLineText(CopyFromState cstate)
 				break;
 			}
 			else if (!cstate->opts.csv_mode)
-
+			{
 				/*
 				 * If we are here, it means we found a backslash followed by
 				 * something other than a period.  In non-CSV mode, anything
@@ -1095,8 +1095,16 @@ CopyReadLineText(CopyFromState cstate)
 				 * backslashes are not special, so we want to process the
 				 * character after the backslash just like a normal character,
 				 * so we don't increment in those cases.
+				 *
+				 * Set 'c' to skip whole character correctly in multi-byte
+				 * encodings.  If we don't have the whole character in the
+				 * buffer yet, we might loop back to process it, after all,
+				 * but that's OK because multi-byte characters cannot have any
+				 * special meaning.
 				 */
 				raw_buf_ptr++;
+				c = c2;
+			}
 		}
 
 		/*
-- 
2.30.0


--------------CFC80B40DFDD66DF067F288D--





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

* Re: commitfest.postgresql.org is no longer fit for purpose
@ 2024-05-19 19:18  Tom Lane <[email protected]>
  0 siblings, 2 replies; 54+ messages in thread

From: Tom Lane @ 2024-05-19 19:18 UTC (permalink / raw)
  To: Dmitry Dolgov <[email protected]>; +Cc: Robert Haas <[email protected]>; pgsql-hackers

Dmitry Dolgov <[email protected]> writes:
> There are lots of good takes on this in the thread. It also makes clear what's
> at stake -- as Melanie pointed out with the patch about EXPLAIN for parallel
> bitmap heap scan, we're loosing potential contributors for no reasons. But I'm
> a bit concerned about what are the next steps: if memory serves, every couple
> of years there is a discussion about everything what goes wrong with the review
> process, commitfests, etc. Yet to my (admittedly limited) insight into the
> community, not many things have changed due to those discussions. How do we
> make sure this time it will be different?

Things *have* changed, if you take the long view.  We didn't have
commitfests at all until around 2007, and we've changed the ground
rules for them a couple of times since then.  We didn't have the CF
app at all until, well, I don't recall when, but the first few CFs
were managed by keeping patch lists on a wiki page.  It's not that
people are unwilling to change this stuff, but that it's hard to
identify what will make things better.

IMV one really fundamental problem is the same as it's been for a
couple of decades: too many patch submissions, too few committers.
We can't fix that by just appointing a ton more committers, at least
not if we want to keep the project's quality up.  We have to grow
qualified committers.  IIRC, one of the main reasons for instituting
the commitfests at all was the hope that if we got more people to
spend time reading the code and reviewing patches, some of them would
learn enough to become good committers.  I think that's worked, again
taking a long view.  I just did some quick statistics on the commit
history, and I see that we were hovering at somewhere around ten
active committers from 1999 to 2012, but since then it's slowly crept
up to about two dozen today.  (I'm counting "active" as "at least 10
commits per year", which is an arbitrary cutoff --- feel free to slice
the data for yourself.)  Meanwhile the number of submissions has also
grown, so I'm not sure how much better the load ratio is.

My point here is not that things are great, but just that we are
indeed improving, and I hope we can continue to.  Let's not be
defeatist about it.

I think this thread has already identified a few things we have
consensus to improve in the CF app, and I hope somebody goes off
and makes those happen (I lack the web skillz to help myself).
However, the app itself is just a tool; what counts more is our
process around it.  I have a couple of thoughts about that:

* Patches that sit in the queue a long time tend to be ones that lack
consensus, either about the goal or the details of how to achieve it.
Sometimes "lacks consensus" really means "nobody but the author thinks
this is a good idea, but we haven't mustered the will to say no".
But I think it's more usually the case that there are plausible
competing opinions about what the patch should do or how it should
do it.  How can we resolve such differences and get something done?

* Another reason for things sitting a long time is that they're too
big to review without an unreasonable amount of effort.  We should
encourage authors to break large patches into smaller stepwise
refinements.  It may seem like that will result in taking forever
to reach the end goal, but dropping a huge patchset on the community
isn't going to give speedy results either.

* Before starting this thread, Robert did a lot of very valuable
review of some individual patches.  I think what prompted him to
start the thread was the realization that he'd only made a small
dent in the problem.  Maybe we could divide and conquer: get a
dozen-or-so senior contributors to split up the list of pending
patches and then look at each one with an eye to what needs to
happen to move it along (*not* to commit it right away, although
in some cases maybe that's the thing to do).  It'd be great if
that could happen just before each commitfest, but that's probably
not practical with the current patch volume.  What I'm thinking
for the moment is to try to make that happen once a year or so.

> What I think wasn't discussed yet in details is the question of motivation.
> Surely, it would be great to have a process that will introduce as less burden
> as possible. But giving more motivation to follow the process / use the tool is
> as important. What motivates folks to review patches, figure out status of a
> complicated patch thread, maintain a list of open items, etc?

Yeah, all this stuff ultimately gets done "for the good of the
project", which isn't the most reliable motivation perhaps.
I don't see a better one...

			regards, tom lane






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

* Re: commitfest.postgresql.org is no longer fit for purpose
@ 2024-05-20 00:20  Bruce Momjian <[email protected]>
  parent: Tom Lane <[email protected]>
  1 sibling, 1 reply; 54+ messages in thread

From: Bruce Momjian @ 2024-05-20 00:20 UTC (permalink / raw)
  To: Tom Lane <[email protected]>; +Cc: Dmitry Dolgov <[email protected]>; Robert Haas <[email protected]>; pgsql-hackers

On Sun, May 19, 2024 at 03:18:11PM -0400, Tom Lane wrote:
> * Another reason for things sitting a long time is that they're too
> big to review without an unreasonable amount of effort.  We should
> encourage authors to break large patches into smaller stepwise
> refinements.  It may seem like that will result in taking forever
> to reach the end goal, but dropping a huge patchset on the community
> isn't going to give speedy results either.

I think it is sometimes hard to incrementally apply patches if the
long-term goal isn't agreed or know to be achievable.

> * Before starting this thread, Robert did a lot of very valuable
> review of some individual patches.  I think what prompted him to
> start the thread was the realization that he'd only made a small
> dent in the problem.  Maybe we could divide and conquer: get a
> dozen-or-so senior contributors to split up the list of pending
> patches and then look at each one with an eye to what needs to
> happen to move it along (*not* to commit it right away, although
> in some cases maybe that's the thing to do).  It'd be great if
> that could happen just before each commitfest, but that's probably
> not practical with the current patch volume.  What I'm thinking
> for the moment is to try to make that happen once a year or so.

For me, if someone already knows what the blocker is, it saves me a lot
of time if they can state that somewhere.

-- 
  Bruce Momjian  <[email protected]>        https://momjian.us
  EDB                                      https://enterprisedb.com

  Only you can decide what is important to you.






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

* Re: commitfest.postgresql.org is no longer fit for purpose
@ 2024-05-20 01:09  Tom Lane <[email protected]>
  parent: Bruce Momjian <[email protected]>
  0 siblings, 3 replies; 54+ messages in thread

From: Tom Lane @ 2024-05-20 01:09 UTC (permalink / raw)
  To: Bruce Momjian <[email protected]>; +Cc: Dmitry Dolgov <[email protected]>; Robert Haas <[email protected]>; pgsql-hackers

Bruce Momjian <[email protected]> writes:
> On Sun, May 19, 2024 at 03:18:11PM -0400, Tom Lane wrote:
>> * Another reason for things sitting a long time is that they're too
>> big to review without an unreasonable amount of effort.  We should
>> encourage authors to break large patches into smaller stepwise
>> refinements.  It may seem like that will result in taking forever
>> to reach the end goal, but dropping a huge patchset on the community
>> isn't going to give speedy results either.

> I think it is sometimes hard to incrementally apply patches if the
> long-term goal isn't agreed or know to be achievable.

True.  The value of the earlier patches in the series can be unclear
if you don't understand what the end goal is.  But I think people
could post a "road map" of how they intend a patch series to go.

Another way of looking at this is that sometimes people do post large
chunks of work in long many-patch sets, but we tend to look at the
whole series as something to review and commit as one (or I do, at
least).  We should be more willing to bite off and push the earlier
patches in such a series even when the later ones aren't entirely
done.

(The cfbot tends to discourage this, since as soon as one of the
patches is committed it no longer knows how to apply the rest.
Can we improve on that tooling somehow?)

			regards, tom lane






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

* Re: commitfest.postgresql.org is no longer fit for purpose
@ 2024-05-20 05:49  Thomas Munro <[email protected]>
  parent: Tom Lane <[email protected]>
  2 siblings, 2 replies; 54+ messages in thread

From: Thomas Munro @ 2024-05-20 05:49 UTC (permalink / raw)
  To: Tom Lane <[email protected]>; +Cc: Bruce Momjian <[email protected]>; Dmitry Dolgov <[email protected]>; Robert Haas <[email protected]>; pgsql-hackers

On Mon, May 20, 2024 at 1:09 PM Tom Lane <[email protected]> wrote:
> (The cfbot tends to discourage this, since as soon as one of the
> patches is committed it no longer knows how to apply the rest.
> Can we improve on that tooling somehow?)

Cfbot currently applies patches with  (GNU) patch
--no-backup-if-mismatch -p1 -V none -f.  The -f means that any
questions of the form "Reversed (or previously applied) patch
detected!  Assume -R? [y]" is answered with "yes", and the operation
fails.  I wondered if --forward would be better.  It's the right idea,
but it seems to be useless in practice for this purpose because the
command still fails:

tmunro@phonebox postgresql % patch --forward -p1 < x.patch || echo XXX
failed $?
patching file 'src/backend/postmaster/postmaster.c'
Ignoring previously applied (or reversed) patch.
1 out of 1 hunks ignored--saving rejects to
'src/backend/postmaster/postmaster.c.rej'
XXX failed 1

I wondered if it might be distinguishable from other kinds of failure
that should stop progress, but nope:

       patch's exit status is 0 if all hunks are applied successfully, 1
       if some hunks cannot be applied or there were merge conflicts,
       and 2 if there is more serious trouble.  When applying a set of
       patches in a loop it behooves you to check this exit status so
       you don't apply a later patch to a partially patched file.

I guess I could parse stdout or whatever that is and detect
all-hunks-ignored condition, but that doesn't sound like fun...

Perhaps cfbot should test explicitly for patches that have already
been applied with something like "git apply --reverse --check", and
skip them.  That would work for exact patches, but of course it would
be confused by any tweaks made before committing.  If going that way,
it might make sense to switch to git apply/am (instead of GNU patch),
to avoid contradictory conclusions.

The reason I was using GNU patch in the first place is that it is/was
a little more tolerant of some of the patches people used to send a
few years back, but now I expect everyone uses git format-patch and
would be prepared to change their ways if not.  In the past we had a
couple of cases of the reverse, that is, GNU patch couldn't apply
something that format-patch produced (some edge case of renaming,
IIRC) and I'm sorry that I never got around to changing that.

Sometimes I question the sanity of the whole thing.  Considering
cfbot's original "zero-effort CI" goal (or I guess "zero-extra-effort"
would be better), I was curious about what other projects had the same
idea, or whether we're really just starting at the "wrong end", and
came up with:

https://github.com/getpatchwork/patchwork
http://vger.kernel.org/bpfconf2022_material/lsfmmbpf2022-bpf-ci.pdf
<-- example user
https://github.com/patchew-project/patchew

Actually cfbot requires more effort than those, because it's driven
first by Commitfest app registration.  Those projects are extremists
IIUC: just write to a mailing list, no other bureaucracy at all (at
least for most participants, presumably administrators can adjust the
status in some database when things go wrong?).  We're actually
halfway to Gitlab et al already, with a web account and interaction
required to start the process of submitting a patch for consideration.
What I'm less clear on is who else has come up with the "bitrot" test
idea, either at the mailing list or web extremist ends of the scale.
Those are also generic tools, and cfbot obviously knows lots of things
about PostgreSQL, like the "highlights" and probably more things I'm
forgetting.






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

* Re: commitfest.postgresql.org is no longer fit for purpose
@ 2024-05-20 08:16  Daniel Gustafsson <[email protected]>
  parent: Thomas Munro <[email protected]>
  1 sibling, 0 replies; 54+ messages in thread

From: Daniel Gustafsson @ 2024-05-20 08:16 UTC (permalink / raw)
  To: Thomas Munro <[email protected]>; +Cc: Tom Lane <[email protected]>; Bruce Momjian <[email protected]>; Dmitry Dolgov <[email protected]>; Robert Haas <[email protected]>; pgsql-hackers

> On 20 May 2024, at 07:49, Thomas Munro <[email protected]> wrote:

> We're actually
> halfway to Gitlab et al already, with a web account and interaction
> required to start the process of submitting a patch for consideration.

Another Web<->Mailinglist extreme is Git themselves who have a Github bridge
for integration with their usual patch-on-mailinglist workflow.

https://github.com/gitgitgadget/gitgitgadget

> What I'm less clear on is who else has come up with the "bitrot" test
> idea, either at the mailing list or web extremist ends of the scale.

Most web based platforms like Github register the patch against the tree at the
time of submitting, and won't refresh unless the user does so.  Github does
detect bitrot and show a "this cannot be merged" error message, but it doesn't
alter any state on the PR etc.

--
Daniel Gustafsson







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

* Re: commitfest.postgresql.org is no longer fit for purpose
@ 2024-05-20 09:20  Mark Cave-Ayland <[email protected]>
  parent: Tom Lane <[email protected]>
  2 siblings, 0 replies; 54+ messages in thread

From: Mark Cave-Ayland @ 2024-05-20 09:20 UTC (permalink / raw)
  To: Tom Lane <[email protected]>; Bruce Momjian <[email protected]>; +Cc: Dmitry Dolgov <[email protected]>; Robert Haas <[email protected]>; pgsql-hackers

On 20/05/2024 06:56, Mark Cave-Ayland wrote:

> In general you find that a series consists of 2 parts: 1) a set of refactorings to 
> enable a new feature and 2) the new feature itself. Even if the details of 2) are 
> still under discussion, often it is possible to merge 1) fairly quickly which also 
> has the knock-on effect of reducing the size of later iterations of the series. This 
> also helps with new contributors since having parts of the series merged sooner helps 
> them feel valued and helps to provide immediate feedback.

[resend due to DKIM header failure]

Something else I also notice is that PostgreSQL doesn't have a MAINTAINERS or 
equivalent file, so when submitting patches it's difficult to know who is expected to 
review and/or commit changes to a particular part of the codebase (this is true both 
with and without the CF system).


ATB,

Mark.






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

* Re: commitfest.postgresql.org is no longer fit for purpose
@ 2024-05-20 11:41  Alvaro Herrera <[email protected]>
  parent: Tom Lane <[email protected]>
  2 siblings, 0 replies; 54+ messages in thread

From: Alvaro Herrera @ 2024-05-20 11:41 UTC (permalink / raw)
  To: Tom Lane <[email protected]>; +Cc: Bruce Momjian <[email protected]>; Dmitry Dolgov <[email protected]>; Robert Haas <[email protected]>; pgsql-hackers

On 2024-May-19, Tom Lane wrote:

> (The cfbot tends to discourage this, since as soon as one of the
> patches is committed it no longer knows how to apply the rest.
> Can we improve on that tooling somehow?)

I think a necessary next step to further improve the cfbot is to get it
integrated in pginfra.  Right now it runs somewhere in Thomas's servers
or something, and there's no real integration with the commitfest proper
except by scraping.

-- 
Álvaro Herrera               48°01'N 7°57'E  —  https://www.EnterpriseDB.com/
"La libertad es como el dinero; el que no la sabe emplear la pierde" (Alvarez)






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

* Re: commitfest.postgresql.org is no longer fit for purpose
@ 2024-05-20 14:16  Robert Haas <[email protected]>
  parent: Tom Lane <[email protected]>
  1 sibling, 1 reply; 54+ messages in thread

From: Robert Haas @ 2024-05-20 14:16 UTC (permalink / raw)
  To: Tom Lane <[email protected]>; +Cc: Dmitry Dolgov <[email protected]>; pgsql-hackers

On Sun, May 19, 2024 at 3:18 PM Tom Lane <[email protected]> wrote:
> Dmitry Dolgov <[email protected]> writes:
> > How do we make sure this time it will be different?
>
> Things *have* changed, if you take the long view.

That's true, but I think Dmitry's point is well-taken all the same: we
haven't really made a significant process improvement in many years,
and in some ways, I think things have been slowly degrading. I don't
believe it's necessary or possible to solve all of the accumulated
problems overnight, but we need to get serious about admitting that
there is a problem which, in my opinion, is an existential threat to
the project.

> IMV one really fundamental problem is the same as it's been for a
> couple of decades: too many patch submissions, too few committers.
> We can't fix that by just appointing a ton more committers, at least
> not if we want to keep the project's quality up.  We have to grow
> qualified committers.  IIRC, one of the main reasons for instituting
> the commitfests at all was the hope that if we got more people to
> spend time reading the code and reviewing patches, some of them would
> learn enough to become good committers.  I think that's worked, again
> taking a long view.  I just did some quick statistics on the commit
> history, and I see that we were hovering at somewhere around ten
> active committers from 1999 to 2012, but since then it's slowly crept
> up to about two dozen today.  (I'm counting "active" as "at least 10
> commits per year", which is an arbitrary cutoff --- feel free to slice
> the data for yourself.)  Meanwhile the number of submissions has also
> grown, so I'm not sure how much better the load ratio is.

That's an interesting statistic. I had not realized that the numbers
had actually grown significantly. However, I think that the
new-patch-submitter experience has not improved; if anything, I think
it may have gotten worse. It's hard to compare the subjective
experience between 2008 when I first got involved and now, especially
since at that time I was a rank newcomer experiencing things as a
newcomer, and now I'm a long-time committer trying to judge the
newcomer experience. But it seems to me that when I started, there was
more of a "middle tier" of people who were not committers but could do
meaningful review of patches and help you push them in the direction
of being something that a committer might not loathe. Now, I feel like
there are a lot of non-committer reviews that aren't actually adding a
lot of value: people come along and say the patch doesn't apply, or a
word is spelled wrong, and we don't get meaningful review of whether
the design makes sense, or if we do, it's wrong. Perhaps this is just
viewing the past with rose-colored glasses: I wasn't in as good a
position to judge the value of reviews that I gave and received at
that point as I am now. But what I do think is happening today is that
a lot of committer energy gets focused on the promising non-committers
who someone thinks might be able to become committers, and other
non-committers struggle to make any headway.

On the plus side, I think we make more of an effort not to be a jerk
to newcomers than we used to. I also have a strong hunch that it may
not be as good as it needs to be.

> * Patches that sit in the queue a long time tend to be ones that lack
> consensus, either about the goal or the details of how to achieve it.
> Sometimes "lacks consensus" really means "nobody but the author thinks
> this is a good idea, but we haven't mustered the will to say no".
> But I think it's more usually the case that there are plausible
> competing opinions about what the patch should do or how it should
> do it.  How can we resolve such differences and get something done?

This is a great question. We need to do better with that.

Also, it would be helpful to have better ways of handling it when the
author has gotten to a certain point with it but doesn't necessarily
have the time/skills/whatever to drive it forward. Such patches are
quite often a good idea, but it's not clear what we can do with them
procedurally other than hit the reject button, which doesn't feel
great.

> * Another reason for things sitting a long time is that they're too
> big to review without an unreasonable amount of effort.  We should
> encourage authors to break large patches into smaller stepwise
> refinements.  It may seem like that will result in taking forever
> to reach the end goal, but dropping a huge patchset on the community
> isn't going to give speedy results either.

Especially if it has a high rate of subtle defects, which is common.

> * Before starting this thread, Robert did a lot of very valuable
> review of some individual patches.  I think what prompted him to
> start the thread was the realization that he'd only made a small
> dent in the problem.  Maybe we could divide and conquer: get a
> dozen-or-so senior contributors to split up the list of pending
> patches and then look at each one with an eye to what needs to
> happen to move it along (*not* to commit it right away, although
> in some cases maybe that's the thing to do).  It'd be great if
> that could happen just before each commitfest, but that's probably
> not practical with the current patch volume.  What I'm thinking
> for the moment is to try to make that happen once a year or so.

I like this idea.

> Yeah, all this stuff ultimately gets done "for the good of the
> project", which isn't the most reliable motivation perhaps.
> I don't see a better one...

This is the really hard part.

-- 
Robert Haas
EDB: http://www.enterprisedb.com






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

* Re: commitfest.postgresql.org is no longer fit for purpose
@ 2024-05-20 21:33  Maciek Sakrejda <[email protected]>
  parent: Thomas Munro <[email protected]>
  1 sibling, 0 replies; 54+ messages in thread

From: Maciek Sakrejda @ 2024-05-20 21:33 UTC (permalink / raw)
  To: Thomas Munro <[email protected]>; +Cc: Tom Lane <[email protected]>; Bruce Momjian <[email protected]>; Dmitry Dolgov <[email protected]>; Robert Haas <[email protected]>; pgsql-hackers

On Sun, May 19, 2024 at 10:50 PM Thomas Munro <[email protected]> wrote:
> Sometimes I question the sanity of the whole thing.  Considering
> cfbot's original "zero-effort CI" goal (or I guess "zero-extra-effort"
> would be better), I was curious about what other projects had the same
> idea, or whether we're really just starting at the "wrong end", and
> came up with:
>
> https://github.com/getpatchwork/patchwork
> http://vger.kernel.org/bpfconf2022_material/lsfmmbpf2022-bpf-ci.pdf
> <-- example user
> https://github.com/patchew-project/patchew
>
> Actually cfbot requires more effort than those, because it's driven
> first by Commitfest app registration.  Those projects are extremists
> IIUC: just write to a mailing list, no other bureaucracy at all (at
> least for most participants, presumably administrators can adjust the
> status in some database when things go wrong?).  We're actually
> halfway to Gitlab et al already, with a web account and interaction
> required to start the process of submitting a patch for consideration.
> What I'm less clear on is who else has come up with the "bitrot" test
> idea, either at the mailing list or web extremist ends of the scale.
> Those are also generic tools, and cfbot obviously knows lots of things
> about PostgreSQL, like the "highlights" and probably more things I'm
> forgetting.

For what it's worth, a few years before cfbot, I had privately
attempted a similar idea for Postgres [1]. The project here is
basically a very simple API and infrastructure for running builds and
make check. A previous version [2] subscribed to the mailing lists and
used Travis CI (and accidentally spammed some Postgres committers
[3]). The project petered out as my work responsibilities shifted (and
to be honest, after I felt sheepish about the spamming).

I think cfbot is way, way ahead of where my project got at this point.
But since you asked about other similar projects, I'm happy to discuss
further if it's helpful to bounce ideas off someone who's thought
about the same problem (though not for a while now, I admit).

Thanks,
Maciek

[1]: https://github.com/msakrejda/pg-quilter
[2]: https://github.com/msakrejda/pg-quilter/blob/2038d9493f9aa7d43d3eb0aec1d299b94624602e/lib/pg-quilter...
[3]: https://www.postgresql.org/message-id/flat/CAM3SWZQboGoVYAJNoPMx%3DuDLE%2BZh5k2MQa4dWk91YPGDxuY-gQ%4...






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

* Re: commitfest.postgresql.org is no longer fit for purpose
@ 2024-05-24 15:40  Tomas Vondra <[email protected]>
  parent: Robert Haas <[email protected]>
  0 siblings, 0 replies; 54+ messages in thread

From: Tomas Vondra @ 2024-05-24 15:40 UTC (permalink / raw)
  To: Robert Haas <[email protected]>; Tom Lane <[email protected]>; +Cc: Dmitry Dolgov <[email protected]>; pgsql-hackers



On 5/20/24 16:16, Robert Haas wrote:
> On Sun, May 19, 2024 at 3:18 PM Tom Lane <[email protected]> wrote:
>
>...
> 
>> * Another reason for things sitting a long time is that they're too
>> big to review without an unreasonable amount of effort.  We should
>> encourage authors to break large patches into smaller stepwise
>> refinements.  It may seem like that will result in taking forever
>> to reach the end goal, but dropping a huge patchset on the community
>> isn't going to give speedy results either.
> 
> Especially if it has a high rate of subtle defects, which is common.
> 

I think breaking large patches into reasonably small parts is a very
important thing. Not only is it really difficult (or perhaps even
practically impossible) to review patches over a certain size, because
you have to grok and review everything at once. But it's also not great
from the cost/benefit POV - the improvement may be very beneficial, but
if it's one huge lump of code that never gets committable as a whole,
there's no benefit in practice. Which makes it less likely I'll even
start looking at the patch very closely, because there's a risk it'd be
just a waste of time in the end.

So I think this is another important reason to advise people to split
patches into smaller parts - not only it makes it easier to review, it
makes it possible to review and commit the parts incrementally, getting
at least some benefits early.

>> * Before starting this thread, Robert did a lot of very valuable
>> review of some individual patches.  I think what prompted him to
>> start the thread was the realization that he'd only made a small
>> dent in the problem.  Maybe we could divide and conquer: get a
>> dozen-or-so senior contributors to split up the list of pending
>> patches and then look at each one with an eye to what needs to
>> happen to move it along (*not* to commit it right away, although
>> in some cases maybe that's the thing to do).  It'd be great if
>> that could happen just before each commitfest, but that's probably
>> not practical with the current patch volume.  What I'm thinking
>> for the moment is to try to make that happen once a year or so.
> 
> I like this idea.
> 

Me too. Do you think it'd happen throughout the whole year, or at some
particular moment?

We used to do a "triage" at the FOSDEM PGDay meeting, but that used to
be more of an ad hoc thing to use the remaining time, with only a small
subset of people. But that seems pretty late in the dev cycle, I guess
we'd want it to happen early, perhaps during the first CF?

FWIW this reminds me the "CAN reports" tracking the current "conditions,
actions and needs" of a ticket. I do that for internal stuff, and I find
that quite helpful (as long as it's kept up to date).

>> Yeah, all this stuff ultimately gets done "for the good of the
>> project", which isn't the most reliable motivation perhaps.
>> I don't see a better one...
> 
> This is the really hard part.
> 

I think we have plenty of motivated people with good intentions. Some
are motivated by personal interest, some by trying to achieve stuff
related to their work/research/... I don't think the exact reasons
matter too much, and it's often a combination.

IMHO we should look at this from the other end - people are motivated to
get a patch reviewed & committed, and if we introduce a process that's
more likely to lead to that result, people will mostly adopt that.

And if we could make that process more convenient by improving the CF
app to support it, that'd be even better ... I'm mostly used to the
mailing list idea, but with the volume it's a constant struggle to keep
track of new patch versions that I wanted/promised to review, etc. The
CF app helps with that a little bit, because I can "become a reviewer"
but I still don't get notifications or anything like that :-(


regards

-- 
Tomas Vondra
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company






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

end of thread, other threads:[~2024-05-24 15:40 UTC | newest]

Thread overview: 54+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2021-02-03 12:00 [PATCH 1/1] Fix a corner-case in COPY FROM backslash processing. Heikki Linnakangas <[email protected]>
2021-02-03 12:00 [PATCH 1/1] Fix a corner-case in COPY FROM backslash processing. Heikki Linnakangas <[email protected]>
2021-02-03 12:00 [PATCH 1/1] Fix a corner-case in COPY FROM backslash processing. Heikki Linnakangas <[email protected]>
2021-02-03 12:00 [PATCH 1/1] Fix a corner-case in COPY FROM backslash processing. Heikki Linnakangas <[email protected]>
2021-02-03 12:00 [PATCH 1/1] Fix a corner-case in COPY FROM backslash processing. Heikki Linnakangas <[email protected]>
2021-02-03 12:00 [PATCH 1/1] Fix a corner-case in COPY FROM backslash processing. Heikki Linnakangas <[email protected]>
2021-02-03 12:00 [PATCH 1/1] Fix a corner-case in COPY FROM backslash processing. Heikki Linnakangas <[email protected]>
2021-02-03 12:00 [PATCH 1/1] Fix a corner-case in COPY FROM backslash processing. Heikki Linnakangas <[email protected]>
2021-02-03 12:00 [PATCH 1/1] Fix a corner-case in COPY FROM backslash processing. Heikki Linnakangas <[email protected]>
2021-02-03 12:00 [PATCH 1/1] Fix a corner-case in COPY FROM backslash processing. Heikki Linnakangas <[email protected]>
2021-02-03 12:00 [PATCH 1/1] Fix a corner-case in COPY FROM backslash processing. Heikki Linnakangas <[email protected]>
2021-02-03 12:00 [PATCH 1/1] Fix a corner-case in COPY FROM backslash processing. Heikki Linnakangas <[email protected]>
2021-02-03 12:00 [PATCH 1/1] Fix a corner-case in COPY FROM backslash processing. Heikki Linnakangas <[email protected]>
2021-02-03 12:00 [PATCH 1/1] Fix a corner-case in COPY FROM backslash processing. Heikki Linnakangas <[email protected]>
2021-02-03 12:00 [PATCH 1/1] Fix a corner-case in COPY FROM backslash processing. Heikki Linnakangas <[email protected]>
2021-02-03 12:00 [PATCH 1/1] Fix a corner-case in COPY FROM backslash processing. Heikki Linnakangas <[email protected]>
2021-02-03 12:00 [PATCH 1/1] Fix a corner-case in COPY FROM backslash processing. Heikki Linnakangas <[email protected]>
2021-02-03 12:00 [PATCH 1/1] Fix a corner-case in COPY FROM backslash processing. Heikki Linnakangas <[email protected]>
2021-02-03 12:00 [PATCH 1/1] Fix a corner-case in COPY FROM backslash processing. Heikki Linnakangas <[email protected]>
2021-02-03 12:00 [PATCH 1/1] Fix a corner-case in COPY FROM backslash processing. Heikki Linnakangas <[email protected]>
2021-02-03 12:00 [PATCH 1/1] Fix a corner-case in COPY FROM backslash processing. Heikki Linnakangas <[email protected]>
2021-02-03 12:00 [PATCH 1/1] Fix a corner-case in COPY FROM backslash processing. Heikki Linnakangas <[email protected]>
2021-02-04 19:36 [PATCH v2 1/1] Fix a corner-case in COPY FROM backslash processing. Heikki Linnakangas <[email protected]>
2021-02-04 19:36 [PATCH v2 1/1] Fix a corner-case in COPY FROM backslash processing. Heikki Linnakangas <[email protected]>
2021-02-04 19:36 [PATCH v2 1/1] Fix a corner-case in COPY FROM backslash processing. Heikki Linnakangas <[email protected]>
2021-02-04 19:36 [PATCH v2 1/1] Fix a corner-case in COPY FROM backslash processing. Heikki Linnakangas <[email protected]>
2021-02-04 19:36 [PATCH v2 1/1] Fix a corner-case in COPY FROM backslash processing. Heikki Linnakangas <[email protected]>
2021-02-04 19:36 [PATCH v2 1/1] Fix a corner-case in COPY FROM backslash processing. Heikki Linnakangas <[email protected]>
2021-02-04 19:36 [PATCH v2 1/1] Fix a corner-case in COPY FROM backslash processing. Heikki Linnakangas <[email protected]>
2021-02-04 19:36 [PATCH v2 1/1] Fix a corner-case in COPY FROM backslash processing. Heikki Linnakangas <[email protected]>
2021-02-04 19:36 [PATCH v2 1/1] Fix a corner-case in COPY FROM backslash processing. Heikki Linnakangas <[email protected]>
2021-02-04 19:36 [PATCH v2 1/1] Fix a corner-case in COPY FROM backslash processing. Heikki Linnakangas <[email protected]>
2021-02-04 19:36 [PATCH v2 1/1] Fix a corner-case in COPY FROM backslash processing. Heikki Linnakangas <[email protected]>
2021-02-04 19:36 [PATCH v2 1/1] Fix a corner-case in COPY FROM backslash processing. Heikki Linnakangas <[email protected]>
2021-02-04 19:36 [PATCH v2 1/1] Fix a corner-case in COPY FROM backslash processing. Heikki Linnakangas <[email protected]>
2021-02-04 19:36 [PATCH v2 1/1] Fix a corner-case in COPY FROM backslash processing. Heikki Linnakangas <[email protected]>
2021-02-04 19:36 [PATCH v2 1/1] Fix a corner-case in COPY FROM backslash processing. Heikki Linnakangas <[email protected]>
2021-02-04 19:36 [PATCH v2 1/1] Fix a corner-case in COPY FROM backslash processing. Heikki Linnakangas <[email protected]>
2021-02-04 19:36 [PATCH v2 1/1] Fix a corner-case in COPY FROM backslash processing. Heikki Linnakangas <[email protected]>
2021-02-04 19:36 [PATCH v2 1/1] Fix a corner-case in COPY FROM backslash processing. Heikki Linnakangas <[email protected]>
2021-02-04 19:36 [PATCH v2 1/1] Fix a corner-case in COPY FROM backslash processing. Heikki Linnakangas <[email protected]>
2021-02-04 19:36 [PATCH v2 1/1] Fix a corner-case in COPY FROM backslash processing. Heikki Linnakangas <[email protected]>
2021-02-04 19:36 [PATCH v2 1/1] Fix a corner-case in COPY FROM backslash processing. Heikki Linnakangas <[email protected]>
2021-02-04 19:36 [PATCH v2 1/1] Fix a corner-case in COPY FROM backslash processing. Heikki Linnakangas <[email protected]>
2024-05-19 19:18 Re: commitfest.postgresql.org is no longer fit for purpose Tom Lane <[email protected]>
2024-05-20 00:20 ` Re: commitfest.postgresql.org is no longer fit for purpose Bruce Momjian <[email protected]>
2024-05-20 01:09   ` Re: commitfest.postgresql.org is no longer fit for purpose Tom Lane <[email protected]>
2024-05-20 05:49     ` Re: commitfest.postgresql.org is no longer fit for purpose Thomas Munro <[email protected]>
2024-05-20 08:16       ` Re: commitfest.postgresql.org is no longer fit for purpose Daniel Gustafsson <[email protected]>
2024-05-20 21:33       ` Re: commitfest.postgresql.org is no longer fit for purpose Maciek Sakrejda <[email protected]>
2024-05-20 09:20     ` Re: commitfest.postgresql.org is no longer fit for purpose Mark Cave-Ayland <[email protected]>
2024-05-20 11:41     ` Re: commitfest.postgresql.org is no longer fit for purpose Alvaro Herrera <[email protected]>
2024-05-20 14:16 ` Re: commitfest.postgresql.org is no longer fit for purpose Robert Haas <[email protected]>
2024-05-24 15:40   ` Re: commitfest.postgresql.org is no longer fit for purpose Tomas Vondra <[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