public inbox for [email protected]
help / color / mirror / Atom feed[PATCH] pageinspect function to decode infomasks
54+ messages / 11 participants
[nested] [flat]
* [PATCH] pageinspect function to decode infomasks
@ 2017-07-20 03:33 Craig Ringer <[email protected]>
2017-07-20 03:38 ` Re: [PATCH] pageinspect function to decode infomasks Craig Ringer <[email protected]>
2017-07-20 03:44 ` Re: [PATCH] pageinspect function to decode infomasks Peter Geoghegan <[email protected]>
0 siblings, 2 replies; 54+ messages in thread
From: Craig Ringer @ 2017-07-20 03:33 UTC (permalink / raw)
To: pgsql-hackers; +Cc: Álvaro Herrera <[email protected]>; abhijit Menon-Sen <[email protected]>; Peter Eisentraut <[email protected]>; Pavan Deolasee <[email protected]>
Hi
Whenever I'm debugging some kind of corruption incident, possible
visibility bug, etc, I always land up staring at integer infomasks or using
a SQL helper function to decode them.
That's silly, so here's a patch to teach pageinspect how to decode
infomasks to a human readable array of flag names.
Example:
SELECT t_infomask, t_infomask2, flags
FROM heap_page_items(get_raw_page('test1', 0)),
LATERAL heap_infomask_flags(t_infomask, t_infomask2, true) m(flags);
t_infomask | t_infomask2 | flags
------------+-------------+----------------------------------------------------------------------------
2816 | 2 |
{HEAP_XMIN_COMMITTED,HEAP_XMIN_INVALID,HEAP_XMAX_INVALID,HEAP_XMIN_FROZEN}
(1 row)
To decode individual mask integers you can just call it directly. It's
strict, so pass 0 for the other mask if you don't have both, e.g.
SELECT heap_infomask_flags(2816, 0);
The patch backports easily to older pageinspect versions for when you're
debugging something old.
BTW, I used text[] not enums. That costs a fair bit of memory, but it
doesn't seem worth worrying too much about in this context.
For convenience it also tests and reports HEAP_LOCKED_UPGRADED and
HEAP_XMAX_IS_LOCKED_ONLY as pseudo-flags.
I decided not to filter
out HEAP_XMIN_COMMITTED,HEAP_XMIN_INVALID,HEAP_XMAX_INVALID
when HEAP_XMIN_FROZEN is set; that doesn't make sense when we
examine HEAP_XMAX_IS_LOCKED_ONLY or HEAP_LOCKED_UPGRADED, and filtering
them out could be just as confusing as leaving them in.
The infomask2 natts mask is ignored. You can bitwise-and it out in SQL
pretty easily if needed. I could output it here as a constructed text
datum, but it seems mostly pointless.
--
Craig Ringer http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
--
Sent via pgsql-hackers mailing list ([email protected])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Attachments:
[text/x-patch] v1-0001-Introduce-heap_infomask_flags-to-decode-infomask-.patch (10.9K, ../../CAMsr+YEY7jeaXOb+oX+RhDyOFuTMdmHjGsBxL=igCm03J0go9Q@mail.gmail.com/3-v1-0001-Introduce-heap_infomask_flags-to-decode-infomask-.patch)
download | inline diff:
From 488a1f69b8082258d508ba681a4f4a5f6fce2267 Mon Sep 17 00:00:00 2001
From: Craig Ringer <[email protected]>
Date: Thu, 20 Jul 2017 11:20:21 +0800
Subject: [PATCH v1] Introduce heap_infomask_flags to decode infomask and
infomask2
---
contrib/pageinspect/Makefile | 3 +-
contrib/pageinspect/expected/page.out | 25 ++++++
contrib/pageinspect/heapfuncs.c | 120 ++++++++++++++++++++++++++
contrib/pageinspect/pageinspect--1.6--1.7.sql | 9 ++
contrib/pageinspect/pageinspect.control | 2 +-
contrib/pageinspect/sql/page.sql | 14 +++
doc/src/sgml/pageinspect.sgml | 32 +++++++
7 files changed, 203 insertions(+), 2 deletions(-)
create mode 100644 contrib/pageinspect/pageinspect--1.6--1.7.sql
diff --git a/contrib/pageinspect/Makefile b/contrib/pageinspect/Makefile
index 0a3cbee..de114c7 100644
--- a/contrib/pageinspect/Makefile
+++ b/contrib/pageinspect/Makefile
@@ -5,7 +5,8 @@ OBJS = rawpage.o heapfuncs.o btreefuncs.o fsmfuncs.o \
brinfuncs.o ginfuncs.o hashfuncs.o $(WIN32RES)
EXTENSION = pageinspect
-DATA = pageinspect--1.5.sql pageinspect--1.5--1.6.sql \
+DATA = pageinspect--1.6--1.7.sql \
+ pageinspect--1.5.sql pageinspect--1.5--1.6.sql \
pageinspect--1.4--1.5.sql pageinspect--1.3--1.4.sql \
pageinspect--1.2--1.3.sql pageinspect--1.1--1.2.sql \
pageinspect--1.0--1.1.sql pageinspect--unpackaged--1.0.sql
diff --git a/contrib/pageinspect/expected/page.out b/contrib/pageinspect/expected/page.out
index 8e15947..054c69d 100644
--- a/contrib/pageinspect/expected/page.out
+++ b/contrib/pageinspect/expected/page.out
@@ -82,6 +82,31 @@ SELECT * FROM fsm_page_contents(get_raw_page('test1', 'fsm', 0));
(1 row)
+-- If we freeze the only tuple on test1, the infomask should
+-- always be the same in all test runs.
+VACUUM FREEZE test1;
+SELECT t_infomask, t_infomask2, flags
+FROM heap_page_items(get_raw_page('test1', 0)),
+ LATERAL heap_infomask_flags(t_infomask, t_infomask2, true) m(flags);
+ t_infomask | t_infomask2 | flags
+------------+-------------+----------------------------------------------------------------------------
+ 2816 | 2 | {HEAP_XMIN_COMMITTED,HEAP_XMIN_INVALID,HEAP_XMAX_INVALID,HEAP_XMIN_FROZEN}
+(1 row)
+
+SELECT t_infomask, t_infomask2, flags
+FROM heap_page_items(get_raw_page('test1', 0)),
+ LATERAL heap_infomask_flags(t_infomask, t_infomask2, false) m(flags);
+ t_infomask | t_infomask2 | flags
+------------+-------------+-----------------------------------------------------------
+ 2816 | 2 | {HEAP_XMIN_COMMITTED,HEAP_XMIN_INVALID,HEAP_XMAX_INVALID}
+(1 row)
+
+SELECT heap_infomask_flags(2816, 0);
+ heap_infomask_flags
+-----------------------------------------------------------
+ {HEAP_XMIN_COMMITTED,HEAP_XMIN_INVALID,HEAP_XMAX_INVALID}
+(1 row)
+
DROP TABLE test1;
-- check that using any of these functions with a partitioned table would fail
create table test_partitioned (a int) partition by range (a);
diff --git a/contrib/pageinspect/heapfuncs.c b/contrib/pageinspect/heapfuncs.c
index 72d1776..17bea2a 100644
--- a/contrib/pageinspect/heapfuncs.c
+++ b/contrib/pageinspect/heapfuncs.c
@@ -478,3 +478,123 @@ tuple_data_split(PG_FUNCTION_ARGS)
PG_RETURN_ARRAYTYPE_P(res);
}
+
+/*
+ * Brian Kernighan's popcount algorithm for counting number of set bits in a
+ * mask. The faster methods aren't worth the complexity here.
+ *
+ * See e.g. http://graphics.stanford.edu/~seander/bithacks.html#CountBitsSetKernighan
+ *
+ * gcc has __builtin_popcount but there's no standard alternative.
+ */
+static inline int
+pg_popcount32(uint32 mask)
+{
+ unsigned int c; // c accumulates the total bits set in v
+ for (c = 0; mask; c++)
+ {
+ mask &= mask - 1; // clear the least significant bit set
+ }
+ return c;
+}
+
+/*
+ * Infomask flag names. The infomask2 values are shifted into the
+ * high 16 bits.
+ */
+struct infomask_details
+{
+ uint32 flag_value;
+ const char flag_name[24];
+};
+
+#define MASKINFO_LENGTH 19
+static struct infomask_details maskinfo[MASKINFO_LENGTH] =
+ {
+ /* infomask1 values */
+ {(uint32)HEAP_HASNULL, "HEAP_HASNULL"},
+ {(uint32)HEAP_HASVARWIDTH, "HEAP_HASVARWIDTH"},
+ {(uint32)HEAP_HASEXTERNAL, "HEAP_HASEXTERNAL"},
+ {(uint32)HEAP_HASOID, "HEAP_HASOID"},
+ {(uint32)HEAP_XMAX_KEYSHR_LOCK, "HEAP_XMAX_KEYSHR_LOCK"},
+ {(uint32)HEAP_COMBOCID, "HEAP_COMBOCID"},
+ {(uint32)HEAP_XMAX_EXCL_LOCK, "HEAP_XMAX_EXCL_LOCK"},
+ {(uint32)HEAP_XMAX_LOCK_ONLY, "HEAP_XMAX_LOCK_ONLY"},
+ {(uint32)HEAP_XMIN_COMMITTED, "HEAP_XMIN_COMMITTED"},
+ {(uint32)HEAP_XMIN_INVALID, "HEAP_XMIN_INVALID"},
+ {(uint32)HEAP_XMAX_COMMITTED, "HEAP_XMAX_COMMITTED"},
+ {(uint32)HEAP_XMAX_INVALID, "HEAP_XMAX_INVALID"},
+ {(uint32)HEAP_XMAX_IS_MULTI, "HEAP_XMAX_IS_MULTI"},
+ {(uint32)HEAP_UPDATED, "HEAP_UPDATED"},
+ {(uint32)HEAP_MOVED_OFF, "HEAP_MOVED_OFF"},
+ {(uint32)HEAP_MOVED_IN, "HEAP_MOVED_IN"},
+ /* infomask2 values */
+ {((uint32)HEAP_KEYS_UPDATED)<<16, "HEAP_KEYS_UPDATED"},
+ {((uint32)HEAP_HOT_UPDATED)<<16, "HEAP_HOT_UPDATED"},
+ {((uint32)HEAP_ONLY_TUPLE)<<16, "HEAP_ONLY_TUPLE"},
+ };
+
+/*
+ * Decode an infomask, per htup_details.c, into human readable
+ * form.
+ */
+PG_FUNCTION_INFO_V1(heap_infomask_flags);
+
+Datum
+heap_infomask_flags(PG_FUNCTION_ARGS)
+{
+ uint16 t_infomask = PG_GETARG_INT16(0);
+ uint16 t_infomask2 = PG_GETARG_INT16(1);
+ bool include_combined = PG_GETARG_BOOL(2);
+ uint32 combomask = (((uint32)t_infomask2) << 16) + (uint32)t_infomask;
+ unsigned int maxarray = pg_popcount32(combomask);
+ Datum *d;
+ ArrayType *a;
+ int i;
+ int insertpos;
+
+ Assert((t_infomask == 0 && t_infomask2 == 0) == (maxarray == 0));
+
+ if (maxarray == 0)
+ {
+ a = construct_empty_array(TEXTOID);
+ PG_RETURN_POINTER(a);
+ }
+
+ if (include_combined)
+ {
+ maxarray += (t_infomask & HEAP_XMIN_FROZEN) == HEAP_XMIN_FROZEN;
+ maxarray += HEAP_XMAX_IS_LOCKED_ONLY(t_infomask);
+ maxarray += HEAP_LOCKED_UPGRADED(t_infomask);
+ }
+
+ d = (Datum *) palloc(sizeof(Datum) * maxarray);
+
+ insertpos = 0;
+ for (i = 0; i < MASKINFO_LENGTH; ++i)
+ {
+ if ((combomask & maskinfo[i].flag_value) == maskinfo[i].flag_value)
+ d[insertpos++] = CStringGetTextDatum(maskinfo[i].flag_name);
+ }
+
+ /*
+ * These tests are useful to report in the mask we output, since they're
+ * much more simply done here than in SQL, and here they won't get out of
+ * sync with what Pg does if we change it later.
+ */
+ if (include_combined)
+ {
+ if ((t_infomask & HEAP_XMIN_FROZEN) == HEAP_XMIN_FROZEN)
+ d[insertpos++] = CStringGetTextDatum("HEAP_XMIN_FROZEN");
+
+ if (HEAP_XMAX_IS_LOCKED_ONLY(t_infomask))
+ d[insertpos++] = CStringGetTextDatum("HEAP_XMAX_IS_LOCKED_ONLY");
+
+ if (HEAP_LOCKED_UPGRADED(t_infomask))
+ d[insertpos++] = CStringGetTextDatum("HEAP_LOCKED_UPGRADED");
+ }
+
+ a = construct_array(d, maxarray - 1, TEXTOID, -1, false, 'i');
+
+ PG_RETURN_POINTER(a);
+}
diff --git a/contrib/pageinspect/pageinspect--1.6--1.7.sql b/contrib/pageinspect/pageinspect--1.6--1.7.sql
new file mode 100644
index 0000000..b3d1fe4
--- /dev/null
+++ b/contrib/pageinspect/pageinspect--1.6--1.7.sql
@@ -0,0 +1,9 @@
+/* contrib/pageinspect/pageinspect--1.6--1.7.sql */
+
+-- complain if script is sourced in psql, rather than via ALTER EXTENSION
+\echo Use "ALTER EXTENSION pageinspect UPDATE TO '1.7'" to load this file. \quit
+
+-- decode infomask flags as human readable flag names
+CREATE FUNCTION heap_infomask_flags(infomask1 integer, infomask2 integer,
+ include_combined boolean DEFAULT true)
+RETURNS text[] STRICT LANGUAGE 'c' AS 'MODULE_PATHNAME','heap_infomask_flags';
diff --git a/contrib/pageinspect/pageinspect.control b/contrib/pageinspect/pageinspect.control
index 1a61c9f..dcfc61f 100644
--- a/contrib/pageinspect/pageinspect.control
+++ b/contrib/pageinspect/pageinspect.control
@@ -1,5 +1,5 @@
# pageinspect extension
comment = 'inspect the contents of database pages at a low level'
-default_version = '1.6'
+default_version = '1.7'
module_pathname = '$libdir/pageinspect'
relocatable = true
diff --git a/contrib/pageinspect/sql/page.sql b/contrib/pageinspect/sql/page.sql
index 493ca9b..b7a1f0b 100644
--- a/contrib/pageinspect/sql/page.sql
+++ b/contrib/pageinspect/sql/page.sql
@@ -31,6 +31,20 @@ SELECT tuple_data_split('test1'::regclass, t_data, t_infomask, t_infomask2, t_bi
SELECT * FROM fsm_page_contents(get_raw_page('test1', 'fsm', 0));
+-- If we freeze the only tuple on test1, the infomask should
+-- always be the same in all test runs.
+VACUUM FREEZE test1;
+
+SELECT t_infomask, t_infomask2, flags
+FROM heap_page_items(get_raw_page('test1', 0)),
+ LATERAL heap_infomask_flags(t_infomask, t_infomask2, true) m(flags);
+
+SELECT t_infomask, t_infomask2, flags
+FROM heap_page_items(get_raw_page('test1', 0)),
+ LATERAL heap_infomask_flags(t_infomask, t_infomask2, false) m(flags);
+
+SELECT heap_infomask_flags(2816, 0);
+
DROP TABLE test1;
-- check that using any of these functions with a partitioned table would fail
diff --git a/doc/src/sgml/pageinspect.sgml b/doc/src/sgml/pageinspect.sgml
index ccdaf3e..24925a0 100644
--- a/doc/src/sgml/pageinspect.sgml
+++ b/doc/src/sgml/pageinspect.sgml
@@ -151,6 +151,10 @@ test=# SELECT * FROM heap_page_items(get_raw_page('pg_class', 0));
<filename>src/include/access/htup_details.h</> for explanations of the fields
returned.
</para>
+ <para>
+ The <function>heap_infomask</function> function can be used to unpack the
+ recognised bits of the infomasks of heap tuples.
+ </para>
</listitem>
</varlistentry>
@@ -206,6 +210,34 @@ test=# SELECT * FROM heap_page_item_attrs(get_raw_page('pg_class', 0), 'pg_class
<varlistentry>
<term>
+ <function>heap_infomask_flags(infomask1 integer, infomask2 integer, show_combined bool) returns text[]</function>
+ <indexterm>
+ <primary>heap_infomask_flags</primary>
+ </indexterm>
+ </term>
+ <listitem>
+ <para>
+ <function>heap_infomask_flags</function> decodes the
+ <structfield>t_infomask1</structfield> and
+ <structfield>t_infomask2</structfield> returned by
+ <function>heap_page_items</function> into a human-readable array of flag
+ names. This can be used to see the tuple hint bits etc.
+ </para>
+ <para>
+ If show_combined is set (the default), combination flags like
+ <literal>HEAP_XMIN_FROZEN</literal> are also output. The original fields
+ are not filtered out, so e.g. a frozen tuple will have
+ <literal>HEAP_XMIN_FROZEN, HEAP_XMIN_COMMITTED, HEAP_XMIN_INVALID</literal>.
+ </para>
+ <para>
+ For the meaning of these flags see
+ <filename>src/include/access/htup_details.h</>
+ </para>
+ </listitem>
+ </varlistentry>
+
+ <varlistentry>
+ <term>
<function>fsm_page_contents(page bytea) returns text</function>
<indexterm>
<primary>fsm_page_contents</primary>
--
2.9.4
^ permalink raw reply [nested|flat] 54+ messages in thread
* Re: [PATCH] pageinspect function to decode infomasks
2017-07-20 03:33 [PATCH] pageinspect function to decode infomasks Craig Ringer <[email protected]>
@ 2017-07-20 03:38 ` Craig Ringer <[email protected]>
1 sibling, 0 replies; 54+ messages in thread
From: Craig Ringer @ 2017-07-20 03:38 UTC (permalink / raw)
To: pgsql-hackers; +Cc: Álvaro Herrera <[email protected]>; abhijit Menon-Sen <[email protected]>; Peter Eisentraut <[email protected]>; Pavan Deolasee <[email protected]>
On 20 July 2017 at 11:33, Craig Ringer <[email protected]> wrote:
> Hi
>
> Whenever I'm debugging some kind of corruption incident, possible
> visibility bug, etc, I always land up staring at integer infomasks or using
> a SQL helper function to decode them.
>
> That's silly, so here's a patch to teach pageinspect how to decode
> infomasks to a human readable array of flag names.
>
> Example:
>
> SELECT t_infomask, t_infomask2, flags
> FROM heap_page_items(get_raw_page('test1', 0)),
> LATERAL heap_infomask_flags(t_infomask, t_infomask2, true) m(flags);
> t_infomask | t_infomask2 | flags
>
> ------------+-------------+---------------------------------
> -------------------------------------------
> 2816 | 2 | {HEAP_XMIN_COMMITTED,HEAP_
> XMIN_INVALID,HEAP_XMAX_INVALID,HEAP_XMIN_FROZEN}
> (1 row)
>
>
> To decode individual mask integers you can just call it directly. It's
> strict, so pass 0 for the other mask if you don't have both, e.g.
>
> SELECT heap_infomask_flags(2816, 0);
>
> The patch backports easily to older pageinspect versions for when you're
> debugging something old.
>
> BTW, I used text[] not enums. That costs a fair bit of memory, but it
> doesn't seem worth worrying too much about in this context.
>
> For convenience it also tests and reports HEAP_LOCKED_UPGRADED and
> HEAP_XMAX_IS_LOCKED_ONLY as pseudo-flags.
>
> I decided not to filter out HEAP_XMIN_COMMITTED,HEAP_XMIN_INVALID,HEAP_XMAX_INVALID
> when HEAP_XMIN_FROZEN is set
>
Er, decided not to filter out HEAP_XMIN_COMMITTED,HEAP_XMIN_INVALID.
Obviously wouldn't filter out HEAP_XMAX_INVALID, that was a copy-paste'o.
I wonder if it's worth dropping the HEAP_ prefix. Meh, anyway, usable as-is.
--
Craig Ringer http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
^ permalink raw reply [nested|flat] 54+ messages in thread
* Re: [PATCH] pageinspect function to decode infomasks
2017-07-20 03:33 [PATCH] pageinspect function to decode infomasks Craig Ringer <[email protected]>
@ 2017-07-20 03:44 ` Peter Geoghegan <[email protected]>
2017-07-20 06:13 ` Re: [PATCH] pageinspect function to decode infomasks Julien Rouhaud <[email protected]>
1 sibling, 1 reply; 54+ messages in thread
From: Peter Geoghegan @ 2017-07-20 03:44 UTC (permalink / raw)
To: Craig Ringer <[email protected]>; +Cc: pgsql-hackers; Álvaro Herrera <[email protected]>; abhijit Menon-Sen <[email protected]>; Peter Eisentraut <[email protected]>; Pavan Deolasee <[email protected]>
On Wed, Jul 19, 2017 at 8:33 PM, Craig Ringer <[email protected]> wrote:
> That's silly, so here's a patch to teach pageinspect how to decode infomasks
> to a human readable array of flag names.
>
> Example:
>
> SELECT t_infomask, t_infomask2, flags
> FROM heap_page_items(get_raw_page('test1', 0)),
> LATERAL heap_infomask_flags(t_infomask, t_infomask2, true) m(flags);
> t_infomask | t_infomask2 | flags
> ------------+-------------+----------------------------------------------------------------------------
> 2816 | 2 |
> {HEAP_XMIN_COMMITTED,HEAP_XMIN_INVALID,HEAP_XMAX_INVALID,HEAP_XMIN_FROZEN}
> (1 row)
Seems like a good idea to me.
--
Peter Geoghegan
--
Sent via pgsql-hackers mailing list ([email protected])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
^ permalink raw reply [nested|flat] 54+ messages in thread
* Re: [PATCH] pageinspect function to decode infomasks
2017-07-20 03:33 [PATCH] pageinspect function to decode infomasks Craig Ringer <[email protected]>
2017-07-20 03:44 ` Re: [PATCH] pageinspect function to decode infomasks Peter Geoghegan <[email protected]>
@ 2017-07-20 06:13 ` Julien Rouhaud <[email protected]>
2017-07-20 06:26 ` Re: [PATCH] pageinspect function to decode infomasks Masahiko Sawada <[email protected]>
0 siblings, 1 reply; 54+ messages in thread
From: Julien Rouhaud @ 2017-07-20 06:13 UTC (permalink / raw)
To: Peter Geoghegan <[email protected]>; +Cc: Craig Ringer <[email protected]>; pgsql-hackers; Álvaro Herrera <[email protected]>; abhijit Menon-Sen <[email protected]>; Peter Eisentraut <[email protected]>; Pavan Deolasee <[email protected]>
On Thu, Jul 20, 2017 at 5:44 AM, Peter Geoghegan <[email protected]> wrote:
> On Wed, Jul 19, 2017 at 8:33 PM, Craig Ringer <[email protected]> wrote:
>> That's silly, so here's a patch to teach pageinspect how to decode infomasks
>> to a human readable array of flag names.
>>
>> Example:
>>
>> SELECT t_infomask, t_infomask2, flags
>> FROM heap_page_items(get_raw_page('test1', 0)),
>> LATERAL heap_infomask_flags(t_infomask, t_infomask2, true) m(flags);
>> t_infomask | t_infomask2 | flags
>> ------------+-------------+----------------------------------------------------------------------------
>> 2816 | 2 |
>> {HEAP_XMIN_COMMITTED,HEAP_XMIN_INVALID,HEAP_XMAX_INVALID,HEAP_XMIN_FROZEN}
>> (1 row)
>
> Seems like a good idea to me.
>
+1, it'll be really helpful.
--
Sent via pgsql-hackers mailing list ([email protected])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
^ permalink raw reply [nested|flat] 54+ messages in thread
* Re: [PATCH] pageinspect function to decode infomasks
2017-07-20 03:33 [PATCH] pageinspect function to decode infomasks Craig Ringer <[email protected]>
2017-07-20 03:44 ` Re: [PATCH] pageinspect function to decode infomasks Peter Geoghegan <[email protected]>
2017-07-20 06:13 ` Re: [PATCH] pageinspect function to decode infomasks Julien Rouhaud <[email protected]>
@ 2017-07-20 06:26 ` Masahiko Sawada <[email protected]>
2017-07-20 11:09 ` Re: [PATCH] pageinspect function to decode infomasks Ashutosh Sharma <[email protected]>
0 siblings, 1 reply; 54+ messages in thread
From: Masahiko Sawada @ 2017-07-20 06:26 UTC (permalink / raw)
To: Julien Rouhaud <[email protected]>; +Cc: Peter Geoghegan <[email protected]>; Craig Ringer <[email protected]>; pgsql-hackers; Álvaro Herrera <[email protected]>; abhijit Menon-Sen <[email protected]>; Peter Eisentraut <[email protected]>; Pavan Deolasee <[email protected]>
On Thu, Jul 20, 2017 at 3:13 PM, Julien Rouhaud <[email protected]> wrote:
> On Thu, Jul 20, 2017 at 5:44 AM, Peter Geoghegan <[email protected]> wrote:
>> On Wed, Jul 19, 2017 at 8:33 PM, Craig Ringer <[email protected]> wrote:
>>> That's silly, so here's a patch to teach pageinspect how to decode infomasks
>>> to a human readable array of flag names.
>>>
>>> Example:
>>>
>>> SELECT t_infomask, t_infomask2, flags
>>> FROM heap_page_items(get_raw_page('test1', 0)),
>>> LATERAL heap_infomask_flags(t_infomask, t_infomask2, true) m(flags);
>>> t_infomask | t_infomask2 | flags
>>> ------------+-------------+----------------------------------------------------------------------------
>>> 2816 | 2 |
>>> {HEAP_XMIN_COMMITTED,HEAP_XMIN_INVALID,HEAP_XMAX_INVALID,HEAP_XMIN_FROZEN}
>>> (1 row)
>>
>> Seems like a good idea to me.
>>
>
> +1, it'll be really helpful.
>
+1.
When I investigated data corruption incident I also wrote a plpgsql
function for the same purpose, and it was very useful. I think we can
have the similar thing for lp_flags as well.
Regards,
--
Masahiko Sawada
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center
--
Sent via pgsql-hackers mailing list ([email protected])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
^ permalink raw reply [nested|flat] 54+ messages in thread
* Re: [PATCH] pageinspect function to decode infomasks
2017-07-20 03:33 [PATCH] pageinspect function to decode infomasks Craig Ringer <[email protected]>
2017-07-20 03:44 ` Re: [PATCH] pageinspect function to decode infomasks Peter Geoghegan <[email protected]>
2017-07-20 06:13 ` Re: [PATCH] pageinspect function to decode infomasks Julien Rouhaud <[email protected]>
2017-07-20 06:26 ` Re: [PATCH] pageinspect function to decode infomasks Masahiko Sawada <[email protected]>
@ 2017-07-20 11:09 ` Ashutosh Sharma <[email protected]>
2017-07-20 11:52 ` Re: [PATCH] pageinspect function to decode infomasks Craig Ringer <[email protected]>
2017-07-21 03:29 ` Re: [PATCH] pageinspect function to decode infomasks Craig Ringer <[email protected]>
0 siblings, 2 replies; 54+ messages in thread
From: Ashutosh Sharma @ 2017-07-20 11:09 UTC (permalink / raw)
To: Masahiko Sawada <[email protected]>; +Cc: Julien Rouhaud <[email protected]>; Peter Geoghegan <[email protected]>; Craig Ringer <[email protected]>; pgsql-hackers; Álvaro Herrera <[email protected]>; abhijit Menon-Sen <[email protected]>; Peter Eisentraut <[email protected]>; Pavan Deolasee <[email protected]>
I had a quick look into this patch and also tested it and following
are my observations.
1) I am seeing a server crash when passing any non meaningful value
for t_infomask2 to heap_infomask_flags().
postgres=# SELECT heap_infomask_flags(2816, 3);
server closed the connection unexpectedly
This probably means the server terminated abnormally
before or while processing the request.
The connection to the server was lost. Attempting reset: Failed.
!> \q
Following is the backtrace,
(gdb) bt
#0 0x0000000000d9c55b in pg_detoast_datum (datum=0x0) at fmgr.c:1833
#1 0x0000000000b87374 in construct_md_array (elems=0x2ad74c0,
nulls=0x0, ndims=1, dims=0x7ffc0b0bbcd0, lbs=0x7ffc0b0bbcc0,
elmtype=25, elmlen=-1,
elmbyval=0 '\000', elmalign=105 'i') at arrayfuncs.c:3382
#2 0x0000000000b8709f in construct_array (elems=0x2ad74c0, nelems=10,
elmtype=25, elmlen=-1, elmbyval=0 '\000', elmalign=105 'i') at
arrayfuncs.c:3316
#3 0x00007fb8001603a5 in heap_infomask_flags (fcinfo=0x2ad3b88) at
heapfuncs.c:597
#4 0x000000000082f4cd in ExecInterpExpr (state=0x2ad3aa0,
econtext=0x2ad3750, isnull=0x7ffc0b0bbf67 "") at execExprInterp.c:672
#5 0x000000000088b832 in ExecEvalExprSwitchContext (state=0x2ad3aa0,
econtext=0x2ad3750, isNull=0x7ffc0b0bbf67 "")
at ../../../src/include/executor/executor.h:290
#6 0x000000000088b8e3 in ExecProject (projInfo=0x2ad3a98) at
../../../src/include/executor/executor.h:324
#7 0x000000000088bb89 in ExecResult (node=0x2ad36b8) at nodeResult.c:132
#8 0x00000000008494fe in ExecProcNode (node=0x2ad36b8) at execProcnode.c:416
#9 0x000000000084125d in ExecutePlan (estate=0x2ad34a0,
planstate=0x2ad36b8, use_parallel_mode=0 '\000', operation=CMD_SELECT,
sendTuples=1 '\001',
numberTuples=0, direction=ForwardScanDirection, dest=0x2ac0ae0,
execute_once=1 '\001') at execMain.c:1693
#10 0x000000000083d54b in standard_ExecutorRun (queryDesc=0x2a42880,
direction=ForwardScanDirection, count=0, execute_once=1 '\001') at
execMain.c:362
#11 0x000000000083d253 in ExecutorRun (queryDesc=0x2a42880,
direction=ForwardScanDirection, count=0, execute_once=1 '\001') at
execMain.c:305
#12 0x0000000000b3dd8f in PortalRunSelect (portal=0x2ad1490, forward=1
'\001', count=0, dest=0x2ac0ae0) at pquery.c:932
#13 0x0000000000b3d7e7 in PortalRun (portal=0x2ad1490,
count=9223372036854775807, isTopLevel=1 '\001', run_once=1 '\001',
dest=0x2ac0ae0, altdest=0x2ac0ae0,
completionTag=0x7ffc0b0bc2c0 "") at pquery.c:773
#14 0x0000000000b31fe4 in exec_simple_query (query_string=0x2a9d9a0
"SELECT heap_infomask_flags(11008, 1111, true);") at postgres.c:1099
#15 0x0000000000b3a727 in PostgresMain (argc=1, argv=0x2a49eb0,
dbname=0x2a1d480 "postgres", username=0x2a49d18 "ashu") at
postgres.c:4090
#16 0x0000000000a2cb3f in BackendRun (port=0x2a3e700) at postmaster.c:4357
#17 0x0000000000a2bc63 in BackendStartup (port=0x2a3e700) at postmaster.c:4029
#18 0x0000000000a248ab in ServerLoop () at postmaster.c:1753
#19 0x0000000000a236a9 in PostmasterMain (argc=3, argv=0x2a1b2b0) at
postmaster.c:1361
#20 0x00000000008d8054 in main (argc=3, argv=0x2a1b2b0) at main.c:228
2) I can see the documentation for heap_infomask(). But, I do not see
it being defined or used anywhere in the patch.
+ <para>
+ The <function>heap_infomask</function> function can be used to unpack the
+ recognised bits of the infomasks of heap tuples.
+ </para>
3) If show_combined flag is set to it's default value and a tuple is
frozen then may i know the reason for not showing it as frozen tuple
when t_infomask2
is passed as zero.
postgres=# SELECT heap_infomask_flags(2816, 0);
heap_infomask_flags
-----------------------------------------------------------
{HEAP_XMIN_COMMITTED,HEAP_XMIN_INVALID,HEAP_XMAX_INVALID}
(1 row)
postgres=# SELECT heap_infomask_flags(2816, 1);
heap_infomask_flags
----------------------------------------------------------------------------
{HEAP_XMIN_COMMITTED,HEAP_XMIN_INVALID,HEAP_XMAX_INVALID,HEAP_XMIN_FROZEN}
(1 row)
4) I think, it would be better to use the same argument name for the
newly added function i.e heap_infomask_flags() in both documentation
and sql file. I am basically refering to 'include_combined' argument.
IF you see the function definition, the argument name used is
'include_combined' whereas in documentation you have mentioned
'show_combined'.
--
With Regards,
Ashutosh Sharma
EnterpriseDB:http://www.enterprisedb.com
On Thu, Jul 20, 2017 at 11:56 AM, Masahiko Sawada <[email protected]> wrote:
> On Thu, Jul 20, 2017 at 3:13 PM, Julien Rouhaud <[email protected]> wrote:
>> On Thu, Jul 20, 2017 at 5:44 AM, Peter Geoghegan <[email protected]> wrote:
>>> On Wed, Jul 19, 2017 at 8:33 PM, Craig Ringer <[email protected]> wrote:
>>>> That's silly, so here's a patch to teach pageinspect how to decode infomasks
>>>> to a human readable array of flag names.
>>>>
>>>> Example:
>>>>
>>>> SELECT t_infomask, t_infomask2, flags
>>>> FROM heap_page_items(get_raw_page('test1', 0)),
>>>> LATERAL heap_infomask_flags(t_infomask, t_infomask2, true) m(flags);
>>>> t_infomask | t_infomask2 | flags
>>>> ------------+-------------+----------------------------------------------------------------------------
>>>> 2816 | 2 |
>>>> {HEAP_XMIN_COMMITTED,HEAP_XMIN_INVALID,HEAP_XMAX_INVALID,HEAP_XMIN_FROZEN}
>>>> (1 row)
>>>
>>> Seems like a good idea to me.
>>>
>>
>> +1, it'll be really helpful.
>>
>
> +1.
> When I investigated data corruption incident I also wrote a plpgsql
> function for the same purpose, and it was very useful. I think we can
> have the similar thing for lp_flags as well.
>
> Regards,
>
> --
> Masahiko Sawada
> NIPPON TELEGRAPH AND TELEPHONE CORPORATION
> NTT Open Source Software Center
>
>
> --
> Sent via pgsql-hackers mailing list ([email protected])
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-hackers
--
Sent via pgsql-hackers mailing list ([email protected])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
^ permalink raw reply [nested|flat] 54+ messages in thread
* Re: [PATCH] pageinspect function to decode infomasks
2017-07-20 03:33 [PATCH] pageinspect function to decode infomasks Craig Ringer <[email protected]>
2017-07-20 03:44 ` Re: [PATCH] pageinspect function to decode infomasks Peter Geoghegan <[email protected]>
2017-07-20 06:13 ` Re: [PATCH] pageinspect function to decode infomasks Julien Rouhaud <[email protected]>
2017-07-20 06:26 ` Re: [PATCH] pageinspect function to decode infomasks Masahiko Sawada <[email protected]>
2017-07-20 11:09 ` Re: [PATCH] pageinspect function to decode infomasks Ashutosh Sharma <[email protected]>
@ 2017-07-20 11:52 ` Craig Ringer <[email protected]>
2017-08-15 01:11 ` Re: [PATCH] pageinspect function to decode infomasks Moon Insung <[email protected]>
1 sibling, 1 reply; 54+ messages in thread
From: Craig Ringer @ 2017-07-20 11:52 UTC (permalink / raw)
To: Ashutosh Sharma <[email protected]>; +Cc: pgsql-hackers; Julien Rouhaud <[email protected]>; Pavan Deolasee <[email protected]>; Álvaro Herrera <[email protected]>; Peter Eisentraut <[email protected]>; Masahiko Sawada <[email protected]>; abhijit Menon-Sen <[email protected]>; Peter Geoghegan <[email protected]>
On 20 Jul. 2017 19:09, "Ashutosh Sharma" <[email protected]> wrote:
I had a quick look into this patch and also tested it and following
are my observations.
Thanks very much.
I'll expand the tests to cover various normal and nonsensical masks and
combinations and fix the identified issues.
This was a quick morning's work in amongst other things so not surprised I
missed a few details. The check is appreciated.
^ permalink raw reply [nested|flat] 54+ messages in thread
* Re: [PATCH] pageinspect function to decode infomasks
2017-07-20 03:33 [PATCH] pageinspect function to decode infomasks Craig Ringer <[email protected]>
2017-07-20 03:44 ` Re: [PATCH] pageinspect function to decode infomasks Peter Geoghegan <[email protected]>
2017-07-20 06:13 ` Re: [PATCH] pageinspect function to decode infomasks Julien Rouhaud <[email protected]>
2017-07-20 06:26 ` Re: [PATCH] pageinspect function to decode infomasks Masahiko Sawada <[email protected]>
2017-07-20 11:09 ` Re: [PATCH] pageinspect function to decode infomasks Ashutosh Sharma <[email protected]>
2017-07-20 11:52 ` Re: [PATCH] pageinspect function to decode infomasks Craig Ringer <[email protected]>
@ 2017-08-15 01:11 ` Moon Insung <[email protected]>
2017-08-15 01:59 ` Re: [PATCH] pageinspect function to decode infomasks Craig Ringer <[email protected]>
0 siblings, 1 reply; 54+ messages in thread
From: Moon Insung @ 2017-08-15 01:11 UTC (permalink / raw)
To: 'Craig Ringer' <[email protected]>; +Cc: pgsql-hackers; 'Julien Rouhaud' <[email protected]>; 'Pavan Deolasee' <[email protected]>; =?utf-8?Q?'=C3=81lvaro_Herrera'?= <[email protected]>; 'Peter Eisentraut' <[email protected]>; 'Masahiko Sawada' <[email protected]>; 'abhijit Menon-Sen' <[email protected]>; 'Peter Geoghegan' <[email protected]>; 'Ashutosh Sharma' <[email protected]>
Dear Craig Ringer
Frist, thank you for implementing the necessary function.
but, i have some question.
question 1) vacuum freeze hint bits
If run a vacuum freeze, bits in the infomask will be 0x0300.
in this case, if output the value of informsk in the run to you modified,
HEAP_XMIN_COMMITTED(0x0100), HEAP_XMIN_INVALID(0x0200), HEAP_XMIN_FROZEN(0x0300)
all outputs to hint bits.
is it normal to output values?
if look at htup_details.h code,
#define HeapTupleHeaderXminInvalid(tup) \
( \
((tup)->t_infomask & (HEAP_XMIN_COMMITTED|HEAP_XMIN_INVALID)) == \
HEAP_XMIN_INVALID \
)
#define HeapTupleHeaderSetXminCommitted(tup) \
( \
AssertMacro(!HeapTupleHeaderXminInvalid(tup)), \
((tup)->t_infomask |= HEAP_XMIN_COMMITTED) \
)
HEAP_XMIN_INVALID and HEAP_XMIN_COMMITTED can not be write simultaneously.
So I think the value of 0x0300 is to HEAP_XMIN_COMMITTED, HEAP_XMIN_FROZEN
Only output needs to be values.
question 2) xmax lock hint bits
similar to the vacuum freezeze question..
Assume that the infomask has a bit of 0x0050
In this case, if run on the code that you modified,
HEAP_XMAX_KEYSHR_LOCK(0x0010), HEAP_XMAX_EXCL_LOCK(0x0040), HEAP_XMAX_IS_LOCKED_ONLY
three hint bits are the output.
if look at htup_details.h code,
#define HEAP_XMAX_IS_SHR_LOCKED(infomask) \
(((infomask) & HEAP_LOCK_MASK) == HEAP_XMAX_SHR_LOCK)
#define HEAP_XMAX_IS_EXCL_LOCKED(infomask) \
(((infomask) & HEAP_LOCK_MASK) == HEAP_XMAX_EXCL_LOCK)
#define HEAP_XMAX_IS_KEYSHR_LOCKED(infomask) \
(((infomask) & HEAP_LOCK_MASK) == HEAP_XMAX_KEYSHR_LOCK)
It is divided into to hint bits.
so I think this part needs to fix.
If my opinion may be wrong. So plz check one more time.
Regards.
Moon
From: [email protected] [mailto:[email protected]] On Behalf Of Craig Ringer
Sent: Thursday, July 20, 2017 8:53 PM
To: Ashutosh Sharma
Cc: PostgreSQL Hackers; Julien Rouhaud; Pavan Deolasee; Álvaro Herrera; Peter Eisentraut; Masahiko Sawada; abhijit Menon-Sen; Peter Geoghegan
Subject: Re: [HACKERS] [PATCH] pageinspect function to decode infomasks
On 20 Jul. 2017 19:09, "Ashutosh Sharma" <[email protected] <mailto:[email protected]> > wrote:
I had a quick look into this patch and also tested it and following
are my observations.
Thanks very much.
I'll expand the tests to cover various normal and nonsensical masks and combinations and fix the identified issues.
This was a quick morning's work in amongst other things so not surprised I missed a few details. The check is appreciated.
^ permalink raw reply [nested|flat] 54+ messages in thread
* Re: [PATCH] pageinspect function to decode infomasks
2017-07-20 03:33 [PATCH] pageinspect function to decode infomasks Craig Ringer <[email protected]>
2017-07-20 03:44 ` Re: [PATCH] pageinspect function to decode infomasks Peter Geoghegan <[email protected]>
2017-07-20 06:13 ` Re: [PATCH] pageinspect function to decode infomasks Julien Rouhaud <[email protected]>
2017-07-20 06:26 ` Re: [PATCH] pageinspect function to decode infomasks Masahiko Sawada <[email protected]>
2017-07-20 11:09 ` Re: [PATCH] pageinspect function to decode infomasks Ashutosh Sharma <[email protected]>
2017-07-20 11:52 ` Re: [PATCH] pageinspect function to decode infomasks Craig Ringer <[email protected]>
2017-08-15 01:11 ` Re: [PATCH] pageinspect function to decode infomasks Moon Insung <[email protected]>
@ 2017-08-15 01:59 ` Craig Ringer <[email protected]>
2017-08-15 13:24 ` Re: [PATCH] pageinspect function to decode infomasks Robert Haas <[email protected]>
0 siblings, 1 reply; 54+ messages in thread
From: Craig Ringer @ 2017-08-15 01:59 UTC (permalink / raw)
To: Moon Insung <[email protected]>; +Cc: pgsql-hackers; Julien Rouhaud <[email protected]>; Pavan Deolasee <[email protected]>; Álvaro Herrera <[email protected]>; Peter Eisentraut <[email protected]>; Masahiko Sawada <[email protected]>; abhijit Menon-Sen <[email protected]>; Peter Geoghegan <[email protected]>; Ashutosh Sharma <[email protected]>
On 15 August 2017 at 09:11, Moon Insung <[email protected]>
wrote:
> Dear Craig Ringer
>
>
>
> Frist, thank you for implementing the necessary function.
>
>
>
> but, i have some question.
>
>
>
> question 1) vacuum freeze hint bits
>
> If run a vacuum freeze, bits in the infomask will be 0x0300.
>
> in this case, if output the value of informsk in the run to you modified,
>
> HEAP_XMIN_COMMITTED(0x0100), HEAP_XMIN_INVALID(0x0200),
> HEAP_XMIN_FROZEN(0x0300)
>
> all outputs to hint bits.
>
>
>
> is it normal to output values?
>
>
>
> if look at htup_details.h code,
>
>
>
> #define HeapTupleHeaderXminInvalid(tup) \
>
> ( \
>
> ((tup)->t_infomask & (HEAP_XMIN_COMMITTED|HEAP_XMIN_INVALID))
> == \
>
> HEAP_XMIN_INVALID \
>
> )
>
> #define HeapTupleHeaderSetXminCommitted(tup) \
>
> ( \
>
> AssertMacro(!HeapTupleHeaderXminInvalid(tup)), \
>
> ((tup)->t_infomask |= HEAP_XMIN_COMMITTED) \
>
> )
>
>
>
> HEAP_XMIN_INVALID and HEAP_XMIN_COMMITTED can not be write simultaneously.
>
The bits are set, those macros just test to exclude the special meaning of
both bits being set at once to mean "frozen".
I was reluctant to filter out HEAP_XMIN_COMMITTED and HEAP_XMIN_INVALID
when we detect that it's frozen, because that could well be misleading when
debugging.
If you think that is useful, then I suggest you add an option so that when
it's outputting the interpreted mask not the raw mask, it suppresses output
of HEAP_XMIN_COMMITTED and HEAP_XMIN_INVALID if HEAP_XMIN_FROZEN.
question 2) xmax lock hint bits
>
> similar to the vacuum freezeze question..
>
> Assume that the infomask has a bit of 0x0050
>
>
>
> In this case, if run on the code that you modified,
>
> HEAP_XMAX_KEYSHR_LOCK(0x0010), HEAP_XMAX_EXCL_LOCK(0x0040),
> HEAP_XMAX_IS_LOCKED_ONLY
>
> three hint bits are the output.
>
>
>
> if look at htup_details.h code,
>
>
>
> #define HEAP_XMAX_IS_SHR_LOCKED(infomask) \
>
> (((infomask) & HEAP_LOCK_MASK) == HEAP_XMAX_SHR_LOCK)
>
> #define HEAP_XMAX_IS_EXCL_LOCKED(infomask) \
>
> (((infomask) & HEAP_LOCK_MASK) == HEAP_XMAX_EXCL_LOCK)
>
> #define HEAP_XMAX_IS_KEYSHR_LOCKED(infomask) \
>
> (((infomask) & HEAP_LOCK_MASK) == HEAP_XMAX_KEYSHR_LOCK)
>
>
>
> It is divided into to hint bits.
>
> so I think this part needs to fix.
>
It's the same issue as above, with the same answer IMO.
If we're showing the raw mask bits we should show the raw mask bits only.
But if we're showing combined bits and masks too, I guess we should filter
out the raw bits when matched by some mask.
I'm not entirely convinced by that, since I think hiding information could
create more confusion than it fixes. I welcome others' views here.
--
Craig Ringer http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
^ permalink raw reply [nested|flat] 54+ messages in thread
* Re: [PATCH] pageinspect function to decode infomasks
2017-07-20 03:33 [PATCH] pageinspect function to decode infomasks Craig Ringer <[email protected]>
2017-07-20 03:44 ` Re: [PATCH] pageinspect function to decode infomasks Peter Geoghegan <[email protected]>
2017-07-20 06:13 ` Re: [PATCH] pageinspect function to decode infomasks Julien Rouhaud <[email protected]>
2017-07-20 06:26 ` Re: [PATCH] pageinspect function to decode infomasks Masahiko Sawada <[email protected]>
2017-07-20 11:09 ` Re: [PATCH] pageinspect function to decode infomasks Ashutosh Sharma <[email protected]>
2017-07-20 11:52 ` Re: [PATCH] pageinspect function to decode infomasks Craig Ringer <[email protected]>
2017-08-15 01:11 ` Re: [PATCH] pageinspect function to decode infomasks Moon Insung <[email protected]>
2017-08-15 01:59 ` Re: [PATCH] pageinspect function to decode infomasks Craig Ringer <[email protected]>
@ 2017-08-15 13:24 ` Robert Haas <[email protected]>
2017-08-15 13:59 ` Re: [PATCH] pageinspect function to decode infomasks Tomas Vondra <[email protected]>
0 siblings, 1 reply; 54+ messages in thread
From: Robert Haas @ 2017-08-15 13:24 UTC (permalink / raw)
To: Craig Ringer <[email protected]>; +Cc: Moon Insung <[email protected]>; pgsql-hackers; Julien Rouhaud <[email protected]>; Pavan Deolasee <[email protected]>; Álvaro Herrera <[email protected]>; Peter Eisentraut <[email protected]>; Masahiko Sawada <[email protected]>; abhijit Menon-Sen <[email protected]>; Peter Geoghegan <[email protected]>; Ashutosh Sharma <[email protected]>
On Mon, Aug 14, 2017 at 9:59 PM, Craig Ringer <[email protected]> wrote:
> The bits are set, those macros just test to exclude the special meaning of
> both bits being set at once to mean "frozen".
>
> I was reluctant to filter out HEAP_XMIN_COMMITTED and HEAP_XMIN_INVALID
> when we detect that it's frozen, because that could well be misleading when
> debugging.
I don't think so -- the "committed" and "invalid" meanings are
effectively canceled when the "frozen" mask is present.
I mean, "committed" and "invalid" contradict each other...
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
--
Sent via pgsql-hackers mailing list ([email protected])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
^ permalink raw reply [nested|flat] 54+ messages in thread
* Re: [PATCH] pageinspect function to decode infomasks
2017-07-20 03:33 [PATCH] pageinspect function to decode infomasks Craig Ringer <[email protected]>
2017-07-20 03:44 ` Re: [PATCH] pageinspect function to decode infomasks Peter Geoghegan <[email protected]>
2017-07-20 06:13 ` Re: [PATCH] pageinspect function to decode infomasks Julien Rouhaud <[email protected]>
2017-07-20 06:26 ` Re: [PATCH] pageinspect function to decode infomasks Masahiko Sawada <[email protected]>
2017-07-20 11:09 ` Re: [PATCH] pageinspect function to decode infomasks Ashutosh Sharma <[email protected]>
2017-07-20 11:52 ` Re: [PATCH] pageinspect function to decode infomasks Craig Ringer <[email protected]>
2017-08-15 01:11 ` Re: [PATCH] pageinspect function to decode infomasks Moon Insung <[email protected]>
2017-08-15 01:59 ` Re: [PATCH] pageinspect function to decode infomasks Craig Ringer <[email protected]>
2017-08-15 13:24 ` Re: [PATCH] pageinspect function to decode infomasks Robert Haas <[email protected]>
@ 2017-08-15 13:59 ` Tomas Vondra <[email protected]>
2017-08-15 14:12 ` Re: [PATCH] pageinspect function to decode infomasks Masahiko Sawada <[email protected]>
2017-08-15 17:54 ` Re: [PATCH] pageinspect function to decode infomasks Robert Haas <[email protected]>
0 siblings, 2 replies; 54+ messages in thread
From: Tomas Vondra @ 2017-08-15 13:59 UTC (permalink / raw)
To: pgsql-hackers
On 08/15/2017 03:24 PM, Robert Haas wrote:
> On Mon, Aug 14, 2017 at 9:59 PM, Craig Ringer <[email protected]> wrote:
>> The bits are set, those macros just test to exclude the special meaning of
>> both bits being set at once to mean "frozen".
>>
>> I was reluctant to filter out HEAP_XMIN_COMMITTED and HEAP_XMIN_INVALID
>> when we detect that it's frozen, because that could well be misleading when
>> debugging.
>
> I don't think so -- the "committed" and "invalid" meanings are
> effectively canceled when the "frozen" mask is present.
>
> I mean, "committed" and "invalid" contradict each other...
>
FWIW I agree with Craig - the functions should output the masks raw,
without any filtering. The reason is that when you're investigating data
corruption or unexpected behavior, all this is very useful when
reasoning about what might (not) have happened.
Or at least make the filtering optional.
regards
--
Tomas Vondra http://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
--
Sent via pgsql-hackers mailing list ([email protected])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
^ permalink raw reply [nested|flat] 54+ messages in thread
* Re: [PATCH] pageinspect function to decode infomasks
2017-07-20 03:33 [PATCH] pageinspect function to decode infomasks Craig Ringer <[email protected]>
2017-07-20 03:44 ` Re: [PATCH] pageinspect function to decode infomasks Peter Geoghegan <[email protected]>
2017-07-20 06:13 ` Re: [PATCH] pageinspect function to decode infomasks Julien Rouhaud <[email protected]>
2017-07-20 06:26 ` Re: [PATCH] pageinspect function to decode infomasks Masahiko Sawada <[email protected]>
2017-07-20 11:09 ` Re: [PATCH] pageinspect function to decode infomasks Ashutosh Sharma <[email protected]>
2017-07-20 11:52 ` Re: [PATCH] pageinspect function to decode infomasks Craig Ringer <[email protected]>
2017-08-15 01:11 ` Re: [PATCH] pageinspect function to decode infomasks Moon Insung <[email protected]>
2017-08-15 01:59 ` Re: [PATCH] pageinspect function to decode infomasks Craig Ringer <[email protected]>
2017-08-15 13:24 ` Re: [PATCH] pageinspect function to decode infomasks Robert Haas <[email protected]>
2017-08-15 13:59 ` Re: [PATCH] pageinspect function to decode infomasks Tomas Vondra <[email protected]>
@ 2017-08-15 14:12 ` Masahiko Sawada <[email protected]>
1 sibling, 0 replies; 54+ messages in thread
From: Masahiko Sawada @ 2017-08-15 14:12 UTC (permalink / raw)
To: Tomas Vondra <[email protected]>; +Cc: pgsql-hackers
On Tue, Aug 15, 2017 at 10:59 PM, Tomas Vondra
<[email protected]> wrote:
>
>
> On 08/15/2017 03:24 PM, Robert Haas wrote:
>>
>> On Mon, Aug 14, 2017 at 9:59 PM, Craig Ringer <[email protected]>
>> wrote:
>>>
>>> The bits are set, those macros just test to exclude the special meaning
>>> of
>>> both bits being set at once to mean "frozen".
>>>
>>> I was reluctant to filter out HEAP_XMIN_COMMITTED and HEAP_XMIN_INVALID
>>> when we detect that it's frozen, because that could well be misleading
>>> when
>>> debugging.
>>
>>
>> I don't think so -- the "committed" and "invalid" meanings are
>> effectively canceled when the "frozen" mask is present.
>>
>> I mean, "committed" and "invalid" contradict each other...
>>
>
> FWIW I agree with Craig - the functions should output the masks raw, without
> any filtering. The reason is that when you're investigating data corruption
> or unexpected behavior, all this is very useful when reasoning about what
> might (not) have happened.
>
> Or at least make the filtering optional.
>
I'd vote for having both and making one optional (perhaps filtering?).
Both are useful to me for the debugging and study purpose.
Regards,
--
Masahiko Sawada
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center
--
Sent via pgsql-hackers mailing list ([email protected])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
^ permalink raw reply [nested|flat] 54+ messages in thread
* Re: [PATCH] pageinspect function to decode infomasks
2017-07-20 03:33 [PATCH] pageinspect function to decode infomasks Craig Ringer <[email protected]>
2017-07-20 03:44 ` Re: [PATCH] pageinspect function to decode infomasks Peter Geoghegan <[email protected]>
2017-07-20 06:13 ` Re: [PATCH] pageinspect function to decode infomasks Julien Rouhaud <[email protected]>
2017-07-20 06:26 ` Re: [PATCH] pageinspect function to decode infomasks Masahiko Sawada <[email protected]>
2017-07-20 11:09 ` Re: [PATCH] pageinspect function to decode infomasks Ashutosh Sharma <[email protected]>
2017-07-20 11:52 ` Re: [PATCH] pageinspect function to decode infomasks Craig Ringer <[email protected]>
2017-08-15 01:11 ` Re: [PATCH] pageinspect function to decode infomasks Moon Insung <[email protected]>
2017-08-15 01:59 ` Re: [PATCH] pageinspect function to decode infomasks Craig Ringer <[email protected]>
2017-08-15 13:24 ` Re: [PATCH] pageinspect function to decode infomasks Robert Haas <[email protected]>
2017-08-15 13:59 ` Re: [PATCH] pageinspect function to decode infomasks Tomas Vondra <[email protected]>
@ 2017-08-15 17:54 ` Robert Haas <[email protected]>
2017-08-15 19:42 ` Re: [PATCH] pageinspect function to decode infomasks Tomas Vondra <[email protected]>
2017-10-12 23:35 ` Re: [PATCH] pageinspect function to decode infomasks Peter Geoghegan <[email protected]>
1 sibling, 2 replies; 54+ messages in thread
From: Robert Haas @ 2017-08-15 17:54 UTC (permalink / raw)
To: Tomas Vondra <[email protected]>; +Cc: pgsql-hackers
On Tue, Aug 15, 2017 at 9:59 AM, Tomas Vondra
<[email protected]> wrote:
>> I don't think so -- the "committed" and "invalid" meanings are
>> effectively canceled when the "frozen" mask is present.
>>
>> I mean, "committed" and "invalid" contradict each other...
>
> FWIW I agree with Craig - the functions should output the masks raw, without
> any filtering. The reason is that when you're investigating data corruption
> or unexpected behavior, all this is very useful when reasoning about what
> might (not) have happened.
>
> Or at least make the filtering optional.
I don't think "filtering" is the right way to think about it. It's
just labeling each combination of bits with the meaning appropriate to
that combination of bits.
I mean, if you were displaying the contents of a CLOG entry, would you
want the value 3 to be displayed as COMMITTED ABORTED SUBCOMMITTED
because TRANSACTION_STATUS_COMMITTED|TRANSACTION_STATUS_ABORTED ==
TRANSACTION_STATUS_SUB_COMMITTED?
I realize that you may be used to thinking of the HEAP_XMIN_COMMITTED
and HEAP_XMAX_COMMITTED bits as two separate bits, but that's not
really true any more. They're a 2-bit field that can have one of four
values: committed, aborted, frozen, or none of the above.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
--
Sent via pgsql-hackers mailing list ([email protected])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
^ permalink raw reply [nested|flat] 54+ messages in thread
* Re: [PATCH] pageinspect function to decode infomasks
2017-07-20 03:33 [PATCH] pageinspect function to decode infomasks Craig Ringer <[email protected]>
2017-07-20 03:44 ` Re: [PATCH] pageinspect function to decode infomasks Peter Geoghegan <[email protected]>
2017-07-20 06:13 ` Re: [PATCH] pageinspect function to decode infomasks Julien Rouhaud <[email protected]>
2017-07-20 06:26 ` Re: [PATCH] pageinspect function to decode infomasks Masahiko Sawada <[email protected]>
2017-07-20 11:09 ` Re: [PATCH] pageinspect function to decode infomasks Ashutosh Sharma <[email protected]>
2017-07-20 11:52 ` Re: [PATCH] pageinspect function to decode infomasks Craig Ringer <[email protected]>
2017-08-15 01:11 ` Re: [PATCH] pageinspect function to decode infomasks Moon Insung <[email protected]>
2017-08-15 01:59 ` Re: [PATCH] pageinspect function to decode infomasks Craig Ringer <[email protected]>
2017-08-15 13:24 ` Re: [PATCH] pageinspect function to decode infomasks Robert Haas <[email protected]>
2017-08-15 13:59 ` Re: [PATCH] pageinspect function to decode infomasks Tomas Vondra <[email protected]>
2017-08-15 17:54 ` Re: [PATCH] pageinspect function to decode infomasks Robert Haas <[email protected]>
@ 2017-08-15 19:42 ` Tomas Vondra <[email protected]>
2017-08-15 19:55 ` Re: [PATCH] pageinspect function to decode infomasks Robert Haas <[email protected]>
2017-08-16 01:37 ` Re: [PATCH] pageinspect function to decode infomasks Craig Ringer <[email protected]>
1 sibling, 2 replies; 54+ messages in thread
From: Tomas Vondra @ 2017-08-15 19:42 UTC (permalink / raw)
To: Robert Haas <[email protected]>; +Cc: pgsql-hackers
On 08/15/2017 07:54 PM, Robert Haas wrote:
> On Tue, Aug 15, 2017 at 9:59 AM, Tomas Vondra
> <[email protected]> wrote:
>>> I don't think so -- the "committed" and "invalid" meanings are
>>> effectively canceled when the "frozen" mask is present.
>>>
>>> I mean, "committed" and "invalid" contradict each other...
>>
>> FWIW I agree with Craig - the functions should output the masks raw, without
>> any filtering. The reason is that when you're investigating data corruption
>> or unexpected behavior, all this is very useful when reasoning about what
>> might (not) have happened.
>>
>> Or at least make the filtering optional.
>
> I don't think "filtering" is the right way to think about it. It's
> just labeling each combination of bits with the meaning appropriate to
> that combination of bits.
>
> I mean, if you were displaying the contents of a CLOG entry, would you
> want the value 3 to be displayed as COMMITTED ABORTED SUBCOMMITTED
> because TRANSACTION_STATUS_COMMITTED|TRANSACTION_STATUS_ABORTED ==
> TRANSACTION_STATUS_SUB_COMMITTED?
>
> I realize that you may be used to thinking of the HEAP_XMIN_COMMITTED
> and HEAP_XMAX_COMMITTED bits as two separate bits, but that's not
> really true any more. They're a 2-bit field that can have one of four
> values: committed, aborted, frozen, or none of the above.
>
All I'm saying is that having the complete information (knowing which
bits are actually set in the bitmask) is valuable when reasoning about
how you might have gotten to the current state. Which I think is what
Craig is after.
What I think we should not do is interpret the bitmasks (omitting some
of the information) assuming all the bits were set correctly.
regards
--
Tomas Vondra http://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
--
Sent via pgsql-hackers mailing list ([email protected])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
^ permalink raw reply [nested|flat] 54+ messages in thread
* Re: [PATCH] pageinspect function to decode infomasks
2017-07-20 03:33 [PATCH] pageinspect function to decode infomasks Craig Ringer <[email protected]>
2017-07-20 03:44 ` Re: [PATCH] pageinspect function to decode infomasks Peter Geoghegan <[email protected]>
2017-07-20 06:13 ` Re: [PATCH] pageinspect function to decode infomasks Julien Rouhaud <[email protected]>
2017-07-20 06:26 ` Re: [PATCH] pageinspect function to decode infomasks Masahiko Sawada <[email protected]>
2017-07-20 11:09 ` Re: [PATCH] pageinspect function to decode infomasks Ashutosh Sharma <[email protected]>
2017-07-20 11:52 ` Re: [PATCH] pageinspect function to decode infomasks Craig Ringer <[email protected]>
2017-08-15 01:11 ` Re: [PATCH] pageinspect function to decode infomasks Moon Insung <[email protected]>
2017-08-15 01:59 ` Re: [PATCH] pageinspect function to decode infomasks Craig Ringer <[email protected]>
2017-08-15 13:24 ` Re: [PATCH] pageinspect function to decode infomasks Robert Haas <[email protected]>
2017-08-15 13:59 ` Re: [PATCH] pageinspect function to decode infomasks Tomas Vondra <[email protected]>
2017-08-15 17:54 ` Re: [PATCH] pageinspect function to decode infomasks Robert Haas <[email protected]>
2017-08-15 19:42 ` Re: [PATCH] pageinspect function to decode infomasks Tomas Vondra <[email protected]>
@ 2017-08-15 19:55 ` Robert Haas <[email protected]>
2017-08-15 20:36 ` Re: [PATCH] pageinspect function to decode infomasks Tomas Vondra <[email protected]>
1 sibling, 1 reply; 54+ messages in thread
From: Robert Haas @ 2017-08-15 19:55 UTC (permalink / raw)
To: Tomas Vondra <[email protected]>; +Cc: pgsql-hackers
On Tue, Aug 15, 2017 at 3:42 PM, Tomas Vondra
<[email protected]> wrote:
> What I think we should not do is interpret the bitmasks (omitting some of
> the information) assuming all the bits were set correctly.
I'm still confused. HEAP_XMIN_COMMITTED|HEAP_XMIN_ABORTED ==
HEAP_XMIN_FROZEN. Nobody is proposing to omit anything; to the
contrary, what's being proposed is not to display the same thing twice
(and in a misleading fashion, to boot).
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
--
Sent via pgsql-hackers mailing list ([email protected])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
^ permalink raw reply [nested|flat] 54+ messages in thread
* Re: [PATCH] pageinspect function to decode infomasks
2017-07-20 03:33 [PATCH] pageinspect function to decode infomasks Craig Ringer <[email protected]>
2017-07-20 03:44 ` Re: [PATCH] pageinspect function to decode infomasks Peter Geoghegan <[email protected]>
2017-07-20 06:13 ` Re: [PATCH] pageinspect function to decode infomasks Julien Rouhaud <[email protected]>
2017-07-20 06:26 ` Re: [PATCH] pageinspect function to decode infomasks Masahiko Sawada <[email protected]>
2017-07-20 11:09 ` Re: [PATCH] pageinspect function to decode infomasks Ashutosh Sharma <[email protected]>
2017-07-20 11:52 ` Re: [PATCH] pageinspect function to decode infomasks Craig Ringer <[email protected]>
2017-08-15 01:11 ` Re: [PATCH] pageinspect function to decode infomasks Moon Insung <[email protected]>
2017-08-15 01:59 ` Re: [PATCH] pageinspect function to decode infomasks Craig Ringer <[email protected]>
2017-08-15 13:24 ` Re: [PATCH] pageinspect function to decode infomasks Robert Haas <[email protected]>
2017-08-15 13:59 ` Re: [PATCH] pageinspect function to decode infomasks Tomas Vondra <[email protected]>
2017-08-15 17:54 ` Re: [PATCH] pageinspect function to decode infomasks Robert Haas <[email protected]>
2017-08-15 19:42 ` Re: [PATCH] pageinspect function to decode infomasks Tomas Vondra <[email protected]>
2017-08-15 19:55 ` Re: [PATCH] pageinspect function to decode infomasks Robert Haas <[email protected]>
@ 2017-08-15 20:36 ` Tomas Vondra <[email protected]>
2017-08-16 01:33 ` Re: [PATCH] pageinspect function to decode infomasks Moon Insung <[email protected]>
2017-08-16 15:14 ` Re: [PATCH] pageinspect function to decode infomasks Robert Haas <[email protected]>
0 siblings, 2 replies; 54+ messages in thread
From: Tomas Vondra @ 2017-08-15 20:36 UTC (permalink / raw)
To: Robert Haas <[email protected]>; +Cc: pgsql-hackers
On 08/15/2017 09:55 PM, Robert Haas wrote:
> On Tue, Aug 15, 2017 at 3:42 PM, Tomas Vondra
> <[email protected]> wrote:
>> What I think we should not do is interpret the bitmasks (omitting some of
>> the information) assuming all the bits were set correctly.
>
> I'm still confused. HEAP_XMIN_COMMITTED|HEAP_XMIN_ABORTED ==
> HEAP_XMIN_FROZEN. Nobody is proposing to omit anything; to the
> contrary, what's being proposed is not to display the same thing
> twice (and in a misleading fashion, to boot).
>
I understand your point. Assume you're looking at this bit of code:
if (HeapTupleHeaderXminCommitted(enumval_tup->t_data))
return;
which is essentially
if (enumval_tup->t_data & HEAP_XMIN_COMMITTED)
return;
If the function only gives you HEAP_XMIN_FROZEN, how likely is it you
miss this actually evaluates as true?
You might say that people investigating issues in this area of code
should be aware of how HEAP_XMIN_FROZEN is defined, and perhaps you're
right ...
regards
--
Tomas Vondra http://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
--
Sent via pgsql-hackers mailing list ([email protected])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
^ permalink raw reply [nested|flat] 54+ messages in thread
* Re: [PATCH] pageinspect function to decode infomasks
2017-07-20 03:33 [PATCH] pageinspect function to decode infomasks Craig Ringer <[email protected]>
2017-07-20 03:44 ` Re: [PATCH] pageinspect function to decode infomasks Peter Geoghegan <[email protected]>
2017-07-20 06:13 ` Re: [PATCH] pageinspect function to decode infomasks Julien Rouhaud <[email protected]>
2017-07-20 06:26 ` Re: [PATCH] pageinspect function to decode infomasks Masahiko Sawada <[email protected]>
2017-07-20 11:09 ` Re: [PATCH] pageinspect function to decode infomasks Ashutosh Sharma <[email protected]>
2017-07-20 11:52 ` Re: [PATCH] pageinspect function to decode infomasks Craig Ringer <[email protected]>
2017-08-15 01:11 ` Re: [PATCH] pageinspect function to decode infomasks Moon Insung <[email protected]>
2017-08-15 01:59 ` Re: [PATCH] pageinspect function to decode infomasks Craig Ringer <[email protected]>
2017-08-15 13:24 ` Re: [PATCH] pageinspect function to decode infomasks Robert Haas <[email protected]>
2017-08-15 13:59 ` Re: [PATCH] pageinspect function to decode infomasks Tomas Vondra <[email protected]>
2017-08-15 17:54 ` Re: [PATCH] pageinspect function to decode infomasks Robert Haas <[email protected]>
2017-08-15 19:42 ` Re: [PATCH] pageinspect function to decode infomasks Tomas Vondra <[email protected]>
2017-08-15 19:55 ` Re: [PATCH] pageinspect function to decode infomasks Robert Haas <[email protected]>
2017-08-15 20:36 ` Re: [PATCH] pageinspect function to decode infomasks Tomas Vondra <[email protected]>
@ 2017-08-16 01:33 ` Moon Insung <[email protected]>
1 sibling, 0 replies; 54+ messages in thread
From: Moon Insung @ 2017-08-16 01:33 UTC (permalink / raw)
To: 'Tomas Vondra' <[email protected]>; 'Robert Haas' <[email protected]>; +Cc: pgsql-hackers
I checked for code related to infomask.
(add flag state -- HEAP_XMIN_COMMITTED, HEAP_XMIN_INVALID, HEAP_XMIN_FROZEN)
first i'm still beginner level about postgresql, so my opinion may be wrong.
if the "HEAP_XMIN_COMMITTED" flag is added, check the function of "HeapTupleHeaderXminInvalid"
if the "HEAP_XMIN_INVALID" flag is added, check the function of "HeapTupleHeaderXminCommitted"
if the "HEAP_XMIN_FROZEN" flag is added, use the "HeapTupleHeaderSetXminFrozen" function or
use the code as
--------------------------------------
xid = HeapTupleHeaderGetXmin(tuple);
if (TransactionIdIsNormal(xid))
{
if (TransactionIdPrecedes(xid, cutoff_xid))
{
frz->t_infomask |= HEAP_XMIN_FROZEN;
changed = true;
}
else
totally_frozen = false;
}
--------------------------------------
to add the flag.
so as a result, HEAP_XMIN_INVALID and HEAP_XMIN_COMMITTED is cannot coexist.
unfortunately, i don't know if HEAP_XMIN_COMMITTED and HEAP_XMIN_FROZEN flags can coexist.
so i think it's also a good idea to output the raw masks, without any filtering.
however, i think the information that is presented to the user should inform us which flags was entered.
Regards.
Moon
> -----Original Message-----
> From: [email protected]
> [mailto:[email protected]] On Behalf Of Tomas Vondra
> Sent: Wednesday, August 16, 2017 5:36 AM
> To: Robert Haas
> Cc: [email protected]
> Subject: Re: [HACKERS] [PATCH] pageinspect function to decode infomasks
>
>
>
> On 08/15/2017 09:55 PM, Robert Haas wrote:
> > On Tue, Aug 15, 2017 at 3:42 PM, Tomas Vondra
> > <[email protected]> wrote:
> >> What I think we should not do is interpret the bitmasks (omitting
> >> some of the information) assuming all the bits were set correctly.
> >
> > I'm still confused. HEAP_XMIN_COMMITTED|HEAP_XMIN_ABORTED ==
> > HEAP_XMIN_FROZEN. Nobody is proposing to omit anything; to the
> > contrary, what's being proposed is not to display the same thing twice
> > (and in a misleading fashion, to boot).
> >
>
> I understand your point. Assume you're looking at this bit of code:
>
> if (HeapTupleHeaderXminCommitted(enumval_tup->t_data))
> return;
>
> which is essentially
>
> if (enumval_tup->t_data & HEAP_XMIN_COMMITTED)
> return;
>
> If the function only gives you HEAP_XMIN_FROZEN, how likely is it you miss
> this actually evaluates as true?
>
> You might say that people investigating issues in this area of code should
> be aware of how HEAP_XMIN_FROZEN is defined, and perhaps you're right ...
>
> regards
>
> --
> Tomas Vondra http://www.2ndQuadrant.com
> PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
>
>
> --
> Sent via pgsql-hackers mailing list ([email protected]) To make
> changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-hackers
--
Sent via pgsql-hackers mailing list ([email protected])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
^ permalink raw reply [nested|flat] 54+ messages in thread
* Re: [PATCH] pageinspect function to decode infomasks
2017-07-20 03:33 [PATCH] pageinspect function to decode infomasks Craig Ringer <[email protected]>
2017-07-20 03:44 ` Re: [PATCH] pageinspect function to decode infomasks Peter Geoghegan <[email protected]>
2017-07-20 06:13 ` Re: [PATCH] pageinspect function to decode infomasks Julien Rouhaud <[email protected]>
2017-07-20 06:26 ` Re: [PATCH] pageinspect function to decode infomasks Masahiko Sawada <[email protected]>
2017-07-20 11:09 ` Re: [PATCH] pageinspect function to decode infomasks Ashutosh Sharma <[email protected]>
2017-07-20 11:52 ` Re: [PATCH] pageinspect function to decode infomasks Craig Ringer <[email protected]>
2017-08-15 01:11 ` Re: [PATCH] pageinspect function to decode infomasks Moon Insung <[email protected]>
2017-08-15 01:59 ` Re: [PATCH] pageinspect function to decode infomasks Craig Ringer <[email protected]>
2017-08-15 13:24 ` Re: [PATCH] pageinspect function to decode infomasks Robert Haas <[email protected]>
2017-08-15 13:59 ` Re: [PATCH] pageinspect function to decode infomasks Tomas Vondra <[email protected]>
2017-08-15 17:54 ` Re: [PATCH] pageinspect function to decode infomasks Robert Haas <[email protected]>
2017-08-15 19:42 ` Re: [PATCH] pageinspect function to decode infomasks Tomas Vondra <[email protected]>
2017-08-15 19:55 ` Re: [PATCH] pageinspect function to decode infomasks Robert Haas <[email protected]>
2017-08-15 20:36 ` Re: [PATCH] pageinspect function to decode infomasks Tomas Vondra <[email protected]>
@ 2017-08-16 15:14 ` Robert Haas <[email protected]>
2017-08-17 00:20 ` Re: [PATCH] pageinspect function to decode infomasks Craig Ringer <[email protected]>
1 sibling, 1 reply; 54+ messages in thread
From: Robert Haas @ 2017-08-16 15:14 UTC (permalink / raw)
To: Tomas Vondra <[email protected]>; +Cc: pgsql-hackers
On Tue, Aug 15, 2017 at 4:36 PM, Tomas Vondra
<[email protected]> wrote:
> You might say that people investigating issues in this area of code should
> be aware of how HEAP_XMIN_FROZEN is defined, and perhaps you're right ...
Yes, I think that's what I would say. I mean, if you happen to NOT
know that committed|invalid == frozen, but you DO know what committed
means and what invalid means, then you're going to be *really*
confused when you see committed and invalid set on the same tuple.
Showing you frozen has got to be clearer.
Now, I agree with you that a test like (enumval_tup->t_data &
HEAP_XMIN_COMMITTED) could be confusing to someone who doesn't realize
that HEAP_XMIN_FROZEN & HEAP_XMIN_COMMITTED == HEAP_XMIN_COMMITTED,
but I think that's just one of those things that unfortunately is
going to require adequate knowledge for people investigating issues.
If there's an action item there, it might be to try to come up with a
way to make the source code clearer.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
--
Sent via pgsql-hackers mailing list ([email protected])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
^ permalink raw reply [nested|flat] 54+ messages in thread
* Re: [PATCH] pageinspect function to decode infomasks
2017-07-20 03:33 [PATCH] pageinspect function to decode infomasks Craig Ringer <[email protected]>
2017-07-20 03:44 ` Re: [PATCH] pageinspect function to decode infomasks Peter Geoghegan <[email protected]>
2017-07-20 06:13 ` Re: [PATCH] pageinspect function to decode infomasks Julien Rouhaud <[email protected]>
2017-07-20 06:26 ` Re: [PATCH] pageinspect function to decode infomasks Masahiko Sawada <[email protected]>
2017-07-20 11:09 ` Re: [PATCH] pageinspect function to decode infomasks Ashutosh Sharma <[email protected]>
2017-07-20 11:52 ` Re: [PATCH] pageinspect function to decode infomasks Craig Ringer <[email protected]>
2017-08-15 01:11 ` Re: [PATCH] pageinspect function to decode infomasks Moon Insung <[email protected]>
2017-08-15 01:59 ` Re: [PATCH] pageinspect function to decode infomasks Craig Ringer <[email protected]>
2017-08-15 13:24 ` Re: [PATCH] pageinspect function to decode infomasks Robert Haas <[email protected]>
2017-08-15 13:59 ` Re: [PATCH] pageinspect function to decode infomasks Tomas Vondra <[email protected]>
2017-08-15 17:54 ` Re: [PATCH] pageinspect function to decode infomasks Robert Haas <[email protected]>
2017-08-15 19:42 ` Re: [PATCH] pageinspect function to decode infomasks Tomas Vondra <[email protected]>
2017-08-15 19:55 ` Re: [PATCH] pageinspect function to decode infomasks Robert Haas <[email protected]>
2017-08-15 20:36 ` Re: [PATCH] pageinspect function to decode infomasks Tomas Vondra <[email protected]>
2017-08-16 15:14 ` Re: [PATCH] pageinspect function to decode infomasks Robert Haas <[email protected]>
@ 2017-08-17 00:20 ` Craig Ringer <[email protected]>
2017-09-14 11:57 ` Re: [PATCH] pageinspect function to decode infomasks Ashutosh Sharma <[email protected]>
0 siblings, 1 reply; 54+ messages in thread
From: Craig Ringer @ 2017-08-17 00:20 UTC (permalink / raw)
To: Robert Haas <[email protected]>; +Cc: Tomas Vondra <[email protected]>; pgsql-hackers
On 16 August 2017 at 23:14, Robert Haas <[email protected]> wrote:
> On Tue, Aug 15, 2017 at 4:36 PM, Tomas Vondra
> <[email protected]> wrote:
> > You might say that people investigating issues in this area of code
> should
> > be aware of how HEAP_XMIN_FROZEN is defined, and perhaps you're right ...
>
> Yes, I think that's what I would say. I mean, if you happen to NOT
> know that committed|invalid == frozen, but you DO know what committed
> means and what invalid means, then you're going to be *really*
> confused when you see committed and invalid set on the same tuple.
> Showing you frozen has got to be clearer.
>
> Now, I agree with you that a test like (enumval_tup->t_data &
> HEAP_XMIN_COMMITTED) could be confusing to someone who doesn't realize
> that HEAP_XMIN_FROZEN & HEAP_XMIN_COMMITTED == HEAP_XMIN_COMMITTED,
> but I think that's just one of those things that unfortunately is
> going to require adequate knowledge for people investigating issues.
> If there's an action item there, it might be to try to come up with a
> way to make the source code clearer.
>
>
For other multi-purpose flags we have macros, and I think it'd make sense
to use them here too.
Eschew direct use of HEAP_XMIN_COMMITTED, HEAP_XMIN_INVALID and
HEAP_XMIN_FROZEN in tests. Instead, consistently use HeapXminIsFrozen(),
HeapXminIsCommitted(), and HeapXminIsInvalid() or something like that.
--
Craig Ringer http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
^ permalink raw reply [nested|flat] 54+ messages in thread
* Re: [PATCH] pageinspect function to decode infomasks
2017-07-20 03:33 [PATCH] pageinspect function to decode infomasks Craig Ringer <[email protected]>
2017-07-20 03:44 ` Re: [PATCH] pageinspect function to decode infomasks Peter Geoghegan <[email protected]>
2017-07-20 06:13 ` Re: [PATCH] pageinspect function to decode infomasks Julien Rouhaud <[email protected]>
2017-07-20 06:26 ` Re: [PATCH] pageinspect function to decode infomasks Masahiko Sawada <[email protected]>
2017-07-20 11:09 ` Re: [PATCH] pageinspect function to decode infomasks Ashutosh Sharma <[email protected]>
2017-07-20 11:52 ` Re: [PATCH] pageinspect function to decode infomasks Craig Ringer <[email protected]>
2017-08-15 01:11 ` Re: [PATCH] pageinspect function to decode infomasks Moon Insung <[email protected]>
2017-08-15 01:59 ` Re: [PATCH] pageinspect function to decode infomasks Craig Ringer <[email protected]>
2017-08-15 13:24 ` Re: [PATCH] pageinspect function to decode infomasks Robert Haas <[email protected]>
2017-08-15 13:59 ` Re: [PATCH] pageinspect function to decode infomasks Tomas Vondra <[email protected]>
2017-08-15 17:54 ` Re: [PATCH] pageinspect function to decode infomasks Robert Haas <[email protected]>
2017-08-15 19:42 ` Re: [PATCH] pageinspect function to decode infomasks Tomas Vondra <[email protected]>
2017-08-15 19:55 ` Re: [PATCH] pageinspect function to decode infomasks Robert Haas <[email protected]>
2017-08-15 20:36 ` Re: [PATCH] pageinspect function to decode infomasks Tomas Vondra <[email protected]>
2017-08-16 15:14 ` Re: [PATCH] pageinspect function to decode infomasks Robert Haas <[email protected]>
2017-08-17 00:20 ` Re: [PATCH] pageinspect function to decode infomasks Craig Ringer <[email protected]>
@ 2017-09-14 11:57 ` Ashutosh Sharma <[email protected]>
2017-09-14 14:00 ` Re: [PATCH] pageinspect function to decode infomasks Craig Ringer <[email protected]>
0 siblings, 1 reply; 54+ messages in thread
From: Ashutosh Sharma @ 2017-09-14 11:57 UTC (permalink / raw)
To: Craig Ringer <[email protected]>; +Cc: Robert Haas <[email protected]>; Tomas Vondra <[email protected]>; pgsql-hackers
Hi Craig,
On Thu, Aug 17, 2017 at 5:50 AM, Craig Ringer <[email protected]> wrote:
> On 16 August 2017 at 23:14, Robert Haas <[email protected]> wrote:
>>
>> On Tue, Aug 15, 2017 at 4:36 PM, Tomas Vondra
>> <[email protected]> wrote:
>> > You might say that people investigating issues in this area of code
>> > should
>> > be aware of how HEAP_XMIN_FROZEN is defined, and perhaps you're right
>> > ...
>>
>> Yes, I think that's what I would say. I mean, if you happen to NOT
>> know that committed|invalid == frozen, but you DO know what committed
>> means and what invalid means, then you're going to be *really*
>> confused when you see committed and invalid set on the same tuple.
>> Showing you frozen has got to be clearer.
>>
>> Now, I agree with you that a test like (enumval_tup->t_data &
>> HEAP_XMIN_COMMITTED) could be confusing to someone who doesn't realize
>> that HEAP_XMIN_FROZEN & HEAP_XMIN_COMMITTED == HEAP_XMIN_COMMITTED,
>> but I think that's just one of those things that unfortunately is
>> going to require adequate knowledge for people investigating issues.
>> If there's an action item there, it might be to try to come up with a
>> way to make the source code clearer.
>>
>
> For other multi-purpose flags we have macros, and I think it'd make sense to
> use them here too.
>
> Eschew direct use of HEAP_XMIN_COMMITTED, HEAP_XMIN_INVALID and
> HEAP_XMIN_FROZEN in tests. Instead, consistently use HeapXminIsFrozen(),
> HeapXminIsCommitted(), and HeapXminIsInvalid() or something like that.
>
> --
Are you planning to work on the review comments from Robert, Moon
Insung and supply the new patch. I just had a quick glance into this
mail thread (after a long time) and could understand Robert's concern
till some extent. I think, he is trying to say that if a tuple is
frozen (committed|invalid) then it shouldn't be shown as COMMITTED and
INVALID together in fact it should just be displayed as FROZEN tuple.
--
With Regards,
Ashutosh Sharma
EnterpriseDB:http://www.enterprisedb.com
--
Sent via pgsql-hackers mailing list ([email protected])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
^ permalink raw reply [nested|flat] 54+ messages in thread
* Re: [PATCH] pageinspect function to decode infomasks
2017-07-20 03:33 [PATCH] pageinspect function to decode infomasks Craig Ringer <[email protected]>
2017-07-20 03:44 ` Re: [PATCH] pageinspect function to decode infomasks Peter Geoghegan <[email protected]>
2017-07-20 06:13 ` Re: [PATCH] pageinspect function to decode infomasks Julien Rouhaud <[email protected]>
2017-07-20 06:26 ` Re: [PATCH] pageinspect function to decode infomasks Masahiko Sawada <[email protected]>
2017-07-20 11:09 ` Re: [PATCH] pageinspect function to decode infomasks Ashutosh Sharma <[email protected]>
2017-07-20 11:52 ` Re: [PATCH] pageinspect function to decode infomasks Craig Ringer <[email protected]>
2017-08-15 01:11 ` Re: [PATCH] pageinspect function to decode infomasks Moon Insung <[email protected]>
2017-08-15 01:59 ` Re: [PATCH] pageinspect function to decode infomasks Craig Ringer <[email protected]>
2017-08-15 13:24 ` Re: [PATCH] pageinspect function to decode infomasks Robert Haas <[email protected]>
2017-08-15 13:59 ` Re: [PATCH] pageinspect function to decode infomasks Tomas Vondra <[email protected]>
2017-08-15 17:54 ` Re: [PATCH] pageinspect function to decode infomasks Robert Haas <[email protected]>
2017-08-15 19:42 ` Re: [PATCH] pageinspect function to decode infomasks Tomas Vondra <[email protected]>
2017-08-15 19:55 ` Re: [PATCH] pageinspect function to decode infomasks Robert Haas <[email protected]>
2017-08-15 20:36 ` Re: [PATCH] pageinspect function to decode infomasks Tomas Vondra <[email protected]>
2017-08-16 15:14 ` Re: [PATCH] pageinspect function to decode infomasks Robert Haas <[email protected]>
2017-08-17 00:20 ` Re: [PATCH] pageinspect function to decode infomasks Craig Ringer <[email protected]>
2017-09-14 11:57 ` Re: [PATCH] pageinspect function to decode infomasks Ashutosh Sharma <[email protected]>
@ 2017-09-14 14:00 ` Craig Ringer <[email protected]>
2017-10-17 07:03 ` Re: [PATCH] pageinspect function to decode infomasks Masahiko Sawada <[email protected]>
0 siblings, 1 reply; 54+ messages in thread
From: Craig Ringer @ 2017-09-14 14:00 UTC (permalink / raw)
To: Ashutosh Sharma <[email protected]>; +Cc: Robert Haas <[email protected]>; Tomas Vondra <[email protected]>; pgsql-hackers
On 14 September 2017 at 19:57, Ashutosh Sharma <[email protected]>
wrote:
>
> Are you planning to work on the review comments from Robert, Moon
> Insung and supply the new patch. I just had a quick glance into this
> mail thread (after a long time) and could understand Robert's concern
> till some extent. I think, he is trying to say that if a tuple is
> frozen (committed|invalid) then it shouldn't be shown as COMMITTED and
> INVALID together in fact it should just be displayed as FROZEN tuple.
Yes, I'd like to, and should have time for it in this CF.
My plan is to emit raw flags by default, so FROZEN would't be shown at all,
only COMMITTED|INVALID. If the bool to decode combined flags is set, then
it'll show things like FROZEN, and hide COMMITTED|INVALID. Similar for
other combos.
--
Craig Ringer http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
^ permalink raw reply [nested|flat] 54+ messages in thread
* Re: [PATCH] pageinspect function to decode infomasks
2017-07-20 03:33 [PATCH] pageinspect function to decode infomasks Craig Ringer <[email protected]>
2017-07-20 03:44 ` Re: [PATCH] pageinspect function to decode infomasks Peter Geoghegan <[email protected]>
2017-07-20 06:13 ` Re: [PATCH] pageinspect function to decode infomasks Julien Rouhaud <[email protected]>
2017-07-20 06:26 ` Re: [PATCH] pageinspect function to decode infomasks Masahiko Sawada <[email protected]>
2017-07-20 11:09 ` Re: [PATCH] pageinspect function to decode infomasks Ashutosh Sharma <[email protected]>
2017-07-20 11:52 ` Re: [PATCH] pageinspect function to decode infomasks Craig Ringer <[email protected]>
2017-08-15 01:11 ` Re: [PATCH] pageinspect function to decode infomasks Moon Insung <[email protected]>
2017-08-15 01:59 ` Re: [PATCH] pageinspect function to decode infomasks Craig Ringer <[email protected]>
2017-08-15 13:24 ` Re: [PATCH] pageinspect function to decode infomasks Robert Haas <[email protected]>
2017-08-15 13:59 ` Re: [PATCH] pageinspect function to decode infomasks Tomas Vondra <[email protected]>
2017-08-15 17:54 ` Re: [PATCH] pageinspect function to decode infomasks Robert Haas <[email protected]>
2017-08-15 19:42 ` Re: [PATCH] pageinspect function to decode infomasks Tomas Vondra <[email protected]>
2017-08-15 19:55 ` Re: [PATCH] pageinspect function to decode infomasks Robert Haas <[email protected]>
2017-08-15 20:36 ` Re: [PATCH] pageinspect function to decode infomasks Tomas Vondra <[email protected]>
2017-08-16 15:14 ` Re: [PATCH] pageinspect function to decode infomasks Robert Haas <[email protected]>
2017-08-17 00:20 ` Re: [PATCH] pageinspect function to decode infomasks Craig Ringer <[email protected]>
2017-09-14 11:57 ` Re: [PATCH] pageinspect function to decode infomasks Ashutosh Sharma <[email protected]>
2017-09-14 14:00 ` Re: [PATCH] pageinspect function to decode infomasks Craig Ringer <[email protected]>
@ 2017-10-17 07:03 ` Masahiko Sawada <[email protected]>
0 siblings, 0 replies; 54+ messages in thread
From: Masahiko Sawada @ 2017-10-17 07:03 UTC (permalink / raw)
To: Craig Ringer <[email protected]>; +Cc: Ashutosh Sharma <[email protected]>; Robert Haas <[email protected]>; Tomas Vondra <[email protected]>; pgsql-hackers
On Thu, Sep 14, 2017 at 11:00 PM, Craig Ringer <[email protected]> wrote:
> On 14 September 2017 at 19:57, Ashutosh Sharma <[email protected]>
> wrote:
>
>>
>>
>> Are you planning to work on the review comments from Robert, Moon
>> Insung and supply the new patch. I just had a quick glance into this
>> mail thread (after a long time) and could understand Robert's concern
>> till some extent. I think, he is trying to say that if a tuple is
>> frozen (committed|invalid) then it shouldn't be shown as COMMITTED and
>> INVALID together in fact it should just be displayed as FROZEN tuple.
>
>
> Yes, I'd like to, and should have time for it in this CF.
>
> My plan is to emit raw flags by default, so FROZEN would't be shown at all,
> only COMMITTED|INVALID. If the bool to decode combined flags is set, then
> it'll show things like FROZEN, and hide COMMITTED|INVALID. Similar for other
> combos.
>
FWIW, I agree with this direction. ISTM the showing the raw flags by
default and having an option to show combined flags would be a right
way.
I sometimes wanted to have the same mechanism for lp_flags but maybe
it should be discussed on a separated thread.
Regards,
--
Masahiko Sawada
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center
--
Sent via pgsql-hackers mailing list ([email protected])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
^ permalink raw reply [nested|flat] 54+ messages in thread
* Re: [PATCH] pageinspect function to decode infomasks
2017-07-20 03:33 [PATCH] pageinspect function to decode infomasks Craig Ringer <[email protected]>
2017-07-20 03:44 ` Re: [PATCH] pageinspect function to decode infomasks Peter Geoghegan <[email protected]>
2017-07-20 06:13 ` Re: [PATCH] pageinspect function to decode infomasks Julien Rouhaud <[email protected]>
2017-07-20 06:26 ` Re: [PATCH] pageinspect function to decode infomasks Masahiko Sawada <[email protected]>
2017-07-20 11:09 ` Re: [PATCH] pageinspect function to decode infomasks Ashutosh Sharma <[email protected]>
2017-07-20 11:52 ` Re: [PATCH] pageinspect function to decode infomasks Craig Ringer <[email protected]>
2017-08-15 01:11 ` Re: [PATCH] pageinspect function to decode infomasks Moon Insung <[email protected]>
2017-08-15 01:59 ` Re: [PATCH] pageinspect function to decode infomasks Craig Ringer <[email protected]>
2017-08-15 13:24 ` Re: [PATCH] pageinspect function to decode infomasks Robert Haas <[email protected]>
2017-08-15 13:59 ` Re: [PATCH] pageinspect function to decode infomasks Tomas Vondra <[email protected]>
2017-08-15 17:54 ` Re: [PATCH] pageinspect function to decode infomasks Robert Haas <[email protected]>
2017-08-15 19:42 ` Re: [PATCH] pageinspect function to decode infomasks Tomas Vondra <[email protected]>
@ 2017-08-16 01:37 ` Craig Ringer <[email protected]>
1 sibling, 0 replies; 54+ messages in thread
From: Craig Ringer @ 2017-08-16 01:37 UTC (permalink / raw)
To: Tomas Vondra <[email protected]>; +Cc: Robert Haas <[email protected]>; pgsql-hackers
On 16 August 2017 at 03:42, Tomas Vondra <[email protected]>
wrote:
>
>
> On 08/15/2017 07:54 PM, Robert Haas wrote:
>
>> On Tue, Aug 15, 2017 at 9:59 AM, Tomas Vondra
>> <[email protected]> wrote:
>>
>>> I don't think so -- the "committed" and "invalid" meanings are
>>>> effectively canceled when the "frozen" mask is present.
>>>>
>>>> I mean, "committed" and "invalid" contradict each other...
>>>>
>>>
>>> FWIW I agree with Craig - the functions should output the masks raw,
>>> without
>>> any filtering. The reason is that when you're investigating data
>>> corruption
>>> or unexpected behavior, all this is very useful when reasoning about what
>>> might (not) have happened.
>>>
>>> Or at least make the filtering optional.
>>>
>>
>> I don't think "filtering" is the right way to think about it. It's
>> just labeling each combination of bits with the meaning appropriate to
>> that combination of bits.
>>
>> I mean, if you were displaying the contents of a CLOG entry, would you
>> want the value 3 to be displayed as COMMITTED ABORTED SUBCOMMITTED
>> because TRANSACTION_STATUS_COMMITTED|TRANSACTION_STATUS_ABORTED ==
>> TRANSACTION_STATUS_SUB_COMMITTED?
>>
>> I realize that you may be used to thinking of the HEAP_XMIN_COMMITTED
>> and HEAP_XMAX_COMMITTED bits as two separate bits, but that's not
>> really true any more. They're a 2-bit field that can have one of four
>> values: committed, aborted, frozen, or none of the above.
>>
>>
> All I'm saying is that having the complete information (knowing which bits
> are actually set in the bitmask) is valuable when reasoning about how you
> might have gotten to the current state. Which I think is what Craig is
> after.
>
> What I think we should not do is interpret the bitmasks (omitting some of
> the information) assuming all the bits were set correctly.
I agree, and the patch already does half of this: it can output just the
raw bit flags, or it can interpret them to show HEAP_XMIN_FROZEN etc.
So the required change, which seems to have broad agreement, is to have the
"interpret the bits" mode show only HEAP_XMIN_FROZEN when it sees
HEAP_XMIN_COMMITTED|HEAP_XMIN_INVALID, etc. We can retain raw-flags output
as-is for when seriously bogus state is suspected.
Any takers?
--
Craig Ringer http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
^ permalink raw reply [nested|flat] 54+ messages in thread
* Re: [PATCH] pageinspect function to decode infomasks
2017-07-20 03:33 [PATCH] pageinspect function to decode infomasks Craig Ringer <[email protected]>
2017-07-20 03:44 ` Re: [PATCH] pageinspect function to decode infomasks Peter Geoghegan <[email protected]>
2017-07-20 06:13 ` Re: [PATCH] pageinspect function to decode infomasks Julien Rouhaud <[email protected]>
2017-07-20 06:26 ` Re: [PATCH] pageinspect function to decode infomasks Masahiko Sawada <[email protected]>
2017-07-20 11:09 ` Re: [PATCH] pageinspect function to decode infomasks Ashutosh Sharma <[email protected]>
2017-07-20 11:52 ` Re: [PATCH] pageinspect function to decode infomasks Craig Ringer <[email protected]>
2017-08-15 01:11 ` Re: [PATCH] pageinspect function to decode infomasks Moon Insung <[email protected]>
2017-08-15 01:59 ` Re: [PATCH] pageinspect function to decode infomasks Craig Ringer <[email protected]>
2017-08-15 13:24 ` Re: [PATCH] pageinspect function to decode infomasks Robert Haas <[email protected]>
2017-08-15 13:59 ` Re: [PATCH] pageinspect function to decode infomasks Tomas Vondra <[email protected]>
2017-08-15 17:54 ` Re: [PATCH] pageinspect function to decode infomasks Robert Haas <[email protected]>
@ 2017-10-12 23:35 ` Peter Geoghegan <[email protected]>
2017-10-13 20:02 ` Re: [PATCH] pageinspect function to decode infomasks Robert Haas <[email protected]>
1 sibling, 1 reply; 54+ messages in thread
From: Peter Geoghegan @ 2017-10-12 23:35 UTC (permalink / raw)
To: Robert Haas <[email protected]>; +Cc: Tomas Vondra <[email protected]>; pgsql-hackers
On Tue, Aug 15, 2017 at 10:54 AM, Robert Haas <[email protected]> wrote:
>> Or at least make the filtering optional.
>
> I don't think "filtering" is the right way to think about it. It's
> just labeling each combination of bits with the meaning appropriate to
> that combination of bits.
I do. -1 to not just showing what's on the page -- if the
HEAP_XMIN_COMMITTED and HEAP_XMIN_ABORTED bits are set, then I think
we should show them. Yeah, I accept that there is a real danger of
confusing people with that. Unfortunately, I think that displaying
HEAP_XMIN_FROZEN will cause even more confusion. I don't think that
HEAP_XMIN_FROZEN is an abstraction at all. It's a notational
convenience.
I don't think it's our place to "interpret" the bits. Are we *also*
going to show HEAP_XMIN_FROZEN when xmin is physically set to
FrozenTransactionId? Where does it end?
I think that we should prominently document that HEAP_XMIN_COMMITTED
|HEAP_XMIN_ABORTED == HEAP_XMIN_FROZEN, rather than trying to hide
complexity that we have no business hiding in a tool like pageinspect.
--
Peter Geoghegan
--
Sent via pgsql-hackers mailing list ([email protected])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
^ permalink raw reply [nested|flat] 54+ messages in thread
* Re: [PATCH] pageinspect function to decode infomasks
2017-07-20 03:33 [PATCH] pageinspect function to decode infomasks Craig Ringer <[email protected]>
2017-07-20 03:44 ` Re: [PATCH] pageinspect function to decode infomasks Peter Geoghegan <[email protected]>
2017-07-20 06:13 ` Re: [PATCH] pageinspect function to decode infomasks Julien Rouhaud <[email protected]>
2017-07-20 06:26 ` Re: [PATCH] pageinspect function to decode infomasks Masahiko Sawada <[email protected]>
2017-07-20 11:09 ` Re: [PATCH] pageinspect function to decode infomasks Ashutosh Sharma <[email protected]>
2017-07-20 11:52 ` Re: [PATCH] pageinspect function to decode infomasks Craig Ringer <[email protected]>
2017-08-15 01:11 ` Re: [PATCH] pageinspect function to decode infomasks Moon Insung <[email protected]>
2017-08-15 01:59 ` Re: [PATCH] pageinspect function to decode infomasks Craig Ringer <[email protected]>
2017-08-15 13:24 ` Re: [PATCH] pageinspect function to decode infomasks Robert Haas <[email protected]>
2017-08-15 13:59 ` Re: [PATCH] pageinspect function to decode infomasks Tomas Vondra <[email protected]>
2017-08-15 17:54 ` Re: [PATCH] pageinspect function to decode infomasks Robert Haas <[email protected]>
2017-10-12 23:35 ` Re: [PATCH] pageinspect function to decode infomasks Peter Geoghegan <[email protected]>
@ 2017-10-13 20:02 ` Robert Haas <[email protected]>
2017-10-13 20:36 ` Re: [PATCH] pageinspect function to decode infomasks Peter Geoghegan <[email protected]>
0 siblings, 1 reply; 54+ messages in thread
From: Robert Haas @ 2017-10-13 20:02 UTC (permalink / raw)
To: Peter Geoghegan <[email protected]>; +Cc: Tomas Vondra <[email protected]>; pgsql-hackers
On Thu, Oct 12, 2017 at 7:35 PM, Peter Geoghegan <[email protected]> wrote:
> On Tue, Aug 15, 2017 at 10:54 AM, Robert Haas <[email protected]> wrote:
>>> Or at least make the filtering optional.
>>
>> I don't think "filtering" is the right way to think about it. It's
>> just labeling each combination of bits with the meaning appropriate to
>> that combination of bits.
>
> I do. -1 to not just showing what's on the page -- if the
> HEAP_XMIN_COMMITTED and HEAP_XMIN_ABORTED bits are set, then I think
> we should show them. Yeah, I accept that there is a real danger of
> confusing people with that. Unfortunately, I think that displaying
> HEAP_XMIN_FROZEN will cause even more confusion. I don't think that
> HEAP_XMIN_FROZEN is an abstraction at all. It's a notational
> convenience.
Well, *I* think that HEAP_XMIN_FROZEN is an abstraction. That's why
we have #define -- to help us create abstractions.
> I don't think it's our place to "interpret" the bits. Are we *also*
> going to show HEAP_XMIN_FROZEN when xmin is physically set to
> FrozenTransactionId?
No, of course not. We're talking about how to display the 256 and 512
bits of t_infomask. Those have four states: nothing, committed,
invalid, frozen. You're arguing that frozen isn't a real state, that
it's somehow just a combination of committed and invalid, but I think
that's the wrong way of thinking about it. When the 256-bit is clear,
the 512-bit tells you whether the xmin is known invalid, but when the
256-bit is set, the 512-bit tells you whether the tuple is also
frozen.
Before HEAP_XMIN_FROZEN existed, it would have been right to display
the state where both bits are set as committed|invalid, because that
would clearly show you that two things had been set that should never
both be set at the same time. But now that's a valid state with a
well-defined meaning and I think we should display the actual meaning
of that state.
> Where does it end?
I guess it ends wherever we decide to stop. This isn't some kind of
crazy slippery slope we're talking about here, where one day we're
labeling informask bits and the next day it's global thermonuclear
war.
> I think that we should prominently document that HEAP_XMIN_COMMITTED
> |HEAP_XMIN_ABORTED == HEAP_XMIN_FROZEN, rather than trying to hide
> complexity that we have no business hiding in a tool like pageinspect.
I respect that opinion, but I don't think I'm trying to hide anything.
I think I'm proposing that we display the information in what I
believed to be the clearest and most accurate way.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
--
Sent via pgsql-hackers mailing list ([email protected])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
^ permalink raw reply [nested|flat] 54+ messages in thread
* Re: [PATCH] pageinspect function to decode infomasks
2017-07-20 03:33 [PATCH] pageinspect function to decode infomasks Craig Ringer <[email protected]>
2017-07-20 03:44 ` Re: [PATCH] pageinspect function to decode infomasks Peter Geoghegan <[email protected]>
2017-07-20 06:13 ` Re: [PATCH] pageinspect function to decode infomasks Julien Rouhaud <[email protected]>
2017-07-20 06:26 ` Re: [PATCH] pageinspect function to decode infomasks Masahiko Sawada <[email protected]>
2017-07-20 11:09 ` Re: [PATCH] pageinspect function to decode infomasks Ashutosh Sharma <[email protected]>
2017-07-20 11:52 ` Re: [PATCH] pageinspect function to decode infomasks Craig Ringer <[email protected]>
2017-08-15 01:11 ` Re: [PATCH] pageinspect function to decode infomasks Moon Insung <[email protected]>
2017-08-15 01:59 ` Re: [PATCH] pageinspect function to decode infomasks Craig Ringer <[email protected]>
2017-08-15 13:24 ` Re: [PATCH] pageinspect function to decode infomasks Robert Haas <[email protected]>
2017-08-15 13:59 ` Re: [PATCH] pageinspect function to decode infomasks Tomas Vondra <[email protected]>
2017-08-15 17:54 ` Re: [PATCH] pageinspect function to decode infomasks Robert Haas <[email protected]>
2017-10-12 23:35 ` Re: [PATCH] pageinspect function to decode infomasks Peter Geoghegan <[email protected]>
2017-10-13 20:02 ` Re: [PATCH] pageinspect function to decode infomasks Robert Haas <[email protected]>
@ 2017-10-13 20:36 ` Peter Geoghegan <[email protected]>
2017-10-14 17:58 ` Re: [PATCH] pageinspect function to decode infomasks Robert Haas <[email protected]>
0 siblings, 1 reply; 54+ messages in thread
From: Peter Geoghegan @ 2017-10-13 20:36 UTC (permalink / raw)
To: Robert Haas <[email protected]>; +Cc: Tomas Vondra <[email protected]>; pgsql-hackers
On Fri, Oct 13, 2017 at 1:02 PM, Robert Haas <[email protected]> wrote:
>> I don't think it's our place to "interpret" the bits. Are we *also*
>> going to show HEAP_XMIN_FROZEN when xmin is physically set to
>> FrozenTransactionId?
>
> No, of course not. We're talking about how to display the 256 and 512
> bits of t_infomask. Those have four states: nothing, committed,
> invalid, frozen. You're arguing that frozen isn't a real state, that
> it's somehow just a combination of committed and invalid, but I think
> that's the wrong way of thinking about it.
No, I'm arguing that they're just bits. Show the bits, rather than
interpreting what is displayed. Document that there are other logical
states that are represented as composites of contradictory/mutually
exclusive states.
Anyone who hopes to interpret these values has to be an expert anyway,
or willing to become something of an expert. There is a good chance
that they've taken an interest because something is already wrong.
> Before HEAP_XMIN_FROZEN existed, it would have been right to display
> the state where both bits are set as committed|invalid, because that
> would clearly show you that two things had been set that should never
> both be set at the same time. But now that's a valid state with a
> well-defined meaning and I think we should display the actual meaning
> of that state.
>
>> Where does it end?
>
> I guess it ends wherever we decide to stop.
You can take what you're saying much further. What about
HEAP_XMAX_SHR_LOCK, and HEAP_MOVED? Code like HEAP_LOCKED_UPGRADED()
pretty strongly undermines the idea that these composite values are
abstractions.
>> I think that we should prominently document that HEAP_XMIN_COMMITTED
>> |HEAP_XMIN_ABORTED == HEAP_XMIN_FROZEN, rather than trying to hide
>> complexity that we have no business hiding in a tool like pageinspect.
>
> I respect that opinion, but I don't think I'm trying to hide anything.
> I think I'm proposing that we display the information in what I
> believed to be the clearest and most accurate way.
pg_filedump doesn't display HEAP_XMIN_FROZEN, either. (Nor does it
ever display any of the other composite t_infomask/t_infomask2
values.)
--
Peter Geoghegan
--
Sent via pgsql-hackers mailing list ([email protected])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
^ permalink raw reply [nested|flat] 54+ messages in thread
* Re: [PATCH] pageinspect function to decode infomasks
2017-07-20 03:33 [PATCH] pageinspect function to decode infomasks Craig Ringer <[email protected]>
2017-07-20 03:44 ` Re: [PATCH] pageinspect function to decode infomasks Peter Geoghegan <[email protected]>
2017-07-20 06:13 ` Re: [PATCH] pageinspect function to decode infomasks Julien Rouhaud <[email protected]>
2017-07-20 06:26 ` Re: [PATCH] pageinspect function to decode infomasks Masahiko Sawada <[email protected]>
2017-07-20 11:09 ` Re: [PATCH] pageinspect function to decode infomasks Ashutosh Sharma <[email protected]>
2017-07-20 11:52 ` Re: [PATCH] pageinspect function to decode infomasks Craig Ringer <[email protected]>
2017-08-15 01:11 ` Re: [PATCH] pageinspect function to decode infomasks Moon Insung <[email protected]>
2017-08-15 01:59 ` Re: [PATCH] pageinspect function to decode infomasks Craig Ringer <[email protected]>
2017-08-15 13:24 ` Re: [PATCH] pageinspect function to decode infomasks Robert Haas <[email protected]>
2017-08-15 13:59 ` Re: [PATCH] pageinspect function to decode infomasks Tomas Vondra <[email protected]>
2017-08-15 17:54 ` Re: [PATCH] pageinspect function to decode infomasks Robert Haas <[email protected]>
2017-10-12 23:35 ` Re: [PATCH] pageinspect function to decode infomasks Peter Geoghegan <[email protected]>
2017-10-13 20:02 ` Re: [PATCH] pageinspect function to decode infomasks Robert Haas <[email protected]>
2017-10-13 20:36 ` Re: [PATCH] pageinspect function to decode infomasks Peter Geoghegan <[email protected]>
@ 2017-10-14 17:58 ` Robert Haas <[email protected]>
2017-10-14 21:47 ` Re: [PATCH] pageinspect function to decode infomasks Peter Geoghegan <[email protected]>
0 siblings, 1 reply; 54+ messages in thread
From: Robert Haas @ 2017-10-14 17:58 UTC (permalink / raw)
To: Peter Geoghegan <[email protected]>; +Cc: Tomas Vondra <[email protected]>; pgsql-hackers
On Fri, Oct 13, 2017 at 4:36 PM, Peter Geoghegan <[email protected]> wrote:
> No, I'm arguing that they're just bits. Show the bits, rather than
> interpreting what is displayed. Document that there are other logical
> states that are represented as composites of contradictory/mutually
> exclusive states.
/me shrugs.
I think it's perfectly sensible to view those 2 bits as making up a
2-bit field with 4 states rather than displaying each bit
individually, but you obviously disagree. Fair enough.
>> I guess it ends wherever we decide to stop.
>
> You can take what you're saying much further. What about
> HEAP_XMAX_SHR_LOCK, and HEAP_MOVED? Code like HEAP_LOCKED_UPGRADED()
> pretty strongly undermines the idea that these composite values are
> abstractions.
HEAP_MOVED is obviously a different kind of thing. The combination of
both bits has no meaning distinct from the meaning of the individual
bits; in fact, I think it's a shouldn't-happen state. Not sure about
HEAP_XMAX_SHR_LOCK.
> pg_filedump doesn't display HEAP_XMIN_FROZEN, either. (Nor does it
> ever display any of the other composite t_infomask/t_infomask2
> values.)
I can think of two possible explanations for that. Number one, the
tool was written before HEAP_XMIN_FROZEN was invented and hasn't been
updated for those changes. Number two, the author of the tool agrees
with your position rather than mine.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
--
Sent via pgsql-hackers mailing list ([email protected])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
^ permalink raw reply [nested|flat] 54+ messages in thread
* Re: [PATCH] pageinspect function to decode infomasks
2017-07-20 03:33 [PATCH] pageinspect function to decode infomasks Craig Ringer <[email protected]>
2017-07-20 03:44 ` Re: [PATCH] pageinspect function to decode infomasks Peter Geoghegan <[email protected]>
2017-07-20 06:13 ` Re: [PATCH] pageinspect function to decode infomasks Julien Rouhaud <[email protected]>
2017-07-20 06:26 ` Re: [PATCH] pageinspect function to decode infomasks Masahiko Sawada <[email protected]>
2017-07-20 11:09 ` Re: [PATCH] pageinspect function to decode infomasks Ashutosh Sharma <[email protected]>
2017-07-20 11:52 ` Re: [PATCH] pageinspect function to decode infomasks Craig Ringer <[email protected]>
2017-08-15 01:11 ` Re: [PATCH] pageinspect function to decode infomasks Moon Insung <[email protected]>
2017-08-15 01:59 ` Re: [PATCH] pageinspect function to decode infomasks Craig Ringer <[email protected]>
2017-08-15 13:24 ` Re: [PATCH] pageinspect function to decode infomasks Robert Haas <[email protected]>
2017-08-15 13:59 ` Re: [PATCH] pageinspect function to decode infomasks Tomas Vondra <[email protected]>
2017-08-15 17:54 ` Re: [PATCH] pageinspect function to decode infomasks Robert Haas <[email protected]>
2017-10-12 23:35 ` Re: [PATCH] pageinspect function to decode infomasks Peter Geoghegan <[email protected]>
2017-10-13 20:02 ` Re: [PATCH] pageinspect function to decode infomasks Robert Haas <[email protected]>
2017-10-13 20:36 ` Re: [PATCH] pageinspect function to decode infomasks Peter Geoghegan <[email protected]>
2017-10-14 17:58 ` Re: [PATCH] pageinspect function to decode infomasks Robert Haas <[email protected]>
@ 2017-10-14 21:47 ` Peter Geoghegan <[email protected]>
2017-10-15 13:28 ` Re: [PATCH] pageinspect function to decode infomasks Vik Fearing <[email protected]>
2017-10-17 00:20 ` Re: [PATCH] pageinspect function to decode infomasks Peter Geoghegan <[email protected]>
0 siblings, 2 replies; 54+ messages in thread
From: Peter Geoghegan @ 2017-10-14 21:47 UTC (permalink / raw)
To: Robert Haas <[email protected]>; +Cc: Tomas Vondra <[email protected]>; pgsql-hackers
On Sat, Oct 14, 2017 at 10:58 AM, Robert Haas <[email protected]> wrote:
> I think it's perfectly sensible to view those 2 bits as making up a
> 2-bit field with 4 states rather than displaying each bit
> individually, but you obviously disagree. Fair enough.
I guess it is that simple.
> I can think of two possible explanations for that. Number one, the
> tool was written before HEAP_XMIN_FROZEN was invented and hasn't been
> updated for those changes.
Have we invented our last t_infomask/t_infomask2 (logical) status already?
> Number two, the author of the tool agrees
> with your position rather than mine.
I am working on an experimental version of pg_filedump, customized to
output XML that can be interpreted by an open source hex editor. The
XML makes the hex editor produce color coded, commented
tags/annotations for any given heap or B-Tree relation. This includes
tooltips with literal values for all status bits (including
t_infomask/t_infomask2 bits, IndexTuple bits, B-Tree meta page status
bits, PD_* page-level bits, ItemId bits, and others). I tweeted about
this several months ago, when it was just a tool I wrote for myself,
and received a surprisingly positive response. It seems like I'm on to
something, and should release the tool to the community.
I mention this project because it very much informs my perspective
here. Having spent quite a while deliberately corrupting test data in
novel ways, just to see what happens, the "work backwards from the
storage format" perspective feels very natural to me. I do think that
I understand where you're coming from too, though.
--
Peter Geoghegan
--
Sent via pgsql-hackers mailing list ([email protected])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
^ permalink raw reply [nested|flat] 54+ messages in thread
* Re: [PATCH] pageinspect function to decode infomasks
2017-07-20 03:33 [PATCH] pageinspect function to decode infomasks Craig Ringer <[email protected]>
2017-07-20 03:44 ` Re: [PATCH] pageinspect function to decode infomasks Peter Geoghegan <[email protected]>
2017-07-20 06:13 ` Re: [PATCH] pageinspect function to decode infomasks Julien Rouhaud <[email protected]>
2017-07-20 06:26 ` Re: [PATCH] pageinspect function to decode infomasks Masahiko Sawada <[email protected]>
2017-07-20 11:09 ` Re: [PATCH] pageinspect function to decode infomasks Ashutosh Sharma <[email protected]>
2017-07-20 11:52 ` Re: [PATCH] pageinspect function to decode infomasks Craig Ringer <[email protected]>
2017-08-15 01:11 ` Re: [PATCH] pageinspect function to decode infomasks Moon Insung <[email protected]>
2017-08-15 01:59 ` Re: [PATCH] pageinspect function to decode infomasks Craig Ringer <[email protected]>
2017-08-15 13:24 ` Re: [PATCH] pageinspect function to decode infomasks Robert Haas <[email protected]>
2017-08-15 13:59 ` Re: [PATCH] pageinspect function to decode infomasks Tomas Vondra <[email protected]>
2017-08-15 17:54 ` Re: [PATCH] pageinspect function to decode infomasks Robert Haas <[email protected]>
2017-10-12 23:35 ` Re: [PATCH] pageinspect function to decode infomasks Peter Geoghegan <[email protected]>
2017-10-13 20:02 ` Re: [PATCH] pageinspect function to decode infomasks Robert Haas <[email protected]>
2017-10-13 20:36 ` Re: [PATCH] pageinspect function to decode infomasks Peter Geoghegan <[email protected]>
2017-10-14 17:58 ` Re: [PATCH] pageinspect function to decode infomasks Robert Haas <[email protected]>
2017-10-14 21:47 ` Re: [PATCH] pageinspect function to decode infomasks Peter Geoghegan <[email protected]>
@ 2017-10-15 13:28 ` Vik Fearing <[email protected]>
1 sibling, 0 replies; 54+ messages in thread
From: Vik Fearing @ 2017-10-15 13:28 UTC (permalink / raw)
To: Peter Geoghegan <[email protected]>; Robert Haas <[email protected]>; +Cc: Tomas Vondra <[email protected]>; pgsql-hackers
On 10/14/2017 11:47 PM, Peter Geoghegan wrote:
> On Sat, Oct 14, 2017 at 10:58 AM, Robert Haas <[email protected]> wrote:
>> I think it's perfectly sensible to view those 2 bits as making up a
>> 2-bit field with 4 states rather than displaying each bit
>> individually, but you obviously disagree. Fair enough.>
> I guess it is that simple.
FWIW, my opinion falls in line with Robert's.
Also, whichever way it goes, this is a patch I've been wanting for a
long time.
--
Vik Fearing +33 6 46 75 15 36
http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support
--
Sent via pgsql-hackers mailing list ([email protected])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
^ permalink raw reply [nested|flat] 54+ messages in thread
* Re: [PATCH] pageinspect function to decode infomasks
2017-07-20 03:33 [PATCH] pageinspect function to decode infomasks Craig Ringer <[email protected]>
2017-07-20 03:44 ` Re: [PATCH] pageinspect function to decode infomasks Peter Geoghegan <[email protected]>
2017-07-20 06:13 ` Re: [PATCH] pageinspect function to decode infomasks Julien Rouhaud <[email protected]>
2017-07-20 06:26 ` Re: [PATCH] pageinspect function to decode infomasks Masahiko Sawada <[email protected]>
2017-07-20 11:09 ` Re: [PATCH] pageinspect function to decode infomasks Ashutosh Sharma <[email protected]>
2017-07-20 11:52 ` Re: [PATCH] pageinspect function to decode infomasks Craig Ringer <[email protected]>
2017-08-15 01:11 ` Re: [PATCH] pageinspect function to decode infomasks Moon Insung <[email protected]>
2017-08-15 01:59 ` Re: [PATCH] pageinspect function to decode infomasks Craig Ringer <[email protected]>
2017-08-15 13:24 ` Re: [PATCH] pageinspect function to decode infomasks Robert Haas <[email protected]>
2017-08-15 13:59 ` Re: [PATCH] pageinspect function to decode infomasks Tomas Vondra <[email protected]>
2017-08-15 17:54 ` Re: [PATCH] pageinspect function to decode infomasks Robert Haas <[email protected]>
2017-10-12 23:35 ` Re: [PATCH] pageinspect function to decode infomasks Peter Geoghegan <[email protected]>
2017-10-13 20:02 ` Re: [PATCH] pageinspect function to decode infomasks Robert Haas <[email protected]>
2017-10-13 20:36 ` Re: [PATCH] pageinspect function to decode infomasks Peter Geoghegan <[email protected]>
2017-10-14 17:58 ` Re: [PATCH] pageinspect function to decode infomasks Robert Haas <[email protected]>
2017-10-14 21:47 ` Re: [PATCH] pageinspect function to decode infomasks Peter Geoghegan <[email protected]>
@ 2017-10-17 00:20 ` Peter Geoghegan <[email protected]>
1 sibling, 0 replies; 54+ messages in thread
From: Peter Geoghegan @ 2017-10-17 00:20 UTC (permalink / raw)
To: Robert Haas <[email protected]>; +Cc: Tomas Vondra <[email protected]>; pgsql-hackers
On Sat, Oct 14, 2017 at 2:47 PM, Peter Geoghegan <[email protected]> wrote:
> I am working on an experimental version of pg_filedump, customized to
> output XML that can be interpreted by an open source hex editor. The
> XML makes the hex editor produce color coded, commented
> tags/annotations for any given heap or B-Tree relation. This includes
> tooltips with literal values for all status bits (including
> t_infomask/t_infomask2 bits, IndexTuple bits, B-Tree meta page status
> bits, PD_* page-level bits, ItemId bits, and others).
This is now available from: https://github.com/petergeoghegan/pg_hexedit
--
Peter Geoghegan
--
Sent via pgsql-hackers mailing list ([email protected])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
^ permalink raw reply [nested|flat] 54+ messages in thread
* Re: [PATCH] pageinspect function to decode infomasks
2017-07-20 03:33 [PATCH] pageinspect function to decode infomasks Craig Ringer <[email protected]>
2017-07-20 03:44 ` Re: [PATCH] pageinspect function to decode infomasks Peter Geoghegan <[email protected]>
2017-07-20 06:13 ` Re: [PATCH] pageinspect function to decode infomasks Julien Rouhaud <[email protected]>
2017-07-20 06:26 ` Re: [PATCH] pageinspect function to decode infomasks Masahiko Sawada <[email protected]>
2017-07-20 11:09 ` Re: [PATCH] pageinspect function to decode infomasks Ashutosh Sharma <[email protected]>
@ 2017-07-21 03:29 ` Craig Ringer <[email protected]>
1 sibling, 0 replies; 54+ messages in thread
From: Craig Ringer @ 2017-07-21 03:29 UTC (permalink / raw)
To: Ashutosh Sharma <[email protected]>; +Cc: Masahiko Sawada <[email protected]>; Julien Rouhaud <[email protected]>; Peter Geoghegan <[email protected]>; pgsql-hackers; Álvaro Herrera <[email protected]>; abhijit Menon-Sen <[email protected]>; Peter Eisentraut <[email protected]>; Pavan Deolasee <[email protected]>
On 20 July 2017 at 19:09, Ashutosh Sharma <[email protected]> wrote:
> I had a quick look into this patch and also tested it and following
> are my observations.
>
> 1) I am seeing a server crash when passing any non meaningful value
> for t_infomask2 to heap_infomask_flags().
>
> postgres=# SELECT heap_infomask_flags(2816, 3);
> server closed the connection unexpectedly
> This probably means the server terminated abnormally
> before or while processing the request.
> The connection to the server was lost. Attempting reset: Failed.
> !> \q
>
> Following is the backtrace,
>
> (gdb) bt
> #0 0x0000000000d9c55b in pg_detoast_datum (datum=0x0) at fmgr.c:1833
> #1 0x0000000000b87374 in construct_md_array (elems=0x2ad74c0,
> nulls=0x0, ndims=1, dims=0x7ffc0b0bbcd0, lbs=0x7ffc0b0bbcc0,
> elmtype=25, elmlen=-1,
> elmbyval=0 '\000', elmalign=105 'i') at arrayfuncs.c:3382
> #2 0x0000000000b8709f in construct_array (elems=0x2ad74c0, nelems=10,
> elmtype=25, elmlen=-1, elmbyval=0 '\000', elmalign=105 'i') at
> arrayfuncs.c:3316
> #3 0x00007fb8001603a5 in heap_infomask_flags (fcinfo=0x2ad3b88) at
> heapfuncs.c:597
>
Fixed.
> 2) I can see the documentation for heap_infomask(). But, I do not see
> it being defined or used anywhere in the patch.
>
> + <para>
> + The <function>heap_infomask</function> function can be used to
> unpack the
> + recognised bits of the infomasks of heap tuples.
> + </para>
>
Fixed. Renamed the function, missed a spot.
> 3) If show_combined flag is set to it's default value and a tuple is
> frozen then may i know the reason for not showing it as frozen tuple
> when t_infomask2
> is passed as zero.
It was a consequence of (1). Fixed.
> 4) I think, it would be better to use the same argument name for the
> newly added function i.e heap_infomask_flags() in both documentation
> and sql file. I am basically refering to 'include_combined' argument.
> IF you see the function definition, the argument name used is
> 'include_combined' whereas in documentation you have mentioned
> 'show_combined'.
>
Fixed, thanks.
I want to find time to expand the tests on this some more and look more
closely, but here it is for now.
--
Craig Ringer http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
--
Sent via pgsql-hackers mailing list ([email protected])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Attachments:
[text/x-patch] v2-0001-Introduce-heap_infomask_flags-to-decode-infomask-.patch (16.5K, ../../CAMsr+YHKpq2LwSpcsGfMRnww3jPohQnYbfdep6Hy+NX-+WUn9Q@mail.gmail.com/3-v2-0001-Introduce-heap_infomask_flags-to-decode-infomask-.patch)
download | inline diff:
From a25c1e50e3b3ff493a018ab594a71dc3d64832ee Mon Sep 17 00:00:00 2001
From: Craig Ringer <[email protected]>
Date: Thu, 20 Jul 2017 11:20:21 +0800
Subject: [PATCH v2] Introduce heap_infomask_flags to decode infomask and
infomask2
---
contrib/pageinspect/Makefile | 3 +-
contrib/pageinspect/expected/page.out | 85 ++++++++++++++++++
contrib/pageinspect/heapfuncs.c | 120 ++++++++++++++++++++++++++
contrib/pageinspect/pageinspect--1.6--1.7.sql | 9 ++
contrib/pageinspect/pageinspect.control | 2 +-
contrib/pageinspect/sql/page.sql | 24 ++++++
doc/src/sgml/pageinspect.sgml | 32 +++++++
7 files changed, 273 insertions(+), 2 deletions(-)
create mode 100644 contrib/pageinspect/pageinspect--1.6--1.7.sql
diff --git a/contrib/pageinspect/Makefile b/contrib/pageinspect/Makefile
index 0a3cbee..de114c7 100644
--- a/contrib/pageinspect/Makefile
+++ b/contrib/pageinspect/Makefile
@@ -5,7 +5,8 @@ OBJS = rawpage.o heapfuncs.o btreefuncs.o fsmfuncs.o \
brinfuncs.o ginfuncs.o hashfuncs.o $(WIN32RES)
EXTENSION = pageinspect
-DATA = pageinspect--1.5.sql pageinspect--1.5--1.6.sql \
+DATA = pageinspect--1.6--1.7.sql \
+ pageinspect--1.5.sql pageinspect--1.5--1.6.sql \
pageinspect--1.4--1.5.sql pageinspect--1.3--1.4.sql \
pageinspect--1.2--1.3.sql pageinspect--1.1--1.2.sql \
pageinspect--1.0--1.1.sql pageinspect--unpackaged--1.0.sql
diff --git a/contrib/pageinspect/expected/page.out b/contrib/pageinspect/expected/page.out
index 8e15947..b335265 100644
--- a/contrib/pageinspect/expected/page.out
+++ b/contrib/pageinspect/expected/page.out
@@ -82,6 +82,91 @@ SELECT * FROM fsm_page_contents(get_raw_page('test1', 'fsm', 0));
(1 row)
+-- If we freeze the only tuple on test1, the infomask should
+-- always be the same in all test runs.
+VACUUM FREEZE test1;
+SELECT t_infomask, t_infomask2, flags
+FROM heap_page_items(get_raw_page('test1', 0)),
+ LATERAL heap_infomask_flags(t_infomask, t_infomask2, true) m(flags);
+ t_infomask | t_infomask2 | flags
+------------+-------------+----------------------------------------------------------------------------
+ 2816 | 2 | {HEAP_XMIN_COMMITTED,HEAP_XMIN_INVALID,HEAP_XMAX_INVALID,HEAP_XMIN_FROZEN}
+(1 row)
+
+SELECT t_infomask, t_infomask2, flags
+FROM heap_page_items(get_raw_page('test1', 0)),
+ LATERAL heap_infomask_flags(t_infomask, t_infomask2, false) m(flags);
+ t_infomask | t_infomask2 | flags
+------------+-------------+-----------------------------------------------------------
+ 2816 | 2 | {HEAP_XMIN_COMMITTED,HEAP_XMIN_INVALID,HEAP_XMAX_INVALID}
+(1 row)
+
+SELECT heap_infomask_flags(2816, 0);
+ heap_infomask_flags
+----------------------------------------------------------------------------
+ {HEAP_XMIN_COMMITTED,HEAP_XMIN_INVALID,HEAP_XMAX_INVALID,HEAP_XMIN_FROZEN}
+(1 row)
+
+SELECT heap_infomask_flags(2816, 0, false);
+ heap_infomask_flags
+-----------------------------------------------------------
+ {HEAP_XMIN_COMMITTED,HEAP_XMIN_INVALID,HEAP_XMAX_INVALID}
+(1 row)
+
+SELECT heap_infomask_flags(2816, 1, true);
+ heap_infomask_flags
+----------------------------------------------------------------------------
+ {HEAP_XMIN_COMMITTED,HEAP_XMIN_INVALID,HEAP_XMAX_INVALID,HEAP_XMIN_FROZEN}
+(1 row)
+
+SELECT heap_infomask_flags(2816, 1, false);
+ heap_infomask_flags
+-----------------------------------------------------------
+ {HEAP_XMIN_COMMITTED,HEAP_XMIN_INVALID,HEAP_XMAX_INVALID}
+(1 row)
+
+SELECT heap_infomask_flags(2816, x'FFFF'::integer, true);
+ heap_infomask_flags
+-------------------------------------------------------------------------------------------------------------------------------
+ {HEAP_XMIN_COMMITTED,HEAP_XMIN_INVALID,HEAP_XMAX_INVALID,HEAP_KEYS_UPDATED,HEAP_HOT_UPDATED,HEAP_ONLY_TUPLE,HEAP_XMIN_FROZEN}
+(1 row)
+
+SELECT heap_infomask_flags(2816, x'FFFF'::integer, false);
+ heap_infomask_flags
+--------------------------------------------------------------------------------------------------------------
+ {HEAP_XMIN_COMMITTED,HEAP_XMIN_INVALID,HEAP_XMAX_INVALID,HEAP_KEYS_UPDATED,HEAP_HOT_UPDATED,HEAP_ONLY_TUPLE}
+(1 row)
+
+SELECT heap_infomask_flags(x'FFFF'::integer, x'FFFF'::integer, false);
+ heap_infomask_flags
+--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
+ {HEAP_HASNULL,HEAP_HASVARWIDTH,HEAP_HASEXTERNAL,HEAP_HASOID,HEAP_XMAX_KEYSHR_LOCK,HEAP_COMBOCID,HEAP_XMAX_EXCL_LOCK,HEAP_XMAX_LOCK_ONLY,HEAP_XMIN_COMMITTED,HEAP_XMIN_INVALID,HEAP_XMAX_COMMITTED,HEAP_XMAX_INVALID,HEAP_XMAX_IS_MULTI,HEAP_UPDATED,HEAP_MOVED_OFF,HEAP_MOVED_IN,HEAP_KEYS_UPDATED,HEAP_HOT_UPDATED,HEAP_ONLY_TUPLE}
+(1 row)
+
+SELECT heap_infomask_flags(x'FFFF'::integer, x'FFFF'::integer, true);
+ heap_infomask_flags
+--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
+ {HEAP_HASNULL,HEAP_HASVARWIDTH,HEAP_HASEXTERNAL,HEAP_HASOID,HEAP_XMAX_KEYSHR_LOCK,HEAP_COMBOCID,HEAP_XMAX_EXCL_LOCK,HEAP_XMAX_LOCK_ONLY,HEAP_XMIN_COMMITTED,HEAP_XMIN_INVALID,HEAP_XMAX_COMMITTED,HEAP_XMAX_INVALID,HEAP_XMAX_IS_MULTI,HEAP_UPDATED,HEAP_MOVED_OFF,HEAP_MOVED_IN,HEAP_KEYS_UPDATED,HEAP_HOT_UPDATED,HEAP_ONLY_TUPLE,HEAP_XMIN_FROZEN,HEAP_XMAX_IS_LOCKED_ONLY}
+(1 row)
+
+SELECT heap_infomask_flags(0, 0, true);
+ heap_infomask_flags
+---------------------
+ {}
+(1 row)
+
+SELECT heap_infomask_flags(0, 0, false);
+ heap_infomask_flags
+---------------------
+ {}
+(1 row)
+
+SELECT heap_infomask_flags(-1, -1, false);
+ heap_infomask_flags
+--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
+ {HEAP_HASNULL,HEAP_HASVARWIDTH,HEAP_HASEXTERNAL,HEAP_HASOID,HEAP_XMAX_KEYSHR_LOCK,HEAP_COMBOCID,HEAP_XMAX_EXCL_LOCK,HEAP_XMAX_LOCK_ONLY,HEAP_XMIN_COMMITTED,HEAP_XMIN_INVALID,HEAP_XMAX_COMMITTED,HEAP_XMAX_INVALID,HEAP_XMAX_IS_MULTI,HEAP_UPDATED,HEAP_MOVED_OFF,HEAP_MOVED_IN,HEAP_KEYS_UPDATED,HEAP_HOT_UPDATED,HEAP_ONLY_TUPLE}
+(1 row)
+
DROP TABLE test1;
-- check that using any of these functions with a partitioned table would fail
create table test_partitioned (a int) partition by range (a);
diff --git a/contrib/pageinspect/heapfuncs.c b/contrib/pageinspect/heapfuncs.c
index 72d1776..2ef5b39 100644
--- a/contrib/pageinspect/heapfuncs.c
+++ b/contrib/pageinspect/heapfuncs.c
@@ -478,3 +478,123 @@ tuple_data_split(PG_FUNCTION_ARGS)
PG_RETURN_ARRAYTYPE_P(res);
}
+
+/*
+ * Brian Kernighan's popcount algorithm for counting number of set bits in a
+ * mask. The faster methods aren't worth the complexity here.
+ *
+ * See e.g. http://graphics.stanford.edu/~seander/bithacks.html#CountBitsSetKernighan
+ *
+ * gcc has __builtin_popcount but there's no standard alternative.
+ */
+static inline int
+pg_popcount32(uint32 mask)
+{
+ unsigned int c; // c accumulates the total bits set in v
+ for (c = 0; mask; c++)
+ {
+ mask &= mask - 1; // clear the least significant bit set
+ }
+ return c;
+}
+
+/*
+ * Infomask flag names. The infomask2 values are shifted into the
+ * high 16 bits.
+ */
+struct infomask_details
+{
+ uint32 flag_value;
+ const char flag_name[24];
+};
+
+#define MASKINFO_LENGTH 19
+static struct infomask_details maskinfo[MASKINFO_LENGTH] =
+ {
+ /* infomask1 values */
+ {(uint32)HEAP_HASNULL, "HEAP_HASNULL"},
+ {(uint32)HEAP_HASVARWIDTH, "HEAP_HASVARWIDTH"},
+ {(uint32)HEAP_HASEXTERNAL, "HEAP_HASEXTERNAL"},
+ {(uint32)HEAP_HASOID, "HEAP_HASOID"},
+ {(uint32)HEAP_XMAX_KEYSHR_LOCK, "HEAP_XMAX_KEYSHR_LOCK"},
+ {(uint32)HEAP_COMBOCID, "HEAP_COMBOCID"},
+ {(uint32)HEAP_XMAX_EXCL_LOCK, "HEAP_XMAX_EXCL_LOCK"},
+ {(uint32)HEAP_XMAX_LOCK_ONLY, "HEAP_XMAX_LOCK_ONLY"},
+ {(uint32)HEAP_XMIN_COMMITTED, "HEAP_XMIN_COMMITTED"},
+ {(uint32)HEAP_XMIN_INVALID, "HEAP_XMIN_INVALID"},
+ {(uint32)HEAP_XMAX_COMMITTED, "HEAP_XMAX_COMMITTED"},
+ {(uint32)HEAP_XMAX_INVALID, "HEAP_XMAX_INVALID"},
+ {(uint32)HEAP_XMAX_IS_MULTI, "HEAP_XMAX_IS_MULTI"},
+ {(uint32)HEAP_UPDATED, "HEAP_UPDATED"},
+ {(uint32)HEAP_MOVED_OFF, "HEAP_MOVED_OFF"},
+ {(uint32)HEAP_MOVED_IN, "HEAP_MOVED_IN"},
+ /* infomask2 values */
+ {((uint32)HEAP_KEYS_UPDATED)<<16, "HEAP_KEYS_UPDATED"},
+ {((uint32)HEAP_HOT_UPDATED)<<16, "HEAP_HOT_UPDATED"},
+ {((uint32)HEAP_ONLY_TUPLE)<<16, "HEAP_ONLY_TUPLE"},
+ };
+
+/*
+ * Decode an infomask, per htup_details.c, into human readable
+ * form.
+ */
+PG_FUNCTION_INFO_V1(heap_infomask_flags);
+
+Datum
+heap_infomask_flags(PG_FUNCTION_ARGS)
+{
+ uint16 t_infomask = PG_GETARG_INT16(0);
+ uint16 t_infomask2 = PG_GETARG_INT16(1);
+ bool include_combined = PG_GETARG_BOOL(2);
+ uint32 combomask = (((uint32)t_infomask2) << 16) + (uint32)t_infomask;
+ unsigned int maxarray = pg_popcount32(combomask);
+ Datum *d;
+ ArrayType *a;
+ int i;
+ int insertpos;
+
+ Assert((t_infomask == 0 && t_infomask2 == 0) == (maxarray == 0));
+
+ if (maxarray == 0)
+ {
+ a = construct_empty_array(TEXTOID);
+ PG_RETURN_POINTER(a);
+ }
+
+ if (include_combined)
+ {
+ maxarray += (t_infomask & HEAP_XMIN_FROZEN) == HEAP_XMIN_FROZEN;
+ maxarray += HEAP_XMAX_IS_LOCKED_ONLY(t_infomask) != 0;
+ maxarray += HEAP_LOCKED_UPGRADED(t_infomask) != 0;
+ }
+
+ d = (Datum *) palloc0(sizeof(Datum) * maxarray);
+
+ insertpos = 0;
+ for (i = 0; i < MASKINFO_LENGTH; ++i)
+ {
+ if ((combomask & maskinfo[i].flag_value) == maskinfo[i].flag_value)
+ d[insertpos++] = CStringGetTextDatum(maskinfo[i].flag_name);
+ }
+
+ /*
+ * These tests are useful to report in the mask we output, since they're
+ * much more simply done here than in SQL, and here they won't get out of
+ * sync with what Pg does if we change it later.
+ */
+ if (include_combined)
+ {
+ if ((t_infomask & HEAP_XMIN_FROZEN) == HEAP_XMIN_FROZEN)
+ d[insertpos++] = CStringGetTextDatum("HEAP_XMIN_FROZEN");
+
+ if (HEAP_XMAX_IS_LOCKED_ONLY(t_infomask))
+ d[insertpos++] = CStringGetTextDatum("HEAP_XMAX_IS_LOCKED_ONLY");
+
+ if (HEAP_LOCKED_UPGRADED(t_infomask))
+ d[insertpos++] = CStringGetTextDatum("HEAP_LOCKED_UPGRADED");
+ }
+
+ a = construct_array(d, insertpos, TEXTOID, -1, false, 'i');
+
+ PG_RETURN_POINTER(a);
+}
diff --git a/contrib/pageinspect/pageinspect--1.6--1.7.sql b/contrib/pageinspect/pageinspect--1.6--1.7.sql
new file mode 100644
index 0000000..b3d1fe4
--- /dev/null
+++ b/contrib/pageinspect/pageinspect--1.6--1.7.sql
@@ -0,0 +1,9 @@
+/* contrib/pageinspect/pageinspect--1.6--1.7.sql */
+
+-- complain if script is sourced in psql, rather than via ALTER EXTENSION
+\echo Use "ALTER EXTENSION pageinspect UPDATE TO '1.7'" to load this file. \quit
+
+-- decode infomask flags as human readable flag names
+CREATE FUNCTION heap_infomask_flags(infomask1 integer, infomask2 integer,
+ include_combined boolean DEFAULT true)
+RETURNS text[] STRICT LANGUAGE 'c' AS 'MODULE_PATHNAME','heap_infomask_flags';
diff --git a/contrib/pageinspect/pageinspect.control b/contrib/pageinspect/pageinspect.control
index 1a61c9f..dcfc61f 100644
--- a/contrib/pageinspect/pageinspect.control
+++ b/contrib/pageinspect/pageinspect.control
@@ -1,5 +1,5 @@
# pageinspect extension
comment = 'inspect the contents of database pages at a low level'
-default_version = '1.6'
+default_version = '1.7'
module_pathname = '$libdir/pageinspect'
relocatable = true
diff --git a/contrib/pageinspect/sql/page.sql b/contrib/pageinspect/sql/page.sql
index 493ca9b..2f06c39 100644
--- a/contrib/pageinspect/sql/page.sql
+++ b/contrib/pageinspect/sql/page.sql
@@ -31,6 +31,30 @@ SELECT tuple_data_split('test1'::regclass, t_data, t_infomask, t_infomask2, t_bi
SELECT * FROM fsm_page_contents(get_raw_page('test1', 'fsm', 0));
+-- If we freeze the only tuple on test1, the infomask should
+-- always be the same in all test runs.
+VACUUM FREEZE test1;
+
+SELECT t_infomask, t_infomask2, flags
+FROM heap_page_items(get_raw_page('test1', 0)),
+ LATERAL heap_infomask_flags(t_infomask, t_infomask2, true) m(flags);
+
+SELECT t_infomask, t_infomask2, flags
+FROM heap_page_items(get_raw_page('test1', 0)),
+ LATERAL heap_infomask_flags(t_infomask, t_infomask2, false) m(flags);
+
+SELECT heap_infomask_flags(2816, 0);
+SELECT heap_infomask_flags(2816, 0, false);
+SELECT heap_infomask_flags(2816, 1, true);
+SELECT heap_infomask_flags(2816, 1, false);
+SELECT heap_infomask_flags(2816, x'FFFF'::integer, true);
+SELECT heap_infomask_flags(2816, x'FFFF'::integer, false);
+SELECT heap_infomask_flags(x'FFFF'::integer, x'FFFF'::integer, false);
+SELECT heap_infomask_flags(x'FFFF'::integer, x'FFFF'::integer, true);
+SELECT heap_infomask_flags(0, 0, true);
+SELECT heap_infomask_flags(0, 0, false);
+SELECT heap_infomask_flags(-1, -1, false);
+
DROP TABLE test1;
-- check that using any of these functions with a partitioned table would fail
diff --git a/doc/src/sgml/pageinspect.sgml b/doc/src/sgml/pageinspect.sgml
index ccdaf3e..52bb9d5 100644
--- a/doc/src/sgml/pageinspect.sgml
+++ b/doc/src/sgml/pageinspect.sgml
@@ -151,6 +151,10 @@ test=# SELECT * FROM heap_page_items(get_raw_page('pg_class', 0));
<filename>src/include/access/htup_details.h</> for explanations of the fields
returned.
</para>
+ <para>
+ The <function>heap_infomask_flags</function> function can be used to unpack the
+ recognised bits of the infomasks of heap tuples.
+ </para>
</listitem>
</varlistentry>
@@ -206,6 +210,34 @@ test=# SELECT * FROM heap_page_item_attrs(get_raw_page('pg_class', 0), 'pg_class
<varlistentry>
<term>
+ <function>heap_infomask_flags(infomask1 integer, infomask2 integer, include_combined bool) returns text[]</function>
+ <indexterm>
+ <primary>heap_infomask_flags</primary>
+ </indexterm>
+ </term>
+ <listitem>
+ <para>
+ <function>heap_infomask_flags</function> decodes the
+ <structfield>t_infomask1</structfield> and
+ <structfield>t_infomask2</structfield> returned by
+ <function>heap_page_items</function> into a human-readable array of flag
+ names. This can be used to see the tuple hint bits etc.
+ </para>
+ <para>
+ If include_combined is set (the default), combination flags like
+ <literal>HEAP_XMIN_FROZEN</literal> are also output. The original fields
+ are not filtered out, so e.g. a frozen tuple will have
+ <literal>HEAP_XMIN_FROZEN, HEAP_XMIN_COMMITTED, HEAP_XMIN_INVALID</literal>.
+ </para>
+ <para>
+ For the meaning of these flags see
+ <filename>src/include/access/htup_details.h</>
+ </para>
+ </listitem>
+ </varlistentry>
+
+ <varlistentry>
+ <term>
<function>fsm_page_contents(page bytea) returns text</function>
<indexterm>
<primary>fsm_page_contents</primary>
--
2.9.4
^ permalink raw reply [nested|flat] 54+ messages in thread
* [PATCH v3 4/5] Fix bugs in the commit to change conversion function signature.
@ 2021-02-01 15:30 Heikki Linnakangas <[email protected]>
0 siblings, 0 replies; 54+ messages in thread
From: Heikki Linnakangas @ 2021-02-01 15:30 UTC (permalink / raw)
Per John Naylor's report, and some bugs I found while testing.
TODO: to be squashed.
---
src/backend/utils/mb/conv.c | 16 ++++++----------
.../utf8_and_iso8859_1/utf8_and_iso8859_1.c | 8 ++++++++
src/backend/utils/mb/mbutils.c | 5 +++--
3 files changed, 17 insertions(+), 12 deletions(-)
diff --git a/src/backend/utils/mb/conv.c b/src/backend/utils/mb/conv.c
index b83358bc7a5..f3bda5753d0 100644
--- a/src/backend/utils/mb/conv.c
+++ b/src/backend/utils/mb/conv.c
@@ -497,7 +497,6 @@ UtfToLocal(const unsigned char *utf, int len,
int l;
const pg_utf_to_local_combined *cp;
const unsigned char *start = utf;
- const unsigned char *cur = utf;
if (!PG_VALID_ENCODING(encoding))
ereport(ERROR,
@@ -511,8 +510,6 @@ UtfToLocal(const unsigned char *utf, int len,
unsigned char b3 = 0;
unsigned char b4 = 0;
- cur = iso;
-
/* "break" cases all represent errors */
if (*utf == '\0')
break;
@@ -648,10 +645,11 @@ UtfToLocal(const unsigned char *utf, int len,
}
/* failed to translate this character */
+ utf -= l;
if (noError)
break;
report_untranslatable_char(PG_UTF8, encoding,
- (const char *) (utf - l), len);
+ (const char *) utf, len);
}
/* if we broke out of loop early, must be invalid input */
@@ -660,7 +658,7 @@ UtfToLocal(const unsigned char *utf, int len,
*iso = '\0';
- return cur - start;
+ return utf - start;
}
/*
@@ -701,7 +699,6 @@ LocalToUtf(const unsigned char *iso, int len,
int l;
const pg_local_to_utf_combined *cp;
const unsigned char *start = iso;
- const unsigned char *cur = iso;
if (!PG_VALID_ENCODING(encoding))
ereport(ERROR,
@@ -715,8 +712,6 @@ LocalToUtf(const unsigned char *iso, int len,
unsigned char b3 = 0;
unsigned char b4 = 0;
- cur = iso;
-
/* "break" cases all represent errors */
if (*iso == '\0')
break;
@@ -799,10 +794,11 @@ LocalToUtf(const unsigned char *iso, int len,
}
/* failed to translate this character */
+ iso -= l;
if (noError)
break;
report_untranslatable_char(encoding, PG_UTF8,
- (const char *) (iso - l), len);
+ (const char *) iso, len);
}
/* if we broke out of loop early, must be invalid input */
@@ -811,5 +807,5 @@ LocalToUtf(const unsigned char *iso, int len,
*utf = '\0';
- return cur - start;
+ return iso - start;
}
diff --git a/src/backend/utils/mb/conversion_procs/utf8_and_iso8859_1/utf8_and_iso8859_1.c b/src/backend/utils/mb/conversion_procs/utf8_and_iso8859_1/utf8_and_iso8859_1.c
index 8ac93604a1b..d0dc4cca378 100644
--- a/src/backend/utils/mb/conversion_procs/utf8_and_iso8859_1/utf8_and_iso8859_1.c
+++ b/src/backend/utils/mb/conversion_procs/utf8_and_iso8859_1/utf8_and_iso8859_1.c
@@ -104,10 +104,18 @@ utf8_to_iso8859_1(PG_FUNCTION_ARGS)
int l = pg_utf_mblen(src);
if (l > len || !pg_utf8_islegal(src, l))
+ {
+ if (noError)
+ break;
report_invalid_encoding(PG_UTF8, (const char *) src, len);
+ }
if (l != 2)
+ {
+ if (noError)
+ break;
report_untranslatable_char(PG_UTF8, PG_LATIN1,
(const char *) src, len);
+ }
c1 = src[1] & 0x3f;
c = ((c & 0x1f) << 6) | c1;
if (c >= 0x80 && c <= 0xff)
diff --git a/src/backend/utils/mb/mbutils.c b/src/backend/utils/mb/mbutils.c
index 3e106027d75..af1cafe523a 100644
--- a/src/backend/utils/mb/mbutils.c
+++ b/src/backend/utils/mb/mbutils.c
@@ -818,12 +818,13 @@ perform_default_encoding_conversion(const char *src, int len,
MemoryContextAllocHuge(CurrentMemoryContext,
(Size) len * MAX_CONVERSION_GROWTH + 1);
- FunctionCall5(flinfo,
+ FunctionCall6(flinfo,
Int32GetDatum(src_encoding),
Int32GetDatum(dest_encoding),
CStringGetDatum(src),
CStringGetDatum(result),
- Int32GetDatum(len));
+ Int32GetDatum(len),
+ BoolGetDatum(false));
/*
* Release extra space if there might be a lot --- see comments in
--
2.29.2
--------------39E46EF15EBFAA37D53332E5
Content-Type: text/x-patch; charset=UTF-8;
name="v3-0005-Do-COPY-FROM-encoding-conversion-verification-in-.patch"
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment;
filename*0="v3-0005-Do-COPY-FROM-encoding-conversion-verification-in-.pa";
filename*1="tch"
^ permalink raw reply [nested|flat] 54+ messages in thread
* [PATCH v3 4/5] Fix bugs in the commit to change conversion function signature.
@ 2021-02-01 15:30 Heikki Linnakangas <[email protected]>
0 siblings, 0 replies; 54+ messages in thread
From: Heikki Linnakangas @ 2021-02-01 15:30 UTC (permalink / raw)
Per John Naylor's report, and some bugs I found while testing.
TODO: to be squashed.
---
src/backend/utils/mb/conv.c | 16 ++++++----------
.../utf8_and_iso8859_1/utf8_and_iso8859_1.c | 8 ++++++++
src/backend/utils/mb/mbutils.c | 5 +++--
3 files changed, 17 insertions(+), 12 deletions(-)
diff --git a/src/backend/utils/mb/conv.c b/src/backend/utils/mb/conv.c
index b83358bc7a5..f3bda5753d0 100644
--- a/src/backend/utils/mb/conv.c
+++ b/src/backend/utils/mb/conv.c
@@ -497,7 +497,6 @@ UtfToLocal(const unsigned char *utf, int len,
int l;
const pg_utf_to_local_combined *cp;
const unsigned char *start = utf;
- const unsigned char *cur = utf;
if (!PG_VALID_ENCODING(encoding))
ereport(ERROR,
@@ -511,8 +510,6 @@ UtfToLocal(const unsigned char *utf, int len,
unsigned char b3 = 0;
unsigned char b4 = 0;
- cur = iso;
-
/* "break" cases all represent errors */
if (*utf == '\0')
break;
@@ -648,10 +645,11 @@ UtfToLocal(const unsigned char *utf, int len,
}
/* failed to translate this character */
+ utf -= l;
if (noError)
break;
report_untranslatable_char(PG_UTF8, encoding,
- (const char *) (utf - l), len);
+ (const char *) utf, len);
}
/* if we broke out of loop early, must be invalid input */
@@ -660,7 +658,7 @@ UtfToLocal(const unsigned char *utf, int len,
*iso = '\0';
- return cur - start;
+ return utf - start;
}
/*
@@ -701,7 +699,6 @@ LocalToUtf(const unsigned char *iso, int len,
int l;
const pg_local_to_utf_combined *cp;
const unsigned char *start = iso;
- const unsigned char *cur = iso;
if (!PG_VALID_ENCODING(encoding))
ereport(ERROR,
@@ -715,8 +712,6 @@ LocalToUtf(const unsigned char *iso, int len,
unsigned char b3 = 0;
unsigned char b4 = 0;
- cur = iso;
-
/* "break" cases all represent errors */
if (*iso == '\0')
break;
@@ -799,10 +794,11 @@ LocalToUtf(const unsigned char *iso, int len,
}
/* failed to translate this character */
+ iso -= l;
if (noError)
break;
report_untranslatable_char(encoding, PG_UTF8,
- (const char *) (iso - l), len);
+ (const char *) iso, len);
}
/* if we broke out of loop early, must be invalid input */
@@ -811,5 +807,5 @@ LocalToUtf(const unsigned char *iso, int len,
*utf = '\0';
- return cur - start;
+ return iso - start;
}
diff --git a/src/backend/utils/mb/conversion_procs/utf8_and_iso8859_1/utf8_and_iso8859_1.c b/src/backend/utils/mb/conversion_procs/utf8_and_iso8859_1/utf8_and_iso8859_1.c
index 8ac93604a1b..d0dc4cca378 100644
--- a/src/backend/utils/mb/conversion_procs/utf8_and_iso8859_1/utf8_and_iso8859_1.c
+++ b/src/backend/utils/mb/conversion_procs/utf8_and_iso8859_1/utf8_and_iso8859_1.c
@@ -104,10 +104,18 @@ utf8_to_iso8859_1(PG_FUNCTION_ARGS)
int l = pg_utf_mblen(src);
if (l > len || !pg_utf8_islegal(src, l))
+ {
+ if (noError)
+ break;
report_invalid_encoding(PG_UTF8, (const char *) src, len);
+ }
if (l != 2)
+ {
+ if (noError)
+ break;
report_untranslatable_char(PG_UTF8, PG_LATIN1,
(const char *) src, len);
+ }
c1 = src[1] & 0x3f;
c = ((c & 0x1f) << 6) | c1;
if (c >= 0x80 && c <= 0xff)
diff --git a/src/backend/utils/mb/mbutils.c b/src/backend/utils/mb/mbutils.c
index 3e106027d75..af1cafe523a 100644
--- a/src/backend/utils/mb/mbutils.c
+++ b/src/backend/utils/mb/mbutils.c
@@ -818,12 +818,13 @@ perform_default_encoding_conversion(const char *src, int len,
MemoryContextAllocHuge(CurrentMemoryContext,
(Size) len * MAX_CONVERSION_GROWTH + 1);
- FunctionCall5(flinfo,
+ FunctionCall6(flinfo,
Int32GetDatum(src_encoding),
Int32GetDatum(dest_encoding),
CStringGetDatum(src),
CStringGetDatum(result),
- Int32GetDatum(len));
+ Int32GetDatum(len),
+ BoolGetDatum(false));
/*
* Release extra space if there might be a lot --- see comments in
--
2.29.2
--------------39E46EF15EBFAA37D53332E5
Content-Type: text/x-patch; charset=UTF-8;
name="v3-0005-Do-COPY-FROM-encoding-conversion-verification-in-.patch"
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment;
filename*0="v3-0005-Do-COPY-FROM-encoding-conversion-verification-in-.pa";
filename*1="tch"
^ permalink raw reply [nested|flat] 54+ messages in thread
* [PATCH v3 4/5] Fix bugs in the commit to change conversion function signature.
@ 2021-02-01 15:30 Heikki Linnakangas <[email protected]>
0 siblings, 0 replies; 54+ messages in thread
From: Heikki Linnakangas @ 2021-02-01 15:30 UTC (permalink / raw)
Per John Naylor's report, and some bugs I found while testing.
TODO: to be squashed.
---
src/backend/utils/mb/conv.c | 16 ++++++----------
.../utf8_and_iso8859_1/utf8_and_iso8859_1.c | 8 ++++++++
src/backend/utils/mb/mbutils.c | 5 +++--
3 files changed, 17 insertions(+), 12 deletions(-)
diff --git a/src/backend/utils/mb/conv.c b/src/backend/utils/mb/conv.c
index b83358bc7a5..f3bda5753d0 100644
--- a/src/backend/utils/mb/conv.c
+++ b/src/backend/utils/mb/conv.c
@@ -497,7 +497,6 @@ UtfToLocal(const unsigned char *utf, int len,
int l;
const pg_utf_to_local_combined *cp;
const unsigned char *start = utf;
- const unsigned char *cur = utf;
if (!PG_VALID_ENCODING(encoding))
ereport(ERROR,
@@ -511,8 +510,6 @@ UtfToLocal(const unsigned char *utf, int len,
unsigned char b3 = 0;
unsigned char b4 = 0;
- cur = iso;
-
/* "break" cases all represent errors */
if (*utf == '\0')
break;
@@ -648,10 +645,11 @@ UtfToLocal(const unsigned char *utf, int len,
}
/* failed to translate this character */
+ utf -= l;
if (noError)
break;
report_untranslatable_char(PG_UTF8, encoding,
- (const char *) (utf - l), len);
+ (const char *) utf, len);
}
/* if we broke out of loop early, must be invalid input */
@@ -660,7 +658,7 @@ UtfToLocal(const unsigned char *utf, int len,
*iso = '\0';
- return cur - start;
+ return utf - start;
}
/*
@@ -701,7 +699,6 @@ LocalToUtf(const unsigned char *iso, int len,
int l;
const pg_local_to_utf_combined *cp;
const unsigned char *start = iso;
- const unsigned char *cur = iso;
if (!PG_VALID_ENCODING(encoding))
ereport(ERROR,
@@ -715,8 +712,6 @@ LocalToUtf(const unsigned char *iso, int len,
unsigned char b3 = 0;
unsigned char b4 = 0;
- cur = iso;
-
/* "break" cases all represent errors */
if (*iso == '\0')
break;
@@ -799,10 +794,11 @@ LocalToUtf(const unsigned char *iso, int len,
}
/* failed to translate this character */
+ iso -= l;
if (noError)
break;
report_untranslatable_char(encoding, PG_UTF8,
- (const char *) (iso - l), len);
+ (const char *) iso, len);
}
/* if we broke out of loop early, must be invalid input */
@@ -811,5 +807,5 @@ LocalToUtf(const unsigned char *iso, int len,
*utf = '\0';
- return cur - start;
+ return iso - start;
}
diff --git a/src/backend/utils/mb/conversion_procs/utf8_and_iso8859_1/utf8_and_iso8859_1.c b/src/backend/utils/mb/conversion_procs/utf8_and_iso8859_1/utf8_and_iso8859_1.c
index 8ac93604a1b..d0dc4cca378 100644
--- a/src/backend/utils/mb/conversion_procs/utf8_and_iso8859_1/utf8_and_iso8859_1.c
+++ b/src/backend/utils/mb/conversion_procs/utf8_and_iso8859_1/utf8_and_iso8859_1.c
@@ -104,10 +104,18 @@ utf8_to_iso8859_1(PG_FUNCTION_ARGS)
int l = pg_utf_mblen(src);
if (l > len || !pg_utf8_islegal(src, l))
+ {
+ if (noError)
+ break;
report_invalid_encoding(PG_UTF8, (const char *) src, len);
+ }
if (l != 2)
+ {
+ if (noError)
+ break;
report_untranslatable_char(PG_UTF8, PG_LATIN1,
(const char *) src, len);
+ }
c1 = src[1] & 0x3f;
c = ((c & 0x1f) << 6) | c1;
if (c >= 0x80 && c <= 0xff)
diff --git a/src/backend/utils/mb/mbutils.c b/src/backend/utils/mb/mbutils.c
index 3e106027d75..af1cafe523a 100644
--- a/src/backend/utils/mb/mbutils.c
+++ b/src/backend/utils/mb/mbutils.c
@@ -818,12 +818,13 @@ perform_default_encoding_conversion(const char *src, int len,
MemoryContextAllocHuge(CurrentMemoryContext,
(Size) len * MAX_CONVERSION_GROWTH + 1);
- FunctionCall5(flinfo,
+ FunctionCall6(flinfo,
Int32GetDatum(src_encoding),
Int32GetDatum(dest_encoding),
CStringGetDatum(src),
CStringGetDatum(result),
- Int32GetDatum(len));
+ Int32GetDatum(len),
+ BoolGetDatum(false));
/*
* Release extra space if there might be a lot --- see comments in
--
2.29.2
--------------39E46EF15EBFAA37D53332E5
Content-Type: text/x-patch; charset=UTF-8;
name="v3-0005-Do-COPY-FROM-encoding-conversion-verification-in-.patch"
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment;
filename*0="v3-0005-Do-COPY-FROM-encoding-conversion-verification-in-.pa";
filename*1="tch"
^ permalink raw reply [nested|flat] 54+ messages in thread
* [PATCH v3 4/5] Fix bugs in the commit to change conversion function signature.
@ 2021-02-01 15:30 Heikki Linnakangas <[email protected]>
0 siblings, 0 replies; 54+ messages in thread
From: Heikki Linnakangas @ 2021-02-01 15:30 UTC (permalink / raw)
Per John Naylor's report, and some bugs I found while testing.
TODO: to be squashed.
---
src/backend/utils/mb/conv.c | 16 ++++++----------
.../utf8_and_iso8859_1/utf8_and_iso8859_1.c | 8 ++++++++
src/backend/utils/mb/mbutils.c | 5 +++--
3 files changed, 17 insertions(+), 12 deletions(-)
diff --git a/src/backend/utils/mb/conv.c b/src/backend/utils/mb/conv.c
index b83358bc7a5..f3bda5753d0 100644
--- a/src/backend/utils/mb/conv.c
+++ b/src/backend/utils/mb/conv.c
@@ -497,7 +497,6 @@ UtfToLocal(const unsigned char *utf, int len,
int l;
const pg_utf_to_local_combined *cp;
const unsigned char *start = utf;
- const unsigned char *cur = utf;
if (!PG_VALID_ENCODING(encoding))
ereport(ERROR,
@@ -511,8 +510,6 @@ UtfToLocal(const unsigned char *utf, int len,
unsigned char b3 = 0;
unsigned char b4 = 0;
- cur = iso;
-
/* "break" cases all represent errors */
if (*utf == '\0')
break;
@@ -648,10 +645,11 @@ UtfToLocal(const unsigned char *utf, int len,
}
/* failed to translate this character */
+ utf -= l;
if (noError)
break;
report_untranslatable_char(PG_UTF8, encoding,
- (const char *) (utf - l), len);
+ (const char *) utf, len);
}
/* if we broke out of loop early, must be invalid input */
@@ -660,7 +658,7 @@ UtfToLocal(const unsigned char *utf, int len,
*iso = '\0';
- return cur - start;
+ return utf - start;
}
/*
@@ -701,7 +699,6 @@ LocalToUtf(const unsigned char *iso, int len,
int l;
const pg_local_to_utf_combined *cp;
const unsigned char *start = iso;
- const unsigned char *cur = iso;
if (!PG_VALID_ENCODING(encoding))
ereport(ERROR,
@@ -715,8 +712,6 @@ LocalToUtf(const unsigned char *iso, int len,
unsigned char b3 = 0;
unsigned char b4 = 0;
- cur = iso;
-
/* "break" cases all represent errors */
if (*iso == '\0')
break;
@@ -799,10 +794,11 @@ LocalToUtf(const unsigned char *iso, int len,
}
/* failed to translate this character */
+ iso -= l;
if (noError)
break;
report_untranslatable_char(encoding, PG_UTF8,
- (const char *) (iso - l), len);
+ (const char *) iso, len);
}
/* if we broke out of loop early, must be invalid input */
@@ -811,5 +807,5 @@ LocalToUtf(const unsigned char *iso, int len,
*utf = '\0';
- return cur - start;
+ return iso - start;
}
diff --git a/src/backend/utils/mb/conversion_procs/utf8_and_iso8859_1/utf8_and_iso8859_1.c b/src/backend/utils/mb/conversion_procs/utf8_and_iso8859_1/utf8_and_iso8859_1.c
index 8ac93604a1b..d0dc4cca378 100644
--- a/src/backend/utils/mb/conversion_procs/utf8_and_iso8859_1/utf8_and_iso8859_1.c
+++ b/src/backend/utils/mb/conversion_procs/utf8_and_iso8859_1/utf8_and_iso8859_1.c
@@ -104,10 +104,18 @@ utf8_to_iso8859_1(PG_FUNCTION_ARGS)
int l = pg_utf_mblen(src);
if (l > len || !pg_utf8_islegal(src, l))
+ {
+ if (noError)
+ break;
report_invalid_encoding(PG_UTF8, (const char *) src, len);
+ }
if (l != 2)
+ {
+ if (noError)
+ break;
report_untranslatable_char(PG_UTF8, PG_LATIN1,
(const char *) src, len);
+ }
c1 = src[1] & 0x3f;
c = ((c & 0x1f) << 6) | c1;
if (c >= 0x80 && c <= 0xff)
diff --git a/src/backend/utils/mb/mbutils.c b/src/backend/utils/mb/mbutils.c
index 3e106027d75..af1cafe523a 100644
--- a/src/backend/utils/mb/mbutils.c
+++ b/src/backend/utils/mb/mbutils.c
@@ -818,12 +818,13 @@ perform_default_encoding_conversion(const char *src, int len,
MemoryContextAllocHuge(CurrentMemoryContext,
(Size) len * MAX_CONVERSION_GROWTH + 1);
- FunctionCall5(flinfo,
+ FunctionCall6(flinfo,
Int32GetDatum(src_encoding),
Int32GetDatum(dest_encoding),
CStringGetDatum(src),
CStringGetDatum(result),
- Int32GetDatum(len));
+ Int32GetDatum(len),
+ BoolGetDatum(false));
/*
* Release extra space if there might be a lot --- see comments in
--
2.29.2
--------------39E46EF15EBFAA37D53332E5
Content-Type: text/x-patch; charset=UTF-8;
name="v3-0005-Do-COPY-FROM-encoding-conversion-verification-in-.patch"
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment;
filename*0="v3-0005-Do-COPY-FROM-encoding-conversion-verification-in-.pa";
filename*1="tch"
^ permalink raw reply [nested|flat] 54+ messages in thread
* [PATCH v3 4/5] Fix bugs in the commit to change conversion function signature.
@ 2021-02-01 15:30 Heikki Linnakangas <[email protected]>
0 siblings, 0 replies; 54+ messages in thread
From: Heikki Linnakangas @ 2021-02-01 15:30 UTC (permalink / raw)
Per John Naylor's report, and some bugs I found while testing.
TODO: to be squashed.
---
src/backend/utils/mb/conv.c | 16 ++++++----------
.../utf8_and_iso8859_1/utf8_and_iso8859_1.c | 8 ++++++++
src/backend/utils/mb/mbutils.c | 5 +++--
3 files changed, 17 insertions(+), 12 deletions(-)
diff --git a/src/backend/utils/mb/conv.c b/src/backend/utils/mb/conv.c
index b83358bc7a5..f3bda5753d0 100644
--- a/src/backend/utils/mb/conv.c
+++ b/src/backend/utils/mb/conv.c
@@ -497,7 +497,6 @@ UtfToLocal(const unsigned char *utf, int len,
int l;
const pg_utf_to_local_combined *cp;
const unsigned char *start = utf;
- const unsigned char *cur = utf;
if (!PG_VALID_ENCODING(encoding))
ereport(ERROR,
@@ -511,8 +510,6 @@ UtfToLocal(const unsigned char *utf, int len,
unsigned char b3 = 0;
unsigned char b4 = 0;
- cur = iso;
-
/* "break" cases all represent errors */
if (*utf == '\0')
break;
@@ -648,10 +645,11 @@ UtfToLocal(const unsigned char *utf, int len,
}
/* failed to translate this character */
+ utf -= l;
if (noError)
break;
report_untranslatable_char(PG_UTF8, encoding,
- (const char *) (utf - l), len);
+ (const char *) utf, len);
}
/* if we broke out of loop early, must be invalid input */
@@ -660,7 +658,7 @@ UtfToLocal(const unsigned char *utf, int len,
*iso = '\0';
- return cur - start;
+ return utf - start;
}
/*
@@ -701,7 +699,6 @@ LocalToUtf(const unsigned char *iso, int len,
int l;
const pg_local_to_utf_combined *cp;
const unsigned char *start = iso;
- const unsigned char *cur = iso;
if (!PG_VALID_ENCODING(encoding))
ereport(ERROR,
@@ -715,8 +712,6 @@ LocalToUtf(const unsigned char *iso, int len,
unsigned char b3 = 0;
unsigned char b4 = 0;
- cur = iso;
-
/* "break" cases all represent errors */
if (*iso == '\0')
break;
@@ -799,10 +794,11 @@ LocalToUtf(const unsigned char *iso, int len,
}
/* failed to translate this character */
+ iso -= l;
if (noError)
break;
report_untranslatable_char(encoding, PG_UTF8,
- (const char *) (iso - l), len);
+ (const char *) iso, len);
}
/* if we broke out of loop early, must be invalid input */
@@ -811,5 +807,5 @@ LocalToUtf(const unsigned char *iso, int len,
*utf = '\0';
- return cur - start;
+ return iso - start;
}
diff --git a/src/backend/utils/mb/conversion_procs/utf8_and_iso8859_1/utf8_and_iso8859_1.c b/src/backend/utils/mb/conversion_procs/utf8_and_iso8859_1/utf8_and_iso8859_1.c
index 8ac93604a1b..d0dc4cca378 100644
--- a/src/backend/utils/mb/conversion_procs/utf8_and_iso8859_1/utf8_and_iso8859_1.c
+++ b/src/backend/utils/mb/conversion_procs/utf8_and_iso8859_1/utf8_and_iso8859_1.c
@@ -104,10 +104,18 @@ utf8_to_iso8859_1(PG_FUNCTION_ARGS)
int l = pg_utf_mblen(src);
if (l > len || !pg_utf8_islegal(src, l))
+ {
+ if (noError)
+ break;
report_invalid_encoding(PG_UTF8, (const char *) src, len);
+ }
if (l != 2)
+ {
+ if (noError)
+ break;
report_untranslatable_char(PG_UTF8, PG_LATIN1,
(const char *) src, len);
+ }
c1 = src[1] & 0x3f;
c = ((c & 0x1f) << 6) | c1;
if (c >= 0x80 && c <= 0xff)
diff --git a/src/backend/utils/mb/mbutils.c b/src/backend/utils/mb/mbutils.c
index 3e106027d75..af1cafe523a 100644
--- a/src/backend/utils/mb/mbutils.c
+++ b/src/backend/utils/mb/mbutils.c
@@ -818,12 +818,13 @@ perform_default_encoding_conversion(const char *src, int len,
MemoryContextAllocHuge(CurrentMemoryContext,
(Size) len * MAX_CONVERSION_GROWTH + 1);
- FunctionCall5(flinfo,
+ FunctionCall6(flinfo,
Int32GetDatum(src_encoding),
Int32GetDatum(dest_encoding),
CStringGetDatum(src),
CStringGetDatum(result),
- Int32GetDatum(len));
+ Int32GetDatum(len),
+ BoolGetDatum(false));
/*
* Release extra space if there might be a lot --- see comments in
--
2.29.2
--------------39E46EF15EBFAA37D53332E5
Content-Type: text/x-patch; charset=UTF-8;
name="v3-0005-Do-COPY-FROM-encoding-conversion-verification-in-.patch"
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment;
filename*0="v3-0005-Do-COPY-FROM-encoding-conversion-verification-in-.pa";
filename*1="tch"
^ permalink raw reply [nested|flat] 54+ messages in thread
* [PATCH v3 4/5] Fix bugs in the commit to change conversion function signature.
@ 2021-02-01 15:30 Heikki Linnakangas <[email protected]>
0 siblings, 0 replies; 54+ messages in thread
From: Heikki Linnakangas @ 2021-02-01 15:30 UTC (permalink / raw)
Per John Naylor's report, and some bugs I found while testing.
TODO: to be squashed.
---
src/backend/utils/mb/conv.c | 16 ++++++----------
.../utf8_and_iso8859_1/utf8_and_iso8859_1.c | 8 ++++++++
src/backend/utils/mb/mbutils.c | 5 +++--
3 files changed, 17 insertions(+), 12 deletions(-)
diff --git a/src/backend/utils/mb/conv.c b/src/backend/utils/mb/conv.c
index b83358bc7a5..f3bda5753d0 100644
--- a/src/backend/utils/mb/conv.c
+++ b/src/backend/utils/mb/conv.c
@@ -497,7 +497,6 @@ UtfToLocal(const unsigned char *utf, int len,
int l;
const pg_utf_to_local_combined *cp;
const unsigned char *start = utf;
- const unsigned char *cur = utf;
if (!PG_VALID_ENCODING(encoding))
ereport(ERROR,
@@ -511,8 +510,6 @@ UtfToLocal(const unsigned char *utf, int len,
unsigned char b3 = 0;
unsigned char b4 = 0;
- cur = iso;
-
/* "break" cases all represent errors */
if (*utf == '\0')
break;
@@ -648,10 +645,11 @@ UtfToLocal(const unsigned char *utf, int len,
}
/* failed to translate this character */
+ utf -= l;
if (noError)
break;
report_untranslatable_char(PG_UTF8, encoding,
- (const char *) (utf - l), len);
+ (const char *) utf, len);
}
/* if we broke out of loop early, must be invalid input */
@@ -660,7 +658,7 @@ UtfToLocal(const unsigned char *utf, int len,
*iso = '\0';
- return cur - start;
+ return utf - start;
}
/*
@@ -701,7 +699,6 @@ LocalToUtf(const unsigned char *iso, int len,
int l;
const pg_local_to_utf_combined *cp;
const unsigned char *start = iso;
- const unsigned char *cur = iso;
if (!PG_VALID_ENCODING(encoding))
ereport(ERROR,
@@ -715,8 +712,6 @@ LocalToUtf(const unsigned char *iso, int len,
unsigned char b3 = 0;
unsigned char b4 = 0;
- cur = iso;
-
/* "break" cases all represent errors */
if (*iso == '\0')
break;
@@ -799,10 +794,11 @@ LocalToUtf(const unsigned char *iso, int len,
}
/* failed to translate this character */
+ iso -= l;
if (noError)
break;
report_untranslatable_char(encoding, PG_UTF8,
- (const char *) (iso - l), len);
+ (const char *) iso, len);
}
/* if we broke out of loop early, must be invalid input */
@@ -811,5 +807,5 @@ LocalToUtf(const unsigned char *iso, int len,
*utf = '\0';
- return cur - start;
+ return iso - start;
}
diff --git a/src/backend/utils/mb/conversion_procs/utf8_and_iso8859_1/utf8_and_iso8859_1.c b/src/backend/utils/mb/conversion_procs/utf8_and_iso8859_1/utf8_and_iso8859_1.c
index 8ac93604a1b..d0dc4cca378 100644
--- a/src/backend/utils/mb/conversion_procs/utf8_and_iso8859_1/utf8_and_iso8859_1.c
+++ b/src/backend/utils/mb/conversion_procs/utf8_and_iso8859_1/utf8_and_iso8859_1.c
@@ -104,10 +104,18 @@ utf8_to_iso8859_1(PG_FUNCTION_ARGS)
int l = pg_utf_mblen(src);
if (l > len || !pg_utf8_islegal(src, l))
+ {
+ if (noError)
+ break;
report_invalid_encoding(PG_UTF8, (const char *) src, len);
+ }
if (l != 2)
+ {
+ if (noError)
+ break;
report_untranslatable_char(PG_UTF8, PG_LATIN1,
(const char *) src, len);
+ }
c1 = src[1] & 0x3f;
c = ((c & 0x1f) << 6) | c1;
if (c >= 0x80 && c <= 0xff)
diff --git a/src/backend/utils/mb/mbutils.c b/src/backend/utils/mb/mbutils.c
index 3e106027d75..af1cafe523a 100644
--- a/src/backend/utils/mb/mbutils.c
+++ b/src/backend/utils/mb/mbutils.c
@@ -818,12 +818,13 @@ perform_default_encoding_conversion(const char *src, int len,
MemoryContextAllocHuge(CurrentMemoryContext,
(Size) len * MAX_CONVERSION_GROWTH + 1);
- FunctionCall5(flinfo,
+ FunctionCall6(flinfo,
Int32GetDatum(src_encoding),
Int32GetDatum(dest_encoding),
CStringGetDatum(src),
CStringGetDatum(result),
- Int32GetDatum(len));
+ Int32GetDatum(len),
+ BoolGetDatum(false));
/*
* Release extra space if there might be a lot --- see comments in
--
2.29.2
--------------39E46EF15EBFAA37D53332E5
Content-Type: text/x-patch; charset=UTF-8;
name="v3-0005-Do-COPY-FROM-encoding-conversion-verification-in-.patch"
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment;
filename*0="v3-0005-Do-COPY-FROM-encoding-conversion-verification-in-.pa";
filename*1="tch"
^ permalink raw reply [nested|flat] 54+ messages in thread
* [PATCH v3 4/5] Fix bugs in the commit to change conversion function signature.
@ 2021-02-01 15:30 Heikki Linnakangas <[email protected]>
0 siblings, 0 replies; 54+ messages in thread
From: Heikki Linnakangas @ 2021-02-01 15:30 UTC (permalink / raw)
Per John Naylor's report, and some bugs I found while testing.
TODO: to be squashed.
---
src/backend/utils/mb/conv.c | 16 ++++++----------
.../utf8_and_iso8859_1/utf8_and_iso8859_1.c | 8 ++++++++
src/backend/utils/mb/mbutils.c | 5 +++--
3 files changed, 17 insertions(+), 12 deletions(-)
diff --git a/src/backend/utils/mb/conv.c b/src/backend/utils/mb/conv.c
index b83358bc7a5..f3bda5753d0 100644
--- a/src/backend/utils/mb/conv.c
+++ b/src/backend/utils/mb/conv.c
@@ -497,7 +497,6 @@ UtfToLocal(const unsigned char *utf, int len,
int l;
const pg_utf_to_local_combined *cp;
const unsigned char *start = utf;
- const unsigned char *cur = utf;
if (!PG_VALID_ENCODING(encoding))
ereport(ERROR,
@@ -511,8 +510,6 @@ UtfToLocal(const unsigned char *utf, int len,
unsigned char b3 = 0;
unsigned char b4 = 0;
- cur = iso;
-
/* "break" cases all represent errors */
if (*utf == '\0')
break;
@@ -648,10 +645,11 @@ UtfToLocal(const unsigned char *utf, int len,
}
/* failed to translate this character */
+ utf -= l;
if (noError)
break;
report_untranslatable_char(PG_UTF8, encoding,
- (const char *) (utf - l), len);
+ (const char *) utf, len);
}
/* if we broke out of loop early, must be invalid input */
@@ -660,7 +658,7 @@ UtfToLocal(const unsigned char *utf, int len,
*iso = '\0';
- return cur - start;
+ return utf - start;
}
/*
@@ -701,7 +699,6 @@ LocalToUtf(const unsigned char *iso, int len,
int l;
const pg_local_to_utf_combined *cp;
const unsigned char *start = iso;
- const unsigned char *cur = iso;
if (!PG_VALID_ENCODING(encoding))
ereport(ERROR,
@@ -715,8 +712,6 @@ LocalToUtf(const unsigned char *iso, int len,
unsigned char b3 = 0;
unsigned char b4 = 0;
- cur = iso;
-
/* "break" cases all represent errors */
if (*iso == '\0')
break;
@@ -799,10 +794,11 @@ LocalToUtf(const unsigned char *iso, int len,
}
/* failed to translate this character */
+ iso -= l;
if (noError)
break;
report_untranslatable_char(encoding, PG_UTF8,
- (const char *) (iso - l), len);
+ (const char *) iso, len);
}
/* if we broke out of loop early, must be invalid input */
@@ -811,5 +807,5 @@ LocalToUtf(const unsigned char *iso, int len,
*utf = '\0';
- return cur - start;
+ return iso - start;
}
diff --git a/src/backend/utils/mb/conversion_procs/utf8_and_iso8859_1/utf8_and_iso8859_1.c b/src/backend/utils/mb/conversion_procs/utf8_and_iso8859_1/utf8_and_iso8859_1.c
index 8ac93604a1b..d0dc4cca378 100644
--- a/src/backend/utils/mb/conversion_procs/utf8_and_iso8859_1/utf8_and_iso8859_1.c
+++ b/src/backend/utils/mb/conversion_procs/utf8_and_iso8859_1/utf8_and_iso8859_1.c
@@ -104,10 +104,18 @@ utf8_to_iso8859_1(PG_FUNCTION_ARGS)
int l = pg_utf_mblen(src);
if (l > len || !pg_utf8_islegal(src, l))
+ {
+ if (noError)
+ break;
report_invalid_encoding(PG_UTF8, (const char *) src, len);
+ }
if (l != 2)
+ {
+ if (noError)
+ break;
report_untranslatable_char(PG_UTF8, PG_LATIN1,
(const char *) src, len);
+ }
c1 = src[1] & 0x3f;
c = ((c & 0x1f) << 6) | c1;
if (c >= 0x80 && c <= 0xff)
diff --git a/src/backend/utils/mb/mbutils.c b/src/backend/utils/mb/mbutils.c
index 3e106027d75..af1cafe523a 100644
--- a/src/backend/utils/mb/mbutils.c
+++ b/src/backend/utils/mb/mbutils.c
@@ -818,12 +818,13 @@ perform_default_encoding_conversion(const char *src, int len,
MemoryContextAllocHuge(CurrentMemoryContext,
(Size) len * MAX_CONVERSION_GROWTH + 1);
- FunctionCall5(flinfo,
+ FunctionCall6(flinfo,
Int32GetDatum(src_encoding),
Int32GetDatum(dest_encoding),
CStringGetDatum(src),
CStringGetDatum(result),
- Int32GetDatum(len));
+ Int32GetDatum(len),
+ BoolGetDatum(false));
/*
* Release extra space if there might be a lot --- see comments in
--
2.29.2
--------------39E46EF15EBFAA37D53332E5
Content-Type: text/x-patch; charset=UTF-8;
name="v3-0005-Do-COPY-FROM-encoding-conversion-verification-in-.patch"
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment;
filename*0="v3-0005-Do-COPY-FROM-encoding-conversion-verification-in-.pa";
filename*1="tch"
^ permalink raw reply [nested|flat] 54+ messages in thread
* [PATCH v3 4/5] Fix bugs in the commit to change conversion function signature.
@ 2021-02-01 15:30 Heikki Linnakangas <[email protected]>
0 siblings, 0 replies; 54+ messages in thread
From: Heikki Linnakangas @ 2021-02-01 15:30 UTC (permalink / raw)
Per John Naylor's report, and some bugs I found while testing.
TODO: to be squashed.
---
src/backend/utils/mb/conv.c | 16 ++++++----------
.../utf8_and_iso8859_1/utf8_and_iso8859_1.c | 8 ++++++++
src/backend/utils/mb/mbutils.c | 5 +++--
3 files changed, 17 insertions(+), 12 deletions(-)
diff --git a/src/backend/utils/mb/conv.c b/src/backend/utils/mb/conv.c
index b83358bc7a5..f3bda5753d0 100644
--- a/src/backend/utils/mb/conv.c
+++ b/src/backend/utils/mb/conv.c
@@ -497,7 +497,6 @@ UtfToLocal(const unsigned char *utf, int len,
int l;
const pg_utf_to_local_combined *cp;
const unsigned char *start = utf;
- const unsigned char *cur = utf;
if (!PG_VALID_ENCODING(encoding))
ereport(ERROR,
@@ -511,8 +510,6 @@ UtfToLocal(const unsigned char *utf, int len,
unsigned char b3 = 0;
unsigned char b4 = 0;
- cur = iso;
-
/* "break" cases all represent errors */
if (*utf == '\0')
break;
@@ -648,10 +645,11 @@ UtfToLocal(const unsigned char *utf, int len,
}
/* failed to translate this character */
+ utf -= l;
if (noError)
break;
report_untranslatable_char(PG_UTF8, encoding,
- (const char *) (utf - l), len);
+ (const char *) utf, len);
}
/* if we broke out of loop early, must be invalid input */
@@ -660,7 +658,7 @@ UtfToLocal(const unsigned char *utf, int len,
*iso = '\0';
- return cur - start;
+ return utf - start;
}
/*
@@ -701,7 +699,6 @@ LocalToUtf(const unsigned char *iso, int len,
int l;
const pg_local_to_utf_combined *cp;
const unsigned char *start = iso;
- const unsigned char *cur = iso;
if (!PG_VALID_ENCODING(encoding))
ereport(ERROR,
@@ -715,8 +712,6 @@ LocalToUtf(const unsigned char *iso, int len,
unsigned char b3 = 0;
unsigned char b4 = 0;
- cur = iso;
-
/* "break" cases all represent errors */
if (*iso == '\0')
break;
@@ -799,10 +794,11 @@ LocalToUtf(const unsigned char *iso, int len,
}
/* failed to translate this character */
+ iso -= l;
if (noError)
break;
report_untranslatable_char(encoding, PG_UTF8,
- (const char *) (iso - l), len);
+ (const char *) iso, len);
}
/* if we broke out of loop early, must be invalid input */
@@ -811,5 +807,5 @@ LocalToUtf(const unsigned char *iso, int len,
*utf = '\0';
- return cur - start;
+ return iso - start;
}
diff --git a/src/backend/utils/mb/conversion_procs/utf8_and_iso8859_1/utf8_and_iso8859_1.c b/src/backend/utils/mb/conversion_procs/utf8_and_iso8859_1/utf8_and_iso8859_1.c
index 8ac93604a1b..d0dc4cca378 100644
--- a/src/backend/utils/mb/conversion_procs/utf8_and_iso8859_1/utf8_and_iso8859_1.c
+++ b/src/backend/utils/mb/conversion_procs/utf8_and_iso8859_1/utf8_and_iso8859_1.c
@@ -104,10 +104,18 @@ utf8_to_iso8859_1(PG_FUNCTION_ARGS)
int l = pg_utf_mblen(src);
if (l > len || !pg_utf8_islegal(src, l))
+ {
+ if (noError)
+ break;
report_invalid_encoding(PG_UTF8, (const char *) src, len);
+ }
if (l != 2)
+ {
+ if (noError)
+ break;
report_untranslatable_char(PG_UTF8, PG_LATIN1,
(const char *) src, len);
+ }
c1 = src[1] & 0x3f;
c = ((c & 0x1f) << 6) | c1;
if (c >= 0x80 && c <= 0xff)
diff --git a/src/backend/utils/mb/mbutils.c b/src/backend/utils/mb/mbutils.c
index 3e106027d75..af1cafe523a 100644
--- a/src/backend/utils/mb/mbutils.c
+++ b/src/backend/utils/mb/mbutils.c
@@ -818,12 +818,13 @@ perform_default_encoding_conversion(const char *src, int len,
MemoryContextAllocHuge(CurrentMemoryContext,
(Size) len * MAX_CONVERSION_GROWTH + 1);
- FunctionCall5(flinfo,
+ FunctionCall6(flinfo,
Int32GetDatum(src_encoding),
Int32GetDatum(dest_encoding),
CStringGetDatum(src),
CStringGetDatum(result),
- Int32GetDatum(len));
+ Int32GetDatum(len),
+ BoolGetDatum(false));
/*
* Release extra space if there might be a lot --- see comments in
--
2.29.2
--------------39E46EF15EBFAA37D53332E5
Content-Type: text/x-patch; charset=UTF-8;
name="v3-0005-Do-COPY-FROM-encoding-conversion-verification-in-.patch"
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment;
filename*0="v3-0005-Do-COPY-FROM-encoding-conversion-verification-in-.pa";
filename*1="tch"
^ permalink raw reply [nested|flat] 54+ messages in thread
* [PATCH v3 4/5] Fix bugs in the commit to change conversion function signature.
@ 2021-02-01 15:30 Heikki Linnakangas <[email protected]>
0 siblings, 0 replies; 54+ messages in thread
From: Heikki Linnakangas @ 2021-02-01 15:30 UTC (permalink / raw)
Per John Naylor's report, and some bugs I found while testing.
TODO: to be squashed.
---
src/backend/utils/mb/conv.c | 16 ++++++----------
.../utf8_and_iso8859_1/utf8_and_iso8859_1.c | 8 ++++++++
src/backend/utils/mb/mbutils.c | 5 +++--
3 files changed, 17 insertions(+), 12 deletions(-)
diff --git a/src/backend/utils/mb/conv.c b/src/backend/utils/mb/conv.c
index b83358bc7a5..f3bda5753d0 100644
--- a/src/backend/utils/mb/conv.c
+++ b/src/backend/utils/mb/conv.c
@@ -497,7 +497,6 @@ UtfToLocal(const unsigned char *utf, int len,
int l;
const pg_utf_to_local_combined *cp;
const unsigned char *start = utf;
- const unsigned char *cur = utf;
if (!PG_VALID_ENCODING(encoding))
ereport(ERROR,
@@ -511,8 +510,6 @@ UtfToLocal(const unsigned char *utf, int len,
unsigned char b3 = 0;
unsigned char b4 = 0;
- cur = iso;
-
/* "break" cases all represent errors */
if (*utf == '\0')
break;
@@ -648,10 +645,11 @@ UtfToLocal(const unsigned char *utf, int len,
}
/* failed to translate this character */
+ utf -= l;
if (noError)
break;
report_untranslatable_char(PG_UTF8, encoding,
- (const char *) (utf - l), len);
+ (const char *) utf, len);
}
/* if we broke out of loop early, must be invalid input */
@@ -660,7 +658,7 @@ UtfToLocal(const unsigned char *utf, int len,
*iso = '\0';
- return cur - start;
+ return utf - start;
}
/*
@@ -701,7 +699,6 @@ LocalToUtf(const unsigned char *iso, int len,
int l;
const pg_local_to_utf_combined *cp;
const unsigned char *start = iso;
- const unsigned char *cur = iso;
if (!PG_VALID_ENCODING(encoding))
ereport(ERROR,
@@ -715,8 +712,6 @@ LocalToUtf(const unsigned char *iso, int len,
unsigned char b3 = 0;
unsigned char b4 = 0;
- cur = iso;
-
/* "break" cases all represent errors */
if (*iso == '\0')
break;
@@ -799,10 +794,11 @@ LocalToUtf(const unsigned char *iso, int len,
}
/* failed to translate this character */
+ iso -= l;
if (noError)
break;
report_untranslatable_char(encoding, PG_UTF8,
- (const char *) (iso - l), len);
+ (const char *) iso, len);
}
/* if we broke out of loop early, must be invalid input */
@@ -811,5 +807,5 @@ LocalToUtf(const unsigned char *iso, int len,
*utf = '\0';
- return cur - start;
+ return iso - start;
}
diff --git a/src/backend/utils/mb/conversion_procs/utf8_and_iso8859_1/utf8_and_iso8859_1.c b/src/backend/utils/mb/conversion_procs/utf8_and_iso8859_1/utf8_and_iso8859_1.c
index 8ac93604a1b..d0dc4cca378 100644
--- a/src/backend/utils/mb/conversion_procs/utf8_and_iso8859_1/utf8_and_iso8859_1.c
+++ b/src/backend/utils/mb/conversion_procs/utf8_and_iso8859_1/utf8_and_iso8859_1.c
@@ -104,10 +104,18 @@ utf8_to_iso8859_1(PG_FUNCTION_ARGS)
int l = pg_utf_mblen(src);
if (l > len || !pg_utf8_islegal(src, l))
+ {
+ if (noError)
+ break;
report_invalid_encoding(PG_UTF8, (const char *) src, len);
+ }
if (l != 2)
+ {
+ if (noError)
+ break;
report_untranslatable_char(PG_UTF8, PG_LATIN1,
(const char *) src, len);
+ }
c1 = src[1] & 0x3f;
c = ((c & 0x1f) << 6) | c1;
if (c >= 0x80 && c <= 0xff)
diff --git a/src/backend/utils/mb/mbutils.c b/src/backend/utils/mb/mbutils.c
index 3e106027d75..af1cafe523a 100644
--- a/src/backend/utils/mb/mbutils.c
+++ b/src/backend/utils/mb/mbutils.c
@@ -818,12 +818,13 @@ perform_default_encoding_conversion(const char *src, int len,
MemoryContextAllocHuge(CurrentMemoryContext,
(Size) len * MAX_CONVERSION_GROWTH + 1);
- FunctionCall5(flinfo,
+ FunctionCall6(flinfo,
Int32GetDatum(src_encoding),
Int32GetDatum(dest_encoding),
CStringGetDatum(src),
CStringGetDatum(result),
- Int32GetDatum(len));
+ Int32GetDatum(len),
+ BoolGetDatum(false));
/*
* Release extra space if there might be a lot --- see comments in
--
2.29.2
--------------39E46EF15EBFAA37D53332E5
Content-Type: text/x-patch; charset=UTF-8;
name="v3-0005-Do-COPY-FROM-encoding-conversion-verification-in-.patch"
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment;
filename*0="v3-0005-Do-COPY-FROM-encoding-conversion-verification-in-.pa";
filename*1="tch"
^ permalink raw reply [nested|flat] 54+ messages in thread
* [PATCH v3 4/5] Fix bugs in the commit to change conversion function signature.
@ 2021-02-01 15:30 Heikki Linnakangas <[email protected]>
0 siblings, 0 replies; 54+ messages in thread
From: Heikki Linnakangas @ 2021-02-01 15:30 UTC (permalink / raw)
Per John Naylor's report, and some bugs I found while testing.
TODO: to be squashed.
---
src/backend/utils/mb/conv.c | 16 ++++++----------
.../utf8_and_iso8859_1/utf8_and_iso8859_1.c | 8 ++++++++
src/backend/utils/mb/mbutils.c | 5 +++--
3 files changed, 17 insertions(+), 12 deletions(-)
diff --git a/src/backend/utils/mb/conv.c b/src/backend/utils/mb/conv.c
index b83358bc7a5..f3bda5753d0 100644
--- a/src/backend/utils/mb/conv.c
+++ b/src/backend/utils/mb/conv.c
@@ -497,7 +497,6 @@ UtfToLocal(const unsigned char *utf, int len,
int l;
const pg_utf_to_local_combined *cp;
const unsigned char *start = utf;
- const unsigned char *cur = utf;
if (!PG_VALID_ENCODING(encoding))
ereport(ERROR,
@@ -511,8 +510,6 @@ UtfToLocal(const unsigned char *utf, int len,
unsigned char b3 = 0;
unsigned char b4 = 0;
- cur = iso;
-
/* "break" cases all represent errors */
if (*utf == '\0')
break;
@@ -648,10 +645,11 @@ UtfToLocal(const unsigned char *utf, int len,
}
/* failed to translate this character */
+ utf -= l;
if (noError)
break;
report_untranslatable_char(PG_UTF8, encoding,
- (const char *) (utf - l), len);
+ (const char *) utf, len);
}
/* if we broke out of loop early, must be invalid input */
@@ -660,7 +658,7 @@ UtfToLocal(const unsigned char *utf, int len,
*iso = '\0';
- return cur - start;
+ return utf - start;
}
/*
@@ -701,7 +699,6 @@ LocalToUtf(const unsigned char *iso, int len,
int l;
const pg_local_to_utf_combined *cp;
const unsigned char *start = iso;
- const unsigned char *cur = iso;
if (!PG_VALID_ENCODING(encoding))
ereport(ERROR,
@@ -715,8 +712,6 @@ LocalToUtf(const unsigned char *iso, int len,
unsigned char b3 = 0;
unsigned char b4 = 0;
- cur = iso;
-
/* "break" cases all represent errors */
if (*iso == '\0')
break;
@@ -799,10 +794,11 @@ LocalToUtf(const unsigned char *iso, int len,
}
/* failed to translate this character */
+ iso -= l;
if (noError)
break;
report_untranslatable_char(encoding, PG_UTF8,
- (const char *) (iso - l), len);
+ (const char *) iso, len);
}
/* if we broke out of loop early, must be invalid input */
@@ -811,5 +807,5 @@ LocalToUtf(const unsigned char *iso, int len,
*utf = '\0';
- return cur - start;
+ return iso - start;
}
diff --git a/src/backend/utils/mb/conversion_procs/utf8_and_iso8859_1/utf8_and_iso8859_1.c b/src/backend/utils/mb/conversion_procs/utf8_and_iso8859_1/utf8_and_iso8859_1.c
index 8ac93604a1b..d0dc4cca378 100644
--- a/src/backend/utils/mb/conversion_procs/utf8_and_iso8859_1/utf8_and_iso8859_1.c
+++ b/src/backend/utils/mb/conversion_procs/utf8_and_iso8859_1/utf8_and_iso8859_1.c
@@ -104,10 +104,18 @@ utf8_to_iso8859_1(PG_FUNCTION_ARGS)
int l = pg_utf_mblen(src);
if (l > len || !pg_utf8_islegal(src, l))
+ {
+ if (noError)
+ break;
report_invalid_encoding(PG_UTF8, (const char *) src, len);
+ }
if (l != 2)
+ {
+ if (noError)
+ break;
report_untranslatable_char(PG_UTF8, PG_LATIN1,
(const char *) src, len);
+ }
c1 = src[1] & 0x3f;
c = ((c & 0x1f) << 6) | c1;
if (c >= 0x80 && c <= 0xff)
diff --git a/src/backend/utils/mb/mbutils.c b/src/backend/utils/mb/mbutils.c
index 3e106027d75..af1cafe523a 100644
--- a/src/backend/utils/mb/mbutils.c
+++ b/src/backend/utils/mb/mbutils.c
@@ -818,12 +818,13 @@ perform_default_encoding_conversion(const char *src, int len,
MemoryContextAllocHuge(CurrentMemoryContext,
(Size) len * MAX_CONVERSION_GROWTH + 1);
- FunctionCall5(flinfo,
+ FunctionCall6(flinfo,
Int32GetDatum(src_encoding),
Int32GetDatum(dest_encoding),
CStringGetDatum(src),
CStringGetDatum(result),
- Int32GetDatum(len));
+ Int32GetDatum(len),
+ BoolGetDatum(false));
/*
* Release extra space if there might be a lot --- see comments in
--
2.29.2
--------------39E46EF15EBFAA37D53332E5
Content-Type: text/x-patch; charset=UTF-8;
name="v3-0005-Do-COPY-FROM-encoding-conversion-verification-in-.patch"
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment;
filename*0="v3-0005-Do-COPY-FROM-encoding-conversion-verification-in-.pa";
filename*1="tch"
^ permalink raw reply [nested|flat] 54+ messages in thread
* [PATCH v3 4/5] Fix bugs in the commit to change conversion function signature.
@ 2021-02-01 15:30 Heikki Linnakangas <[email protected]>
0 siblings, 0 replies; 54+ messages in thread
From: Heikki Linnakangas @ 2021-02-01 15:30 UTC (permalink / raw)
Per John Naylor's report, and some bugs I found while testing.
TODO: to be squashed.
---
src/backend/utils/mb/conv.c | 16 ++++++----------
.../utf8_and_iso8859_1/utf8_and_iso8859_1.c | 8 ++++++++
src/backend/utils/mb/mbutils.c | 5 +++--
3 files changed, 17 insertions(+), 12 deletions(-)
diff --git a/src/backend/utils/mb/conv.c b/src/backend/utils/mb/conv.c
index b83358bc7a5..f3bda5753d0 100644
--- a/src/backend/utils/mb/conv.c
+++ b/src/backend/utils/mb/conv.c
@@ -497,7 +497,6 @@ UtfToLocal(const unsigned char *utf, int len,
int l;
const pg_utf_to_local_combined *cp;
const unsigned char *start = utf;
- const unsigned char *cur = utf;
if (!PG_VALID_ENCODING(encoding))
ereport(ERROR,
@@ -511,8 +510,6 @@ UtfToLocal(const unsigned char *utf, int len,
unsigned char b3 = 0;
unsigned char b4 = 0;
- cur = iso;
-
/* "break" cases all represent errors */
if (*utf == '\0')
break;
@@ -648,10 +645,11 @@ UtfToLocal(const unsigned char *utf, int len,
}
/* failed to translate this character */
+ utf -= l;
if (noError)
break;
report_untranslatable_char(PG_UTF8, encoding,
- (const char *) (utf - l), len);
+ (const char *) utf, len);
}
/* if we broke out of loop early, must be invalid input */
@@ -660,7 +658,7 @@ UtfToLocal(const unsigned char *utf, int len,
*iso = '\0';
- return cur - start;
+ return utf - start;
}
/*
@@ -701,7 +699,6 @@ LocalToUtf(const unsigned char *iso, int len,
int l;
const pg_local_to_utf_combined *cp;
const unsigned char *start = iso;
- const unsigned char *cur = iso;
if (!PG_VALID_ENCODING(encoding))
ereport(ERROR,
@@ -715,8 +712,6 @@ LocalToUtf(const unsigned char *iso, int len,
unsigned char b3 = 0;
unsigned char b4 = 0;
- cur = iso;
-
/* "break" cases all represent errors */
if (*iso == '\0')
break;
@@ -799,10 +794,11 @@ LocalToUtf(const unsigned char *iso, int len,
}
/* failed to translate this character */
+ iso -= l;
if (noError)
break;
report_untranslatable_char(encoding, PG_UTF8,
- (const char *) (iso - l), len);
+ (const char *) iso, len);
}
/* if we broke out of loop early, must be invalid input */
@@ -811,5 +807,5 @@ LocalToUtf(const unsigned char *iso, int len,
*utf = '\0';
- return cur - start;
+ return iso - start;
}
diff --git a/src/backend/utils/mb/conversion_procs/utf8_and_iso8859_1/utf8_and_iso8859_1.c b/src/backend/utils/mb/conversion_procs/utf8_and_iso8859_1/utf8_and_iso8859_1.c
index 8ac93604a1b..d0dc4cca378 100644
--- a/src/backend/utils/mb/conversion_procs/utf8_and_iso8859_1/utf8_and_iso8859_1.c
+++ b/src/backend/utils/mb/conversion_procs/utf8_and_iso8859_1/utf8_and_iso8859_1.c
@@ -104,10 +104,18 @@ utf8_to_iso8859_1(PG_FUNCTION_ARGS)
int l = pg_utf_mblen(src);
if (l > len || !pg_utf8_islegal(src, l))
+ {
+ if (noError)
+ break;
report_invalid_encoding(PG_UTF8, (const char *) src, len);
+ }
if (l != 2)
+ {
+ if (noError)
+ break;
report_untranslatable_char(PG_UTF8, PG_LATIN1,
(const char *) src, len);
+ }
c1 = src[1] & 0x3f;
c = ((c & 0x1f) << 6) | c1;
if (c >= 0x80 && c <= 0xff)
diff --git a/src/backend/utils/mb/mbutils.c b/src/backend/utils/mb/mbutils.c
index 3e106027d75..af1cafe523a 100644
--- a/src/backend/utils/mb/mbutils.c
+++ b/src/backend/utils/mb/mbutils.c
@@ -818,12 +818,13 @@ perform_default_encoding_conversion(const char *src, int len,
MemoryContextAllocHuge(CurrentMemoryContext,
(Size) len * MAX_CONVERSION_GROWTH + 1);
- FunctionCall5(flinfo,
+ FunctionCall6(flinfo,
Int32GetDatum(src_encoding),
Int32GetDatum(dest_encoding),
CStringGetDatum(src),
CStringGetDatum(result),
- Int32GetDatum(len));
+ Int32GetDatum(len),
+ BoolGetDatum(false));
/*
* Release extra space if there might be a lot --- see comments in
--
2.29.2
--------------39E46EF15EBFAA37D53332E5
Content-Type: text/x-patch; charset=UTF-8;
name="v3-0005-Do-COPY-FROM-encoding-conversion-verification-in-.patch"
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment;
filename*0="v3-0005-Do-COPY-FROM-encoding-conversion-verification-in-.pa";
filename*1="tch"
^ permalink raw reply [nested|flat] 54+ messages in thread
* [PATCH v3 4/5] Fix bugs in the commit to change conversion function signature.
@ 2021-02-01 15:30 Heikki Linnakangas <[email protected]>
0 siblings, 0 replies; 54+ messages in thread
From: Heikki Linnakangas @ 2021-02-01 15:30 UTC (permalink / raw)
Per John Naylor's report, and some bugs I found while testing.
TODO: to be squashed.
---
src/backend/utils/mb/conv.c | 16 ++++++----------
.../utf8_and_iso8859_1/utf8_and_iso8859_1.c | 8 ++++++++
src/backend/utils/mb/mbutils.c | 5 +++--
3 files changed, 17 insertions(+), 12 deletions(-)
diff --git a/src/backend/utils/mb/conv.c b/src/backend/utils/mb/conv.c
index b83358bc7a5..f3bda5753d0 100644
--- a/src/backend/utils/mb/conv.c
+++ b/src/backend/utils/mb/conv.c
@@ -497,7 +497,6 @@ UtfToLocal(const unsigned char *utf, int len,
int l;
const pg_utf_to_local_combined *cp;
const unsigned char *start = utf;
- const unsigned char *cur = utf;
if (!PG_VALID_ENCODING(encoding))
ereport(ERROR,
@@ -511,8 +510,6 @@ UtfToLocal(const unsigned char *utf, int len,
unsigned char b3 = 0;
unsigned char b4 = 0;
- cur = iso;
-
/* "break" cases all represent errors */
if (*utf == '\0')
break;
@@ -648,10 +645,11 @@ UtfToLocal(const unsigned char *utf, int len,
}
/* failed to translate this character */
+ utf -= l;
if (noError)
break;
report_untranslatable_char(PG_UTF8, encoding,
- (const char *) (utf - l), len);
+ (const char *) utf, len);
}
/* if we broke out of loop early, must be invalid input */
@@ -660,7 +658,7 @@ UtfToLocal(const unsigned char *utf, int len,
*iso = '\0';
- return cur - start;
+ return utf - start;
}
/*
@@ -701,7 +699,6 @@ LocalToUtf(const unsigned char *iso, int len,
int l;
const pg_local_to_utf_combined *cp;
const unsigned char *start = iso;
- const unsigned char *cur = iso;
if (!PG_VALID_ENCODING(encoding))
ereport(ERROR,
@@ -715,8 +712,6 @@ LocalToUtf(const unsigned char *iso, int len,
unsigned char b3 = 0;
unsigned char b4 = 0;
- cur = iso;
-
/* "break" cases all represent errors */
if (*iso == '\0')
break;
@@ -799,10 +794,11 @@ LocalToUtf(const unsigned char *iso, int len,
}
/* failed to translate this character */
+ iso -= l;
if (noError)
break;
report_untranslatable_char(encoding, PG_UTF8,
- (const char *) (iso - l), len);
+ (const char *) iso, len);
}
/* if we broke out of loop early, must be invalid input */
@@ -811,5 +807,5 @@ LocalToUtf(const unsigned char *iso, int len,
*utf = '\0';
- return cur - start;
+ return iso - start;
}
diff --git a/src/backend/utils/mb/conversion_procs/utf8_and_iso8859_1/utf8_and_iso8859_1.c b/src/backend/utils/mb/conversion_procs/utf8_and_iso8859_1/utf8_and_iso8859_1.c
index 8ac93604a1b..d0dc4cca378 100644
--- a/src/backend/utils/mb/conversion_procs/utf8_and_iso8859_1/utf8_and_iso8859_1.c
+++ b/src/backend/utils/mb/conversion_procs/utf8_and_iso8859_1/utf8_and_iso8859_1.c
@@ -104,10 +104,18 @@ utf8_to_iso8859_1(PG_FUNCTION_ARGS)
int l = pg_utf_mblen(src);
if (l > len || !pg_utf8_islegal(src, l))
+ {
+ if (noError)
+ break;
report_invalid_encoding(PG_UTF8, (const char *) src, len);
+ }
if (l != 2)
+ {
+ if (noError)
+ break;
report_untranslatable_char(PG_UTF8, PG_LATIN1,
(const char *) src, len);
+ }
c1 = src[1] & 0x3f;
c = ((c & 0x1f) << 6) | c1;
if (c >= 0x80 && c <= 0xff)
diff --git a/src/backend/utils/mb/mbutils.c b/src/backend/utils/mb/mbutils.c
index 3e106027d75..af1cafe523a 100644
--- a/src/backend/utils/mb/mbutils.c
+++ b/src/backend/utils/mb/mbutils.c
@@ -818,12 +818,13 @@ perform_default_encoding_conversion(const char *src, int len,
MemoryContextAllocHuge(CurrentMemoryContext,
(Size) len * MAX_CONVERSION_GROWTH + 1);
- FunctionCall5(flinfo,
+ FunctionCall6(flinfo,
Int32GetDatum(src_encoding),
Int32GetDatum(dest_encoding),
CStringGetDatum(src),
CStringGetDatum(result),
- Int32GetDatum(len));
+ Int32GetDatum(len),
+ BoolGetDatum(false));
/*
* Release extra space if there might be a lot --- see comments in
--
2.29.2
--------------39E46EF15EBFAA37D53332E5
Content-Type: text/x-patch; charset=UTF-8;
name="v3-0005-Do-COPY-FROM-encoding-conversion-verification-in-.patch"
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment;
filename*0="v3-0005-Do-COPY-FROM-encoding-conversion-verification-in-.pa";
filename*1="tch"
^ permalink raw reply [nested|flat] 54+ messages in thread
* [PATCH v3 4/5] Fix bugs in the commit to change conversion function signature.
@ 2021-02-01 15:30 Heikki Linnakangas <[email protected]>
0 siblings, 0 replies; 54+ messages in thread
From: Heikki Linnakangas @ 2021-02-01 15:30 UTC (permalink / raw)
Per John Naylor's report, and some bugs I found while testing.
TODO: to be squashed.
---
src/backend/utils/mb/conv.c | 16 ++++++----------
.../utf8_and_iso8859_1/utf8_and_iso8859_1.c | 8 ++++++++
src/backend/utils/mb/mbutils.c | 5 +++--
3 files changed, 17 insertions(+), 12 deletions(-)
diff --git a/src/backend/utils/mb/conv.c b/src/backend/utils/mb/conv.c
index b83358bc7a5..f3bda5753d0 100644
--- a/src/backend/utils/mb/conv.c
+++ b/src/backend/utils/mb/conv.c
@@ -497,7 +497,6 @@ UtfToLocal(const unsigned char *utf, int len,
int l;
const pg_utf_to_local_combined *cp;
const unsigned char *start = utf;
- const unsigned char *cur = utf;
if (!PG_VALID_ENCODING(encoding))
ereport(ERROR,
@@ -511,8 +510,6 @@ UtfToLocal(const unsigned char *utf, int len,
unsigned char b3 = 0;
unsigned char b4 = 0;
- cur = iso;
-
/* "break" cases all represent errors */
if (*utf == '\0')
break;
@@ -648,10 +645,11 @@ UtfToLocal(const unsigned char *utf, int len,
}
/* failed to translate this character */
+ utf -= l;
if (noError)
break;
report_untranslatable_char(PG_UTF8, encoding,
- (const char *) (utf - l), len);
+ (const char *) utf, len);
}
/* if we broke out of loop early, must be invalid input */
@@ -660,7 +658,7 @@ UtfToLocal(const unsigned char *utf, int len,
*iso = '\0';
- return cur - start;
+ return utf - start;
}
/*
@@ -701,7 +699,6 @@ LocalToUtf(const unsigned char *iso, int len,
int l;
const pg_local_to_utf_combined *cp;
const unsigned char *start = iso;
- const unsigned char *cur = iso;
if (!PG_VALID_ENCODING(encoding))
ereport(ERROR,
@@ -715,8 +712,6 @@ LocalToUtf(const unsigned char *iso, int len,
unsigned char b3 = 0;
unsigned char b4 = 0;
- cur = iso;
-
/* "break" cases all represent errors */
if (*iso == '\0')
break;
@@ -799,10 +794,11 @@ LocalToUtf(const unsigned char *iso, int len,
}
/* failed to translate this character */
+ iso -= l;
if (noError)
break;
report_untranslatable_char(encoding, PG_UTF8,
- (const char *) (iso - l), len);
+ (const char *) iso, len);
}
/* if we broke out of loop early, must be invalid input */
@@ -811,5 +807,5 @@ LocalToUtf(const unsigned char *iso, int len,
*utf = '\0';
- return cur - start;
+ return iso - start;
}
diff --git a/src/backend/utils/mb/conversion_procs/utf8_and_iso8859_1/utf8_and_iso8859_1.c b/src/backend/utils/mb/conversion_procs/utf8_and_iso8859_1/utf8_and_iso8859_1.c
index 8ac93604a1b..d0dc4cca378 100644
--- a/src/backend/utils/mb/conversion_procs/utf8_and_iso8859_1/utf8_and_iso8859_1.c
+++ b/src/backend/utils/mb/conversion_procs/utf8_and_iso8859_1/utf8_and_iso8859_1.c
@@ -104,10 +104,18 @@ utf8_to_iso8859_1(PG_FUNCTION_ARGS)
int l = pg_utf_mblen(src);
if (l > len || !pg_utf8_islegal(src, l))
+ {
+ if (noError)
+ break;
report_invalid_encoding(PG_UTF8, (const char *) src, len);
+ }
if (l != 2)
+ {
+ if (noError)
+ break;
report_untranslatable_char(PG_UTF8, PG_LATIN1,
(const char *) src, len);
+ }
c1 = src[1] & 0x3f;
c = ((c & 0x1f) << 6) | c1;
if (c >= 0x80 && c <= 0xff)
diff --git a/src/backend/utils/mb/mbutils.c b/src/backend/utils/mb/mbutils.c
index 3e106027d75..af1cafe523a 100644
--- a/src/backend/utils/mb/mbutils.c
+++ b/src/backend/utils/mb/mbutils.c
@@ -818,12 +818,13 @@ perform_default_encoding_conversion(const char *src, int len,
MemoryContextAllocHuge(CurrentMemoryContext,
(Size) len * MAX_CONVERSION_GROWTH + 1);
- FunctionCall5(flinfo,
+ FunctionCall6(flinfo,
Int32GetDatum(src_encoding),
Int32GetDatum(dest_encoding),
CStringGetDatum(src),
CStringGetDatum(result),
- Int32GetDatum(len));
+ Int32GetDatum(len),
+ BoolGetDatum(false));
/*
* Release extra space if there might be a lot --- see comments in
--
2.29.2
--------------39E46EF15EBFAA37D53332E5
Content-Type: text/x-patch; charset=UTF-8;
name="v3-0005-Do-COPY-FROM-encoding-conversion-verification-in-.patch"
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment;
filename*0="v3-0005-Do-COPY-FROM-encoding-conversion-verification-in-.pa";
filename*1="tch"
^ permalink raw reply [nested|flat] 54+ messages in thread
* [PATCH v3 4/5] Fix bugs in the commit to change conversion function signature.
@ 2021-02-01 15:30 Heikki Linnakangas <[email protected]>
0 siblings, 0 replies; 54+ messages in thread
From: Heikki Linnakangas @ 2021-02-01 15:30 UTC (permalink / raw)
Per John Naylor's report, and some bugs I found while testing.
TODO: to be squashed.
---
src/backend/utils/mb/conv.c | 16 ++++++----------
.../utf8_and_iso8859_1/utf8_and_iso8859_1.c | 8 ++++++++
src/backend/utils/mb/mbutils.c | 5 +++--
3 files changed, 17 insertions(+), 12 deletions(-)
diff --git a/src/backend/utils/mb/conv.c b/src/backend/utils/mb/conv.c
index b83358bc7a5..f3bda5753d0 100644
--- a/src/backend/utils/mb/conv.c
+++ b/src/backend/utils/mb/conv.c
@@ -497,7 +497,6 @@ UtfToLocal(const unsigned char *utf, int len,
int l;
const pg_utf_to_local_combined *cp;
const unsigned char *start = utf;
- const unsigned char *cur = utf;
if (!PG_VALID_ENCODING(encoding))
ereport(ERROR,
@@ -511,8 +510,6 @@ UtfToLocal(const unsigned char *utf, int len,
unsigned char b3 = 0;
unsigned char b4 = 0;
- cur = iso;
-
/* "break" cases all represent errors */
if (*utf == '\0')
break;
@@ -648,10 +645,11 @@ UtfToLocal(const unsigned char *utf, int len,
}
/* failed to translate this character */
+ utf -= l;
if (noError)
break;
report_untranslatable_char(PG_UTF8, encoding,
- (const char *) (utf - l), len);
+ (const char *) utf, len);
}
/* if we broke out of loop early, must be invalid input */
@@ -660,7 +658,7 @@ UtfToLocal(const unsigned char *utf, int len,
*iso = '\0';
- return cur - start;
+ return utf - start;
}
/*
@@ -701,7 +699,6 @@ LocalToUtf(const unsigned char *iso, int len,
int l;
const pg_local_to_utf_combined *cp;
const unsigned char *start = iso;
- const unsigned char *cur = iso;
if (!PG_VALID_ENCODING(encoding))
ereport(ERROR,
@@ -715,8 +712,6 @@ LocalToUtf(const unsigned char *iso, int len,
unsigned char b3 = 0;
unsigned char b4 = 0;
- cur = iso;
-
/* "break" cases all represent errors */
if (*iso == '\0')
break;
@@ -799,10 +794,11 @@ LocalToUtf(const unsigned char *iso, int len,
}
/* failed to translate this character */
+ iso -= l;
if (noError)
break;
report_untranslatable_char(encoding, PG_UTF8,
- (const char *) (iso - l), len);
+ (const char *) iso, len);
}
/* if we broke out of loop early, must be invalid input */
@@ -811,5 +807,5 @@ LocalToUtf(const unsigned char *iso, int len,
*utf = '\0';
- return cur - start;
+ return iso - start;
}
diff --git a/src/backend/utils/mb/conversion_procs/utf8_and_iso8859_1/utf8_and_iso8859_1.c b/src/backend/utils/mb/conversion_procs/utf8_and_iso8859_1/utf8_and_iso8859_1.c
index 8ac93604a1b..d0dc4cca378 100644
--- a/src/backend/utils/mb/conversion_procs/utf8_and_iso8859_1/utf8_and_iso8859_1.c
+++ b/src/backend/utils/mb/conversion_procs/utf8_and_iso8859_1/utf8_and_iso8859_1.c
@@ -104,10 +104,18 @@ utf8_to_iso8859_1(PG_FUNCTION_ARGS)
int l = pg_utf_mblen(src);
if (l > len || !pg_utf8_islegal(src, l))
+ {
+ if (noError)
+ break;
report_invalid_encoding(PG_UTF8, (const char *) src, len);
+ }
if (l != 2)
+ {
+ if (noError)
+ break;
report_untranslatable_char(PG_UTF8, PG_LATIN1,
(const char *) src, len);
+ }
c1 = src[1] & 0x3f;
c = ((c & 0x1f) << 6) | c1;
if (c >= 0x80 && c <= 0xff)
diff --git a/src/backend/utils/mb/mbutils.c b/src/backend/utils/mb/mbutils.c
index 3e106027d75..af1cafe523a 100644
--- a/src/backend/utils/mb/mbutils.c
+++ b/src/backend/utils/mb/mbutils.c
@@ -818,12 +818,13 @@ perform_default_encoding_conversion(const char *src, int len,
MemoryContextAllocHuge(CurrentMemoryContext,
(Size) len * MAX_CONVERSION_GROWTH + 1);
- FunctionCall5(flinfo,
+ FunctionCall6(flinfo,
Int32GetDatum(src_encoding),
Int32GetDatum(dest_encoding),
CStringGetDatum(src),
CStringGetDatum(result),
- Int32GetDatum(len));
+ Int32GetDatum(len),
+ BoolGetDatum(false));
/*
* Release extra space if there might be a lot --- see comments in
--
2.29.2
--------------39E46EF15EBFAA37D53332E5
Content-Type: text/x-patch; charset=UTF-8;
name="v3-0005-Do-COPY-FROM-encoding-conversion-verification-in-.patch"
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment;
filename*0="v3-0005-Do-COPY-FROM-encoding-conversion-verification-in-.pa";
filename*1="tch"
^ permalink raw reply [nested|flat] 54+ messages in thread
* [PATCH v3 4/5] Fix bugs in the commit to change conversion function signature.
@ 2021-02-01 15:30 Heikki Linnakangas <[email protected]>
0 siblings, 0 replies; 54+ messages in thread
From: Heikki Linnakangas @ 2021-02-01 15:30 UTC (permalink / raw)
Per John Naylor's report, and some bugs I found while testing.
TODO: to be squashed.
---
src/backend/utils/mb/conv.c | 16 ++++++----------
.../utf8_and_iso8859_1/utf8_and_iso8859_1.c | 8 ++++++++
src/backend/utils/mb/mbutils.c | 5 +++--
3 files changed, 17 insertions(+), 12 deletions(-)
diff --git a/src/backend/utils/mb/conv.c b/src/backend/utils/mb/conv.c
index b83358bc7a5..f3bda5753d0 100644
--- a/src/backend/utils/mb/conv.c
+++ b/src/backend/utils/mb/conv.c
@@ -497,7 +497,6 @@ UtfToLocal(const unsigned char *utf, int len,
int l;
const pg_utf_to_local_combined *cp;
const unsigned char *start = utf;
- const unsigned char *cur = utf;
if (!PG_VALID_ENCODING(encoding))
ereport(ERROR,
@@ -511,8 +510,6 @@ UtfToLocal(const unsigned char *utf, int len,
unsigned char b3 = 0;
unsigned char b4 = 0;
- cur = iso;
-
/* "break" cases all represent errors */
if (*utf == '\0')
break;
@@ -648,10 +645,11 @@ UtfToLocal(const unsigned char *utf, int len,
}
/* failed to translate this character */
+ utf -= l;
if (noError)
break;
report_untranslatable_char(PG_UTF8, encoding,
- (const char *) (utf - l), len);
+ (const char *) utf, len);
}
/* if we broke out of loop early, must be invalid input */
@@ -660,7 +658,7 @@ UtfToLocal(const unsigned char *utf, int len,
*iso = '\0';
- return cur - start;
+ return utf - start;
}
/*
@@ -701,7 +699,6 @@ LocalToUtf(const unsigned char *iso, int len,
int l;
const pg_local_to_utf_combined *cp;
const unsigned char *start = iso;
- const unsigned char *cur = iso;
if (!PG_VALID_ENCODING(encoding))
ereport(ERROR,
@@ -715,8 +712,6 @@ LocalToUtf(const unsigned char *iso, int len,
unsigned char b3 = 0;
unsigned char b4 = 0;
- cur = iso;
-
/* "break" cases all represent errors */
if (*iso == '\0')
break;
@@ -799,10 +794,11 @@ LocalToUtf(const unsigned char *iso, int len,
}
/* failed to translate this character */
+ iso -= l;
if (noError)
break;
report_untranslatable_char(encoding, PG_UTF8,
- (const char *) (iso - l), len);
+ (const char *) iso, len);
}
/* if we broke out of loop early, must be invalid input */
@@ -811,5 +807,5 @@ LocalToUtf(const unsigned char *iso, int len,
*utf = '\0';
- return cur - start;
+ return iso - start;
}
diff --git a/src/backend/utils/mb/conversion_procs/utf8_and_iso8859_1/utf8_and_iso8859_1.c b/src/backend/utils/mb/conversion_procs/utf8_and_iso8859_1/utf8_and_iso8859_1.c
index 8ac93604a1b..d0dc4cca378 100644
--- a/src/backend/utils/mb/conversion_procs/utf8_and_iso8859_1/utf8_and_iso8859_1.c
+++ b/src/backend/utils/mb/conversion_procs/utf8_and_iso8859_1/utf8_and_iso8859_1.c
@@ -104,10 +104,18 @@ utf8_to_iso8859_1(PG_FUNCTION_ARGS)
int l = pg_utf_mblen(src);
if (l > len || !pg_utf8_islegal(src, l))
+ {
+ if (noError)
+ break;
report_invalid_encoding(PG_UTF8, (const char *) src, len);
+ }
if (l != 2)
+ {
+ if (noError)
+ break;
report_untranslatable_char(PG_UTF8, PG_LATIN1,
(const char *) src, len);
+ }
c1 = src[1] & 0x3f;
c = ((c & 0x1f) << 6) | c1;
if (c >= 0x80 && c <= 0xff)
diff --git a/src/backend/utils/mb/mbutils.c b/src/backend/utils/mb/mbutils.c
index 3e106027d75..af1cafe523a 100644
--- a/src/backend/utils/mb/mbutils.c
+++ b/src/backend/utils/mb/mbutils.c
@@ -818,12 +818,13 @@ perform_default_encoding_conversion(const char *src, int len,
MemoryContextAllocHuge(CurrentMemoryContext,
(Size) len * MAX_CONVERSION_GROWTH + 1);
- FunctionCall5(flinfo,
+ FunctionCall6(flinfo,
Int32GetDatum(src_encoding),
Int32GetDatum(dest_encoding),
CStringGetDatum(src),
CStringGetDatum(result),
- Int32GetDatum(len));
+ Int32GetDatum(len),
+ BoolGetDatum(false));
/*
* Release extra space if there might be a lot --- see comments in
--
2.29.2
--------------39E46EF15EBFAA37D53332E5
Content-Type: text/x-patch; charset=UTF-8;
name="v3-0005-Do-COPY-FROM-encoding-conversion-verification-in-.patch"
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment;
filename*0="v3-0005-Do-COPY-FROM-encoding-conversion-verification-in-.pa";
filename*1="tch"
^ permalink raw reply [nested|flat] 54+ messages in thread
* [PATCH v3 4/5] Fix bugs in the commit to change conversion function signature.
@ 2021-02-01 15:30 Heikki Linnakangas <[email protected]>
0 siblings, 0 replies; 54+ messages in thread
From: Heikki Linnakangas @ 2021-02-01 15:30 UTC (permalink / raw)
Per John Naylor's report, and some bugs I found while testing.
TODO: to be squashed.
---
src/backend/utils/mb/conv.c | 16 ++++++----------
.../utf8_and_iso8859_1/utf8_and_iso8859_1.c | 8 ++++++++
src/backend/utils/mb/mbutils.c | 5 +++--
3 files changed, 17 insertions(+), 12 deletions(-)
diff --git a/src/backend/utils/mb/conv.c b/src/backend/utils/mb/conv.c
index b83358bc7a5..f3bda5753d0 100644
--- a/src/backend/utils/mb/conv.c
+++ b/src/backend/utils/mb/conv.c
@@ -497,7 +497,6 @@ UtfToLocal(const unsigned char *utf, int len,
int l;
const pg_utf_to_local_combined *cp;
const unsigned char *start = utf;
- const unsigned char *cur = utf;
if (!PG_VALID_ENCODING(encoding))
ereport(ERROR,
@@ -511,8 +510,6 @@ UtfToLocal(const unsigned char *utf, int len,
unsigned char b3 = 0;
unsigned char b4 = 0;
- cur = iso;
-
/* "break" cases all represent errors */
if (*utf == '\0')
break;
@@ -648,10 +645,11 @@ UtfToLocal(const unsigned char *utf, int len,
}
/* failed to translate this character */
+ utf -= l;
if (noError)
break;
report_untranslatable_char(PG_UTF8, encoding,
- (const char *) (utf - l), len);
+ (const char *) utf, len);
}
/* if we broke out of loop early, must be invalid input */
@@ -660,7 +658,7 @@ UtfToLocal(const unsigned char *utf, int len,
*iso = '\0';
- return cur - start;
+ return utf - start;
}
/*
@@ -701,7 +699,6 @@ LocalToUtf(const unsigned char *iso, int len,
int l;
const pg_local_to_utf_combined *cp;
const unsigned char *start = iso;
- const unsigned char *cur = iso;
if (!PG_VALID_ENCODING(encoding))
ereport(ERROR,
@@ -715,8 +712,6 @@ LocalToUtf(const unsigned char *iso, int len,
unsigned char b3 = 0;
unsigned char b4 = 0;
- cur = iso;
-
/* "break" cases all represent errors */
if (*iso == '\0')
break;
@@ -799,10 +794,11 @@ LocalToUtf(const unsigned char *iso, int len,
}
/* failed to translate this character */
+ iso -= l;
if (noError)
break;
report_untranslatable_char(encoding, PG_UTF8,
- (const char *) (iso - l), len);
+ (const char *) iso, len);
}
/* if we broke out of loop early, must be invalid input */
@@ -811,5 +807,5 @@ LocalToUtf(const unsigned char *iso, int len,
*utf = '\0';
- return cur - start;
+ return iso - start;
}
diff --git a/src/backend/utils/mb/conversion_procs/utf8_and_iso8859_1/utf8_and_iso8859_1.c b/src/backend/utils/mb/conversion_procs/utf8_and_iso8859_1/utf8_and_iso8859_1.c
index 8ac93604a1b..d0dc4cca378 100644
--- a/src/backend/utils/mb/conversion_procs/utf8_and_iso8859_1/utf8_and_iso8859_1.c
+++ b/src/backend/utils/mb/conversion_procs/utf8_and_iso8859_1/utf8_and_iso8859_1.c
@@ -104,10 +104,18 @@ utf8_to_iso8859_1(PG_FUNCTION_ARGS)
int l = pg_utf_mblen(src);
if (l > len || !pg_utf8_islegal(src, l))
+ {
+ if (noError)
+ break;
report_invalid_encoding(PG_UTF8, (const char *) src, len);
+ }
if (l != 2)
+ {
+ if (noError)
+ break;
report_untranslatable_char(PG_UTF8, PG_LATIN1,
(const char *) src, len);
+ }
c1 = src[1] & 0x3f;
c = ((c & 0x1f) << 6) | c1;
if (c >= 0x80 && c <= 0xff)
diff --git a/src/backend/utils/mb/mbutils.c b/src/backend/utils/mb/mbutils.c
index 3e106027d75..af1cafe523a 100644
--- a/src/backend/utils/mb/mbutils.c
+++ b/src/backend/utils/mb/mbutils.c
@@ -818,12 +818,13 @@ perform_default_encoding_conversion(const char *src, int len,
MemoryContextAllocHuge(CurrentMemoryContext,
(Size) len * MAX_CONVERSION_GROWTH + 1);
- FunctionCall5(flinfo,
+ FunctionCall6(flinfo,
Int32GetDatum(src_encoding),
Int32GetDatum(dest_encoding),
CStringGetDatum(src),
CStringGetDatum(result),
- Int32GetDatum(len));
+ Int32GetDatum(len),
+ BoolGetDatum(false));
/*
* Release extra space if there might be a lot --- see comments in
--
2.29.2
--------------39E46EF15EBFAA37D53332E5
Content-Type: text/x-patch; charset=UTF-8;
name="v3-0005-Do-COPY-FROM-encoding-conversion-verification-in-.patch"
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment;
filename*0="v3-0005-Do-COPY-FROM-encoding-conversion-verification-in-.pa";
filename*1="tch"
^ permalink raw reply [nested|flat] 54+ messages in thread
* [PATCH v3 4/5] Fix bugs in the commit to change conversion function signature.
@ 2021-02-01 15:30 Heikki Linnakangas <[email protected]>
0 siblings, 0 replies; 54+ messages in thread
From: Heikki Linnakangas @ 2021-02-01 15:30 UTC (permalink / raw)
Per John Naylor's report, and some bugs I found while testing.
TODO: to be squashed.
---
src/backend/utils/mb/conv.c | 16 ++++++----------
.../utf8_and_iso8859_1/utf8_and_iso8859_1.c | 8 ++++++++
src/backend/utils/mb/mbutils.c | 5 +++--
3 files changed, 17 insertions(+), 12 deletions(-)
diff --git a/src/backend/utils/mb/conv.c b/src/backend/utils/mb/conv.c
index b83358bc7a5..f3bda5753d0 100644
--- a/src/backend/utils/mb/conv.c
+++ b/src/backend/utils/mb/conv.c
@@ -497,7 +497,6 @@ UtfToLocal(const unsigned char *utf, int len,
int l;
const pg_utf_to_local_combined *cp;
const unsigned char *start = utf;
- const unsigned char *cur = utf;
if (!PG_VALID_ENCODING(encoding))
ereport(ERROR,
@@ -511,8 +510,6 @@ UtfToLocal(const unsigned char *utf, int len,
unsigned char b3 = 0;
unsigned char b4 = 0;
- cur = iso;
-
/* "break" cases all represent errors */
if (*utf == '\0')
break;
@@ -648,10 +645,11 @@ UtfToLocal(const unsigned char *utf, int len,
}
/* failed to translate this character */
+ utf -= l;
if (noError)
break;
report_untranslatable_char(PG_UTF8, encoding,
- (const char *) (utf - l), len);
+ (const char *) utf, len);
}
/* if we broke out of loop early, must be invalid input */
@@ -660,7 +658,7 @@ UtfToLocal(const unsigned char *utf, int len,
*iso = '\0';
- return cur - start;
+ return utf - start;
}
/*
@@ -701,7 +699,6 @@ LocalToUtf(const unsigned char *iso, int len,
int l;
const pg_local_to_utf_combined *cp;
const unsigned char *start = iso;
- const unsigned char *cur = iso;
if (!PG_VALID_ENCODING(encoding))
ereport(ERROR,
@@ -715,8 +712,6 @@ LocalToUtf(const unsigned char *iso, int len,
unsigned char b3 = 0;
unsigned char b4 = 0;
- cur = iso;
-
/* "break" cases all represent errors */
if (*iso == '\0')
break;
@@ -799,10 +794,11 @@ LocalToUtf(const unsigned char *iso, int len,
}
/* failed to translate this character */
+ iso -= l;
if (noError)
break;
report_untranslatable_char(encoding, PG_UTF8,
- (const char *) (iso - l), len);
+ (const char *) iso, len);
}
/* if we broke out of loop early, must be invalid input */
@@ -811,5 +807,5 @@ LocalToUtf(const unsigned char *iso, int len,
*utf = '\0';
- return cur - start;
+ return iso - start;
}
diff --git a/src/backend/utils/mb/conversion_procs/utf8_and_iso8859_1/utf8_and_iso8859_1.c b/src/backend/utils/mb/conversion_procs/utf8_and_iso8859_1/utf8_and_iso8859_1.c
index 8ac93604a1b..d0dc4cca378 100644
--- a/src/backend/utils/mb/conversion_procs/utf8_and_iso8859_1/utf8_and_iso8859_1.c
+++ b/src/backend/utils/mb/conversion_procs/utf8_and_iso8859_1/utf8_and_iso8859_1.c
@@ -104,10 +104,18 @@ utf8_to_iso8859_1(PG_FUNCTION_ARGS)
int l = pg_utf_mblen(src);
if (l > len || !pg_utf8_islegal(src, l))
+ {
+ if (noError)
+ break;
report_invalid_encoding(PG_UTF8, (const char *) src, len);
+ }
if (l != 2)
+ {
+ if (noError)
+ break;
report_untranslatable_char(PG_UTF8, PG_LATIN1,
(const char *) src, len);
+ }
c1 = src[1] & 0x3f;
c = ((c & 0x1f) << 6) | c1;
if (c >= 0x80 && c <= 0xff)
diff --git a/src/backend/utils/mb/mbutils.c b/src/backend/utils/mb/mbutils.c
index 3e106027d75..af1cafe523a 100644
--- a/src/backend/utils/mb/mbutils.c
+++ b/src/backend/utils/mb/mbutils.c
@@ -818,12 +818,13 @@ perform_default_encoding_conversion(const char *src, int len,
MemoryContextAllocHuge(CurrentMemoryContext,
(Size) len * MAX_CONVERSION_GROWTH + 1);
- FunctionCall5(flinfo,
+ FunctionCall6(flinfo,
Int32GetDatum(src_encoding),
Int32GetDatum(dest_encoding),
CStringGetDatum(src),
CStringGetDatum(result),
- Int32GetDatum(len));
+ Int32GetDatum(len),
+ BoolGetDatum(false));
/*
* Release extra space if there might be a lot --- see comments in
--
2.29.2
--------------39E46EF15EBFAA37D53332E5
Content-Type: text/x-patch; charset=UTF-8;
name="v3-0005-Do-COPY-FROM-encoding-conversion-verification-in-.patch"
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment;
filename*0="v3-0005-Do-COPY-FROM-encoding-conversion-verification-in-.pa";
filename*1="tch"
^ permalink raw reply [nested|flat] 54+ messages in thread
* [PATCH v3 4/5] Fix bugs in the commit to change conversion function signature.
@ 2021-02-01 15:30 Heikki Linnakangas <[email protected]>
0 siblings, 0 replies; 54+ messages in thread
From: Heikki Linnakangas @ 2021-02-01 15:30 UTC (permalink / raw)
Per John Naylor's report, and some bugs I found while testing.
TODO: to be squashed.
---
src/backend/utils/mb/conv.c | 16 ++++++----------
.../utf8_and_iso8859_1/utf8_and_iso8859_1.c | 8 ++++++++
src/backend/utils/mb/mbutils.c | 5 +++--
3 files changed, 17 insertions(+), 12 deletions(-)
diff --git a/src/backend/utils/mb/conv.c b/src/backend/utils/mb/conv.c
index b83358bc7a5..f3bda5753d0 100644
--- a/src/backend/utils/mb/conv.c
+++ b/src/backend/utils/mb/conv.c
@@ -497,7 +497,6 @@ UtfToLocal(const unsigned char *utf, int len,
int l;
const pg_utf_to_local_combined *cp;
const unsigned char *start = utf;
- const unsigned char *cur = utf;
if (!PG_VALID_ENCODING(encoding))
ereport(ERROR,
@@ -511,8 +510,6 @@ UtfToLocal(const unsigned char *utf, int len,
unsigned char b3 = 0;
unsigned char b4 = 0;
- cur = iso;
-
/* "break" cases all represent errors */
if (*utf == '\0')
break;
@@ -648,10 +645,11 @@ UtfToLocal(const unsigned char *utf, int len,
}
/* failed to translate this character */
+ utf -= l;
if (noError)
break;
report_untranslatable_char(PG_UTF8, encoding,
- (const char *) (utf - l), len);
+ (const char *) utf, len);
}
/* if we broke out of loop early, must be invalid input */
@@ -660,7 +658,7 @@ UtfToLocal(const unsigned char *utf, int len,
*iso = '\0';
- return cur - start;
+ return utf - start;
}
/*
@@ -701,7 +699,6 @@ LocalToUtf(const unsigned char *iso, int len,
int l;
const pg_local_to_utf_combined *cp;
const unsigned char *start = iso;
- const unsigned char *cur = iso;
if (!PG_VALID_ENCODING(encoding))
ereport(ERROR,
@@ -715,8 +712,6 @@ LocalToUtf(const unsigned char *iso, int len,
unsigned char b3 = 0;
unsigned char b4 = 0;
- cur = iso;
-
/* "break" cases all represent errors */
if (*iso == '\0')
break;
@@ -799,10 +794,11 @@ LocalToUtf(const unsigned char *iso, int len,
}
/* failed to translate this character */
+ iso -= l;
if (noError)
break;
report_untranslatable_char(encoding, PG_UTF8,
- (const char *) (iso - l), len);
+ (const char *) iso, len);
}
/* if we broke out of loop early, must be invalid input */
@@ -811,5 +807,5 @@ LocalToUtf(const unsigned char *iso, int len,
*utf = '\0';
- return cur - start;
+ return iso - start;
}
diff --git a/src/backend/utils/mb/conversion_procs/utf8_and_iso8859_1/utf8_and_iso8859_1.c b/src/backend/utils/mb/conversion_procs/utf8_and_iso8859_1/utf8_and_iso8859_1.c
index 8ac93604a1b..d0dc4cca378 100644
--- a/src/backend/utils/mb/conversion_procs/utf8_and_iso8859_1/utf8_and_iso8859_1.c
+++ b/src/backend/utils/mb/conversion_procs/utf8_and_iso8859_1/utf8_and_iso8859_1.c
@@ -104,10 +104,18 @@ utf8_to_iso8859_1(PG_FUNCTION_ARGS)
int l = pg_utf_mblen(src);
if (l > len || !pg_utf8_islegal(src, l))
+ {
+ if (noError)
+ break;
report_invalid_encoding(PG_UTF8, (const char *) src, len);
+ }
if (l != 2)
+ {
+ if (noError)
+ break;
report_untranslatable_char(PG_UTF8, PG_LATIN1,
(const char *) src, len);
+ }
c1 = src[1] & 0x3f;
c = ((c & 0x1f) << 6) | c1;
if (c >= 0x80 && c <= 0xff)
diff --git a/src/backend/utils/mb/mbutils.c b/src/backend/utils/mb/mbutils.c
index 3e106027d75..af1cafe523a 100644
--- a/src/backend/utils/mb/mbutils.c
+++ b/src/backend/utils/mb/mbutils.c
@@ -818,12 +818,13 @@ perform_default_encoding_conversion(const char *src, int len,
MemoryContextAllocHuge(CurrentMemoryContext,
(Size) len * MAX_CONVERSION_GROWTH + 1);
- FunctionCall5(flinfo,
+ FunctionCall6(flinfo,
Int32GetDatum(src_encoding),
Int32GetDatum(dest_encoding),
CStringGetDatum(src),
CStringGetDatum(result),
- Int32GetDatum(len));
+ Int32GetDatum(len),
+ BoolGetDatum(false));
/*
* Release extra space if there might be a lot --- see comments in
--
2.29.2
--------------39E46EF15EBFAA37D53332E5
Content-Type: text/x-patch; charset=UTF-8;
name="v3-0005-Do-COPY-FROM-encoding-conversion-verification-in-.patch"
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment;
filename*0="v3-0005-Do-COPY-FROM-encoding-conversion-verification-in-.pa";
filename*1="tch"
^ permalink raw reply [nested|flat] 54+ messages in thread
* [PATCH v3 4/5] Fix bugs in the commit to change conversion function signature.
@ 2021-02-01 15:30 Heikki Linnakangas <[email protected]>
0 siblings, 0 replies; 54+ messages in thread
From: Heikki Linnakangas @ 2021-02-01 15:30 UTC (permalink / raw)
Per John Naylor's report, and some bugs I found while testing.
TODO: to be squashed.
---
src/backend/utils/mb/conv.c | 16 ++++++----------
.../utf8_and_iso8859_1/utf8_and_iso8859_1.c | 8 ++++++++
src/backend/utils/mb/mbutils.c | 5 +++--
3 files changed, 17 insertions(+), 12 deletions(-)
diff --git a/src/backend/utils/mb/conv.c b/src/backend/utils/mb/conv.c
index b83358bc7a5..f3bda5753d0 100644
--- a/src/backend/utils/mb/conv.c
+++ b/src/backend/utils/mb/conv.c
@@ -497,7 +497,6 @@ UtfToLocal(const unsigned char *utf, int len,
int l;
const pg_utf_to_local_combined *cp;
const unsigned char *start = utf;
- const unsigned char *cur = utf;
if (!PG_VALID_ENCODING(encoding))
ereport(ERROR,
@@ -511,8 +510,6 @@ UtfToLocal(const unsigned char *utf, int len,
unsigned char b3 = 0;
unsigned char b4 = 0;
- cur = iso;
-
/* "break" cases all represent errors */
if (*utf == '\0')
break;
@@ -648,10 +645,11 @@ UtfToLocal(const unsigned char *utf, int len,
}
/* failed to translate this character */
+ utf -= l;
if (noError)
break;
report_untranslatable_char(PG_UTF8, encoding,
- (const char *) (utf - l), len);
+ (const char *) utf, len);
}
/* if we broke out of loop early, must be invalid input */
@@ -660,7 +658,7 @@ UtfToLocal(const unsigned char *utf, int len,
*iso = '\0';
- return cur - start;
+ return utf - start;
}
/*
@@ -701,7 +699,6 @@ LocalToUtf(const unsigned char *iso, int len,
int l;
const pg_local_to_utf_combined *cp;
const unsigned char *start = iso;
- const unsigned char *cur = iso;
if (!PG_VALID_ENCODING(encoding))
ereport(ERROR,
@@ -715,8 +712,6 @@ LocalToUtf(const unsigned char *iso, int len,
unsigned char b3 = 0;
unsigned char b4 = 0;
- cur = iso;
-
/* "break" cases all represent errors */
if (*iso == '\0')
break;
@@ -799,10 +794,11 @@ LocalToUtf(const unsigned char *iso, int len,
}
/* failed to translate this character */
+ iso -= l;
if (noError)
break;
report_untranslatable_char(encoding, PG_UTF8,
- (const char *) (iso - l), len);
+ (const char *) iso, len);
}
/* if we broke out of loop early, must be invalid input */
@@ -811,5 +807,5 @@ LocalToUtf(const unsigned char *iso, int len,
*utf = '\0';
- return cur - start;
+ return iso - start;
}
diff --git a/src/backend/utils/mb/conversion_procs/utf8_and_iso8859_1/utf8_and_iso8859_1.c b/src/backend/utils/mb/conversion_procs/utf8_and_iso8859_1/utf8_and_iso8859_1.c
index 8ac93604a1b..d0dc4cca378 100644
--- a/src/backend/utils/mb/conversion_procs/utf8_and_iso8859_1/utf8_and_iso8859_1.c
+++ b/src/backend/utils/mb/conversion_procs/utf8_and_iso8859_1/utf8_and_iso8859_1.c
@@ -104,10 +104,18 @@ utf8_to_iso8859_1(PG_FUNCTION_ARGS)
int l = pg_utf_mblen(src);
if (l > len || !pg_utf8_islegal(src, l))
+ {
+ if (noError)
+ break;
report_invalid_encoding(PG_UTF8, (const char *) src, len);
+ }
if (l != 2)
+ {
+ if (noError)
+ break;
report_untranslatable_char(PG_UTF8, PG_LATIN1,
(const char *) src, len);
+ }
c1 = src[1] & 0x3f;
c = ((c & 0x1f) << 6) | c1;
if (c >= 0x80 && c <= 0xff)
diff --git a/src/backend/utils/mb/mbutils.c b/src/backend/utils/mb/mbutils.c
index 3e106027d75..af1cafe523a 100644
--- a/src/backend/utils/mb/mbutils.c
+++ b/src/backend/utils/mb/mbutils.c
@@ -818,12 +818,13 @@ perform_default_encoding_conversion(const char *src, int len,
MemoryContextAllocHuge(CurrentMemoryContext,
(Size) len * MAX_CONVERSION_GROWTH + 1);
- FunctionCall5(flinfo,
+ FunctionCall6(flinfo,
Int32GetDatum(src_encoding),
Int32GetDatum(dest_encoding),
CStringGetDatum(src),
CStringGetDatum(result),
- Int32GetDatum(len));
+ Int32GetDatum(len),
+ BoolGetDatum(false));
/*
* Release extra space if there might be a lot --- see comments in
--
2.29.2
--------------39E46EF15EBFAA37D53332E5
Content-Type: text/x-patch; charset=UTF-8;
name="v3-0005-Do-COPY-FROM-encoding-conversion-verification-in-.patch"
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment;
filename*0="v3-0005-Do-COPY-FROM-encoding-conversion-verification-in-.pa";
filename*1="tch"
^ permalink raw reply [nested|flat] 54+ messages in thread
* [PATCH v3 4/5] Fix bugs in the commit to change conversion function signature.
@ 2021-02-01 15:30 Heikki Linnakangas <[email protected]>
0 siblings, 0 replies; 54+ messages in thread
From: Heikki Linnakangas @ 2021-02-01 15:30 UTC (permalink / raw)
Per John Naylor's report, and some bugs I found while testing.
TODO: to be squashed.
---
src/backend/utils/mb/conv.c | 16 ++++++----------
.../utf8_and_iso8859_1/utf8_and_iso8859_1.c | 8 ++++++++
src/backend/utils/mb/mbutils.c | 5 +++--
3 files changed, 17 insertions(+), 12 deletions(-)
diff --git a/src/backend/utils/mb/conv.c b/src/backend/utils/mb/conv.c
index b83358bc7a5..f3bda5753d0 100644
--- a/src/backend/utils/mb/conv.c
+++ b/src/backend/utils/mb/conv.c
@@ -497,7 +497,6 @@ UtfToLocal(const unsigned char *utf, int len,
int l;
const pg_utf_to_local_combined *cp;
const unsigned char *start = utf;
- const unsigned char *cur = utf;
if (!PG_VALID_ENCODING(encoding))
ereport(ERROR,
@@ -511,8 +510,6 @@ UtfToLocal(const unsigned char *utf, int len,
unsigned char b3 = 0;
unsigned char b4 = 0;
- cur = iso;
-
/* "break" cases all represent errors */
if (*utf == '\0')
break;
@@ -648,10 +645,11 @@ UtfToLocal(const unsigned char *utf, int len,
}
/* failed to translate this character */
+ utf -= l;
if (noError)
break;
report_untranslatable_char(PG_UTF8, encoding,
- (const char *) (utf - l), len);
+ (const char *) utf, len);
}
/* if we broke out of loop early, must be invalid input */
@@ -660,7 +658,7 @@ UtfToLocal(const unsigned char *utf, int len,
*iso = '\0';
- return cur - start;
+ return utf - start;
}
/*
@@ -701,7 +699,6 @@ LocalToUtf(const unsigned char *iso, int len,
int l;
const pg_local_to_utf_combined *cp;
const unsigned char *start = iso;
- const unsigned char *cur = iso;
if (!PG_VALID_ENCODING(encoding))
ereport(ERROR,
@@ -715,8 +712,6 @@ LocalToUtf(const unsigned char *iso, int len,
unsigned char b3 = 0;
unsigned char b4 = 0;
- cur = iso;
-
/* "break" cases all represent errors */
if (*iso == '\0')
break;
@@ -799,10 +794,11 @@ LocalToUtf(const unsigned char *iso, int len,
}
/* failed to translate this character */
+ iso -= l;
if (noError)
break;
report_untranslatable_char(encoding, PG_UTF8,
- (const char *) (iso - l), len);
+ (const char *) iso, len);
}
/* if we broke out of loop early, must be invalid input */
@@ -811,5 +807,5 @@ LocalToUtf(const unsigned char *iso, int len,
*utf = '\0';
- return cur - start;
+ return iso - start;
}
diff --git a/src/backend/utils/mb/conversion_procs/utf8_and_iso8859_1/utf8_and_iso8859_1.c b/src/backend/utils/mb/conversion_procs/utf8_and_iso8859_1/utf8_and_iso8859_1.c
index 8ac93604a1b..d0dc4cca378 100644
--- a/src/backend/utils/mb/conversion_procs/utf8_and_iso8859_1/utf8_and_iso8859_1.c
+++ b/src/backend/utils/mb/conversion_procs/utf8_and_iso8859_1/utf8_and_iso8859_1.c
@@ -104,10 +104,18 @@ utf8_to_iso8859_1(PG_FUNCTION_ARGS)
int l = pg_utf_mblen(src);
if (l > len || !pg_utf8_islegal(src, l))
+ {
+ if (noError)
+ break;
report_invalid_encoding(PG_UTF8, (const char *) src, len);
+ }
if (l != 2)
+ {
+ if (noError)
+ break;
report_untranslatable_char(PG_UTF8, PG_LATIN1,
(const char *) src, len);
+ }
c1 = src[1] & 0x3f;
c = ((c & 0x1f) << 6) | c1;
if (c >= 0x80 && c <= 0xff)
diff --git a/src/backend/utils/mb/mbutils.c b/src/backend/utils/mb/mbutils.c
index 3e106027d75..af1cafe523a 100644
--- a/src/backend/utils/mb/mbutils.c
+++ b/src/backend/utils/mb/mbutils.c
@@ -818,12 +818,13 @@ perform_default_encoding_conversion(const char *src, int len,
MemoryContextAllocHuge(CurrentMemoryContext,
(Size) len * MAX_CONVERSION_GROWTH + 1);
- FunctionCall5(flinfo,
+ FunctionCall6(flinfo,
Int32GetDatum(src_encoding),
Int32GetDatum(dest_encoding),
CStringGetDatum(src),
CStringGetDatum(result),
- Int32GetDatum(len));
+ Int32GetDatum(len),
+ BoolGetDatum(false));
/*
* Release extra space if there might be a lot --- see comments in
--
2.29.2
--------------39E46EF15EBFAA37D53332E5
Content-Type: text/x-patch; charset=UTF-8;
name="v3-0005-Do-COPY-FROM-encoding-conversion-verification-in-.patch"
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment;
filename*0="v3-0005-Do-COPY-FROM-encoding-conversion-verification-in-.pa";
filename*1="tch"
^ permalink raw reply [nested|flat] 54+ messages in thread
* [PATCH v3 4/5] Fix bugs in the commit to change conversion function signature.
@ 2021-02-01 15:30 Heikki Linnakangas <[email protected]>
0 siblings, 0 replies; 54+ messages in thread
From: Heikki Linnakangas @ 2021-02-01 15:30 UTC (permalink / raw)
Per John Naylor's report, and some bugs I found while testing.
TODO: to be squashed.
---
src/backend/utils/mb/conv.c | 16 ++++++----------
.../utf8_and_iso8859_1/utf8_and_iso8859_1.c | 8 ++++++++
src/backend/utils/mb/mbutils.c | 5 +++--
3 files changed, 17 insertions(+), 12 deletions(-)
diff --git a/src/backend/utils/mb/conv.c b/src/backend/utils/mb/conv.c
index b83358bc7a5..f3bda5753d0 100644
--- a/src/backend/utils/mb/conv.c
+++ b/src/backend/utils/mb/conv.c
@@ -497,7 +497,6 @@ UtfToLocal(const unsigned char *utf, int len,
int l;
const pg_utf_to_local_combined *cp;
const unsigned char *start = utf;
- const unsigned char *cur = utf;
if (!PG_VALID_ENCODING(encoding))
ereport(ERROR,
@@ -511,8 +510,6 @@ UtfToLocal(const unsigned char *utf, int len,
unsigned char b3 = 0;
unsigned char b4 = 0;
- cur = iso;
-
/* "break" cases all represent errors */
if (*utf == '\0')
break;
@@ -648,10 +645,11 @@ UtfToLocal(const unsigned char *utf, int len,
}
/* failed to translate this character */
+ utf -= l;
if (noError)
break;
report_untranslatable_char(PG_UTF8, encoding,
- (const char *) (utf - l), len);
+ (const char *) utf, len);
}
/* if we broke out of loop early, must be invalid input */
@@ -660,7 +658,7 @@ UtfToLocal(const unsigned char *utf, int len,
*iso = '\0';
- return cur - start;
+ return utf - start;
}
/*
@@ -701,7 +699,6 @@ LocalToUtf(const unsigned char *iso, int len,
int l;
const pg_local_to_utf_combined *cp;
const unsigned char *start = iso;
- const unsigned char *cur = iso;
if (!PG_VALID_ENCODING(encoding))
ereport(ERROR,
@@ -715,8 +712,6 @@ LocalToUtf(const unsigned char *iso, int len,
unsigned char b3 = 0;
unsigned char b4 = 0;
- cur = iso;
-
/* "break" cases all represent errors */
if (*iso == '\0')
break;
@@ -799,10 +794,11 @@ LocalToUtf(const unsigned char *iso, int len,
}
/* failed to translate this character */
+ iso -= l;
if (noError)
break;
report_untranslatable_char(encoding, PG_UTF8,
- (const char *) (iso - l), len);
+ (const char *) iso, len);
}
/* if we broke out of loop early, must be invalid input */
@@ -811,5 +807,5 @@ LocalToUtf(const unsigned char *iso, int len,
*utf = '\0';
- return cur - start;
+ return iso - start;
}
diff --git a/src/backend/utils/mb/conversion_procs/utf8_and_iso8859_1/utf8_and_iso8859_1.c b/src/backend/utils/mb/conversion_procs/utf8_and_iso8859_1/utf8_and_iso8859_1.c
index 8ac93604a1b..d0dc4cca378 100644
--- a/src/backend/utils/mb/conversion_procs/utf8_and_iso8859_1/utf8_and_iso8859_1.c
+++ b/src/backend/utils/mb/conversion_procs/utf8_and_iso8859_1/utf8_and_iso8859_1.c
@@ -104,10 +104,18 @@ utf8_to_iso8859_1(PG_FUNCTION_ARGS)
int l = pg_utf_mblen(src);
if (l > len || !pg_utf8_islegal(src, l))
+ {
+ if (noError)
+ break;
report_invalid_encoding(PG_UTF8, (const char *) src, len);
+ }
if (l != 2)
+ {
+ if (noError)
+ break;
report_untranslatable_char(PG_UTF8, PG_LATIN1,
(const char *) src, len);
+ }
c1 = src[1] & 0x3f;
c = ((c & 0x1f) << 6) | c1;
if (c >= 0x80 && c <= 0xff)
diff --git a/src/backend/utils/mb/mbutils.c b/src/backend/utils/mb/mbutils.c
index 3e106027d75..af1cafe523a 100644
--- a/src/backend/utils/mb/mbutils.c
+++ b/src/backend/utils/mb/mbutils.c
@@ -818,12 +818,13 @@ perform_default_encoding_conversion(const char *src, int len,
MemoryContextAllocHuge(CurrentMemoryContext,
(Size) len * MAX_CONVERSION_GROWTH + 1);
- FunctionCall5(flinfo,
+ FunctionCall6(flinfo,
Int32GetDatum(src_encoding),
Int32GetDatum(dest_encoding),
CStringGetDatum(src),
CStringGetDatum(result),
- Int32GetDatum(len));
+ Int32GetDatum(len),
+ BoolGetDatum(false));
/*
* Release extra space if there might be a lot --- see comments in
--
2.29.2
--------------39E46EF15EBFAA37D53332E5
Content-Type: text/x-patch; charset=UTF-8;
name="v3-0005-Do-COPY-FROM-encoding-conversion-verification-in-.patch"
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment;
filename*0="v3-0005-Do-COPY-FROM-encoding-conversion-verification-in-.pa";
filename*1="tch"
^ permalink raw reply [nested|flat] 54+ messages in thread
* [PATCH v3 4/5] Fix bugs in the commit to change conversion function signature.
@ 2021-02-01 15:30 Heikki Linnakangas <[email protected]>
0 siblings, 0 replies; 54+ messages in thread
From: Heikki Linnakangas @ 2021-02-01 15:30 UTC (permalink / raw)
Per John Naylor's report, and some bugs I found while testing.
TODO: to be squashed.
---
src/backend/utils/mb/conv.c | 16 ++++++----------
.../utf8_and_iso8859_1/utf8_and_iso8859_1.c | 8 ++++++++
src/backend/utils/mb/mbutils.c | 5 +++--
3 files changed, 17 insertions(+), 12 deletions(-)
diff --git a/src/backend/utils/mb/conv.c b/src/backend/utils/mb/conv.c
index b83358bc7a5..f3bda5753d0 100644
--- a/src/backend/utils/mb/conv.c
+++ b/src/backend/utils/mb/conv.c
@@ -497,7 +497,6 @@ UtfToLocal(const unsigned char *utf, int len,
int l;
const pg_utf_to_local_combined *cp;
const unsigned char *start = utf;
- const unsigned char *cur = utf;
if (!PG_VALID_ENCODING(encoding))
ereport(ERROR,
@@ -511,8 +510,6 @@ UtfToLocal(const unsigned char *utf, int len,
unsigned char b3 = 0;
unsigned char b4 = 0;
- cur = iso;
-
/* "break" cases all represent errors */
if (*utf == '\0')
break;
@@ -648,10 +645,11 @@ UtfToLocal(const unsigned char *utf, int len,
}
/* failed to translate this character */
+ utf -= l;
if (noError)
break;
report_untranslatable_char(PG_UTF8, encoding,
- (const char *) (utf - l), len);
+ (const char *) utf, len);
}
/* if we broke out of loop early, must be invalid input */
@@ -660,7 +658,7 @@ UtfToLocal(const unsigned char *utf, int len,
*iso = '\0';
- return cur - start;
+ return utf - start;
}
/*
@@ -701,7 +699,6 @@ LocalToUtf(const unsigned char *iso, int len,
int l;
const pg_local_to_utf_combined *cp;
const unsigned char *start = iso;
- const unsigned char *cur = iso;
if (!PG_VALID_ENCODING(encoding))
ereport(ERROR,
@@ -715,8 +712,6 @@ LocalToUtf(const unsigned char *iso, int len,
unsigned char b3 = 0;
unsigned char b4 = 0;
- cur = iso;
-
/* "break" cases all represent errors */
if (*iso == '\0')
break;
@@ -799,10 +794,11 @@ LocalToUtf(const unsigned char *iso, int len,
}
/* failed to translate this character */
+ iso -= l;
if (noError)
break;
report_untranslatable_char(encoding, PG_UTF8,
- (const char *) (iso - l), len);
+ (const char *) iso, len);
}
/* if we broke out of loop early, must be invalid input */
@@ -811,5 +807,5 @@ LocalToUtf(const unsigned char *iso, int len,
*utf = '\0';
- return cur - start;
+ return iso - start;
}
diff --git a/src/backend/utils/mb/conversion_procs/utf8_and_iso8859_1/utf8_and_iso8859_1.c b/src/backend/utils/mb/conversion_procs/utf8_and_iso8859_1/utf8_and_iso8859_1.c
index 8ac93604a1b..d0dc4cca378 100644
--- a/src/backend/utils/mb/conversion_procs/utf8_and_iso8859_1/utf8_and_iso8859_1.c
+++ b/src/backend/utils/mb/conversion_procs/utf8_and_iso8859_1/utf8_and_iso8859_1.c
@@ -104,10 +104,18 @@ utf8_to_iso8859_1(PG_FUNCTION_ARGS)
int l = pg_utf_mblen(src);
if (l > len || !pg_utf8_islegal(src, l))
+ {
+ if (noError)
+ break;
report_invalid_encoding(PG_UTF8, (const char *) src, len);
+ }
if (l != 2)
+ {
+ if (noError)
+ break;
report_untranslatable_char(PG_UTF8, PG_LATIN1,
(const char *) src, len);
+ }
c1 = src[1] & 0x3f;
c = ((c & 0x1f) << 6) | c1;
if (c >= 0x80 && c <= 0xff)
diff --git a/src/backend/utils/mb/mbutils.c b/src/backend/utils/mb/mbutils.c
index 3e106027d75..af1cafe523a 100644
--- a/src/backend/utils/mb/mbutils.c
+++ b/src/backend/utils/mb/mbutils.c
@@ -818,12 +818,13 @@ perform_default_encoding_conversion(const char *src, int len,
MemoryContextAllocHuge(CurrentMemoryContext,
(Size) len * MAX_CONVERSION_GROWTH + 1);
- FunctionCall5(flinfo,
+ FunctionCall6(flinfo,
Int32GetDatum(src_encoding),
Int32GetDatum(dest_encoding),
CStringGetDatum(src),
CStringGetDatum(result),
- Int32GetDatum(len));
+ Int32GetDatum(len),
+ BoolGetDatum(false));
/*
* Release extra space if there might be a lot --- see comments in
--
2.29.2
--------------39E46EF15EBFAA37D53332E5
Content-Type: text/x-patch; charset=UTF-8;
name="v3-0005-Do-COPY-FROM-encoding-conversion-verification-in-.patch"
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment;
filename*0="v3-0005-Do-COPY-FROM-encoding-conversion-verification-in-.pa";
filename*1="tch"
^ permalink raw reply [nested|flat] 54+ messages in thread
* Re: Simplify if/else logic of walsender CreateReplicationSlot
@ 2023-11-20 08:07 Michael Paquier <[email protected]>
0 siblings, 0 replies; 54+ messages in thread
From: Michael Paquier @ 2023-11-20 08:07 UTC (permalink / raw)
To: Peter Smith <[email protected]>; +Cc: PostgreSQL Hackers <[email protected]>
On Mon, Nov 20, 2023 at 06:01:42PM +1100, Peter Smith wrote:
> While reviewing another patch I was looking at the walsender's static
> function CreateReplicationSlot
>
> I found that the current logic seemed to have some unnecessary if/else
> checking which can be simplified.
Good idea. What you are suggesting here improves the readability of
this code, so +1.
--
Michael
Attachments:
[application/pgp-signature] signature.asc (833B, ../../[email protected]/2-signature.asc)
download
^ permalink raw reply [nested|flat] 54+ messages in thread
end of thread, other threads:[~2023-11-20 08:07 UTC | newest]
Thread overview: 54+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2017-07-20 03:33 [PATCH] pageinspect function to decode infomasks Craig Ringer <[email protected]>
2017-07-20 03:38 ` Craig Ringer <[email protected]>
2017-07-20 03:44 ` Peter Geoghegan <[email protected]>
2017-07-20 06:13 ` Julien Rouhaud <[email protected]>
2017-07-20 06:26 ` Masahiko Sawada <[email protected]>
2017-07-20 11:09 ` Ashutosh Sharma <[email protected]>
2017-07-20 11:52 ` Craig Ringer <[email protected]>
2017-08-15 01:11 ` Moon Insung <[email protected]>
2017-08-15 01:59 ` Craig Ringer <[email protected]>
2017-08-15 13:24 ` Robert Haas <[email protected]>
2017-08-15 13:59 ` Tomas Vondra <[email protected]>
2017-08-15 14:12 ` Masahiko Sawada <[email protected]>
2017-08-15 17:54 ` Robert Haas <[email protected]>
2017-08-15 19:42 ` Tomas Vondra <[email protected]>
2017-08-15 19:55 ` Robert Haas <[email protected]>
2017-08-15 20:36 ` Tomas Vondra <[email protected]>
2017-08-16 01:33 ` Moon Insung <[email protected]>
2017-08-16 15:14 ` Robert Haas <[email protected]>
2017-08-17 00:20 ` Craig Ringer <[email protected]>
2017-09-14 11:57 ` Ashutosh Sharma <[email protected]>
2017-09-14 14:00 ` Craig Ringer <[email protected]>
2017-10-17 07:03 ` Masahiko Sawada <[email protected]>
2017-08-16 01:37 ` Craig Ringer <[email protected]>
2017-10-12 23:35 ` Peter Geoghegan <[email protected]>
2017-10-13 20:02 ` Robert Haas <[email protected]>
2017-10-13 20:36 ` Peter Geoghegan <[email protected]>
2017-10-14 17:58 ` Robert Haas <[email protected]>
2017-10-14 21:47 ` Peter Geoghegan <[email protected]>
2017-10-15 13:28 ` Vik Fearing <[email protected]>
2017-10-17 00:20 ` Peter Geoghegan <[email protected]>
2017-07-21 03:29 ` Craig Ringer <[email protected]>
2021-02-01 15:30 [PATCH v3 4/5] Fix bugs in the commit to change conversion function signature. Heikki Linnakangas <[email protected]>
2021-02-01 15:30 [PATCH v3 4/5] Fix bugs in the commit to change conversion function signature. Heikki Linnakangas <[email protected]>
2021-02-01 15:30 [PATCH v3 4/5] Fix bugs in the commit to change conversion function signature. Heikki Linnakangas <[email protected]>
2021-02-01 15:30 [PATCH v3 4/5] Fix bugs in the commit to change conversion function signature. Heikki Linnakangas <[email protected]>
2021-02-01 15:30 [PATCH v3 4/5] Fix bugs in the commit to change conversion function signature. Heikki Linnakangas <[email protected]>
2021-02-01 15:30 [PATCH v3 4/5] Fix bugs in the commit to change conversion function signature. Heikki Linnakangas <[email protected]>
2021-02-01 15:30 [PATCH v3 4/5] Fix bugs in the commit to change conversion function signature. Heikki Linnakangas <[email protected]>
2021-02-01 15:30 [PATCH v3 4/5] Fix bugs in the commit to change conversion function signature. Heikki Linnakangas <[email protected]>
2021-02-01 15:30 [PATCH v3 4/5] Fix bugs in the commit to change conversion function signature. Heikki Linnakangas <[email protected]>
2021-02-01 15:30 [PATCH v3 4/5] Fix bugs in the commit to change conversion function signature. Heikki Linnakangas <[email protected]>
2021-02-01 15:30 [PATCH v3 4/5] Fix bugs in the commit to change conversion function signature. Heikki Linnakangas <[email protected]>
2021-02-01 15:30 [PATCH v3 4/5] Fix bugs in the commit to change conversion function signature. Heikki Linnakangas <[email protected]>
2021-02-01 15:30 [PATCH v3 4/5] Fix bugs in the commit to change conversion function signature. Heikki Linnakangas <[email protected]>
2021-02-01 15:30 [PATCH v3 4/5] Fix bugs in the commit to change conversion function signature. Heikki Linnakangas <[email protected]>
2021-02-01 15:30 [PATCH v3 4/5] Fix bugs in the commit to change conversion function signature. Heikki Linnakangas <[email protected]>
2021-02-01 15:30 [PATCH v3 4/5] Fix bugs in the commit to change conversion function signature. Heikki Linnakangas <[email protected]>
2021-02-01 15:30 [PATCH v3 4/5] Fix bugs in the commit to change conversion function signature. Heikki Linnakangas <[email protected]>
2021-02-01 15:30 [PATCH v3 4/5] Fix bugs in the commit to change conversion function signature. Heikki Linnakangas <[email protected]>
2021-02-01 15:30 [PATCH v3 4/5] Fix bugs in the commit to change conversion function signature. Heikki Linnakangas <[email protected]>
2021-02-01 15:30 [PATCH v3 4/5] Fix bugs in the commit to change conversion function signature. Heikki Linnakangas <[email protected]>
2021-02-01 15:30 [PATCH v3 4/5] Fix bugs in the commit to change conversion function signature. Heikki Linnakangas <[email protected]>
2021-02-01 15:30 [PATCH v3 4/5] Fix bugs in the commit to change conversion function signature. Heikki Linnakangas <[email protected]>
2023-11-20 08:07 Re: Simplify if/else logic of walsender CreateReplicationSlot Michael Paquier <[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