agora inbox for [email protected]help / color / mirror / Atom feed
Re: Range Merge Join v1 30+ messages / 5 participants [nested] [flat]
* Re: Range Merge Join v1 @ 2017-05-31 06:44 Andrew Borodin <[email protected]> 0 siblings, 1 reply; 30+ messages in thread From: Andrew Borodin @ 2017-05-31 06:44 UTC (permalink / raw) To: Andrew Borodin <[email protected]>; +Cc: Jeff Davis <[email protected]>; pgsql-hackers Hi, Jeff! Sorry for being late. Actually, I had several unsuccessful attempts to find something wrong with the patch. Here's my review. in pathkey.c ecs = (EquivalenceClass **) palloc(nClauses * sizeof(EquivalenceClass *)); scores = (int *) palloc(nClauses * sizeof(int)); range_ecs = palloc(nClauses * sizeof(bool)); Third assignment has no cast. And I have few questions: 1. Are there any types, which could benefit from Range Merge and are not covered by this patch? 2. Can Range Merge handle merge of different ranges? Like int4range() && int8range() ? My perf test script from the previous message was broken, here's fixed one in the attachment. This patch implements feature, contains new tests and passes old tests, is documented and spec compliant. I do not see any reason why not mark it "Ready for committer". Best regards, Andrey Borodin. \timing create table r as select int4range(g, g+10) ir, g g from generate_series(1,1000000) g order by random(); create index r_idx on r using gist (ir); create table s as select int4range(g+5, g+15) ir,g g from generate_series(1,1000000) g order by random(); create index s_idx on s using gist (ir); vacuum analyze r; vacuum analyze s; --baseline performance using GiST set enable_mergejoin=false; explain analyze select * from r, s where r.ir && s.ir; explain analyze select * from r, s where r.ir && s.ir; --performance without GiST set enable_mergejoin=true; explain analyze select * from r, s where r.ir && s.ir; explain analyze select * from r, s where r.ir && s.ir; --performance in presence of expression index create index r_idx1 on r(int4range(r.g, r.g+10)); create index s_idx1 on s(int4range(s.g+5, s.g+15)); explain analyze select * from r, s where int4range(r.g, r.g+10) && int4range(s.g+5, s.g+15); explain analyze select * from r, s where int4range(r.g, r.g+10) && int4range(s.g+5, s.g+15); --performance in precence of direct B-tree index create index r_idx2 on r(ir); create index s_idx2 on s(ir); explain analyze select * from r, s where r.ir && s.ir; explain analyze select * from r, s where r.ir && s.ir; drop table r; drop table s; --here we test that performance is not affected by payload of other attributes in heap tuples create table r as select int4range(g, g+10) ir, g g, 1::float pl1,1::float pl2,1::float pl3,1::float pl4,1::float pl5,1::float pl6,1::float pl7,1::float pl8,1::float pl9,1::float pl0 from generate_series(1,1000000) g order by random(); create table s as select int4range(g+5, g+15) ir,g g, 1::float pl1,1::float pl2,1::float pl3,1::float pl4,1::float pl5,1::float pl6,1::float pl7,1::float pl8,1::float pl9,1::float pl0 from generate_series(1,1000000) g order by random(); explain analyze select r.ir,s.ir from r, s where r.ir && s.ir; explain analyze select r.ir,s.ir from r, s where r.ir && s.ir; drop table r; drop table s; -- Sent via pgsql-hackers mailing list ([email protected]) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers Attachments: [text/plain] perf2_fixed.sql (2.0K, ../../CAJEAwVEvjgRG4AJspJo2_nm267HUacYwJ3-7stHDqt0+ry5oYQ@mail.gmail.com/2-perf2_fixed.sql) download | inline: \timing create table r as select int4range(g, g+10) ir, g g from generate_series(1,1000000) g order by random(); create index r_idx on r using gist (ir); create table s as select int4range(g+5, g+15) ir,g g from generate_series(1,1000000) g order by random(); create index s_idx on s using gist (ir); vacuum analyze r; vacuum analyze s; --baseline performance using GiST set enable_mergejoin=false; explain analyze select * from r, s where r.ir && s.ir; explain analyze select * from r, s where r.ir && s.ir; --performance without GiST set enable_mergejoin=true; explain analyze select * from r, s where r.ir && s.ir; explain analyze select * from r, s where r.ir && s.ir; --performance in presence of expression index create index r_idx1 on r(int4range(r.g, r.g+10)); create index s_idx1 on s(int4range(s.g+5, s.g+15)); explain analyze select * from r, s where int4range(r.g, r.g+10) && int4range(s.g+5, s.g+15); explain analyze select * from r, s where int4range(r.g, r.g+10) && int4range(s.g+5, s.g+15); --performance in precence of direct B-tree index create index r_idx2 on r(ir); create index s_idx2 on s(ir); explain analyze select * from r, s where r.ir && s.ir; explain analyze select * from r, s where r.ir && s.ir; drop table r; drop table s; --here we test that performance is not affected by payload of other attributes in heap tuples create table r as select int4range(g, g+10) ir, g g, 1::float pl1,1::float pl2,1::float pl3,1::float pl4,1::float pl5,1::float pl6,1::float pl7,1::float pl8,1::float pl9,1::float pl0 from generate_series(1,1000000) g order by random(); create table s as select int4range(g+5, g+15) ir,g g, 1::float pl1,1::float pl2,1::float pl3,1::float pl4,1::float pl5,1::float pl6,1::float pl7,1::float pl8,1::float pl9,1::float pl0 from generate_series(1,1000000) g order by random(); explain analyze select r.ir,s.ir from r, s where r.ir && s.ir; explain analyze select r.ir,s.ir from r, s where r.ir && s.ir; drop table r; drop table s; ^ permalink raw reply [nested|flat] 30+ messages in thread
* Re: Range Merge Join v1 @ 2017-06-02 14:42 Jeff Davis <[email protected]> parent: Andrew Borodin <[email protected]> 0 siblings, 1 reply; 30+ messages in thread From: Jeff Davis @ 2017-06-02 14:42 UTC (permalink / raw) To: Andrew Borodin <[email protected]>; +Cc: pgsql-hackers On Tue, May 30, 2017 at 11:44 PM, Andrew Borodin <[email protected]> wrote: > Hi, Jeff! Hi! > Sorry for being late. Actually, I had several unsuccessful attempts to > find something wrong with the patch. > Here's my review. > > in pathkey.c > > ecs = (EquivalenceClass **) palloc(nClauses * sizeof(EquivalenceClass *)); > scores = (int *) palloc(nClauses * sizeof(int)); > range_ecs = palloc(nClauses * sizeof(bool)); > > Third assignment has no cast. Will fix. > And I have few questions: > 1. Are there any types, which could benefit from Range Merge and are > not covered by this patch? I thought about this for a while, and the only thing I can think of are range joins that don't explicitly use range types. > 2. Can Range Merge handle merge of different ranges? Like int4range() > && int8range() ? Right now, there aren't even casts between range types. I think the best way to handle that at this point would be to add casts among the numeric ranges. There may be advantages to supporting any two ranges where the contained types are part of the same opfamily, but it seems a little early to add that complication. > My perf test script from the previous message was broken, here's fixed > one in the attachment. > > This patch implements feature, contains new tests and passes old > tests, is documented and spec compliant. I do not see any reason why > not mark it "Ready for committer". Great! I think there are a couple more things that could be done if we want to. Let me know if you think these things should be done now, or if they should be a separate patch later when the need arises: * Support for r1 @> r2 joins (join on "contains" rather than "overlaps"). * Better integration with the catalog so that users could add their own types that support range merge join. Thank you for the review. Regards, Jeff Davis -- 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] 30+ messages in thread
* Re: Range Merge Join v1 @ 2017-06-03 06:26 Andrew Borodin <[email protected]> parent: Jeff Davis <[email protected]> 0 siblings, 0 replies; 30+ messages in thread From: Andrew Borodin @ 2017-06-03 06:26 UTC (permalink / raw) To: Jeff Davis <[email protected]>; +Cc: pgsql-hackers 2017-06-02 19:42 GMT+05:00 Jeff Davis <[email protected]>: > On Tue, May 30, 2017 at 11:44 PM, Andrew Borodin <[email protected]> wrote: >> 1. Are there any types, which could benefit from Range Merge and are >> not covered by this patch? > > I thought about this for a while, and the only thing I can think of > are range joins that don't explicitly use range types. Let me try to write && in SQL select * from a join b where (a.min<=b.max and a.min>=b.min) or (a.max<=b.max and a.max>=b.min); Quite complicated. Here user knows that min <= max, but DB don't. If we write it precisely we get hell lot of permutations. Here's what I think: 1. For me, this feature seems hard to implement. 2. This feature also will be hard to commit solely, since it's use case will be rare. 3. If this could yield unexpected performance for queries like select * from t where x<y and b>c or a!=z or [other conditions] and optimizer could think: "Aha! I'll sort it and do it fast" it'd be cool. I do not think range joins that don't explicitly use range types are possible right now... >> 2. Can Range Merge handle merge of different ranges? Like int4range() >> && int8range() ? > > Right now, there aren't even casts between range types. I think the > best way to handle that at this point would be to add casts among the > numeric ranges. There may be advantages to supporting any two ranges > where the contained types are part of the same opfamily, but it seems > a little early to add that complication. Agree. > I think there are a couple more things that could be done if we want > to. Let me know if you think these things should be done now, or if > they should be a separate patch later when the need arises: > > * Support for r1 @> r2 joins (join on "contains" rather than "overlaps"). > * Better integration with the catalog so that users could add their > own types that support range merge join. 1. I believe this changes can be incremental. They constitute value for the end user, then they are committable. This patch is not overly complicated, but it's easier to do this stuff in small pieces. 2. The commitfest is 3 months away. If you will have new versions of the patch, I'll review again. Maybe will spot some new things :) Best regards, Andrey Borodin, Octonica. -- 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] 30+ messages in thread
* [PATCH v6 07/10] is vs are plural @ 2019-04-04 23:57 Justin Pryzby <[email protected]> 0 siblings, 0 replies; 30+ messages in thread From: Justin Pryzby @ 2019-04-04 23:57 UTC (permalink / raw) Should backpatch to v12 --- doc/src/sgml/ref/alter_table.sgml | 2 +- doc/src/sgml/sources.sgml | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/doc/src/sgml/ref/alter_table.sgml b/doc/src/sgml/ref/alter_table.sgml index b2eb7097a9..fa848e0bdf 100644 --- a/doc/src/sgml/ref/alter_table.sgml +++ b/doc/src/sgml/ref/alter_table.sgml @@ -889,7 +889,7 @@ WITH ( MODULUS <replaceable class="parameter">numeric_literal</replaceable>, REM from the parent table will be created in the partition, if they don't already exist. If any of the <literal>CHECK</literal> constraints of the table being - attached is marked <literal>NO INHERIT</literal>, the command will fail; + attached are marked <literal>NO INHERIT</literal>, the command will fail; such constraints must be recreated without the <literal>NO INHERIT</literal> clause. </para> diff --git a/doc/src/sgml/sources.sgml b/doc/src/sgml/sources.sgml index 283c3e0357..12704c6fdf 100644 --- a/doc/src/sgml/sources.sgml +++ b/doc/src/sgml/sources.sgml @@ -466,8 +466,8 @@ Hint: the addendum enough for error messages. Detail and hint messages can be relegated to a verbose mode, or perhaps a pop-up error-details window. Also, details and hints would normally be suppressed from the server log to save - space. Reference to implementation details is best avoided since users - aren't expected to know the details. + space. References to implementation details are best avoided since users + aren't expected to know them. </para> </simplesect> -- 2.17.0 --C+ts3FVlLX8+P6JN Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v6-0008-doc-backup-manifests.patch" ^ permalink raw reply [nested|flat] 30+ messages in thread
* [PATCH v6 07/10] is vs are plural @ 2019-04-04 23:57 Justin Pryzby <[email protected]> 0 siblings, 0 replies; 30+ messages in thread From: Justin Pryzby @ 2019-04-04 23:57 UTC (permalink / raw) Should backpatch to v12 --- doc/src/sgml/ref/alter_table.sgml | 2 +- doc/src/sgml/sources.sgml | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/doc/src/sgml/ref/alter_table.sgml b/doc/src/sgml/ref/alter_table.sgml index b2eb7097a9..fa848e0bdf 100644 --- a/doc/src/sgml/ref/alter_table.sgml +++ b/doc/src/sgml/ref/alter_table.sgml @@ -889,7 +889,7 @@ WITH ( MODULUS <replaceable class="parameter">numeric_literal</replaceable>, REM from the parent table will be created in the partition, if they don't already exist. If any of the <literal>CHECK</literal> constraints of the table being - attached is marked <literal>NO INHERIT</literal>, the command will fail; + attached are marked <literal>NO INHERIT</literal>, the command will fail; such constraints must be recreated without the <literal>NO INHERIT</literal> clause. </para> diff --git a/doc/src/sgml/sources.sgml b/doc/src/sgml/sources.sgml index 283c3e0357..12704c6fdf 100644 --- a/doc/src/sgml/sources.sgml +++ b/doc/src/sgml/sources.sgml @@ -466,8 +466,8 @@ Hint: the addendum enough for error messages. Detail and hint messages can be relegated to a verbose mode, or perhaps a pop-up error-details window. Also, details and hints would normally be suppressed from the server log to save - space. Reference to implementation details is best avoided since users - aren't expected to know the details. + space. References to implementation details are best avoided since users + aren't expected to know them. </para> </simplesect> -- 2.17.0 --C+ts3FVlLX8+P6JN Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v6-0008-doc-backup-manifests.patch" ^ permalink raw reply [nested|flat] 30+ messages in thread
* [PATCH v6 07/10] is vs are plural @ 2019-04-04 23:57 Justin Pryzby <[email protected]> 0 siblings, 0 replies; 30+ messages in thread From: Justin Pryzby @ 2019-04-04 23:57 UTC (permalink / raw) Should backpatch to v12 --- doc/src/sgml/ref/alter_table.sgml | 2 +- doc/src/sgml/sources.sgml | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/doc/src/sgml/ref/alter_table.sgml b/doc/src/sgml/ref/alter_table.sgml index b2eb7097a9..fa848e0bdf 100644 --- a/doc/src/sgml/ref/alter_table.sgml +++ b/doc/src/sgml/ref/alter_table.sgml @@ -889,7 +889,7 @@ WITH ( MODULUS <replaceable class="parameter">numeric_literal</replaceable>, REM from the parent table will be created in the partition, if they don't already exist. If any of the <literal>CHECK</literal> constraints of the table being - attached is marked <literal>NO INHERIT</literal>, the command will fail; + attached are marked <literal>NO INHERIT</literal>, the command will fail; such constraints must be recreated without the <literal>NO INHERIT</literal> clause. </para> diff --git a/doc/src/sgml/sources.sgml b/doc/src/sgml/sources.sgml index 283c3e0357..12704c6fdf 100644 --- a/doc/src/sgml/sources.sgml +++ b/doc/src/sgml/sources.sgml @@ -466,8 +466,8 @@ Hint: the addendum enough for error messages. Detail and hint messages can be relegated to a verbose mode, or perhaps a pop-up error-details window. Also, details and hints would normally be suppressed from the server log to save - space. Reference to implementation details is best avoided since users - aren't expected to know the details. + space. References to implementation details are best avoided since users + aren't expected to know them. </para> </simplesect> -- 2.17.0 --C+ts3FVlLX8+P6JN Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v6-0008-doc-backup-manifests.patch" ^ permalink raw reply [nested|flat] 30+ messages in thread
* [PATCH v6 07/10] is vs are plural @ 2019-04-04 23:57 Justin Pryzby <[email protected]> 0 siblings, 0 replies; 30+ messages in thread From: Justin Pryzby @ 2019-04-04 23:57 UTC (permalink / raw) Should backpatch to v12 --- doc/src/sgml/ref/alter_table.sgml | 2 +- doc/src/sgml/sources.sgml | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/doc/src/sgml/ref/alter_table.sgml b/doc/src/sgml/ref/alter_table.sgml index b2eb7097a9..fa848e0bdf 100644 --- a/doc/src/sgml/ref/alter_table.sgml +++ b/doc/src/sgml/ref/alter_table.sgml @@ -889,7 +889,7 @@ WITH ( MODULUS <replaceable class="parameter">numeric_literal</replaceable>, REM from the parent table will be created in the partition, if they don't already exist. If any of the <literal>CHECK</literal> constraints of the table being - attached is marked <literal>NO INHERIT</literal>, the command will fail; + attached are marked <literal>NO INHERIT</literal>, the command will fail; such constraints must be recreated without the <literal>NO INHERIT</literal> clause. </para> diff --git a/doc/src/sgml/sources.sgml b/doc/src/sgml/sources.sgml index 283c3e0357..12704c6fdf 100644 --- a/doc/src/sgml/sources.sgml +++ b/doc/src/sgml/sources.sgml @@ -466,8 +466,8 @@ Hint: the addendum enough for error messages. Detail and hint messages can be relegated to a verbose mode, or perhaps a pop-up error-details window. Also, details and hints would normally be suppressed from the server log to save - space. Reference to implementation details is best avoided since users - aren't expected to know the details. + space. References to implementation details are best avoided since users + aren't expected to know them. </para> </simplesect> -- 2.17.0 --C+ts3FVlLX8+P6JN Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v6-0008-doc-backup-manifests.patch" ^ permalink raw reply [nested|flat] 30+ messages in thread
* [PATCH v6 07/10] is vs are plural @ 2019-04-04 23:57 Justin Pryzby <[email protected]> 0 siblings, 0 replies; 30+ messages in thread From: Justin Pryzby @ 2019-04-04 23:57 UTC (permalink / raw) Should backpatch to v12 --- doc/src/sgml/ref/alter_table.sgml | 2 +- doc/src/sgml/sources.sgml | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/doc/src/sgml/ref/alter_table.sgml b/doc/src/sgml/ref/alter_table.sgml index b2eb7097a9..fa848e0bdf 100644 --- a/doc/src/sgml/ref/alter_table.sgml +++ b/doc/src/sgml/ref/alter_table.sgml @@ -889,7 +889,7 @@ WITH ( MODULUS <replaceable class="parameter">numeric_literal</replaceable>, REM from the parent table will be created in the partition, if they don't already exist. If any of the <literal>CHECK</literal> constraints of the table being - attached is marked <literal>NO INHERIT</literal>, the command will fail; + attached are marked <literal>NO INHERIT</literal>, the command will fail; such constraints must be recreated without the <literal>NO INHERIT</literal> clause. </para> diff --git a/doc/src/sgml/sources.sgml b/doc/src/sgml/sources.sgml index 283c3e0357..12704c6fdf 100644 --- a/doc/src/sgml/sources.sgml +++ b/doc/src/sgml/sources.sgml @@ -466,8 +466,8 @@ Hint: the addendum enough for error messages. Detail and hint messages can be relegated to a verbose mode, or perhaps a pop-up error-details window. Also, details and hints would normally be suppressed from the server log to save - space. Reference to implementation details is best avoided since users - aren't expected to know the details. + space. References to implementation details are best avoided since users + aren't expected to know them. </para> </simplesect> -- 2.17.0 --C+ts3FVlLX8+P6JN Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v6-0008-doc-backup-manifests.patch" ^ permalink raw reply [nested|flat] 30+ messages in thread
* [PATCH v6 07/10] is vs are plural @ 2019-04-04 23:57 Justin Pryzby <[email protected]> 0 siblings, 0 replies; 30+ messages in thread From: Justin Pryzby @ 2019-04-04 23:57 UTC (permalink / raw) Should backpatch to v12 --- doc/src/sgml/ref/alter_table.sgml | 2 +- doc/src/sgml/sources.sgml | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/doc/src/sgml/ref/alter_table.sgml b/doc/src/sgml/ref/alter_table.sgml index b2eb7097a9..fa848e0bdf 100644 --- a/doc/src/sgml/ref/alter_table.sgml +++ b/doc/src/sgml/ref/alter_table.sgml @@ -889,7 +889,7 @@ WITH ( MODULUS <replaceable class="parameter">numeric_literal</replaceable>, REM from the parent table will be created in the partition, if they don't already exist. If any of the <literal>CHECK</literal> constraints of the table being - attached is marked <literal>NO INHERIT</literal>, the command will fail; + attached are marked <literal>NO INHERIT</literal>, the command will fail; such constraints must be recreated without the <literal>NO INHERIT</literal> clause. </para> diff --git a/doc/src/sgml/sources.sgml b/doc/src/sgml/sources.sgml index 283c3e0357..12704c6fdf 100644 --- a/doc/src/sgml/sources.sgml +++ b/doc/src/sgml/sources.sgml @@ -466,8 +466,8 @@ Hint: the addendum enough for error messages. Detail and hint messages can be relegated to a verbose mode, or perhaps a pop-up error-details window. Also, details and hints would normally be suppressed from the server log to save - space. Reference to implementation details is best avoided since users - aren't expected to know the details. + space. References to implementation details are best avoided since users + aren't expected to know them. </para> </simplesect> -- 2.17.0 --C+ts3FVlLX8+P6JN Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v6-0008-doc-backup-manifests.patch" ^ permalink raw reply [nested|flat] 30+ messages in thread
* [PATCH v6 07/10] is vs are plural @ 2019-04-04 23:57 Justin Pryzby <[email protected]> 0 siblings, 0 replies; 30+ messages in thread From: Justin Pryzby @ 2019-04-04 23:57 UTC (permalink / raw) Should backpatch to v12 --- doc/src/sgml/ref/alter_table.sgml | 2 +- doc/src/sgml/sources.sgml | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/doc/src/sgml/ref/alter_table.sgml b/doc/src/sgml/ref/alter_table.sgml index b2eb7097a9..fa848e0bdf 100644 --- a/doc/src/sgml/ref/alter_table.sgml +++ b/doc/src/sgml/ref/alter_table.sgml @@ -889,7 +889,7 @@ WITH ( MODULUS <replaceable class="parameter">numeric_literal</replaceable>, REM from the parent table will be created in the partition, if they don't already exist. If any of the <literal>CHECK</literal> constraints of the table being - attached is marked <literal>NO INHERIT</literal>, the command will fail; + attached are marked <literal>NO INHERIT</literal>, the command will fail; such constraints must be recreated without the <literal>NO INHERIT</literal> clause. </para> diff --git a/doc/src/sgml/sources.sgml b/doc/src/sgml/sources.sgml index 283c3e0357..12704c6fdf 100644 --- a/doc/src/sgml/sources.sgml +++ b/doc/src/sgml/sources.sgml @@ -466,8 +466,8 @@ Hint: the addendum enough for error messages. Detail and hint messages can be relegated to a verbose mode, or perhaps a pop-up error-details window. Also, details and hints would normally be suppressed from the server log to save - space. Reference to implementation details is best avoided since users - aren't expected to know the details. + space. References to implementation details are best avoided since users + aren't expected to know them. </para> </simplesect> -- 2.17.0 --C+ts3FVlLX8+P6JN Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v6-0008-doc-backup-manifests.patch" ^ permalink raw reply [nested|flat] 30+ messages in thread
* [PATCH v6 07/10] is vs are plural @ 2019-04-04 23:57 Justin Pryzby <[email protected]> 0 siblings, 0 replies; 30+ messages in thread From: Justin Pryzby @ 2019-04-04 23:57 UTC (permalink / raw) Should backpatch to v12 --- doc/src/sgml/ref/alter_table.sgml | 2 +- doc/src/sgml/sources.sgml | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/doc/src/sgml/ref/alter_table.sgml b/doc/src/sgml/ref/alter_table.sgml index b2eb7097a9..fa848e0bdf 100644 --- a/doc/src/sgml/ref/alter_table.sgml +++ b/doc/src/sgml/ref/alter_table.sgml @@ -889,7 +889,7 @@ WITH ( MODULUS <replaceable class="parameter">numeric_literal</replaceable>, REM from the parent table will be created in the partition, if they don't already exist. If any of the <literal>CHECK</literal> constraints of the table being - attached is marked <literal>NO INHERIT</literal>, the command will fail; + attached are marked <literal>NO INHERIT</literal>, the command will fail; such constraints must be recreated without the <literal>NO INHERIT</literal> clause. </para> diff --git a/doc/src/sgml/sources.sgml b/doc/src/sgml/sources.sgml index 283c3e0357..12704c6fdf 100644 --- a/doc/src/sgml/sources.sgml +++ b/doc/src/sgml/sources.sgml @@ -466,8 +466,8 @@ Hint: the addendum enough for error messages. Detail and hint messages can be relegated to a verbose mode, or perhaps a pop-up error-details window. Also, details and hints would normally be suppressed from the server log to save - space. Reference to implementation details is best avoided since users - aren't expected to know the details. + space. References to implementation details are best avoided since users + aren't expected to know them. </para> </simplesect> -- 2.17.0 --C+ts3FVlLX8+P6JN Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v6-0008-doc-backup-manifests.patch" ^ permalink raw reply [nested|flat] 30+ messages in thread
* [PATCH v6 07/10] is vs are plural @ 2019-04-04 23:57 Justin Pryzby <[email protected]> 0 siblings, 0 replies; 30+ messages in thread From: Justin Pryzby @ 2019-04-04 23:57 UTC (permalink / raw) Should backpatch to v12 --- doc/src/sgml/ref/alter_table.sgml | 2 +- doc/src/sgml/sources.sgml | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/doc/src/sgml/ref/alter_table.sgml b/doc/src/sgml/ref/alter_table.sgml index b2eb7097a9..fa848e0bdf 100644 --- a/doc/src/sgml/ref/alter_table.sgml +++ b/doc/src/sgml/ref/alter_table.sgml @@ -889,7 +889,7 @@ WITH ( MODULUS <replaceable class="parameter">numeric_literal</replaceable>, REM from the parent table will be created in the partition, if they don't already exist. If any of the <literal>CHECK</literal> constraints of the table being - attached is marked <literal>NO INHERIT</literal>, the command will fail; + attached are marked <literal>NO INHERIT</literal>, the command will fail; such constraints must be recreated without the <literal>NO INHERIT</literal> clause. </para> diff --git a/doc/src/sgml/sources.sgml b/doc/src/sgml/sources.sgml index 283c3e0357..12704c6fdf 100644 --- a/doc/src/sgml/sources.sgml +++ b/doc/src/sgml/sources.sgml @@ -466,8 +466,8 @@ Hint: the addendum enough for error messages. Detail and hint messages can be relegated to a verbose mode, or perhaps a pop-up error-details window. Also, details and hints would normally be suppressed from the server log to save - space. Reference to implementation details is best avoided since users - aren't expected to know the details. + space. References to implementation details are best avoided since users + aren't expected to know them. </para> </simplesect> -- 2.17.0 --C+ts3FVlLX8+P6JN Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v6-0008-doc-backup-manifests.patch" ^ permalink raw reply [nested|flat] 30+ messages in thread
* [PATCH v6 07/10] is vs are plural @ 2019-04-04 23:57 Justin Pryzby <[email protected]> 0 siblings, 0 replies; 30+ messages in thread From: Justin Pryzby @ 2019-04-04 23:57 UTC (permalink / raw) Should backpatch to v12 --- doc/src/sgml/ref/alter_table.sgml | 2 +- doc/src/sgml/sources.sgml | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/doc/src/sgml/ref/alter_table.sgml b/doc/src/sgml/ref/alter_table.sgml index b2eb7097a9..fa848e0bdf 100644 --- a/doc/src/sgml/ref/alter_table.sgml +++ b/doc/src/sgml/ref/alter_table.sgml @@ -889,7 +889,7 @@ WITH ( MODULUS <replaceable class="parameter">numeric_literal</replaceable>, REM from the parent table will be created in the partition, if they don't already exist. If any of the <literal>CHECK</literal> constraints of the table being - attached is marked <literal>NO INHERIT</literal>, the command will fail; + attached are marked <literal>NO INHERIT</literal>, the command will fail; such constraints must be recreated without the <literal>NO INHERIT</literal> clause. </para> diff --git a/doc/src/sgml/sources.sgml b/doc/src/sgml/sources.sgml index 283c3e0357..12704c6fdf 100644 --- a/doc/src/sgml/sources.sgml +++ b/doc/src/sgml/sources.sgml @@ -466,8 +466,8 @@ Hint: the addendum enough for error messages. Detail and hint messages can be relegated to a verbose mode, or perhaps a pop-up error-details window. Also, details and hints would normally be suppressed from the server log to save - space. Reference to implementation details is best avoided since users - aren't expected to know the details. + space. References to implementation details are best avoided since users + aren't expected to know them. </para> </simplesect> -- 2.17.0 --C+ts3FVlLX8+P6JN Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v6-0008-doc-backup-manifests.patch" ^ permalink raw reply [nested|flat] 30+ messages in thread
* [PATCH v6 07/10] is vs are plural @ 2019-04-04 23:57 Justin Pryzby <[email protected]> 0 siblings, 0 replies; 30+ messages in thread From: Justin Pryzby @ 2019-04-04 23:57 UTC (permalink / raw) Should backpatch to v12 --- doc/src/sgml/ref/alter_table.sgml | 2 +- doc/src/sgml/sources.sgml | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/doc/src/sgml/ref/alter_table.sgml b/doc/src/sgml/ref/alter_table.sgml index b2eb7097a9..fa848e0bdf 100644 --- a/doc/src/sgml/ref/alter_table.sgml +++ b/doc/src/sgml/ref/alter_table.sgml @@ -889,7 +889,7 @@ WITH ( MODULUS <replaceable class="parameter">numeric_literal</replaceable>, REM from the parent table will be created in the partition, if they don't already exist. If any of the <literal>CHECK</literal> constraints of the table being - attached is marked <literal>NO INHERIT</literal>, the command will fail; + attached are marked <literal>NO INHERIT</literal>, the command will fail; such constraints must be recreated without the <literal>NO INHERIT</literal> clause. </para> diff --git a/doc/src/sgml/sources.sgml b/doc/src/sgml/sources.sgml index 283c3e0357..12704c6fdf 100644 --- a/doc/src/sgml/sources.sgml +++ b/doc/src/sgml/sources.sgml @@ -466,8 +466,8 @@ Hint: the addendum enough for error messages. Detail and hint messages can be relegated to a verbose mode, or perhaps a pop-up error-details window. Also, details and hints would normally be suppressed from the server log to save - space. Reference to implementation details is best avoided since users - aren't expected to know the details. + space. References to implementation details are best avoided since users + aren't expected to know them. </para> </simplesect> -- 2.17.0 --C+ts3FVlLX8+P6JN Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v6-0008-doc-backup-manifests.patch" ^ permalink raw reply [nested|flat] 30+ messages in thread
* [PATCH v6 07/10] is vs are plural @ 2019-04-04 23:57 Justin Pryzby <[email protected]> 0 siblings, 0 replies; 30+ messages in thread From: Justin Pryzby @ 2019-04-04 23:57 UTC (permalink / raw) Should backpatch to v12 --- doc/src/sgml/ref/alter_table.sgml | 2 +- doc/src/sgml/sources.sgml | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/doc/src/sgml/ref/alter_table.sgml b/doc/src/sgml/ref/alter_table.sgml index b2eb7097a9..fa848e0bdf 100644 --- a/doc/src/sgml/ref/alter_table.sgml +++ b/doc/src/sgml/ref/alter_table.sgml @@ -889,7 +889,7 @@ WITH ( MODULUS <replaceable class="parameter">numeric_literal</replaceable>, REM from the parent table will be created in the partition, if they don't already exist. If any of the <literal>CHECK</literal> constraints of the table being - attached is marked <literal>NO INHERIT</literal>, the command will fail; + attached are marked <literal>NO INHERIT</literal>, the command will fail; such constraints must be recreated without the <literal>NO INHERIT</literal> clause. </para> diff --git a/doc/src/sgml/sources.sgml b/doc/src/sgml/sources.sgml index 283c3e0357..12704c6fdf 100644 --- a/doc/src/sgml/sources.sgml +++ b/doc/src/sgml/sources.sgml @@ -466,8 +466,8 @@ Hint: the addendum enough for error messages. Detail and hint messages can be relegated to a verbose mode, or perhaps a pop-up error-details window. Also, details and hints would normally be suppressed from the server log to save - space. Reference to implementation details is best avoided since users - aren't expected to know the details. + space. References to implementation details are best avoided since users + aren't expected to know them. </para> </simplesect> -- 2.17.0 --C+ts3FVlLX8+P6JN Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v6-0008-doc-backup-manifests.patch" ^ permalink raw reply [nested|flat] 30+ messages in thread
* [PATCH v6 07/10] is vs are plural @ 2019-04-04 23:57 Justin Pryzby <[email protected]> 0 siblings, 0 replies; 30+ messages in thread From: Justin Pryzby @ 2019-04-04 23:57 UTC (permalink / raw) Should backpatch to v12 --- doc/src/sgml/ref/alter_table.sgml | 2 +- doc/src/sgml/sources.sgml | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/doc/src/sgml/ref/alter_table.sgml b/doc/src/sgml/ref/alter_table.sgml index b2eb7097a9..fa848e0bdf 100644 --- a/doc/src/sgml/ref/alter_table.sgml +++ b/doc/src/sgml/ref/alter_table.sgml @@ -889,7 +889,7 @@ WITH ( MODULUS <replaceable class="parameter">numeric_literal</replaceable>, REM from the parent table will be created in the partition, if they don't already exist. If any of the <literal>CHECK</literal> constraints of the table being - attached is marked <literal>NO INHERIT</literal>, the command will fail; + attached are marked <literal>NO INHERIT</literal>, the command will fail; such constraints must be recreated without the <literal>NO INHERIT</literal> clause. </para> diff --git a/doc/src/sgml/sources.sgml b/doc/src/sgml/sources.sgml index 283c3e0357..12704c6fdf 100644 --- a/doc/src/sgml/sources.sgml +++ b/doc/src/sgml/sources.sgml @@ -466,8 +466,8 @@ Hint: the addendum enough for error messages. Detail and hint messages can be relegated to a verbose mode, or perhaps a pop-up error-details window. Also, details and hints would normally be suppressed from the server log to save - space. Reference to implementation details is best avoided since users - aren't expected to know the details. + space. References to implementation details are best avoided since users + aren't expected to know them. </para> </simplesect> -- 2.17.0 --C+ts3FVlLX8+P6JN Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v6-0008-doc-backup-manifests.patch" ^ permalink raw reply [nested|flat] 30+ messages in thread
* [PATCH v6 07/10] is vs are plural @ 2019-04-04 23:57 Justin Pryzby <[email protected]> 0 siblings, 0 replies; 30+ messages in thread From: Justin Pryzby @ 2019-04-04 23:57 UTC (permalink / raw) Should backpatch to v12 --- doc/src/sgml/ref/alter_table.sgml | 2 +- doc/src/sgml/sources.sgml | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/doc/src/sgml/ref/alter_table.sgml b/doc/src/sgml/ref/alter_table.sgml index b2eb7097a9..fa848e0bdf 100644 --- a/doc/src/sgml/ref/alter_table.sgml +++ b/doc/src/sgml/ref/alter_table.sgml @@ -889,7 +889,7 @@ WITH ( MODULUS <replaceable class="parameter">numeric_literal</replaceable>, REM from the parent table will be created in the partition, if they don't already exist. If any of the <literal>CHECK</literal> constraints of the table being - attached is marked <literal>NO INHERIT</literal>, the command will fail; + attached are marked <literal>NO INHERIT</literal>, the command will fail; such constraints must be recreated without the <literal>NO INHERIT</literal> clause. </para> diff --git a/doc/src/sgml/sources.sgml b/doc/src/sgml/sources.sgml index 283c3e0357..12704c6fdf 100644 --- a/doc/src/sgml/sources.sgml +++ b/doc/src/sgml/sources.sgml @@ -466,8 +466,8 @@ Hint: the addendum enough for error messages. Detail and hint messages can be relegated to a verbose mode, or perhaps a pop-up error-details window. Also, details and hints would normally be suppressed from the server log to save - space. Reference to implementation details is best avoided since users - aren't expected to know the details. + space. References to implementation details are best avoided since users + aren't expected to know them. </para> </simplesect> -- 2.17.0 --C+ts3FVlLX8+P6JN Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v6-0008-doc-backup-manifests.patch" ^ permalink raw reply [nested|flat] 30+ messages in thread
* [PATCH v6 07/10] is vs are plural @ 2019-04-04 23:57 Justin Pryzby <[email protected]> 0 siblings, 0 replies; 30+ messages in thread From: Justin Pryzby @ 2019-04-04 23:57 UTC (permalink / raw) Should backpatch to v12 --- doc/src/sgml/ref/alter_table.sgml | 2 +- doc/src/sgml/sources.sgml | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/doc/src/sgml/ref/alter_table.sgml b/doc/src/sgml/ref/alter_table.sgml index b2eb7097a9..fa848e0bdf 100644 --- a/doc/src/sgml/ref/alter_table.sgml +++ b/doc/src/sgml/ref/alter_table.sgml @@ -889,7 +889,7 @@ WITH ( MODULUS <replaceable class="parameter">numeric_literal</replaceable>, REM from the parent table will be created in the partition, if they don't already exist. If any of the <literal>CHECK</literal> constraints of the table being - attached is marked <literal>NO INHERIT</literal>, the command will fail; + attached are marked <literal>NO INHERIT</literal>, the command will fail; such constraints must be recreated without the <literal>NO INHERIT</literal> clause. </para> diff --git a/doc/src/sgml/sources.sgml b/doc/src/sgml/sources.sgml index 283c3e0357..12704c6fdf 100644 --- a/doc/src/sgml/sources.sgml +++ b/doc/src/sgml/sources.sgml @@ -466,8 +466,8 @@ Hint: the addendum enough for error messages. Detail and hint messages can be relegated to a verbose mode, or perhaps a pop-up error-details window. Also, details and hints would normally be suppressed from the server log to save - space. Reference to implementation details is best avoided since users - aren't expected to know the details. + space. References to implementation details are best avoided since users + aren't expected to know them. </para> </simplesect> -- 2.17.0 --C+ts3FVlLX8+P6JN Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v6-0008-doc-backup-manifests.patch" ^ permalink raw reply [nested|flat] 30+ messages in thread
* [PATCH v6 07/10] is vs are plural @ 2019-04-04 23:57 Justin Pryzby <[email protected]> 0 siblings, 0 replies; 30+ messages in thread From: Justin Pryzby @ 2019-04-04 23:57 UTC (permalink / raw) Should backpatch to v12 --- doc/src/sgml/ref/alter_table.sgml | 2 +- doc/src/sgml/sources.sgml | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/doc/src/sgml/ref/alter_table.sgml b/doc/src/sgml/ref/alter_table.sgml index b2eb7097a9..fa848e0bdf 100644 --- a/doc/src/sgml/ref/alter_table.sgml +++ b/doc/src/sgml/ref/alter_table.sgml @@ -889,7 +889,7 @@ WITH ( MODULUS <replaceable class="parameter">numeric_literal</replaceable>, REM from the parent table will be created in the partition, if they don't already exist. If any of the <literal>CHECK</literal> constraints of the table being - attached is marked <literal>NO INHERIT</literal>, the command will fail; + attached are marked <literal>NO INHERIT</literal>, the command will fail; such constraints must be recreated without the <literal>NO INHERIT</literal> clause. </para> diff --git a/doc/src/sgml/sources.sgml b/doc/src/sgml/sources.sgml index 283c3e0357..12704c6fdf 100644 --- a/doc/src/sgml/sources.sgml +++ b/doc/src/sgml/sources.sgml @@ -466,8 +466,8 @@ Hint: the addendum enough for error messages. Detail and hint messages can be relegated to a verbose mode, or perhaps a pop-up error-details window. Also, details and hints would normally be suppressed from the server log to save - space. Reference to implementation details is best avoided since users - aren't expected to know the details. + space. References to implementation details are best avoided since users + aren't expected to know them. </para> </simplesect> -- 2.17.0 --C+ts3FVlLX8+P6JN Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v6-0008-doc-backup-manifests.patch" ^ permalink raw reply [nested|flat] 30+ messages in thread
* [PATCH v6 07/10] is vs are plural @ 2019-04-04 23:57 Justin Pryzby <[email protected]> 0 siblings, 0 replies; 30+ messages in thread From: Justin Pryzby @ 2019-04-04 23:57 UTC (permalink / raw) Should backpatch to v12 --- doc/src/sgml/ref/alter_table.sgml | 2 +- doc/src/sgml/sources.sgml | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/doc/src/sgml/ref/alter_table.sgml b/doc/src/sgml/ref/alter_table.sgml index b2eb7097a9..fa848e0bdf 100644 --- a/doc/src/sgml/ref/alter_table.sgml +++ b/doc/src/sgml/ref/alter_table.sgml @@ -889,7 +889,7 @@ WITH ( MODULUS <replaceable class="parameter">numeric_literal</replaceable>, REM from the parent table will be created in the partition, if they don't already exist. If any of the <literal>CHECK</literal> constraints of the table being - attached is marked <literal>NO INHERIT</literal>, the command will fail; + attached are marked <literal>NO INHERIT</literal>, the command will fail; such constraints must be recreated without the <literal>NO INHERIT</literal> clause. </para> diff --git a/doc/src/sgml/sources.sgml b/doc/src/sgml/sources.sgml index 283c3e0357..12704c6fdf 100644 --- a/doc/src/sgml/sources.sgml +++ b/doc/src/sgml/sources.sgml @@ -466,8 +466,8 @@ Hint: the addendum enough for error messages. Detail and hint messages can be relegated to a verbose mode, or perhaps a pop-up error-details window. Also, details and hints would normally be suppressed from the server log to save - space. Reference to implementation details is best avoided since users - aren't expected to know the details. + space. References to implementation details are best avoided since users + aren't expected to know them. </para> </simplesect> -- 2.17.0 --C+ts3FVlLX8+P6JN Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v6-0008-doc-backup-manifests.patch" ^ permalink raw reply [nested|flat] 30+ messages in thread
* [PATCH v7 06/10] is vs are plural @ 2019-04-04 23:57 Justin Pryzby <[email protected]> 0 siblings, 0 replies; 30+ messages in thread From: Justin Pryzby @ 2019-04-04 23:57 UTC (permalink / raw) Introduced: commit f0e44751d7175fa3394da2c8f85e3ceb3cdbfe63 Author: Robert Haas <[email protected]> Date: Wed Dec 7 13:17:43 2016 -0500 --- doc/src/sgml/ref/alter_table.sgml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/src/sgml/ref/alter_table.sgml b/doc/src/sgml/ref/alter_table.sgml index b2eb7097a9..fa848e0bdf 100644 --- a/doc/src/sgml/ref/alter_table.sgml +++ b/doc/src/sgml/ref/alter_table.sgml @@ -889,7 +889,7 @@ WITH ( MODULUS <replaceable class="parameter">numeric_literal</replaceable>, REM from the parent table will be created in the partition, if they don't already exist. If any of the <literal>CHECK</literal> constraints of the table being - attached is marked <literal>NO INHERIT</literal>, the command will fail; + attached are marked <literal>NO INHERIT</literal>, the command will fail; such constraints must be recreated without the <literal>NO INHERIT</literal> clause. </para> -- 2.17.0 --AsxXAMtlQ5JHofzM Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v7-0007-doc-backup-manifests.patch" ^ permalink raw reply [nested|flat] 30+ messages in thread
* [PATCH v1 19/19] is vs are plural @ 2019-04-04 23:57 Justin Pryzby <[email protected]> 0 siblings, 0 replies; 30+ messages in thread From: Justin Pryzby @ 2019-04-04 23:57 UTC (permalink / raw) Should backpatch to v12 --- doc/src/sgml/auto-explain.sgml | 4 ++-- doc/src/sgml/func.sgml | 2 +- doc/src/sgml/ref/alter_table.sgml | 2 +- doc/src/sgml/sources.sgml | 4 ++-- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/doc/src/sgml/auto-explain.sgml b/doc/src/sgml/auto-explain.sgml index d4d29c4a64..828a2d102c 100644 --- a/doc/src/sgml/auto-explain.sgml +++ b/doc/src/sgml/auto-explain.sgml @@ -200,8 +200,8 @@ LOAD 'auto_explain'; <listitem> <para> <varname>auto_explain.log_settings</varname> controls whether information - about modified configuration options are printed when execution plan is logged. - Only options affecting query planning with value different from the built-in + about modified configuration options is printed when an execution plan is logged. + Only those options which affect query planning and whose value differs from its built-in default value are included in the output. This parameter is off by default. Only superusers can change this setting. </para> diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml index b708b79229..1d6016ff73 100644 --- a/doc/src/sgml/func.sgml +++ b/doc/src/sgml/func.sgml @@ -10006,7 +10006,7 @@ gen_random_uuid() returns uuid <title>Producing XML Content</title> <para> - A set of functions and function-like expressions are available for + A set of functions and function-like expressions is available for producing XML content from SQL data. As such, they are particularly suitable for formatting query results into XML documents for processing in client applications. diff --git a/doc/src/sgml/ref/alter_table.sgml b/doc/src/sgml/ref/alter_table.sgml index fe0f077d1f..0beb08a5a2 100644 --- a/doc/src/sgml/ref/alter_table.sgml +++ b/doc/src/sgml/ref/alter_table.sgml @@ -884,7 +884,7 @@ WITH ( MODULUS <replaceable class="parameter">numeric_literal</replaceable>, REM from the parent table will be created in the partition, if they don't already exist. If any of the <literal>CHECK</literal> constraints of the table being - attached is marked <literal>NO INHERIT</literal>, the command will fail; + attached are marked <literal>NO INHERIT</literal>, the command will fail; such constraints must be recreated without the <literal>NO INHERIT</literal> clause. </para> diff --git a/doc/src/sgml/sources.sgml b/doc/src/sgml/sources.sgml index 25f79d2a1f..5a8dbcb4d3 100644 --- a/doc/src/sgml/sources.sgml +++ b/doc/src/sgml/sources.sgml @@ -466,8 +466,8 @@ Hint: the addendum enough for error messages. Detail and hint messages can be relegated to a verbose mode, or perhaps a pop-up error-details window. Also, details and hints would normally be suppressed from the server log to save - space. Reference to implementation details is best avoided since users - aren't expected to know the details. + space. References to implementation details are best avoided since users + aren't expected to know them. </para> </simplesect> -- 2.17.0 --4ybNbZnZ8tziJ7D6-- ^ permalink raw reply [nested|flat] 30+ messages in thread
* [PATCH v2 12/12] is vs are plural @ 2019-04-04 23:57 Justin Pryzby <[email protected]> 0 siblings, 0 replies; 30+ messages in thread From: Justin Pryzby @ 2019-04-04 23:57 UTC (permalink / raw) Should backpatch to v12 --- doc/src/sgml/auto-explain.sgml | 6 +++--- doc/src/sgml/ref/alter_table.sgml | 2 +- doc/src/sgml/sources.sgml | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/doc/src/sgml/auto-explain.sgml b/doc/src/sgml/auto-explain.sgml index d4d29c4a64..de2be61bff 100644 --- a/doc/src/sgml/auto-explain.sgml +++ b/doc/src/sgml/auto-explain.sgml @@ -200,9 +200,9 @@ LOAD 'auto_explain'; <listitem> <para> <varname>auto_explain.log_settings</varname> controls whether information - about modified configuration options are printed when execution plan is logged. - Only options affecting query planning with value different from the built-in - default value are included in the output. This parameter is off by default. + about modified configuration options is printed when an execution plan is logged. + Only those options which affect query planning and whose value differs from their + built-in default are included in the output. This parameter is off by default. Only superusers can change this setting. </para> </listitem> diff --git a/doc/src/sgml/ref/alter_table.sgml b/doc/src/sgml/ref/alter_table.sgml index fe0f077d1f..0beb08a5a2 100644 --- a/doc/src/sgml/ref/alter_table.sgml +++ b/doc/src/sgml/ref/alter_table.sgml @@ -884,7 +884,7 @@ WITH ( MODULUS <replaceable class="parameter">numeric_literal</replaceable>, REM from the parent table will be created in the partition, if they don't already exist. If any of the <literal>CHECK</literal> constraints of the table being - attached is marked <literal>NO INHERIT</literal>, the command will fail; + attached are marked <literal>NO INHERIT</literal>, the command will fail; such constraints must be recreated without the <literal>NO INHERIT</literal> clause. </para> diff --git a/doc/src/sgml/sources.sgml b/doc/src/sgml/sources.sgml index 25f79d2a1f..5a8dbcb4d3 100644 --- a/doc/src/sgml/sources.sgml +++ b/doc/src/sgml/sources.sgml @@ -466,8 +466,8 @@ Hint: the addendum enough for error messages. Detail and hint messages can be relegated to a verbose mode, or perhaps a pop-up error-details window. Also, details and hints would normally be suppressed from the server log to save - space. Reference to implementation details is best avoided since users - aren't expected to know the details. + space. References to implementation details are best avoided since users + aren't expected to know them. </para> </simplesect> -- 2.17.0 --xYeFQzU4VZLrHqxU-- ^ permalink raw reply [nested|flat] 30+ messages in thread
* [PATCH v3 17/20] is vs are plural @ 2019-04-04 23:57 Justin Pryzby <[email protected]> 0 siblings, 0 replies; 30+ messages in thread From: Justin Pryzby @ 2019-04-04 23:57 UTC (permalink / raw) Should backpatch to v12 --- doc/src/sgml/auto-explain.sgml | 6 +++--- doc/src/sgml/ref/alter_table.sgml | 2 +- doc/src/sgml/sources.sgml | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/doc/src/sgml/auto-explain.sgml b/doc/src/sgml/auto-explain.sgml index d4d29c4a64..de2be61bff 100644 --- a/doc/src/sgml/auto-explain.sgml +++ b/doc/src/sgml/auto-explain.sgml @@ -200,9 +200,9 @@ LOAD 'auto_explain'; <listitem> <para> <varname>auto_explain.log_settings</varname> controls whether information - about modified configuration options are printed when execution plan is logged. - Only options affecting query planning with value different from the built-in - default value are included in the output. This parameter is off by default. + about modified configuration options is printed when an execution plan is logged. + Only those options which affect query planning and whose value differs from their + built-in default are included in the output. This parameter is off by default. Only superusers can change this setting. </para> </listitem> diff --git a/doc/src/sgml/ref/alter_table.sgml b/doc/src/sgml/ref/alter_table.sgml index 9c136dbce3..f74eb049cc 100644 --- a/doc/src/sgml/ref/alter_table.sgml +++ b/doc/src/sgml/ref/alter_table.sgml @@ -889,7 +889,7 @@ WITH ( MODULUS <replaceable class="parameter">numeric_literal</replaceable>, REM from the parent table will be created in the partition, if they don't already exist. If any of the <literal>CHECK</literal> constraints of the table being - attached is marked <literal>NO INHERIT</literal>, the command will fail; + attached are marked <literal>NO INHERIT</literal>, the command will fail; such constraints must be recreated without the <literal>NO INHERIT</literal> clause. </para> diff --git a/doc/src/sgml/sources.sgml b/doc/src/sgml/sources.sgml index 25f79d2a1f..5a8dbcb4d3 100644 --- a/doc/src/sgml/sources.sgml +++ b/doc/src/sgml/sources.sgml @@ -466,8 +466,8 @@ Hint: the addendum enough for error messages. Detail and hint messages can be relegated to a verbose mode, or perhaps a pop-up error-details window. Also, details and hints would normally be suppressed from the server log to save - space. Reference to implementation details is best avoided since users - aren't expected to know the details. + space. References to implementation details are best avoided since users + aren't expected to know them. </para> </simplesect> -- 2.17.0 --doUn1Hmx68n+7ij2 Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v3-0018-doc-Rename-the-recovery-related-wait-events.patch" ^ permalink raw reply [nested|flat] 30+ messages in thread
* [PATCH v4 10/12] is vs are plural @ 2019-04-04 23:57 Justin Pryzby <[email protected]> 0 siblings, 0 replies; 30+ messages in thread From: Justin Pryzby @ 2019-04-04 23:57 UTC (permalink / raw) Should backpatch to v12 --- doc/src/sgml/auto-explain.sgml | 6 +++--- doc/src/sgml/ref/alter_table.sgml | 2 +- doc/src/sgml/sources.sgml | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/doc/src/sgml/auto-explain.sgml b/doc/src/sgml/auto-explain.sgml index 192d6574c3..de2be61bff 100644 --- a/doc/src/sgml/auto-explain.sgml +++ b/doc/src/sgml/auto-explain.sgml @@ -200,9 +200,9 @@ LOAD 'auto_explain'; <listitem> <para> <varname>auto_explain.log_settings</varname> controls whether information - about modified configuration options is printed when execution plan is logged. - Only options affecting query planning with value different from the built-in - default value are included in the output. This parameter is off by default. + about modified configuration options is printed when an execution plan is logged. + Only those options which affect query planning and whose value differs from their + built-in default are included in the output. This parameter is off by default. Only superusers can change this setting. </para> </listitem> diff --git a/doc/src/sgml/ref/alter_table.sgml b/doc/src/sgml/ref/alter_table.sgml index 6563bd5ab2..52211612c6 100644 --- a/doc/src/sgml/ref/alter_table.sgml +++ b/doc/src/sgml/ref/alter_table.sgml @@ -891,7 +891,7 @@ WITH ( MODULUS <replaceable class="parameter">numeric_literal</replaceable>, REM from the parent table will be created in the partition, if they don't already exist. If any of the <literal>CHECK</literal> constraints of the table being - attached is marked <literal>NO INHERIT</literal>, the command will fail; + attached are marked <literal>NO INHERIT</literal>, the command will fail; such constraints must be recreated without the <literal>NO INHERIT</literal> clause. </para> diff --git a/doc/src/sgml/sources.sgml b/doc/src/sgml/sources.sgml index 283c3e0357..12704c6fdf 100644 --- a/doc/src/sgml/sources.sgml +++ b/doc/src/sgml/sources.sgml @@ -466,8 +466,8 @@ Hint: the addendum enough for error messages. Detail and hint messages can be relegated to a verbose mode, or perhaps a pop-up error-details window. Also, details and hints would normally be suppressed from the server log to save - space. Reference to implementation details is best avoided since users - aren't expected to know the details. + space. References to implementation details are best avoided since users + aren't expected to know them. </para> </simplesect> -- 2.17.0 --DiL7RhKs8rK9YGuF Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v4-0011-doc-Rename-the-recovery-related-wait-events.patch" ^ permalink raw reply [nested|flat] 30+ messages in thread
* [PATCH v6 07/10] is vs are plural @ 2019-04-04 23:57 Justin Pryzby <[email protected]> 0 siblings, 0 replies; 30+ messages in thread From: Justin Pryzby @ 2019-04-04 23:57 UTC (permalink / raw) Should backpatch to v12 --- doc/src/sgml/ref/alter_table.sgml | 2 +- doc/src/sgml/sources.sgml | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/doc/src/sgml/ref/alter_table.sgml b/doc/src/sgml/ref/alter_table.sgml index b2eb7097a9..fa848e0bdf 100644 --- a/doc/src/sgml/ref/alter_table.sgml +++ b/doc/src/sgml/ref/alter_table.sgml @@ -889,7 +889,7 @@ WITH ( MODULUS <replaceable class="parameter">numeric_literal</replaceable>, REM from the parent table will be created in the partition, if they don't already exist. If any of the <literal>CHECK</literal> constraints of the table being - attached is marked <literal>NO INHERIT</literal>, the command will fail; + attached are marked <literal>NO INHERIT</literal>, the command will fail; such constraints must be recreated without the <literal>NO INHERIT</literal> clause. </para> diff --git a/doc/src/sgml/sources.sgml b/doc/src/sgml/sources.sgml index 283c3e0357..12704c6fdf 100644 --- a/doc/src/sgml/sources.sgml +++ b/doc/src/sgml/sources.sgml @@ -466,8 +466,8 @@ Hint: the addendum enough for error messages. Detail and hint messages can be relegated to a verbose mode, or perhaps a pop-up error-details window. Also, details and hints would normally be suppressed from the server log to save - space. Reference to implementation details is best avoided since users - aren't expected to know the details. + space. References to implementation details are best avoided since users + aren't expected to know them. </para> </simplesect> -- 2.17.0 --C+ts3FVlLX8+P6JN Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v6-0008-doc-backup-manifests.patch" ^ permalink raw reply [nested|flat] 30+ messages in thread
* [PATCH v8 06/14] is vs are plural @ 2019-04-04 23:57 Justin Pryzby <[email protected]> 0 siblings, 0 replies; 30+ messages in thread From: Justin Pryzby @ 2019-04-04 23:57 UTC (permalink / raw) Introduced: commit f0e44751d7175fa3394da2c8f85e3ceb3cdbfe63 Author: Robert Haas <[email protected]> Date: Wed Dec 7 13:17:43 2016 -0500 --- doc/src/sgml/ref/alter_table.sgml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/src/sgml/ref/alter_table.sgml b/doc/src/sgml/ref/alter_table.sgml index b2eb7097a9..fa848e0bdf 100644 --- a/doc/src/sgml/ref/alter_table.sgml +++ b/doc/src/sgml/ref/alter_table.sgml @@ -889,7 +889,7 @@ WITH ( MODULUS <replaceable class="parameter">numeric_literal</replaceable>, REM from the parent table will be created in the partition, if they don't already exist. If any of the <literal>CHECK</literal> constraints of the table being - attached is marked <literal>NO INHERIT</literal>, the command will fail; + attached are marked <literal>NO INHERIT</literal>, the command will fail; such constraints must be recreated without the <literal>NO INHERIT</literal> clause. </para> -- 2.17.0 --0eh6TmSyL6TZE2Uz Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v8-0007-doc-backup-manifests.patch" ^ permalink raw reply [nested|flat] 30+ messages in thread
* [PATCH v6 07/10] is vs are plural @ 2019-04-04 23:57 Justin Pryzby <[email protected]> 0 siblings, 0 replies; 30+ messages in thread From: Justin Pryzby @ 2019-04-04 23:57 UTC (permalink / raw) Should backpatch to v12 --- doc/src/sgml/ref/alter_table.sgml | 2 +- doc/src/sgml/sources.sgml | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/doc/src/sgml/ref/alter_table.sgml b/doc/src/sgml/ref/alter_table.sgml index b2eb7097a9..fa848e0bdf 100644 --- a/doc/src/sgml/ref/alter_table.sgml +++ b/doc/src/sgml/ref/alter_table.sgml @@ -889,7 +889,7 @@ WITH ( MODULUS <replaceable class="parameter">numeric_literal</replaceable>, REM from the parent table will be created in the partition, if they don't already exist. If any of the <literal>CHECK</literal> constraints of the table being - attached is marked <literal>NO INHERIT</literal>, the command will fail; + attached are marked <literal>NO INHERIT</literal>, the command will fail; such constraints must be recreated without the <literal>NO INHERIT</literal> clause. </para> diff --git a/doc/src/sgml/sources.sgml b/doc/src/sgml/sources.sgml index 283c3e0357..12704c6fdf 100644 --- a/doc/src/sgml/sources.sgml +++ b/doc/src/sgml/sources.sgml @@ -466,8 +466,8 @@ Hint: the addendum enough for error messages. Detail and hint messages can be relegated to a verbose mode, or perhaps a pop-up error-details window. Also, details and hints would normally be suppressed from the server log to save - space. Reference to implementation details is best avoided since users - aren't expected to know the details. + space. References to implementation details are best avoided since users + aren't expected to know them. </para> </simplesect> -- 2.17.0 --C+ts3FVlLX8+P6JN Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v6-0008-doc-backup-manifests.patch" ^ permalink raw reply [nested|flat] 30+ messages in thread
* Re: Fix some ubsan/asan related issues @ 2024-02-06 03:53 Tristan Partin <[email protected]> 0 siblings, 1 reply; 30+ messages in thread From: Tristan Partin @ 2024-02-06 03:53 UTC (permalink / raw) To: Andres Freund <[email protected]>; Alexander Lakhin <[email protected]>; +Cc: pgsql-hackers; Heikki Linnakangas <[email protected]> On Tue Jan 30, 2024 at 3:58 PM CST, Andres Freund wrote: > Hi, > > On 2024-01-30 09:59:25 -0600, Tristan Partin wrote: > > From 331cec1c9db6ff60dcc6d9ba62a9c8be4e5e95ed Mon Sep 17 00:00:00 2001 > > From: Tristan Partin <[email protected]> > > Date: Mon, 29 Jan 2024 18:03:39 -0600 > > Subject: [PATCH v1 1/3] Refuse to register message in LogLogicalMessage if > > NULL > > > If this occurs, the memcpy of rdata_data in CopyXLogRecordToWAL breaks > > the API contract of memcpy in glibc. The two pointer arguments are > > marked as nonnull, even in the event the amount to copy is 0 bytes. > > It seems pretty odd to call LogLogicalMessage() with a NULL argument. Why is > that something useful? Dropped. Will change on the Neon side. Should we add an assert somewhere for good measure? Near the memcpy in question? > > From dc9488f3fdee69b981b52c985fb77106d7d301ff Mon Sep 17 00:00:00 2001 > > From: Tristan Partin <[email protected]> > > Date: Wed, 24 Jan 2024 17:07:01 -0600 > > Subject: [PATCH v1 2/3] meson: Support compiling with -Db_sanitize=address > > > > The ecpg is parser is extremely leaky, so we need to silence leak > > detection. > > This does stuff beyond epcg... Dropped. > > +if get_option('b_sanitize').contains('address') > > + cdata.set('USE_ADDRESS_SANITIZER', 1) > > +endif > > > > ############################################################### > > # NLS / Gettext > > diff --git a/src/bin/initdb/initdb.c b/src/bin/initdb/initdb.c > > index ac409b0006..e18e716d9c 100644 > > --- a/src/bin/initdb/initdb.c > > +++ b/src/bin/initdb/initdb.c > > @@ -338,6 +338,17 @@ do { \ > > output_failed = true, output_errno = errno; \ > > } while (0) > > > > +#ifdef USE_ADDRESS_SANITIZER > > When asan is used __SANITIZE_ADDRESS__ is defined, so we don't need to > implement this ourselves. Thanks! > > +const char *__asan_default_options(void); > > + > > +const char *__asan_default_options(void) > > +{ > > + return "detect_leaks=0"; > > +} > > + > > +#endif > > Wonder if we should move this into some static library and link it into all > binaries that don't want leak detection? It doesn't seem great to have to > adjust this in a bunch of files if we want to adjust the options... See attached patches. Here is what I found to be necessary to get a -Db_sanitize=address,undefined build to successfully make it through tests. I do have a few concerns about the patch. 1. For whatever reason, __SANITIZE_LEAK__ is not defined when the leak sanitizer is enabled. So you will see me use this, to make some include directives work. I don't like this as a final solution because someone could use -fsanitize=leak. 2. I tracked down what seems to be a valid leak in adt/xml.c. Attached (test.sql) is a fairly minimal reproduction of what is needed to show the leak. I didn't spend too much time tracking it down. Might get to it later, who knows. Below you will find the backtrace, and whoever wants to try their hand at fixing it will need to comment out xmlNewNode in the leak.supp file. 3. I don't love the new library name. Maybe it should be name more lsan specific. 4. Should pg_attribute_no_asan be renamed to pg_attribute_no_sanitize_address? That would match pg_attribute_no_sanitize_alignment. I will also attach a Meson test log for good measure. I didn't try testing anything that requires PG_TEST_EXTRA, but I suspect that everything will be fine. Alexander, I haven't yet gotten to the things you pointed out in the sibling thread. ==221848==ERROR: LeakSanitizer: detected memory leaks Direct leak of 120 byte(s) in 1 object(s) allocated from: #0 0x7fac4a6d92ef in malloc (/lib64/libasan.so.8+0xd92ef) (BuildId: 7fcb7759bc17ef47f9682414b6d99732d6a6ab0c) #1 0x7fac4a48427d in xmlNewNode (/lib64/libxml2.so.2+0x5d27d) (BuildId: 3074681c8fa9b17e0cbed09bc61c25ada5c28e7c) #2 0x22107a6 in xmltotext_with_options ../src/backend/utils/adt/xml.c:754 #3 0xead047 in ExecEvalXmlExpr ../src/backend/executor/execExprInterp.c:4020 #4 0xe8c119 in ExecInterpExpr ../src/backend/executor/execExprInterp.c:1537 #5 0xe91f2c in ExecInterpExprStillValid ../src/backend/executor/execExprInterp.c:1881 #6 0x109632d in ExecEvalExprSwitchContext ../src/include/executor/executor.h:355 #7 0x109655d in ExecProject ../src/include/executor/executor.h:389 #8 0x1097186 in ExecResult ../src/backend/executor/nodeResult.c:136 #9 0xf0f90c in ExecProcNodeFirst ../src/backend/executor/execProcnode.c:464 #10 0xec9bec in ExecProcNode ../src/include/executor/executor.h:273 #11 0xed875c in ExecutePlan ../src/backend/executor/execMain.c:1670 #12 0xecbee0 in standard_ExecutorRun ../src/backend/executor/execMain.c:365 #13 0xecb529 in ExecutorRun ../src/backend/executor/execMain.c:309 #14 0x1ae89f6 in PortalRunSelect ../src/backend/tcop/pquery.c:924 #15 0x1ae7c06 in PortalRun ../src/backend/tcop/pquery.c:768 #16 0x1ad1b43 in exec_simple_query ../src/backend/tcop/postgres.c:1273 #17 0x1adf8de in PostgresMain ../src/backend/tcop/postgres.c:4653 #18 0x170dbce in BackendRun ../src/backend/postmaster/postmaster.c:4464 #19 0x170bf70 in BackendStartup ../src/backend/postmaster/postmaster.c:4140 #20 0x170263f in ServerLoop ../src/backend/postmaster/postmaster.c:1776 #21 0x1701052 in PostmasterMain ../src/backend/postmaster/postmaster.c:1475 #22 0x11a5f67 in main ../src/backend/main/main.c:198 #23 0x7fac48e46149 in __libc_start_call_main (/lib64/libc.so.6+0x28149) (BuildId: 7ea8d85df0e89b90c63ac7ed2b3578b2e7728756) #24 0x7fac48e4620a in __libc_start_main_impl (/lib64/libc.so.6+0x2820a) (BuildId: 7ea8d85df0e89b90c63ac7ed2b3578b2e7728756) #25 0x49c5d4 in _start (/home/tristan957/Projects/work/postgresql/build/tmp_install/home/tristan957/.opt/postgresql/bin/postgres+0x49c5d4) (BuildId: c8ca341e1303be0f9dc0b0271c55c4b9e42af89b) Indirect leak of 13 byte(s) in 1 object(s) allocated from: #0 0x7fac4a6d92ef in malloc (/lib64/libasan.so.8+0xd92ef) (BuildId: 7fcb7759bc17ef47f9682414b6d99732d6a6ab0c) #1 0x7fac4a4e106f in xmlStrndup (/lib64/libxml2.so.2+0xba06f) (BuildId: 3074681c8fa9b17e0cbed09bc61c25ada5c28e7c) #2 0x7fac4a4842c0 in xmlNewNode (/lib64/libxml2.so.2+0x5d2c0) (BuildId: 3074681c8fa9b17e0cbed09bc61c25ada5c28e7c) #3 0x22107a6 in xmltotext_with_options ../src/backend/utils/adt/xml.c:754 #4 0xead047 in ExecEvalXmlExpr ../src/backend/executor/execExprInterp.c:4020 #5 0xe8c119 in ExecInterpExpr ../src/backend/executor/execExprInterp.c:1537 #6 0xe91f2c in ExecInterpExprStillValid ../src/backend/executor/execExprInterp.c:1881 #7 0x109632d in ExecEvalExprSwitchContext ../src/include/executor/executor.h:355 #8 0x109655d in ExecProject ../src/include/executor/executor.h:389 #9 0x1097186 in ExecResult ../src/backend/executor/nodeResult.c:136 #10 0xf0f90c in ExecProcNodeFirst ../src/backend/executor/execProcnode.c:464 #11 0xec9bec in ExecProcNode ../src/include/executor/executor.h:273 #12 0xed875c in ExecutePlan ../src/backend/executor/execMain.c:1670 #13 0xecbee0 in standard_ExecutorRun ../src/backend/executor/execMain.c:365 #14 0xecb529 in ExecutorRun ../src/backend/executor/execMain.c:309 #15 0x1ae89f6 in PortalRunSelect ../src/backend/tcop/pquery.c:924 #16 0x1ae7c06 in PortalRun ../src/backend/tcop/pquery.c:768 #17 0x1ad1b43 in exec_simple_query ../src/backend/tcop/postgres.c:1273 #18 0x1adf8de in PostgresMain ../src/backend/tcop/postgres.c:4653 #19 0x170dbce in BackendRun ../src/backend/postmaster/postmaster.c:4464 #20 0x170bf70 in BackendStartup ../src/backend/postmaster/postmaster.c:4140 #21 0x170263f in ServerLoop ../src/backend/postmaster/postmaster.c:1776 #22 0x1701052 in PostmasterMain ../src/backend/postmaster/postmaster.c:1475 #23 0x11a5f67 in main ../src/backend/main/main.c:198 #24 0x7fac48e46149 in __libc_start_call_main (/lib64/libc.so.6+0x28149) (BuildId: 7ea8d85df0e89b90c63ac7ed2b3578b2e7728756) #25 0x7fac48e4620a in __libc_start_main_impl (/lib64/libc.so.6+0x2820a) (BuildId: 7ea8d85df0e89b90c63ac7ed2b3578b2e7728756) #26 0x49c5d4 in _start (/home/tristan957/Projects/work/postgresql/build/tmp_install/home/tristan957/.opt/postgresql/bin/postgres+0x49c5d4) (BuildId: c8ca341e1303be0f9dc0b0271c55c4b9e42af89b) SUMMARY: AddressSanitizer: 133 byte(s) leaked in 2 allocation(s). -- Tristan Partin Neon (https://neon.tech) Attachments: [application/sql] test.sql (33.0K, ../../[email protected]/2-test.sql) download [text/x-patch] v2-0001-Add-pg_attribute_no_asan.patch (952B, ../../[email protected]/3-v2-0001-Add-pg_attribute_no_asan.patch) download | inline diff: From 40f56251ef629eed2ebb6d9b2f875eac06678cab Mon Sep 17 00:00:00 2001 From: Tristan Partin <[email protected]> Date: Mon, 5 Feb 2024 20:18:59 -0600 Subject: [PATCH v2 1/4] Add pg_attribute_no_asan Putting this attribute on a function will disable AddressSanitizer instrumentation in a function. --- src/include/c.h | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/include/c.h b/src/include/c.h index 2e3ea206e1..11e7df0fd7 100644 --- a/src/include/c.h +++ b/src/include/c.h @@ -272,6 +272,13 @@ #endif /* defined(__MINGW64__) && __GNUC__ == 8 && * __GNUC_MINOR__ == 1 */ + +#if __SANITIZE_ADDRESS__ +#define pg_attribute_no_asan __attribute__((no_sanitize("address"))) +#else +#define pg_attribute_no_asan +#endif + /* * Mark a point as unreachable in a portable fashion. This should preferably * be something that the compiler understands, to aid code generation. -- Tristan Partin Neon (https://neon.tech) [text/x-patch] v2-0002-Fix-stack-depth-checking-with-AddressSanitizer-en.patch (1.6K, ../../[email protected]/4-v2-0002-Fix-stack-depth-checking-with-AddressSanitizer-en.patch) download | inline diff: From 7192614059f6a7bddc5e32df21e5956aa714b820 Mon Sep 17 00:00:00 2001 From: Tristan Partin <[email protected]> Date: Mon, 5 Feb 2024 20:21:13 -0600 Subject: [PATCH v2 2/4] Fix stack depth checking with AddressSanitizer enabled The AddressSanitizer instrumentation caused stack depth checking to fail. --- src/backend/tcop/postgres.c | 4 ++-- src/include/miscadmin.h | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c index 1a34bd3715..7cc18dbed9 100644 --- a/src/backend/tcop/postgres.c +++ b/src/backend/tcop/postgres.c @@ -3466,7 +3466,7 @@ ProcessInterrupts(void) * * Returns the old reference point, if any. */ -pg_stack_base_t +pg_stack_base_t pg_attribute_no_asan set_stack_base(void) { #ifndef HAVE__BUILTIN_FRAME_ADDRESS @@ -3530,7 +3530,7 @@ check_stack_depth(void) } } -bool +bool pg_attribute_no_asan stack_is_too_deep(void) { char stack_top_loc; diff --git a/src/include/miscadmin.h b/src/include/miscadmin.h index 0b01c1f093..538adfe757 100644 --- a/src/include/miscadmin.h +++ b/src/include/miscadmin.h @@ -292,10 +292,10 @@ extern PGDLLIMPORT bool VacuumCostActive; typedef char *pg_stack_base_t; -extern pg_stack_base_t set_stack_base(void); +extern pg_stack_base_t set_stack_base(void) pg_attribute_no_asan; extern void restore_stack_base(pg_stack_base_t base); extern void check_stack_depth(void); -extern bool stack_is_too_deep(void); +extern bool stack_is_too_deep(void) pg_attribute_no_asan; /* in tcop/utility.c */ extern void PreventCommandIfReadOnly(const char *cmdname); -- Tristan Partin Neon (https://neon.tech) [text/x-patch] v2-0003-Add-libpgbincommon.patch (1.7K, ../../[email protected]/5-v2-0003-Add-libpgbincommon.patch) download | inline diff: From f40310fddfc361d5ed24ec1fb5043ce211179e35 Mon Sep 17 00:00:00 2001 From: Tristan Partin <[email protected]> Date: Mon, 5 Feb 2024 20:24:27 -0600 Subject: [PATCH v2 3/4] Add libpgbincommon This library is supposed to be linked wholly into executables. It defines a function which turns leak detection off when using the AddressSanitizer. --- src/bin/common/asan.c | 12 ++++++++++++ src/bin/common/meson.build | 8 ++++++++ src/bin/meson.build | 1 + 3 files changed, 21 insertions(+) create mode 100644 src/bin/common/asan.c create mode 100644 src/bin/common/meson.build diff --git a/src/bin/common/asan.c b/src/bin/common/asan.c new file mode 100644 index 0000000000..d2739b2ad7 --- /dev/null +++ b/src/bin/common/asan.c @@ -0,0 +1,12 @@ +/* + * Copyright (c) 2024, PostgreSQL Global Development Group + * + * src/bin/common/asan.c + */ + +const char *__asan_default_options(void); + +const char *__asan_default_options(void) +{ + return "detect_leaks=0"; +} diff --git a/src/bin/common/meson.build b/src/bin/common/meson.build new file mode 100644 index 0000000000..f63b9d664b --- /dev/null +++ b/src/bin/common/meson.build @@ -0,0 +1,8 @@ +# Copyright (c) 2024, PostgreSQL Global Development Group + +libpgbincommon = static_library( + 'pgbincommon', + 'asan.c', +) + +libpgbincommon_dep = declare_dependency(link_whole: libpgbincommon) diff --git a/src/bin/meson.build b/src/bin/meson.build index aa60ebaa30..fb18d618be 100644 --- a/src/bin/meson.build +++ b/src/bin/meson.build @@ -1,5 +1,6 @@ # Copyright (c) 2022-2024, PostgreSQL Global Development Group +subdir('common') subdir('initdb') subdir('pg_amcheck') subdir('pg_archivecleanup') -- Tristan Partin Neon (https://neon.tech) [text/x-patch] v2-0004-Add-instrumentation-to-disable-the-LeakSanitizer-.patch (18.5K, ../../[email protected]/6-v2-0004-Add-instrumentation-to-disable-the-LeakSanitizer-.patch) download | inline diff: From 0c1b9bf1ab5c178ffa4c731b541deedfc9a552ed Mon Sep 17 00:00:00 2001 From: Tristan Partin <[email protected]> Date: Mon, 5 Feb 2024 20:29:38 -0600 Subject: [PATCH v2 4/4] Add instrumentation to disable the LeakSanitizer in various paths We knowingly leak memory in certain executables, so we should turn leak detection off. Other code paths also leak memory, like plpython, when registering global data structures. --- leak.supp | 4 ++++ meson.build | 3 ++- src/bin/initdb/meson.build | 2 +- src/bin/pg_amcheck/meson.build | 2 +- src/bin/pg_basebackup/meson.build | 2 +- src/bin/pg_combinebackup/meson.build | 2 +- src/bin/pg_config/meson.build | 2 +- src/bin/pg_controldata/meson.build | 2 +- src/bin/pg_ctl/meson.build | 2 +- src/bin/pg_dump/meson.build | 6 +++--- src/bin/pg_resetwal/meson.build | 2 +- src/bin/pg_rewind/meson.build | 2 +- src/bin/pg_upgrade/meson.build | 2 +- src/bin/pg_verifybackup/meson.build | 2 +- src/bin/pg_waldump/meson.build | 2 +- src/bin/pg_walsummary/meson.build | 2 +- src/bin/pgbench/meson.build | 2 +- src/bin/psql/meson.build | 2 +- src/bin/scripts/meson.build | 2 +- src/interfaces/ecpg/preproc/meson.build | 2 +- src/interfaces/ecpg/test/meson.build | 4 ++-- src/interfaces/ecpg/test/thread/meson.build | 2 +- src/interfaces/libpq/test/meson.build | 2 +- src/pl/plpython/plpy_exec.c | 12 ++++++++++++ src/pl/plpython/plpy_main.c | 14 ++++++++++++++ src/pl/plpython/plpy_procedure.c | 14 ++++++++++++++ src/test/isolation/meson.build | 4 ++-- src/test/modules/libpq_pipeline/meson.build | 2 +- src/test/regress/meson.build | 2 +- 29 files changed, 74 insertions(+), 29 deletions(-) create mode 100644 leak.supp diff --git a/leak.supp b/leak.supp new file mode 100644 index 0000000000..d72c5b368f --- /dev/null +++ b/leak.supp @@ -0,0 +1,4 @@ +leak:PyImport_ImportModule +leak:RAND_status +leak:save_ps_display_args +leak:xmlNewNode diff --git a/meson.build b/meson.build index 8ed51b6aae..03fe3e0d89 100644 --- a/meson.build +++ b/meson.build @@ -3100,6 +3100,7 @@ if library_path_var != '' test_env.prepend(library_path_var, test_install_location / get_option('libdir')) endif +test_env.set('LSAN_OPTIONS', 'print_suppressions=0:suppressions=@0@'.format(meson.source_root() / 'leak.supp')) # Create (and remove old) initdb template directory. Tests use that, where # possible, to make it cheaper to run tests. @@ -3215,7 +3216,7 @@ foreach test_dir : tests test_kwargs = { 'protocol': 'tap', 'priority': 10, - 'timeout': 1000, + 'timeout': get_option('b_sanitize') == '' ? 1000 : 2000, 'depends': test_deps + t.get('deps', []), 'env': env, } + t.get('test_kwargs', {}) diff --git a/src/bin/initdb/meson.build b/src/bin/initdb/meson.build index 7dc5ed6e77..7afe6cd326 100644 --- a/src/bin/initdb/meson.build +++ b/src/bin/initdb/meson.build @@ -21,7 +21,7 @@ initdb = executable('initdb', # shared library from a different PG version. Define # USE_PRIVATE_ENCODING_FUNCS to ensure that that happens. c_args: ['-DUSE_PRIVATE_ENCODING_FUNCS'], - dependencies: [frontend_code, libpq, icu, icu_i18n], + dependencies: [frontend_code, libpq, icu, icu_i18n, libpgbincommon_dep], kwargs: default_bin_args, ) bin_targets += initdb diff --git a/src/bin/pg_amcheck/meson.build b/src/bin/pg_amcheck/meson.build index 292b33eb09..8bcaab36db 100644 --- a/src/bin/pg_amcheck/meson.build +++ b/src/bin/pg_amcheck/meson.build @@ -12,7 +12,7 @@ endif pg_amcheck = executable('pg_amcheck', pg_amcheck_sources, - dependencies: [frontend_code, libpq], + dependencies: [frontend_code, libpq, libpgbincommon_dep], kwargs: default_bin_args, ) bin_targets += pg_amcheck diff --git a/src/bin/pg_basebackup/meson.build b/src/bin/pg_basebackup/meson.build index f7e60e6670..157fe95c0c 100644 --- a/src/bin/pg_basebackup/meson.build +++ b/src/bin/pg_basebackup/meson.build @@ -12,7 +12,7 @@ common_sources = files( 'walmethods.c', ) -pg_basebackup_deps = [frontend_code, libpq, lz4, zlib, zstd] +pg_basebackup_deps = [frontend_code, libpq, libpgbincommon_dep, lz4, zlib, zstd] pg_basebackup_common = static_library('libpg_basebackup_common', common_sources, dependencies: pg_basebackup_deps, diff --git a/src/bin/pg_combinebackup/meson.build b/src/bin/pg_combinebackup/meson.build index 30dbbaa6cf..ff7e4e796e 100644 --- a/src/bin/pg_combinebackup/meson.build +++ b/src/bin/pg_combinebackup/meson.build @@ -17,7 +17,7 @@ endif pg_combinebackup = executable('pg_combinebackup', pg_combinebackup_sources, - dependencies: [frontend_code], + dependencies: [frontend_code, libpgbincommon_dep], kwargs: default_bin_args, ) bin_targets += pg_combinebackup diff --git a/src/bin/pg_config/meson.build b/src/bin/pg_config/meson.build index b4fddd297a..9d717072d4 100644 --- a/src/bin/pg_config/meson.build +++ b/src/bin/pg_config/meson.build @@ -12,7 +12,7 @@ endif pg_config = executable('pg_config', pg_config_sources, - dependencies: [frontend_code], + dependencies: [frontend_code, libpgbincommon_dep], kwargs: default_bin_args, ) bin_targets += pg_config diff --git a/src/bin/pg_controldata/meson.build b/src/bin/pg_controldata/meson.build index 8009bd8c9a..ff54169084 100644 --- a/src/bin/pg_controldata/meson.build +++ b/src/bin/pg_controldata/meson.build @@ -12,7 +12,7 @@ endif pg_controldata = executable('pg_controldata', pg_controldata_sources, - dependencies: [frontend_code], + dependencies: [frontend_code, libpgbincommon_dep], kwargs: default_bin_args, ) bin_targets += pg_controldata diff --git a/src/bin/pg_ctl/meson.build b/src/bin/pg_ctl/meson.build index da05bd8a8d..da013dec86 100644 --- a/src/bin/pg_ctl/meson.build +++ b/src/bin/pg_ctl/meson.build @@ -12,7 +12,7 @@ endif pg_ctl = executable('pg_ctl', pg_ctl_sources, - dependencies: [frontend_code, libpq], + dependencies: [frontend_code, libpq, libpgbincommon_dep], kwargs: default_bin_args, ) bin_targets += pg_ctl diff --git a/src/bin/pg_dump/meson.build b/src/bin/pg_dump/meson.build index ecd0a0a0e1..3e465f9711 100644 --- a/src/bin/pg_dump/meson.build +++ b/src/bin/pg_dump/meson.build @@ -41,7 +41,7 @@ endif pg_dump = executable('pg_dump', pg_dump_sources, link_with: [pg_dump_common], - dependencies: [frontend_code, libpq, zlib], + dependencies: [frontend_code, libpq, libpgbincommon_dep, zlib], kwargs: default_bin_args, ) bin_targets += pg_dump @@ -60,7 +60,7 @@ endif pg_dumpall = executable('pg_dumpall', pg_dumpall_sources, link_with: [pg_dump_common], - dependencies: [frontend_code, libpq, zlib], + dependencies: [frontend_code, libpq, libpgbincommon_dep, zlib], kwargs: default_bin_args, ) bin_targets += pg_dumpall @@ -79,7 +79,7 @@ endif pg_restore = executable('pg_restore', pg_restore_sources, link_with: [pg_dump_common], - dependencies: [frontend_code, libpq, zlib], + dependencies: [frontend_code, libpq, libpgbincommon_dep, zlib], kwargs: default_bin_args, ) bin_targets += pg_restore diff --git a/src/bin/pg_resetwal/meson.build b/src/bin/pg_resetwal/meson.build index c1239528db..d1397fb7ce 100644 --- a/src/bin/pg_resetwal/meson.build +++ b/src/bin/pg_resetwal/meson.build @@ -12,7 +12,7 @@ endif pg_resetwal = executable('pg_resetwal', pg_resetwal_sources, - dependencies: [frontend_code], + dependencies: [frontend_code, libpgbincommon_dep], kwargs: default_bin_args, ) bin_targets += pg_resetwal diff --git a/src/bin/pg_rewind/meson.build b/src/bin/pg_rewind/meson.build index e0f88bde22..75b74e89ae 100644 --- a/src/bin/pg_rewind/meson.build +++ b/src/bin/pg_rewind/meson.build @@ -21,7 +21,7 @@ endif pg_rewind = executable('pg_rewind', pg_rewind_sources, - dependencies: [frontend_code, libpq, lz4, zstd], + dependencies: [frontend_code, libpq, libpgbincommon_dep, lz4, zstd], c_args: ['-DFRONTEND'], # needed for xlogreader et al kwargs: default_bin_args, ) diff --git a/src/bin/pg_upgrade/meson.build b/src/bin/pg_upgrade/meson.build index 9825fa3305..11e55a2bbe 100644 --- a/src/bin/pg_upgrade/meson.build +++ b/src/bin/pg_upgrade/meson.build @@ -27,7 +27,7 @@ endif pg_upgrade = executable('pg_upgrade', pg_upgrade_sources, c_pch: pch_postgres_fe_h, - dependencies: [frontend_code, libpq], + dependencies: [frontend_code, libpq, libpgbincommon_dep], kwargs: default_bin_args, ) bin_targets += pg_upgrade diff --git a/src/bin/pg_verifybackup/meson.build b/src/bin/pg_verifybackup/meson.build index 7c7d31a035..13c583ef55 100644 --- a/src/bin/pg_verifybackup/meson.build +++ b/src/bin/pg_verifybackup/meson.build @@ -12,7 +12,7 @@ endif pg_verifybackup = executable('pg_verifybackup', pg_verifybackup_sources, - dependencies: [frontend_code, libpq], + dependencies: [frontend_code, libpq, libpgbincommon_dep], kwargs: default_bin_args, ) bin_targets += pg_verifybackup diff --git a/src/bin/pg_waldump/meson.build b/src/bin/pg_waldump/meson.build index bb30f0fe08..05d23f7fee 100644 --- a/src/bin/pg_waldump/meson.build +++ b/src/bin/pg_waldump/meson.build @@ -18,7 +18,7 @@ endif pg_waldump = executable('pg_waldump', pg_waldump_sources, - dependencies: [frontend_code, lz4, zstd], + dependencies: [frontend_code, libpgbincommon_dep, lz4, zstd], c_args: ['-DFRONTEND'], # needed for xlogreader et al kwargs: default_bin_args, ) diff --git a/src/bin/pg_walsummary/meson.build b/src/bin/pg_walsummary/meson.build index f886a4cb36..bac71a5776 100644 --- a/src/bin/pg_walsummary/meson.build +++ b/src/bin/pg_walsummary/meson.build @@ -12,7 +12,7 @@ endif pg_walsummary = executable('pg_walsummary', pg_walsummary_sources, - dependencies: [frontend_code], + dependencies: [frontend_code, libpgbincommon_dep], kwargs: default_bin_args, ) bin_targets += pg_walsummary diff --git a/src/bin/pgbench/meson.build b/src/bin/pgbench/meson.build index d330bb5eb0..5a4eab226f 100644 --- a/src/bin/pgbench/meson.build +++ b/src/bin/pgbench/meson.build @@ -27,7 +27,7 @@ endif pgbench = executable('pgbench', pgbench_sources, - dependencies: [frontend_code, libpq, thread_dep], + dependencies: [frontend_code, libpq, libpgbincommon_dep, thread_dep], include_directories: include_directories('.'), c_pch: pch_postgres_fe_h, c_args: host_system == 'windows' ? ['-DFD_SETSIZE=1024'] : [], diff --git a/src/bin/psql/meson.build b/src/bin/psql/meson.build index f3a6392138..9614a1fbd0 100644 --- a/src/bin/psql/meson.build +++ b/src/bin/psql/meson.build @@ -48,7 +48,7 @@ psql = executable('psql', psql_sources, c_pch: pch_postgres_fe_h, include_directories: include_directories('.'), - dependencies: [frontend_code, libpq, readline], + dependencies: [frontend_code, libpq, libpgbincommon_dep, readline], kwargs: default_bin_args, ) bin_targets += psql diff --git a/src/bin/scripts/meson.build b/src/bin/scripts/meson.build index cef24d7806..dc06580f9e 100644 --- a/src/bin/scripts/meson.build +++ b/src/bin/scripts/meson.build @@ -29,7 +29,7 @@ foreach binary : binaries binary = executable(binary, binary_sources, link_with: [scripts_common], - dependencies: [frontend_code, libpq], + dependencies: [frontend_code, libpq, libpgbincommon_dep], kwargs: default_bin_args, ) bin_targets += binary diff --git a/src/interfaces/ecpg/preproc/meson.build b/src/interfaces/ecpg/preproc/meson.build index ddd7a66547..fc21e3fc36 100644 --- a/src/interfaces/ecpg/preproc/meson.build +++ b/src/interfaces/ecpg/preproc/meson.build @@ -94,7 +94,7 @@ ecpg_exe = executable('ecpg', ecpg_sources, include_directories: ['.', ecpg_inc, postgres_inc, libpq_inc], c_pch: pch_postgres_fe_h, - dependencies: [frontend_code], + dependencies: [frontend_code, libpgbincommon_dep], kwargs: default_bin_args, ) ecpg_targets += ecpg_exe diff --git a/src/interfaces/ecpg/test/meson.build b/src/interfaces/ecpg/test/meson.build index c1e508ccc8..c4c5a3e602 100644 --- a/src/interfaces/ecpg/test/meson.build +++ b/src/interfaces/ecpg/test/meson.build @@ -18,7 +18,7 @@ pg_regress_ecpg = executable('pg_regress_ecpg', pg_regress_ecpg_sources, c_args: pg_regress_cflags, include_directories: [pg_regress_inc, include_directories('.')], - dependencies: [frontend_code, libpq], + dependencies: [frontend_code, libpq, libpgbincommon_dep], kwargs: default_bin_args + { 'install': false }, @@ -27,7 +27,7 @@ testprep_targets += pg_regress_ecpg # create .c files and executables from .pgc files ecpg_test_exec_kw = { - 'dependencies': [frontend_code, libpq], + 'dependencies': [frontend_code, libpq, libpgbincommon_dep], 'include_directories': [ecpg_inc], 'link_with': [ecpglib_so, ecpg_compat_so, ecpg_pgtypes_so], 'build_by_default': false, diff --git a/src/interfaces/ecpg/test/thread/meson.build b/src/interfaces/ecpg/test/thread/meson.build index 5ed67ccbcd..d7e590db61 100644 --- a/src/interfaces/ecpg/test/thread/meson.build +++ b/src/interfaces/ecpg/test/thread/meson.build @@ -18,6 +18,6 @@ foreach pgc_file : pgc_files ecpg_test_dependencies += executable(pgc_file, exe_input, - kwargs: ecpg_test_exec_kw + {'dependencies': [frontend_code, libpq, thread_dep,]}, + kwargs: ecpg_test_exec_kw + {'dependencies': [frontend_code, libpq, libpgbincommon_dep, thread_dep,]}, ) endforeach diff --git a/src/interfaces/libpq/test/meson.build b/src/interfaces/libpq/test/meson.build index 21dd37f69b..070c2cc72f 100644 --- a/src/interfaces/libpq/test/meson.build +++ b/src/interfaces/libpq/test/meson.build @@ -12,7 +12,7 @@ endif testprep_targets += executable('libpq_uri_regress', libpq_uri_regress_sources, - dependencies: [frontend_code, libpq], + dependencies: [frontend_code, libpq, libpgbincommon_dep], kwargs: default_bin_args + { 'install': false, } diff --git a/src/pl/plpython/plpy_exec.c b/src/pl/plpython/plpy_exec.c index e06fde1dd9..e7a8372b30 100644 --- a/src/pl/plpython/plpy_exec.c +++ b/src/pl/plpython/plpy_exec.c @@ -23,6 +23,10 @@ #include "utils/rel.h" #include "utils/typcache.h" +#ifdef __SANITIZE_ADDRESS__ +#include <sanitizer/lsan_interface.h> +#endif + /* saved state for a set-returning function */ typedef struct PLySRFState { @@ -1040,6 +1044,10 @@ PLy_procedure_call(PLyProcedure *proc, const char *kargs, PyObject *vargs) PG_TRY(); { +#ifdef __SANITIZE_ADDRESS__ + __lsan_disable(); +#endif + #if PY_VERSION_HEX >= 0x03020000 rv = PyEval_EvalCode(proc->code, proc->globals, proc->globals); @@ -1048,6 +1056,10 @@ PLy_procedure_call(PLyProcedure *proc, const char *kargs, PyObject *vargs) proc->globals, proc->globals); #endif +#ifdef __SANITIZE_ADDRESS__ + __lsan_enable(); +#endif + /* * Since plpy will only let you close subtransactions that you * started, you cannot *unnest* subtransactions, only *nest* them diff --git a/src/pl/plpython/plpy_main.c b/src/pl/plpython/plpy_main.c index 010a97378c..86d409ca35 100644 --- a/src/pl/plpython/plpy_main.c +++ b/src/pl/plpython/plpy_main.c @@ -24,6 +24,10 @@ #include "utils/rel.h" #include "utils/syscache.h" +#ifdef __SANITIZE_ADDRESS__ +#include <sanitizer/lsan_interface.h> +#endif + /* * exported functions */ @@ -113,14 +117,24 @@ PLy_initialize(void) if (inited) return; +#ifdef __SANITIZE_ADDRESS__ + __lsan_disable(); +#endif + PyImport_AppendInittab("plpy", PyInit_plpy); Py_Initialize(); PyImport_ImportModule("plpy"); PLy_init_interp(); PLy_init_plpy(); + +#ifdef __SANITIZE_ADDRESS__ + __lsan_enable(); +#endif + if (PyErr_Occurred()) PLy_elog(FATAL, "untrapped error in initialization"); + init_procedure_caches(); explicit_subtransactions = NIL; diff --git a/src/pl/plpython/plpy_procedure.c b/src/pl/plpython/plpy_procedure.c index 79b6ef6a44..2ff4337d2a 100644 --- a/src/pl/plpython/plpy_procedure.c +++ b/src/pl/plpython/plpy_procedure.c @@ -22,6 +22,10 @@ #include "utils/memutils.h" #include "utils/syscache.h" +#ifdef __SANITIZE_ADDRESS__ +#include <sanitizer/lsan_interface.h> +#endif + static HTAB *PLy_procedure_cache = NULL; static PLyProcedure *PLy_procedure_create(HeapTuple procTup, Oid fn_oid, bool is_trigger); @@ -370,7 +374,17 @@ PLy_procedure_compile(PLyProcedure *proc, const char *src) msrc = PLy_procedure_munge_source(proc->pyname, src); /* Save the mangled source for later inclusion in tracebacks */ proc->src = MemoryContextStrdup(proc->mcxt, msrc); + +#ifdef __SANITIZE_ADDRESS__ + __lsan_disable(); +#endif + crv = PyRun_String(msrc, Py_file_input, proc->globals, NULL); + +#ifdef __SANITIZE_ADDRESS__ + __lsan_enable(); +#endif + pfree(msrc); if (crv != NULL) diff --git a/src/test/isolation/meson.build b/src/test/isolation/meson.build index 1082887a44..e2c54ba268 100644 --- a/src/test/isolation/meson.build +++ b/src/test/isolation/meson.build @@ -35,7 +35,7 @@ pg_isolation_regress = executable('pg_isolation_regress', isolation_sources, c_args: pg_regress_cflags, include_directories: pg_regress_inc, - dependencies: [frontend_code, libpq], + dependencies: [frontend_code, libpq, libpgbincommon_dep], kwargs: default_bin_args + { 'install_dir': dir_pgxs / 'src/test/isolation', }, @@ -52,7 +52,7 @@ endif isolationtester = executable('isolationtester', isolationtester_sources, include_directories: include_directories('.'), - dependencies: [frontend_code, libpq], + dependencies: [frontend_code, libpq, libpgbincommon_dep], kwargs: default_bin_args + { 'install_dir': dir_pgxs / 'src/test/isolation', }, diff --git a/src/test/modules/libpq_pipeline/meson.build b/src/test/modules/libpq_pipeline/meson.build index 727963ee68..4aa732f09d 100644 --- a/src/test/modules/libpq_pipeline/meson.build +++ b/src/test/modules/libpq_pipeline/meson.build @@ -12,7 +12,7 @@ endif libpq_pipeline = executable('libpq_pipeline', libpq_pipeline_sources, - dependencies: [frontend_code, libpq], + dependencies: [frontend_code, libpq, libpgbincommon_dep], kwargs: default_bin_args + { 'install': false, }, diff --git a/src/test/regress/meson.build b/src/test/regress/meson.build index 5a9be73531..a2a9b87e3f 100644 --- a/src/test/regress/meson.build +++ b/src/test/regress/meson.build @@ -30,7 +30,7 @@ endif pg_regress = executable('pg_regress', regress_sources, c_args: pg_regress_cflags, - dependencies: [frontend_code, libpq], + dependencies: [frontend_code, libpq, libpgbincommon_dep], kwargs: default_bin_args + { 'install_dir': dir_pgxs / 'src/test/regress', }, -- Tristan Partin Neon (https://neon.tech) ^ permalink raw reply [nested|flat] 30+ messages in thread
* Re: Fix some ubsan/asan related issues @ 2024-09-16 13:29 Junwang Zhao <[email protected]> parent: Tristan Partin <[email protected]> 0 siblings, 0 replies; 30+ messages in thread From: Junwang Zhao @ 2024-09-16 13:29 UTC (permalink / raw) To: Tristan Partin <[email protected]>; +Cc: Andres Freund <[email protected]>; Alexander Lakhin <[email protected]>; pgsql-hackers; Heikki Linnakangas <[email protected]> Hi Tristan, On Tue, Feb 6, 2024 at 11:53 AM Tristan Partin <[email protected]> wrote: > > On Tue Jan 30, 2024 at 3:58 PM CST, Andres Freund wrote: > > Hi, > > > > On 2024-01-30 09:59:25 -0600, Tristan Partin wrote: > > > From 331cec1c9db6ff60dcc6d9ba62a9c8be4e5e95ed Mon Sep 17 00:00:00 2001 > > > From: Tristan Partin <[email protected]> > > > Date: Mon, 29 Jan 2024 18:03:39 -0600 > > > Subject: [PATCH v1 1/3] Refuse to register message in LogLogicalMessage if > > > NULL > > > > > If this occurs, the memcpy of rdata_data in CopyXLogRecordToWAL breaks > > > the API contract of memcpy in glibc. The two pointer arguments are > > > marked as nonnull, even in the event the amount to copy is 0 bytes. > > > > It seems pretty odd to call LogLogicalMessage() with a NULL argument. Why is > > that something useful? > > Dropped. Will change on the Neon side. Should we add an assert > somewhere for good measure? Near the memcpy in question? > > > > From dc9488f3fdee69b981b52c985fb77106d7d301ff Mon Sep 17 00:00:00 2001 > > > From: Tristan Partin <[email protected]> > > > Date: Wed, 24 Jan 2024 17:07:01 -0600 > > > Subject: [PATCH v1 2/3] meson: Support compiling with -Db_sanitize=address > > > > > > The ecpg is parser is extremely leaky, so we need to silence leak > > > detection. > > > > This does stuff beyond epcg... > > Dropped. > > > > +if get_option('b_sanitize').contains('address') > > > + cdata.set('USE_ADDRESS_SANITIZER', 1) > > > +endif > > > > > > ############################################################### > > > # NLS / Gettext > > > diff --git a/src/bin/initdb/initdb.c b/src/bin/initdb/initdb.c > > > index ac409b0006..e18e716d9c 100644 > > > --- a/src/bin/initdb/initdb.c > > > +++ b/src/bin/initdb/initdb.c > > > @@ -338,6 +338,17 @@ do { \ > > > output_failed = true, output_errno = errno; \ > > > } while (0) > > > > > > +#ifdef USE_ADDRESS_SANITIZER > > > > When asan is used __SANITIZE_ADDRESS__ is defined, so we don't need to > > implement this ourselves. > > Thanks! > > > > +const char *__asan_default_options(void); > > > + > > > +const char *__asan_default_options(void) > > > +{ > > > + return "detect_leaks=0"; > > > +} > > > + > > > +#endif > > > > Wonder if we should move this into some static library and link it into all > > binaries that don't want leak detection? It doesn't seem great to have to > > adjust this in a bunch of files if we want to adjust the options... > > See attached patches. Here is what I found to be necessary to get > a -Db_sanitize=address,undefined build to successfully make it through > tests. I do have a few concerns about the patch. > > 1. For whatever reason, __SANITIZE_LEAK__ is not defined when the leak > sanitizer is enabled. So you will see me use this, to make some > include directives work. I don't like this as a final solution > because someone could use -fsanitize=leak. > 2. I tracked down what seems to be a valid leak in adt/xml.c. Attached > (test.sql) is a fairly minimal reproduction of what is needed to show > the leak. I didn't spend too much time tracking it down. Might get to > it later, who knows. Below you will find the backtrace, and whoever > wants to try their hand at fixing it will need to comment out > xmlNewNode in the leak.supp file. > 3. I don't love the new library name. Maybe it should be name more lsan > specific. > 4. Should pg_attribute_no_asan be renamed to > pg_attribute_no_sanitize_address? That would match > pg_attribute_no_sanitize_alignment. > > I will also attach a Meson test log for good measure. I didn't try > testing anything that requires PG_TEST_EXTRA, but I suspect that > everything will be fine. > > Alexander, I haven't yet gotten to the things you pointed out in the > sibling thread. > > ==221848==ERROR: LeakSanitizer: detected memory leaks > > Direct leak of 120 byte(s) in 1 object(s) allocated from: > #0 0x7fac4a6d92ef in malloc (/lib64/libasan.so.8+0xd92ef) (BuildId: 7fcb7759bc17ef47f9682414b6d99732d6a6ab0c) > #1 0x7fac4a48427d in xmlNewNode (/lib64/libxml2.so.2+0x5d27d) (BuildId: 3074681c8fa9b17e0cbed09bc61c25ada5c28e7c) > #2 0x22107a6 in xmltotext_with_options ../src/backend/utils/adt/xml.c:754 > #3 0xead047 in ExecEvalXmlExpr ../src/backend/executor/execExprInterp.c:4020 > #4 0xe8c119 in ExecInterpExpr ../src/backend/executor/execExprInterp.c:1537 > #5 0xe91f2c in ExecInterpExprStillValid ../src/backend/executor/execExprInterp.c:1881 > #6 0x109632d in ExecEvalExprSwitchContext ../src/include/executor/executor.h:355 > #7 0x109655d in ExecProject ../src/include/executor/executor.h:389 > #8 0x1097186 in ExecResult ../src/backend/executor/nodeResult.c:136 > #9 0xf0f90c in ExecProcNodeFirst ../src/backend/executor/execProcnode.c:464 > #10 0xec9bec in ExecProcNode ../src/include/executor/executor.h:273 > #11 0xed875c in ExecutePlan ../src/backend/executor/execMain.c:1670 > #12 0xecbee0 in standard_ExecutorRun ../src/backend/executor/execMain.c:365 > #13 0xecb529 in ExecutorRun ../src/backend/executor/execMain.c:309 > #14 0x1ae89f6 in PortalRunSelect ../src/backend/tcop/pquery.c:924 > #15 0x1ae7c06 in PortalRun ../src/backend/tcop/pquery.c:768 > #16 0x1ad1b43 in exec_simple_query ../src/backend/tcop/postgres.c:1273 > #17 0x1adf8de in PostgresMain ../src/backend/tcop/postgres.c:4653 > #18 0x170dbce in BackendRun ../src/backend/postmaster/postmaster.c:4464 > #19 0x170bf70 in BackendStartup ../src/backend/postmaster/postmaster.c:4140 > #20 0x170263f in ServerLoop ../src/backend/postmaster/postmaster.c:1776 > #21 0x1701052 in PostmasterMain ../src/backend/postmaster/postmaster.c:1475 > #22 0x11a5f67 in main ../src/backend/main/main.c:198 > #23 0x7fac48e46149 in __libc_start_call_main (/lib64/libc.so.6+0x28149) (BuildId: 7ea8d85df0e89b90c63ac7ed2b3578b2e7728756) > #24 0x7fac48e4620a in __libc_start_main_impl (/lib64/libc.so.6+0x2820a) (BuildId: 7ea8d85df0e89b90c63ac7ed2b3578b2e7728756) > #25 0x49c5d4 in _start (/home/tristan957/Projects/work/postgresql/build/tmp_install/home/tristan957/.opt/postgresql/bin/postgres+0x49c5d4) (BuildId: c8ca341e1303be0f9dc0b0271c55c4b9e42af89b) > > Indirect leak of 13 byte(s) in 1 object(s) allocated from: > #0 0x7fac4a6d92ef in malloc (/lib64/libasan.so.8+0xd92ef) (BuildId: 7fcb7759bc17ef47f9682414b6d99732d6a6ab0c) > #1 0x7fac4a4e106f in xmlStrndup (/lib64/libxml2.so.2+0xba06f) (BuildId: 3074681c8fa9b17e0cbed09bc61c25ada5c28e7c) > #2 0x7fac4a4842c0 in xmlNewNode (/lib64/libxml2.so.2+0x5d2c0) (BuildId: 3074681c8fa9b17e0cbed09bc61c25ada5c28e7c) > #3 0x22107a6 in xmltotext_with_options ../src/backend/utils/adt/xml.c:754 > #4 0xead047 in ExecEvalXmlExpr ../src/backend/executor/execExprInterp.c:4020 > #5 0xe8c119 in ExecInterpExpr ../src/backend/executor/execExprInterp.c:1537 > #6 0xe91f2c in ExecInterpExprStillValid ../src/backend/executor/execExprInterp.c:1881 > #7 0x109632d in ExecEvalExprSwitchContext ../src/include/executor/executor.h:355 > #8 0x109655d in ExecProject ../src/include/executor/executor.h:389 > #9 0x1097186 in ExecResult ../src/backend/executor/nodeResult.c:136 > #10 0xf0f90c in ExecProcNodeFirst ../src/backend/executor/execProcnode.c:464 > #11 0xec9bec in ExecProcNode ../src/include/executor/executor.h:273 > #12 0xed875c in ExecutePlan ../src/backend/executor/execMain.c:1670 > #13 0xecbee0 in standard_ExecutorRun ../src/backend/executor/execMain.c:365 > #14 0xecb529 in ExecutorRun ../src/backend/executor/execMain.c:309 > #15 0x1ae89f6 in PortalRunSelect ../src/backend/tcop/pquery.c:924 > #16 0x1ae7c06 in PortalRun ../src/backend/tcop/pquery.c:768 > #17 0x1ad1b43 in exec_simple_query ../src/backend/tcop/postgres.c:1273 > #18 0x1adf8de in PostgresMain ../src/backend/tcop/postgres.c:4653 > #19 0x170dbce in BackendRun ../src/backend/postmaster/postmaster.c:4464 > #20 0x170bf70 in BackendStartup ../src/backend/postmaster/postmaster.c:4140 > #21 0x170263f in ServerLoop ../src/backend/postmaster/postmaster.c:1776 > #22 0x1701052 in PostmasterMain ../src/backend/postmaster/postmaster.c:1475 > #23 0x11a5f67 in main ../src/backend/main/main.c:198 > #24 0x7fac48e46149 in __libc_start_call_main (/lib64/libc.so.6+0x28149) (BuildId: 7ea8d85df0e89b90c63ac7ed2b3578b2e7728756) > #25 0x7fac48e4620a in __libc_start_main_impl (/lib64/libc.so.6+0x2820a) (BuildId: 7ea8d85df0e89b90c63ac7ed2b3578b2e7728756) > #26 0x49c5d4 in _start (/home/tristan957/Projects/work/postgresql/build/tmp_install/home/tristan957/.opt/postgresql/bin/postgres+0x49c5d4) (BuildId: c8ca341e1303be0f9dc0b0271c55c4b9e42af89b) > > SUMMARY: AddressSanitizer: 133 byte(s) leaked in 2 allocation(s). > > -- > Tristan Partin > Neon (https://neon.tech) I tried your v1-0002, it works at compile phase but failed to run initdb with the following leak detected: ================================================================= ==64983==ERROR: LeakSanitizer: detected memory leaks Direct leak of 248 byte(s) in 1 object(s) allocated from: #0 0x7fc7729df9cf in __interceptor_malloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:69 #1 0x55bff5480e8b in save_ps_display_args ../postgres/src/backend/utils/misc/ps_status.c:190 #2 0x55bff4a5a298 in main ../postgres/src/backend/main/main.c:90 #3 0x7fc771924249 in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58 Indirect leak of 19 byte(s) in 1 object(s) allocated from: #0 0x7fc77299777b in __interceptor_strdup ../../../../src/libsanitizer/asan/asan_interceptors.cpp:439 #1 0x55bff5480f41 in save_ps_display_args ../postgres/src/backend/utils/misc/ps_status.c:198 #2 0x55bff4a5a298 in main ../postgres/src/backend/main/main.c:90 #3 0x7fc771924249 in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58 I worked around by moving *new_environ* as a global variable, I think this is false positive report so maybe this deserves a patch? I tried to apply your v2 patch set but v2-0004 seems out of date. -- Regards Junwang Zhao ^ permalink raw reply [nested|flat] 30+ messages in thread
end of thread, other threads:[~2024-09-16 13:29 UTC | newest] Thread overview: 30+ messages (download: mbox mbox.gz follow: Atom feed) -- links below jump to the message on this page -- 2017-05-31 06:44 Re: Range Merge Join v1 Andrew Borodin <[email protected]> 2017-06-02 14:42 ` Jeff Davis <[email protected]> 2017-06-03 06:26 ` Andrew Borodin <[email protected]> 2019-04-04 23:57 [PATCH v6 07/10] is vs are plural Justin Pryzby <[email protected]> 2019-04-04 23:57 [PATCH v8 06/14] is vs are plural Justin Pryzby <[email protected]> 2019-04-04 23:57 [PATCH v6 07/10] is vs are plural Justin Pryzby <[email protected]> 2019-04-04 23:57 [PATCH v6 07/10] is vs are plural Justin Pryzby <[email protected]> 2019-04-04 23:57 [PATCH v6 07/10] is vs are plural Justin Pryzby <[email protected]> 2019-04-04 23:57 [PATCH v6 07/10] is vs are plural Justin Pryzby <[email protected]> 2019-04-04 23:57 [PATCH v6 07/10] is vs are plural Justin Pryzby <[email protected]> 2019-04-04 23:57 [PATCH v6 07/10] is vs are plural Justin Pryzby <[email protected]> 2019-04-04 23:57 [PATCH v6 07/10] is vs are plural Justin Pryzby <[email protected]> 2019-04-04 23:57 [PATCH v2 12/12] is vs are plural Justin Pryzby <[email protected]> 2019-04-04 23:57 [PATCH v4 10/12] is vs are plural Justin Pryzby <[email protected]> 2019-04-04 23:57 [PATCH v6 07/10] is vs are plural Justin Pryzby <[email protected]> 2019-04-04 23:57 [PATCH v7 06/10] is vs are plural Justin Pryzby <[email protected]> 2019-04-04 23:57 [PATCH v6 07/10] is vs are plural Justin Pryzby <[email protected]> 2019-04-04 23:57 [PATCH v3 17/20] is vs are plural Justin Pryzby <[email protected]> 2019-04-04 23:57 [PATCH v6 07/10] is vs are plural Justin Pryzby <[email protected]> 2019-04-04 23:57 [PATCH v6 07/10] is vs are plural Justin Pryzby <[email protected]> 2019-04-04 23:57 [PATCH v6 07/10] is vs are plural Justin Pryzby <[email protected]> 2019-04-04 23:57 [PATCH v6 07/10] is vs are plural Justin Pryzby <[email protected]> 2019-04-04 23:57 [PATCH v6 07/10] is vs are plural Justin Pryzby <[email protected]> 2019-04-04 23:57 [PATCH v6 07/10] is vs are plural Justin Pryzby <[email protected]> 2019-04-04 23:57 [PATCH v1 19/19] is vs are plural Justin Pryzby <[email protected]> 2019-04-04 23:57 [PATCH v6 07/10] is vs are plural Justin Pryzby <[email protected]> 2019-04-04 23:57 [PATCH v6 07/10] is vs are plural Justin Pryzby <[email protected]> 2019-04-04 23:57 [PATCH v6 07/10] is vs are plural Justin Pryzby <[email protected]> 2024-02-06 03:53 Re: Fix some ubsan/asan related issues Tristan Partin <[email protected]> 2024-09-16 13:29 ` Re: Fix some ubsan/asan related issues Junwang Zhao <[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