Cast json array to postgres array and preserve order of elements
When I want t to convert json array into postgres array, I do:
with t(j) as(
> select '{"my_arr":[3,1,2]}'::json
> )
> SELECT ARRAY(SELECT json_array_elements_text(j->'my_arr')) from t
It works like a charm and I never noticed any problem, but I'm asking here
just to make sure, order of elements will be preserved always?
Is that guaranteed in above example, or not?
Thanks.
Re: [PATCH] Incremental sort (was: PoC: Partial sort)
On Sat, May 09, 2020 at 06:48:02PM -0700, Peter Geoghegan wrote: On Sat, May 9, 2020 at 3:19 PM Tomas Vondra wrote: I'm generally OK with most of this - I'd probably keep the single-line format, but I don't feel very strongly about that and if others think using two lines is better ... Barring objections I'll get this polished and pushed soon-ish (say, early next week). I see something about starting a new thread on the Open Items page. Please CC me on this. Speaking in my capacity as an RMT member: Glad to see that you plan to close this item out next week. (I had planned on giving you a nudge about this, but it looks like I don't really have to now.) Not sure about about the new thread - the discussion continues on the main incremental sort thread, I don't see any proposal to start a new thread there. IMO it'd be pointless at this point. regards -- Tomas Vondra http://www.2ndQuadrant.com PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Re: valgrind error
On 4/18/20 9:15 AM, Andrew Dunstan wrote:
> I was just trying to revive lousyjack, my valgrind buildfarm animal
> which has been offline for 12 days, after having upgraded the machine
> (fedora 31, gcc 9.3.1, valgrind 3.15) and noticed lots of errors like this:
>
>
> 2020-04-17 19:26:03.483 EDT [63741:3] pg_regress LOG: statement: CREATE
> DATABASE "regression" TEMPLATE=template0
> ==63717== VALGRINDERROR-BEGIN
> ==63717== Use of uninitialised value of size 8
> ==63717== at 0xAC5BB5: pg_comp_crc32c_sb8 (pg_crc32c_sb8.c:82)
> ==63717== by 0x55A98B: XLogRecordAssemble (xloginsert.c:785)
> ==63717== by 0x55A268: XLogInsert (xloginsert.c:461)
> ==63717== by 0x8BC9E0: LogCurrentRunningXacts (standby.c:1005)
> ==63717== by 0x8BC8F9: LogStandbySnapshot (standby.c:961)
> ==63717== by 0x550CB3: CreateCheckPoint (xlog.c:8937)
> ==63717== by 0x82A3B2: CheckpointerMain (checkpointer.c:441)
> ==63717== by 0x56347D: AuxiliaryProcessMain (bootstrap.c:453)
> ==63717== by 0x83CA18: StartChildProcess (postmaster.c:5474)
> ==63717== by 0x83A120: reaper (postmaster.c:3045)
> ==63717== by 0x4874B1F: ??? (in /usr/lib64/libpthread-2.30.so)
> ==63717== by 0x5056F29: select (in /usr/lib64/libc-2.30.so)
> ==63717== by 0x8380A0: ServerLoop (postmaster.c:1691)
> ==63717== by 0x837A1F: PostmasterMain (postmaster.c:1400)
> ==63717== by 0x74A71D: main (main.c:210)
> ==63717== Uninitialised value was created by a stack allocation
> ==63717== at 0x8BC942: LogCurrentRunningXacts (standby.c:984)
> ==63717==
> ==63717== VALGRINDERROR-END
> {
>
> Memcheck:Value8
> fun:pg_comp_crc32c_sb8
> fun:XLogRecordAssemble
> fun:XLogInsert
> fun:LogCurrentRunningXacts
> fun:LogStandbySnapshot
> fun:CreateCheckPoint
> fun:CheckpointerMain
> fun:AuxiliaryProcessMain
> fun:StartChildProcess
> fun:reaper
> obj:/usr/lib64/libpthread-2.30.so
> fun:select
> fun:ServerLoop
> fun:PostmasterMain
> fun:main
> }
>
>
After many hours of testing I have a culprit for this. The error appears
with valgrind 3.15.0 with everything else held constant. 3.14.0 does
not produce the problem. So lousyjack will be back on the air before long.
Here are the build flags it's using:
CFLAGS=-Wall -Wmissing-prototypes -Wpointer-arith
-Wdeclaration-after-statement -Werror=vla -Wendif-labels
-Wmissing-format-attribute -Wformat-security -fno-strict-aliasing
-fwrapv -fexcess-precision=standard -Wno-format-truncation
-Wno-stringop-truncatio
n -g -fno-omit-frame-pointer -O0 -fPIC
CPPFLAGS=-DUSE_VALGRIND -DRELCACHE_FORCE_RELEASE -D_GNU_SOURCE
-I/usr/include/libxml2
and valgrind is invoked like this:
valgrind --quiet --trace-children=yes --track-origins=yes
--read-var-info=yes --num-callers=20 --leak-check=no
--gen-suppressions=all --error-limit=no
--suppressions=../pgsql/src/tools/valgrind.supp
--error-markers=VALGRINDERROR-BEGIN,VALGRINDERROR-END bin/postgres -D data-C
Does anyone see anything here that needs tweaking?
Note that this is quite an old machine:
andrew@freddo:bf (master)*$ lscpu
Architecture: x86_64
CPU op-mode(s): 32-bit, 64-bit
Byte Order: Little Endian
CPU(s): 2
On-line CPU(s) list: 0,1
Thread(s) per core: 1
Core(s) per socket: 2
Socket(s): 1
NUMA node(s): 1
Vendor ID: AuthenticAMD
CPU family: 16
Model: 6
Model name: AMD Athlon(tm) II X2 215 Processor
Stepping: 2
CPU MHz: 2700.000
CPU max MHz: 2700.
CPU min MHz: 800.
BogoMIPS: 5425.13
Virtualization: AMD-V
L1d cache: 64K
L1i cache: 64K
L2 cache: 512K
NUMA node0 CPU(s): 0,1
Flags: fpu vme de pse tsc msr pae mce cx8 apic sep mtrr
pge mca cmov pat pse36 clflush mmx fxsr sse sse2 ht syscall nx mmxext
fxsr_opt pdpe1gb rdtscp lm 3dnowext 3dnow constant_tsc rep_good nopl
nonstop_tsc cpuid extd_apicid pni monitor cx16 popcnt lahf_lm cmp_legacy
svm extapic cr8_legacy abm sse4a misalignsse 3dnowprefetch osvw ibs
skinit wdt hw_pstate vmmcall npt lbrv svm_lock nrip_save
I did not manage to reproduce this anywhere else, tried on various
physical, Virtualbox and Docker instances.
cheers
andrew
--
Andrew Dunstanhttps://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Re: Strange decreasing value of pg_last_wal_receive_lsn()
synchronous_standby_names=ANY 1(host1, host2) synchronous_commit=on So to understand which standby wrote last data to disk I should know receive_lsn or write_lsn. Sent from my iPhone > On 9 May 2020, at 13:48, Michael Paquier wrote: > > On Fri, May 08, 2020 at 03:02:26PM +0500, godjan • wrote: >> Can you recommend what to use to determine which quorum standby >> should be promoted in such case? >> We planned to use pg_last_wal_receive_lsn() to determine which has >> fresh data but if it returns the beginning of the segment on both >> replicas we can’t determine which standby confirmed that write >> transaction to disk. > > If you want to preserve transaction-level consistency across those > notes, what is your configuration for synchronous_standby_names and > synchronous_commit on the primary? Cannot you rely on that? > -- > Michael
Re: Cast json array to postgres array and preserve order of elements
On 5/10/20 8:21 AM, otar shavadze wrote:
> When I want t to convert json array into postgres array, I do:
>
> with t(j) as(
> select '{"my_arr":[3,1,2]}'::json
> )
> SELECT ARRAY(SELECT json_array_elements_text(j->'my_arr')) from t
>
>
> It works like a charm and I never noticed any problem, but I'm asking
> here just to make sure, order of elements will be preserved always?
> Is that guaranteed in above example, or not?
>
>
yes. The order is significant and the elements are produced in array order.
cheers
andrew
--
Andrew Dunstanhttps://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Re: Cast json array to postgres array and preserve order of elements
Great, thanks very much Andrew!
On Sun, May 10, 2020 at 7:08 PM Andrew Dunstan <
andrew.duns...@2ndquadrant.com> wrote:
>
> On 5/10/20 8:21 AM, otar shavadze wrote:
> > When I want t to convert json array into postgres array, I do:
> >
> > with t(j) as(
> > select '{"my_arr":[3,1,2]}'::json
> > )
> > SELECT ARRAY(SELECT json_array_elements_text(j->'my_arr')) from t
> >
> >
> > It works like a charm and I never noticed any problem, but I'm asking
> > here just to make sure, order of elements will be preserved always?
> > Is that guaranteed in above example, or not?
> >
> >
>
>
> yes. The order is significant and the elements are produced in array order.
>
>
> cheers
>
>
> andrew
>
>
> --
> Andrew Dunstanhttps://www.2ndQuadrant.com
> PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
>
>
Re: should INSERT SELECT use a BulkInsertState?
On Fri, May 08, 2020 at 02:25:45AM -0500, Justin Pryzby wrote:
> Seems to me it should, at least conditionally. At least if there's a function
> scan or a relation or ..
>
> I mentioned a bit about our use-case here:
> https://www.postgresql.org/message-id/20200219173742.GA30939%40telsasoft.com
> => I'd prefer our loaders to write their own data rather than dirtying large
> fractions of buffer cache and leaving it around for other backends to clean
> up.
Nobody suggested otherwise so I added here and cleaned up to pass tests.
https://commitfest.postgresql.org/28/2553/
--
Justin
>From ba5cf05960a097cf82c10a29af81f4f66a9274a6 Mon Sep 17 00:00:00 2001
From: Justin Pryzby
Date: Fri, 8 May 2020 02:17:32 -0500
Subject: [PATCH v1] Make INSERT SELECT use a BulkInsertState
---
src/backend/executor/nodeModifyTable.c | 21 +++--
src/include/nodes/execnodes.h | 2 ++
2 files changed, 21 insertions(+), 2 deletions(-)
diff --git a/src/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.c
index 20a4c474cc..aa85245f39 100644
--- a/src/backend/executor/nodeModifyTable.c
+++ b/src/backend/executor/nodeModifyTable.c
@@ -578,7 +578,7 @@ ExecInsert(ModifyTableState *mtstate,
table_tuple_insert_speculative(resultRelationDesc, slot,
estate->es_output_cid,
0,
- NULL,
+ NULL, /* Bulk insert not supported */
specToken);
/* insert index entries for tuple */
@@ -617,7 +617,7 @@ ExecInsert(ModifyTableState *mtstate,
/* insert the tuple normally */
table_tuple_insert(resultRelationDesc, slot,
estate->es_output_cid,
- 0, NULL);
+ 0, mtstate->bistate);
/* insert index entries for tuple */
if (resultRelInfo->ri_NumIndices > 0)
@@ -2332,6 +2332,17 @@ ExecInitModifyTable(ModifyTable *node, EState *estate, int eflags)
mtstate->mt_arowmarks = (List **) palloc0(sizeof(List *) * nplans);
mtstate->mt_nplans = nplans;
+ mtstate->bistate = NULL;
+ if (operation == CMD_INSERT &&
+ node->onConflictAction != ONCONFLICT_UPDATE &&
+ node->rootResultRelIndex < 0)
+ {
+ Plan *p = linitial(node->plans);
+ Assert(nplans == 1);
+
+ if (!IsA(p, Result) && !IsA(p, ProjectSet) && !IsA(p, ValuesScan))
+ mtstate->bistate = GetBulkInsertState();
+ }
/* set up epqstate with dummy subplan data for the moment */
EvalPlanQualInit(&mtstate->mt_epqstate, estate, NULL, NIL, node->epqParam);
@@ -2776,6 +2787,12 @@ ExecEndModifyTable(ModifyTableState *node)
resultRelInfo);
}
+ if (node->bistate)
+ {
+ FreeBulkInsertState(node->bistate);
+ table_finish_bulk_insert((getTargetResultRelInfo(node))->ri_RelationDesc, 0);
+ }
+
/*
* Close all the partitioned tables, leaf partitions, and their indices
* and release the slot used for tuple routing, if set.
diff --git a/src/include/nodes/execnodes.h b/src/include/nodes/execnodes.h
index 4fee043bb2..daf365f181 100644
--- a/src/include/nodes/execnodes.h
+++ b/src/include/nodes/execnodes.h
@@ -14,6 +14,7 @@
#ifndef EXECNODES_H
#define EXECNODES_H
+#include "access/heapam.h"
#include "access/tupconvert.h"
#include "executor/instrument.h"
#include "fmgr.h"
@@ -1177,6 +1178,7 @@ typedef struct ModifyTableState
List **mt_arowmarks; /* per-subplan ExecAuxRowMark lists */
EPQState mt_epqstate; /* for evaluating EvalPlanQual rechecks */
bool fireBSTriggers; /* do we need to fire stmt triggers? */
+ BulkInsertState bistate; /* State for bulk insert like INSERT SELECT */
/*
* Slot for storing tuples in the root partitioned table's rowtype during
--
2.17.0
gcov coverage data not full with immediate stop
Hello hackers,
I've found that gcov coverage data miss some information when a postgres
node stopped in 'immediate' mode.
For example, on the master branch:
make coverage-clean; time make check -C src/test/recovery/; make
coverage-html
generates a coverage report with 106193 lines/6318 functions for me
(`make check` takes 1m34s).
But with the attached simple patch I get a coverage report with 106540
lines/6332 functions (and `make check` takes 2m5s).
(IMO, the slowdown of the test is significant.)
So if we want to make the coverage reports more precise, I see the three
ways:
1. Change the stop mode in teardown_node to fast (probably only when
configured with --enable-coverage);
2. Explicitly stop nodes in TAP tests (where it's important) -- seems
too tedious and troublesome;
3. Explicitly call __gcov_flush in SIGQUIT handler (quickdie)?
Best regards,
Alexander
diff --git a/src/test/perl/PostgresNode.pm b/src/test/perl/PostgresNode.pm
index af13c320e9c..99c1e85e1a3 100644
--- a/src/test/perl/PostgresNode.pm
+++ b/src/test/perl/PostgresNode.pm
@@ -1242,7 +1242,7 @@ sub teardown_node
{
my $self = shift;
- $self->stop('immediate');
+ $self->stop();
return;
}
Re: Another modest proposal for docs formatting: catalog descriptions
Just FTR, here's a complete patch for this. I successfully regenerated
the column names, types, and ordering from the system catalogs, and
plugged the descriptions back into that by dint of parsing them out of
the XML. The "references" data was based on findoidjoins' results plus
hand additions to cover all the cases we are calling out now (there are
a lot of "references" links for attnums and a few other non-OID columns,
plus references links for views which findoidjoins doesn't consider).
So I have pretty high confidence that this info is OK. I'd be too
embarrassed to show anybody the code though ;-) ... it was just a brute
force hack.
regards, tom lane
catalog-reformat.patch.gz
Description: catalog-reformat.patch.gz
--- main.css~ 2020-05-10 12:18:39.773129302 -0400
+++ main.css 2020-05-09 21:46:22.424776849 -0400
@@ -792,13 +792,6 @@
}
/** Formatting for entries in tables of functions: indent all but first line **/
-#docContent table.table th.functableentry,
-#docContent table.table td.functableentry {
- padding-left: 4em;
- text-indent: -3.5em;
- text-align: left;
-}
-
#docContent table.table th.func_table_entry p,
#docContent table.table td.func_table_entry p {
margin-top: 0.1em;
@@ -820,6 +813,38 @@
padding-left: 4em;
}
+/** Formatting for entries in tables of catalog/view columns **/
+#docContent table.table th.catalog_table_entry p,
+#docContent table.table td.catalog_table_entry p {
+ margin-top: 0.1em;
+ margin-bottom: 0.1em;
+ padding-left: 4em;
+ text-align: left;
+}
+
+#docContent table.table th.catalog_table_entry p.column_definition {
+ text-indent: -3.5em;
+ word-spacing: 0.25em;
+}
+
+#docContent table.table td.catalog_table_entry p.column_definition {
+ text-indent: -3.5em;
+}
+
+#docContent table.table p.column_definition code.type {
+ padding-left: 0.25em;
+ padding-right: 0.25em;
+}
+
+#docContent table.table td.catalog_table_entry pre.programlisting {
+ background-color: inherit;
+ border: 0;
+ margin-bottom: 0.1em;
+ margin-top: 0.1em;
+ padding: 0;
+ padding-left: 4em;
+}
+
/**
* Titles, Navigation
*/
Re: Index Skip Scan
Sorry for late reply. > On Tue, Apr 14, 2020 at 09:19:22PM +1200, David Rowley wrote: > > I've not yet looked at the latest patch, but I did put some thoughts > into an email on the other thread that's been discussing UniqueKeys > [1]. > > I'm keen to hear thoughts on the plan I mentioned over there. Likely > it would be best to discuss the specifics of what additional features > we need to add to UniqueKeys for skip scans over here, but discuss any > chances which affect both patches over there. We certainly can't have > two separate implementations of UniqueKeys, so I believe the skip > scans UniqueKeys patch should most likely be based on the one in [1] > or some descendant of it. > > [1] > https://www.postgresql.org/message-id/CAApHDvpx1qED1uLqubcKJ=ohatcmd7ptukkdq0b72_08nbr...@mail.gmail.com Yes, I've come to the same conclusion, although I have my concerns about having such a dependency between patches. Will look at the suggested patches soon.
Re: Index Skip Scan
> On Sat, Apr 11, 2020 at 03:17:25PM +0530, Dilip Kumar wrote:
>
> Some more comments...
Thanks for reviewing. Since this patch took much longer than I expected,
it's useful to have someone to look at it with a "fresh eyes".
> + so->skipScanKey->nextkey = ScanDirectionIsForward(dir);
> + _bt_update_skip_scankeys(scan, indexRel);
> +
> ...
> + /*
> + * We haven't found scan key within the current page, so let's scan from
> + * the root. Use _bt_search and _bt_binsrch to get the buffer and offset
> + * number
> + */
> + so->skipScanKey->nextkey = ScanDirectionIsForward(dir);
> + stack = _bt_search(scan->indexRelation, so->skipScanKey,
> +&buf, BT_READ, scan->xs_snapshot);
>
> Why do we need to set so->skipScanKey->nextkey =
> ScanDirectionIsForward(dir); multiple times? I think it is fine to
> just set it once?
I believe it was necessary for previous implementations, but in the
current version we can avoid this, you're right.
> +static inline bool
> +_bt_scankey_within_page(IndexScanDesc scan, BTScanInsert key,
> + Buffer buf, ScanDirection dir)
> +{
> + OffsetNumber low, high;
> + Page page = BufferGetPage(buf);
> + BTPageOpaque opaque = (BTPageOpaque) PageGetSpecialPointer(page);
> +
> + low = P_FIRSTDATAKEY(opaque);
> + high = PageGetMaxOffsetNumber(page);
> +
> + if (unlikely(high < low))
> + return false;
> +
> + return (_bt_compare(scan->indexRelation, key, page, low) > 0 &&
> + _bt_compare(scan->indexRelation, key, page, high) < 1);
> +}
>
> I think the high key condition should be changed to
> _bt_compare(scan->indexRelation, key, page, high) < 0 ? Because if
> prefix qual is equal to the high key then also there is no point in
> searching on the current page so we can directly skip.
>From nbtree/README and comments to functions like _bt_split I've got an
impression that the high key could be equal to the last item on the leaf
page, so there is a point in searching. Is that incorrect?
> + /* Check if an index skip scan is possible. */
> + can_skip = enable_indexskipscan & index->amcanskip;
> +
> + /*
> + * Skip scan is not supported when there are qual conditions, which are not
> + * covered by index. The reason for that is that those conditions are
> + * evaluated later, already after skipping was applied.
> + *
> + * TODO: This implementation is too restrictive, and doesn't allow e.g.
> + * index expressions. For that we need to examine index_clauses too.
> + */
> + if (root->parse->jointree != NULL)
> + {
> + ListCell *lc;
> +
> + foreach(lc, (List *)root->parse->jointree->quals)
> + {
> + Node *expr, *qual = (Node *) lfirst(lc);
> + Var *var;
>
> I think we can avoid checking for expression if can_skip is already false.
Yes, that makes sense. I'll include your suggestions into the next
rebased version I'm preparing.
Re: Another modest proposal for docs formatting: catalog descriptions
On 5/10/20 12:27 PM, Tom Lane wrote: > Just FTR, here's a complete patch for this. Cool. I'll play around with it tonight once I clear out release work. Per upthread reference, I believe you've become a CSS maven yourself. > I successfully regenerated > the column names, types, and ordering from the system catalogs, and > plugged the descriptions back into that by dint of parsing them out of > the XML. The "references" data was based on findoidjoins' results plus > hand additions to cover all the cases we are calling out now (there are > a lot of "references" links for attnums and a few other non-OID columns, > plus references links for views which findoidjoins doesn't consider). > So I have pretty high confidence that this info is OK. I'd be too > embarrassed to show anybody the code though ;-) ... it was just a brute > force hack. If it works it works ;) Jonathan signature.asc Description: OpenPGP digital signature
Re: Back-branch minor release notes are up for review
Fujii Masao writes: > The empty paragraph needs to be removed. Ah, thanks for catching that. > I'd like to add the note about the following commit that I pushed recently. > https://git.postgresql.org/gitweb/?p=postgresql.git;a=commitdiff;h=683e0ef5530f449f0f913de579b4f7bcd31c91fd I revised this a bit and included it in today's updates. regards, tom lane
Re: PG 13 release notes, first draft (ltree dot star)
> In ltree, when using adjacent asterisks with braces, e.g. "*{2}.*{3}",
> properly interpret that as "*{5}" (Nikita Glukhov)
I think that should say ".*" not "*", as in:
> In ltree, when using adjacent asterisks with braces, e.g. ".*{2}.*{3}",
> properly interpret that as "*{5}" (Nikita Glukhov)
The existing text clearly came from the commit message, which (based on its
regression tests) I think was the source of the missing dot.
commit 9950c8aadf0edd31baec74a729d47d94af636c06
Author: Tom Lane
Date: Sat Mar 28 18:31:05 2020 -0400
Fix lquery's behavior for consecutive '*' items.
Something like "*{2}.*{3}" should presumably mean the same as
"*{5}", but it didn't. Improve that.
...
--
Justin
calling procedures is slow and consumes extra much memory against calling function
Hi I try to use procedures in Orafce package, and I did some easy performance tests. I found some hard problems: 1. test case create or replace procedure p1(inout r int, inout v int) as $$ begin v := random() * r; end $$ language plpgsql; This command requires do $$ declare r int default 100; x int; begin for i in 1..30 loop call p1(r, x); end loop; end; $$; about 2.2GB RAM and 10 sec. When I rewrite same to functions then create or replace function p1func2(inout r int, inout v int) as $$ begin v := random() * r; end $$ language plpgsql; do $$ declare r int default 100; x int; re record; begin for i in 1..30 loop re := p1func2(r, x); end loop; end; $$; Then execution is about 1 sec, and memory requirements are +/- zero. Minimally it looks so CALL statements has a memory issue. Regards Pavel
deferred primary key and logical replication
Hi,
While looking at an old wal2json issue, I stumbled on a scenario that a
table
with a deferred primary key is not updatable in logical replication. AFAICS
it
has been like that since the beginning of logical decoding and seems to be
an
oversight while designing logical decoding. I don't envision a problem with
a
deferred primary key in an after commit scenario. Am I missing something?
Just in case, I'm attaching a patch to fix it and also add a test to cover
this
scenario.
--
Euler Taveira http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
From 746385e6adaa1668f3be7c7d037e3868ecadae60 Mon Sep 17 00:00:00 2001
From: Euler Taveira
Date: Sun, 19 Apr 2020 20:04:39 -0300
Subject: [PATCH] Table with deferred PK is not updatable in logical
replication
In logical replication, an UPDATE or DELETE cannot be executed if a
table has a primary key that is marked as deferred. RelationGetIndexList
does not fill rd_replidindex accordingly. The consequence is that UPDATE
or DELETE cannot be executed in a deferred PK table. Deferrable primary
key cannot prevent a primary key to be used as replica identity.
---
src/backend/utils/cache/relcache.c | 7 +++
src/test/subscription/t/001_rep_changes.pl | 21 +++--
2 files changed, 22 insertions(+), 6 deletions(-)
diff --git a/src/backend/utils/cache/relcache.c b/src/backend/utils/cache/relcache.c
index 9f1f11d0c1..fb35af4962 100644
--- a/src/backend/utils/cache/relcache.c
+++ b/src/backend/utils/cache/relcache.c
@@ -4555,12 +4555,11 @@ RelationGetIndexList(Relation relation)
result = lappend_oid(result, index->indexrelid);
/*
- * Invalid, non-unique, non-immediate or predicate indexes aren't
- * interesting for either oid indexes or replication identity indexes,
- * so don't check them.
+ * Invalid, non-unique or predicate indexes aren't interesting for
+ * either oid indexes or replication identity indexes, so don't check
+ * them.
*/
if (!index->indisvalid || !index->indisunique ||
- !index->indimmediate ||
!heap_attisnull(htup, Anum_pg_index_indpred, NULL))
continue;
diff --git a/src/test/subscription/t/001_rep_changes.pl b/src/test/subscription/t/001_rep_changes.pl
index 6da7f71ca3..eacdf2aa00 100644
--- a/src/test/subscription/t/001_rep_changes.pl
+++ b/src/test/subscription/t/001_rep_changes.pl
@@ -3,7 +3,7 @@ use strict;
use warnings;
use PostgresNode;
use TestLib;
-use Test::More tests => 23;
+use Test::More tests => 24;
# Initialize publisher node
my $node_publisher = get_new_node('publisher');
@@ -34,6 +34,8 @@ $node_publisher->safe_psql('postgres',
$node_publisher->safe_psql('postgres',
"CREATE TABLE tab_include (a int, b text, CONSTRAINT covering PRIMARY KEY(a) INCLUDE(b))"
);
+$node_publisher->safe_psql('postgres',
+ "CREATE TABLE tab_deferred_pk (a int PRIMARY KEY DEFERRABLE)");
# Let this table with REPLICA IDENTITY NOTHING, allowing only INSERT changes.
$node_publisher->safe_psql('postgres', "CREATE TABLE tab_nothing (a int)");
$node_publisher->safe_psql('postgres',
@@ -58,13 +60,17 @@ $node_subscriber->safe_psql('postgres',
"CREATE TABLE tab_include (a int, b text, CONSTRAINT covering PRIMARY KEY(a) INCLUDE(b))"
);
+# replication of the table with deferrable primary key
+$node_subscriber->safe_psql('postgres',
+ "CREATE TABLE tab_deferred_pk (a int PRIMARY KEY DEFERRABLE)");
+
# Setup logical replication
my $publisher_connstr = $node_publisher->connstr . ' dbname=postgres';
$node_publisher->safe_psql('postgres', "CREATE PUBLICATION tap_pub");
$node_publisher->safe_psql('postgres',
"CREATE PUBLICATION tap_pub_ins_only WITH (publish = insert)");
$node_publisher->safe_psql('postgres',
- "ALTER PUBLICATION tap_pub ADD TABLE tab_rep, tab_full, tab_full2, tab_mixed, tab_include, tab_nothing"
+ "ALTER PUBLICATION tap_pub ADD TABLE tab_rep, tab_full, tab_full2, tab_mixed, tab_include, tab_nothing, tab_deferred_pk"
);
$node_publisher->safe_psql('postgres',
"ALTER PUBLICATION tap_pub_ins_only ADD TABLE tab_ins");
@@ -111,6 +117,13 @@ $node_publisher->safe_psql('postgres',
"DELETE FROM tab_include WHERE a > 20");
$node_publisher->safe_psql('postgres', "UPDATE tab_include SET a = -a");
+$node_publisher->safe_psql('postgres',
+ "INSERT INTO tab_deferred_pk VALUES (1),(2),(3)");
+$node_publisher->safe_psql('postgres',
+ "UPDATE tab_deferred_pk SET a = 11 WHERE a = 1");
+$node_publisher->safe_psql('postgres',
+ "DELETE FROM tab_deferred_pk WHERE a = 2");
+
$node_publisher->wait_for_catchup('tap_sub');
$result = $node_subscriber->safe_psql('postgres',
@@ -134,6 +147,10 @@ $result = $node_subscriber->safe_psql('postgres',
is($result, qq(20|-20|-1),
'check replicated changes with primary key index with included columns');
+$result = $node_subscriber->safe_psql('postgres',
+ "SELECT count(*), min(a), max(a) FROM tab_deferred_pk");
+is($result, qq(2|3|11), 'check replicated changes with deferred prim
Re: Problem with logical replication
On Mon, 20 Apr 2020 at 10:25, Masahiko Sawada <
masahiko.saw...@2ndquadrant.com> wrote:
> On Thu, 16 Apr 2020 at 17:48, Dilip Kumar wrote:
>
> I could reproduce this issue by the steps you shared. For the bug fix
> patch, I basically agree to remove that assertion from
> build_replindex_scan_key() but I think it's better to update the
> assertion instead of removal and update the following comment:
>
> IMO the assertion is using the wrong function because it should test a
replica
identity or primary key (GetRelationIdentityOrPK). RelationGetReplicaIndex
returns InvalidOid even though the table has a primary key.
GetRelationIdentityOrPK tries to obtain a replica identity and if it fails,
it
tries a primary key. That's exact what this assertion should use. We should
also notice that FindReplTupleInLocalRel uses GetRelationIdentityOrPK and
after
a code path like RelationFindReplTupleByIndex -> build_replindex_scan_key it
should also use the same function.
Since GetRelationIdentityOrPK is a fallback function that
uses RelationGetReplicaIndex and RelationGetPrimaryKeyIndex, I propose that
we
move this static function to execReplication.c.
I attached a patch with the described solution. I also included a test that
covers this scenario.
--
Euler Taveira http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
From 37be6d62bf26a20ec3cba8825082ad92fdd1c6d7 Mon Sep 17 00:00:00 2001
From: Euler Taveira
Date: Sat, 9 May 2020 20:41:50 -0300
Subject: [PATCH] Fix assert failure with REPLICA IDENTITY FULL in the
subscriber
This assertion failure can be reproduced using a replicated table in the
subscriber with REPLICA IDENTITY FULL. Since FindReplTupleInLocalRel
uses GetRelationIdentityOrPK to obtain a replica identity or primary
key, the follow path RelationFindReplTupleByIndex and then
build_replindex_scan_key should also use the same function
(GetRelationIdentityOrPK) to test an assertion condition. The Assert is
using RelationGetReplicaIndex that returns InvalidOid because REPLICA
IDENTITY is FULL (see RelationGetIndexList).
---
src/backend/executor/execReplication.c | 2 +-
src/backend/replication/logical/worker.c | 18 --
src/backend/utils/cache/relcache.c | 19 +++
src/include/utils/relcache.h | 1 +
src/test/subscription/t/001_rep_changes.pl | 12 +++-
5 files changed, 32 insertions(+), 20 deletions(-)
diff --git a/src/backend/executor/execReplication.c b/src/backend/executor/execReplication.c
index 1418746eb8..d9d473a4cd 100644
--- a/src/backend/executor/execReplication.c
+++ b/src/backend/executor/execReplication.c
@@ -57,7 +57,7 @@ build_replindex_scan_key(ScanKey skey, Relation rel, Relation idxrel,
int2vector *indkey = &idxrel->rd_index->indkey;
bool hasnulls = false;
- Assert(RelationGetReplicaIndex(rel) == RelationGetRelid(idxrel));
+ Assert(GetRelationIdentityOrPK(rel) == RelationGetRelid(idxrel));
indclassDatum = SysCacheGetAttr(INDEXRELID, idxrel->rd_indextuple,
Anum_pg_index_indclass, &isnull);
diff --git a/src/backend/replication/logical/worker.c b/src/backend/replication/logical/worker.c
index a752a1224d..a2d78e74eb 100644
--- a/src/backend/replication/logical/worker.c
+++ b/src/backend/replication/logical/worker.c
@@ -584,24 +584,6 @@ apply_handle_type(StringInfo s)
logicalrep_typmap_update(&typ);
}
-/*
- * Get replica identity index or if it is not defined a primary key.
- *
- * If neither is defined, returns InvalidOid
- */
-static Oid
-GetRelationIdentityOrPK(Relation rel)
-{
- Oid idxoid;
-
- idxoid = RelationGetReplicaIndex(rel);
-
- if (!OidIsValid(idxoid))
- idxoid = RelationGetPrimaryKeyIndex(rel);
-
- return idxoid;
-}
-
/*
* Handle INSERT message.
*/
diff --git a/src/backend/utils/cache/relcache.c b/src/backend/utils/cache/relcache.c
index 9f1f11d0c1..c5849f15d1 100644
--- a/src/backend/utils/cache/relcache.c
+++ b/src/backend/utils/cache/relcache.c
@@ -4727,6 +4727,25 @@ RelationGetReplicaIndex(Relation relation)
return relation->rd_replidindex;
}
+/*
+ * GetRelationIdentityOrPK -- get relation's replica identity index or, if it
+ * is not defined, the primary key
+ *
+ * If neither is defined, returns InvalidOid.
+ */
+Oid
+GetRelationIdentityOrPK(Relation rel)
+{
+ Oid idxoid;
+
+ idxoid = RelationGetReplicaIndex(rel);
+
+ if (!OidIsValid(idxoid))
+ idxoid = RelationGetPrimaryKeyIndex(rel);
+
+ return idxoid;
+}
+
/*
* RelationGetIndexExpressions -- get the index expressions for an index
*
diff --git a/src/include/utils/relcache.h b/src/include/utils/relcache.h
index 9a85b7dd57..c0f7675cdc 100644
--- a/src/include/utils/relcache.h
+++ b/src/include/utils/relcache.h
@@ -48,6 +48,7 @@ extern List *RelationGetIndexList(Relation relation);
extern List *RelationGetStatExtList(Relation relation);
extern Oid RelationGetPrimaryKeyIndex(Relation relation);
extern Oid RelationGetReplicaIndex(Relation relation);
Re: Back-branch minor release notes are up for review
On 2020/05/11 4:07, Tom Lane wrote: Fujii Masao writes: The empty paragraph needs to be removed. Ah, thanks for catching that. I'd like to add the note about the following commit that I pushed recently. https://git.postgresql.org/gitweb/?p=postgresql.git;a=commitdiff;h=683e0ef5530f449f0f913de579b4f7bcd31c91fd I revised this a bit and included it in today's updates. Thanks a lot! Regards, -- Fujii Masao Advanced Computing Technology Center Research and Development Headquarters NTT DATA CORPORATION
A comment fix
Hello. I happened to notice a wrong function name in the comment of XLogReadDetermineTimeline. * The caller must also make sure it doesn't read past the current replay * position (using GetWalRcvWriteRecPtr) if executing in recovery, so it The comment is mentioning "replay position" and the callers are actually using GetXLogReplayRecPtr to check TLI and target LSN. The comment was written in that way when the function is introduced by 1148e22a82. The attached fixes that. The function GetWalRcvWriteRecPtr is not called from anywhere in core but I don't think we need to bother removing it since it is a public function. regards. -- Kyotaro Horiguchi NTT Open Source Software Center diff --git a/src/backend/access/transam/xlogutils.c b/src/backend/access/transam/xlogutils.c index bbd801513a..0bb69447c2 100644 --- a/src/backend/access/transam/xlogutils.c +++ b/src/backend/access/transam/xlogutils.c @@ -681,10 +681,10 @@ XLogTruncateRelation(RelFileNode rnode, ForkNumber forkNum, * copied to a new timeline. * * The caller must also make sure it doesn't read past the current replay - * position (using GetWalRcvWriteRecPtr) if executing in recovery, so it + * position (using GetXLogReplayRecPtr) if executing in recovery, so it * doesn't fail to notice that the current timeline became historical. The * caller must also update ThisTimeLineID with the result of - * GetWalRcvWriteRecPtr and must check RecoveryInProgress(). + * GetXLogReplayRecPtr and must check RecoveryInProgress(). */ void XLogReadDetermineTimeline(XLogReaderState *state, XLogRecPtr wantPage, uint32 wantLength)
No core file generated after PostgresNode->start
Hi:
When I run make -C subscription check, then I see the following logs
in ./tmp_check/log/013_partition_publisher.log
2020-05-11 09:37:40.778 CST [69541] sub_viaroot WARNING: terminating
connection because of crash of another server process
2020-05-11 09:37:40.778 CST [69541] sub_viaroot DETAIL: The postmaster
has commanded this server process to roll back the current transaction and
exit,
because another server process exited abnormally and possibly corrupted
shared memory.
However there is no core file generated. In my other cases(like start pg
manually with bin/postgres xxx) can generate core file successfully at
the same machine. What might be the problem for PostgresNode case?
I tried this modification, but it doesn't help.
--- a/src/test/perl/PostgresNode.pm
+++ b/src/test/perl/PostgresNode.pm
@@ -766,7 +766,7 @@ sub start
# Note: We set the cluster_name here, not in
postgresql.conf (in
# sub init) so that it does not get copied to standbys.
- $ret = TestLib::system_log('pg_ctl', '-D', $self->data_dir,
'-l',
+ $ret = TestLib::system_log('pg_ctl', "-c", '-D',
$self->data_dir, '-l',
$self->logfile, '-o', "--cluster-name=$name",
'start');
}
Best Regards
Andy Fan
2020-05-14 Press Release Draft
Hi, Attached is a draft of the press release for the 2020-05-14 cumulative update. Please let me know your feedback by 2020-05-13 :) Thanks, Jonathan 2020-05-14 Cumulative Update Release The PostgreSQL Global Development Group has released an update to all supported versions of our database system, including 12.3, 11.8, 10.13, 9.6.18, and 9.5.22. This release fixes more than 75 bugs reported over the last three months. Users should plan to update at their earliest convenience. Bug Fixes and Improvements -- This update also fixes over 75 bugs that were reported in the last several months. Some of these issues affect only version 12, but may also affect all supported versions. Some of these fixes include: * Several fixes for GENERATED columns, including an issue where it was possible to crash or corrupt data in a table when the output of the generated column was the exact copy of a physical column on the table. * Several fixes for `ALTER TABLE`, including ensuring the `SET STORAGE` directive is propagated to a table's indexes. * Fix a potential race condition when using `DROP OWNED BY` while another session is deleting the same objects. * Ensure that a detatched partition has triggers that come from its former parent removed. * Several fixes for `REINDEX CONCURRENTLY`, particular with dealing with issue when a `REINDEX CONCURRENTLY` operation fails. * Fix crash when COLLATE is applied to an uncollatable type in a partition bound expression. * Fix performance regression in floating point overflow/underflow detection. * Several fixes for full text search, particularly with phrase searching. * Fix query-lifespan memory leak for a set-returning function used in a query's FROM clause. * Several reporting fixes for the output of `VACUUM VERBOSE`. * Allow input of type circle to accept the format `(x,y),r`, which is specified in the documentation. * Allow for the `get_bit()` and `set_bit()` functions to not fail on `bytea` strings longer than 256MB. * Avoid premature recycling of WAL segments during crash recovery, which could lead to WAL segments being recycled before being archived. * Avoid scanning irrelevant timelines during archive recovery, which can eliminate attempts to fetch nonexistent WAL files from archive storage. * Several fixes for logical replication and replication slots. * Fix several race conditions in synchronous standby management, including one that occurred when changing the `synchronous_standby_names` setting. * Several fixes for GSSAPI support, include a fix for a memory leak that occurred when using GSSAPI encryption. * Ensure that members of the `pg_read_all_stats` role can read all statistics views. * Fix performance regression in information_schema.triggers view. * Fix memory leak in libpq when using `sslmode=verify-full`. * Fix crash in `psql` when attempting to re-establish a failed connection. * Allow tab-completion of the filename argument to `\gx` command in `psql`. * Add `pg_dump` support for `ALTER ... DEPENDS ON EXTENSION`. * Several other fixes for `pg_dump`, which include dumping comments on RLS policies and postponing restore of event triggers until the end. * Ensure `pg_basebackup` generates valid tar files. * `pg_checksums` skips tablespace subdirectories that belong to a different PostgreSQL major version * Several Windows compatibility fixes This update also contains tzdata release 2020a for DST law changes in Morocco and the Canadian Yukon, plus historical corrections for Shanghai. The America/Godthab zone has been renamed to America/Nuuk to reflect current English usage; however, the old name remains available as a compatibility link. This also updates initdb's list of known Windows time zone names to include recent additions. For the full list of changes available, please review the [release notes](https://www.postgresql.org/docs/current/release.html). Updating All PostgreSQL update releases are cumulative. As with other minor releases, users are not required to dump and reload their database or use `pg_upgrade` in order to apply this update release; you may simply shutdown PostgreSQL and update its binaries. Users who have skipped one or more update releases may need to run additional, post-update steps; please see the release notes for earlier versions for details. For more details, please see the [release notes](https://www.postgresql.org/docs/current/release.html). Links - * [Download](https://www.postgresql.org/download/) * [Release Notes](https://www.postgresql.org/docs/current/release.html) * [Security Page](https://www.postgresql.org/support/security/) * [Versioning Policy](https://www.postgresql.org/support/versioning/) * [Follow @postgresql on Twitter](https://twitter.com/postgresql) signature.asc Description: OpenPGP digital signature
Re: No core file generated after PostgresNode->start
On Mon, May 11, 2020 at 9:48 AM Andy Fan wrote: > Hi: > > > 2020-05-11 09:37:40.778 CST [69541] sub_viaroot WARNING: terminating > connection because of crash of another server process > > Looks this doesn't mean a crash. If the test case(subscription/t/ 013_partition.pl) failed, test framework kill some process, which leads the above message. So you can ignore this issue now. Thanks Best Regards Andy Fan
Re: gcov coverage data not full with immediate stop
(Strangely, I was just thinking about these branches of mine as I
closed my week last Friday...)
On 2020-May-10, Alexander Lakhin wrote:
> So if we want to make the coverage reports more precise, I see the three
> ways:
> 1. Change the stop mode in teardown_node to fast (probably only when
> configured with --enable-coverage);
> 2. Explicitly stop nodes in TAP tests (where it's important) -- seems
> too tedious and troublesome;
> 3. Explicitly call __gcov_flush in SIGQUIT handler (quickdie)?
I tried your idea 3 a long time ago and my experiments didn't show an
increase in coverage [1]. But I like this idea the best, and maybe I
did something wrong. Attached is the patch I had (on top of
fc115d0f9fc6), but I don't know if it still applies.
(The second attachment is another branch I had on this, I don't remember
why; that one was on top of 438e51987dcc. The curious thing is that I
didn't add the __gcov_flush to quickdie in this one. Maybe what we need
is a mix of both.)
I think we should definitely get this fixed for pg13 ...
[1] https://postgr.es/m/20190531170503.GA24057@alvherre.pgsql
--
Álvaro Herrerahttps://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
>From 9803714aa0e493c603e04e282a241cfa89507da3 Mon Sep 17 00:00:00 2001
From: Alvaro Herrera
Date: Fri, 31 May 2019 13:09:46 -0400
Subject: [PATCH] add gcov_flush call in quickdie
---
configure | 4
configure.in| 4 +++-
src/backend/tcop/postgres.c | 8
src/include/pg_config.h.in | 3 +++
4 files changed, 18 insertions(+), 1 deletion(-)
diff --git a/configure b/configure
index fd61bf6472..c0cf19d662 100755
--- a/configure
+++ b/configure
@@ -3516,6 +3516,10 @@ fi
if test -z "$GENHTML"; then
as_fn_error $? "genhtml not found" "$LINENO" 5
fi
+
+$as_echo "#define USE_GCOV_COVERAGE 1" >>confdefs.h
+
+
;;
no)
:
diff --git a/configure.in b/configure.in
index 4586a1716c..21465bbaa6 100644
--- a/configure.in
+++ b/configure.in
@@ -222,7 +222,9 @@ fi
PGAC_PATH_PROGS(GENHTML, genhtml)
if test -z "$GENHTML"; then
AC_MSG_ERROR([genhtml not found])
-fi])
+fi
+AC_DEFINE([USE_GCOV_COVERAGE], 1, [Define to use gcov coverage support stuff])
+])
AC_SUBST(enable_coverage)
#
diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c
index 44a59e1d4f..a483eba454 100644
--- a/src/backend/tcop/postgres.c
+++ b/src/backend/tcop/postgres.c
@@ -2729,6 +2729,14 @@ quickdie(SIGNAL_ARGS)
errhint("In a moment you should be able to reconnect to the"
" database and repeat your command.")));
+#ifdef USE_GCOV_COVERAGE
+ /*
+ * We still want to flush coverage data down to disk, which gcov's atexit
+ * callback would do, but we're preventing that below.
+ */
+ __gcov_flush();
+#endif
+
/*
* We DO NOT want to run proc_exit() or atexit() callbacks -- we're here
* because shared memory may be corrupted, so we don't want to try to
diff --git a/src/include/pg_config.h.in b/src/include/pg_config.h.in
index 6cd4cfed0a..5fb1a545ed 100644
--- a/src/include/pg_config.h.in
+++ b/src/include/pg_config.h.in
@@ -916,6 +916,9 @@
(--enable-float8-byval) */
#undef USE_FLOAT8_BYVAL
+/* Define to use gcov coverage support stuff */
+#undef USE_GCOV_COVERAGE
+
/* Define to build with ICU support. (--with-icu) */
#undef USE_ICU
--
2.20.1
>From 853d4c901ce4b53285b43ddf44cb567f774a2dd8 Mon Sep 17 00:00:00 2001
From: Alvaro Herrera
Date: Fri, 31 May 2019 11:20:23 -0400
Subject: [PATCH] gcov_flush stuff
---
config/c-compiler.m4 | 15 +++
configure| 102 +++
configure.in | 9
3 files changed, 126 insertions(+)
diff --git a/config/c-compiler.m4 b/config/c-compiler.m4
index 71b645839d..0a73fd4624 100644
--- a/config/c-compiler.m4
+++ b/config/c-compiler.m4
@@ -394,6 +394,21 @@ AC_DEFINE_UNQUOTED(AS_TR_CPP([HAVE$1]), 1,
[Define to 1 if your compiler understands $1.])
fi])# PGAC_CHECK_BUILTIN_FUNC
+AC_DEFUN([PGAC_CHECK_BUILTIN_FUNC0],
+[AC_CACHE_CHECK(for $1, pgac_cv$1,
+[AC_LINK_IFELSE([AC_LANG_PROGRAM([
+int
+call$1()
+{
+return $1();
+}], [])],
+[pgac_cv$1=yes],
+[pgac_cv$1=no])])
+if test x"${pgac_cv$1}" = xyes ; then
+AC_DEFINE_UNQUOTED(AS_TR_CPP([HAVE$1]), 1,
+ [Define to 1 if your compiler understands $1.])
+fi])# PGAC_CHECK_BUILTIN_FUNC0
+
# PGAC_PROG_VARCC_VARFLAGS_OPT
diff --git a/configure b/configure
index fd61bf6472..28ebab733f 100755
--- a/configure
+++ b/configure
@@ -11915,6 +11915,69 @@ fi
fi
fi
+if test "$enable_coverage" = yes ; then
+ if test "$PORTNAME" != "win32"; then
+{ $as_echo "$as_me:${as_lineno-$LINENO}: checking for library containing __gcov_flush" >&5
+$as_echo_n "checking for library containing __gcov_flush... " >&6; }
+if ${ac_cv_search___gcov_flush+:} false; then :
+ $as_echo_n "(cached) " >&6
+else
+ ac_func_search_save_LIBS=$LIBS
+cat confdefs.h
Re: Add -Wold-style-definition to CFLAGS?
On 2020-May-09, Tom Lane wrote: > If we do want to push this sort of thing now, the nearby changes > to enable fallthrough warnings should go in too. I'll get that sorted out tomorrow. -- Álvaro Herrerahttps://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Re: Add "-Wimplicit-fallthrough" to default flags (was Re: pgsql: Support FETCH FIRST WITH TIES)
On 2020-May-06, Alvaro Herrera wrote: > This doesn't allow whitespace between "fall" and "through", which means > we generate 217 such warnings currently. Or we can just use > -Wimplicit-fallthrough=3, which does allow whitespace (among other > detritus). If we're OK with patching all those places, I volunteer to do so. Any objections? Or I can keep it at level 3, which can be done with minimal patching. -- Álvaro Herrerahttps://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Re: +(pg_lsn, int8) and -(pg_lsn, int8) operators
At Sat, 9 May 2020 23:40:15 +0900, Fujii Masao wrote in > > > On 2020/05/08 12:10, Kyotaro Horiguchi wrote: > > At Fri, 8 May 2020 11:31:42 +0900, Fujii Masao > > wrote in > You mean that pg_lsn_pli() and pg_lsn_mii() should emit an error like > "the number of bytes to add/subtract cannnot be NaN" when NaN is > specified? > >>> The function is called while executing an expression, so "NaN cannot > >>> be used in this expression" or something like that would work. > >> > >> This sounds ambiguous. I like to use clearer messages like > >> > >> cannot add NaN to pg_lsn > >> cannot subtract NaN from pg_lsn > > They works fine to me. > > Ok, I updated pg_lsn_pli() and pg_lsn_mii() so that they emit an error > when NaN is specified as the number of bytes. It's fine with me. > > Sorry, I misread the patch as it rejected -1 for *nbytes*, by seeing > > numeric_pg_lsn. > > Finally, I'm convinced that we lack required integer arithmetic > > infrastructure to perform the objective. > > The patch looks good to me except the size of buf[], but I don't > > strongly object to that. > > Ok, I changed the size of buf[] to 32. > Attached is the updated version of the patch. Thank you very much! The patch looks good to me. regard. -- Kyotaro Horiguchi NTT Open Source Software Center
Re: 2020-05-14 Press Release Draft
At Sun, 10 May 2020 22:08:46 -0400, "Jonathan S. Katz" wrote in > Attached is a draft of the press release for the 2020-05-14 cumulative > update. Please let me know your feedback by 2020-05-13 :) Thank you. I found a typo in it. > * Ensure that a detatched partition has triggers that come from its former > parent removed. s/detatched/detached/ ? regards. -- Kyotaro Horiguchi NTT Open Source Software Center
Re: Add "-Wimplicit-fallthrough" to default flags (was Re: pgsql: Support FETCH FIRST WITH TIES)
Alvaro Herrera writes: > On 2020-May-06, Alvaro Herrera wrote: >> This doesn't allow whitespace between "fall" and "through", which means >> we generate 217 such warnings currently. Or we can just use >> -Wimplicit-fallthrough=3, which does allow whitespace (among other >> detritus). > If we're OK with patching all those places, I volunteer to do so. Any > objections? Or I can keep it at level 3, which can be done with minimal > patching. If we're gonna do it at all, I think we should go for the level 4 warnings. Level 3's idea of a fallthrough comment is too liberal for my tastes... regards, tom lane
Re: gcov coverage data not full with immediate stop
Alvaro Herrera writes: > On 2020-May-10, Alexander Lakhin wrote: >> 3. Explicitly call __gcov_flush in SIGQUIT handler (quickdie)? > I tried your idea 3 a long time ago and my experiments didn't show an > increase in coverage [1]. But I like this idea the best, and maybe I > did something wrong. Attached is the patch I had (on top of > fc115d0f9fc6), but I don't know if it still applies. Putting ill-defined, not-controlled-by-us work into a quickdie signal handler sounds like a really bad idea to me. Maybe it's all right, since presumably it would only appear in specialized test builds; but even so, how much could you trust the results? > I think we should definitely get this fixed for pg13 ... -1 for shoving in such a thing so late in the cycle. We've survived without it for years, we can do so for a few months more. regards, tom lane
Re: A comment fix
On Mon, May 11, 2020 at 10:16:19AM +0900, Kyotaro Horiguchi wrote: > The comment is mentioning "replay position" and the callers are > actually using GetXLogReplayRecPtr to check TLI and target LSN. The > comment was written in that way when the function is introduced by > 1148e22a82. The attached fixes that. Looks right to me, so will fix if there are no objections. read_local_xlog_page() uses the replay location when in recovery. > The function GetWalRcvWriteRecPtr is not called from anywhere in core > but I don't think we need to bother removing it since it is a public > function. Yes, I don't think that's removable (just look at the log message of d140f2f3), and the function is dead simple so that's not really going to break even if this is dead in-core now. Worth noting some future WAL prefetch stuff may actually use it. -- Michael signature.asc Description: PGP signature
Re: PG 13 release notes, first draft
On Sat, May 9, 2020 at 11:16 AM Amit Kapila wrote: > > On Tue, May 5, 2020 at 8:46 AM Bruce Momjian wrote: > > > > I have committed the first draft of the PG 13 release notes. You can > > see them here: > > > > https://momjian.us/pgsql_docs/release-13.html > > > > Thanks for the work. I was today going through the release notes and > was wondering whether we should consider adding information about some > other work done for PG13. > 1. We have allowed an (auto)vacuum to display additional information > about heap or index in case of an error in commit b61d161c14 [1]. > Now, in general, it might not be worth saying much about error > information but I think this one could help users in case they have > some corruption. For example, if one of the indexes on a relation has > some corrupted data (due to bad hardware or some bug), it will let the > user know the index information, and the user can take appropriate > action like either Reindex or maybe drop and recreate the index to > overcome the problem. > 2. In the "Source Code" section, we can add information about > infrastructure enhancement for parallelism. Basically, "Allow > relation extension and page lock to conflict among parallel-group > members" [2][3]. This will allow improving the parallelism further in > many cases like (a) we can allow multiple workers to operate on a heap > and index in a parallel vacuum, (b) we can allow parallel Inserts, > etc. > One more observation: Allow inserts to trigger autovacuum activity (Laurenz Albe, Darafei Praliaskouski) This new behavior allows pages to be set as all-visible, which then allows index-only scans, ... The above sentence sounds to mean that this feature allows index-only scans in more number of cases after this feature. Is that what you intend to say? If so, is that correct? Because I think this will allow index-only scans to skip "Heap Fetches" in more cases. -- With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.com
Re: calling procedures is slow and consumes extra much memory against calling function
Hi
ne 10. 5. 2020 v 22:20 odesílatel Pavel Stehule
napsal:
> Hi
>
> I try to use procedures in Orafce package, and I did some easy performance
> tests. I found some hard problems:
>
> 1. test case
>
> create or replace procedure p1(inout r int, inout v int) as $$
> begin v := random() * r; end
> $$ language plpgsql;
>
> This command requires
>
> do $$
> declare r int default 100; x int;
> begin
> for i in 1..30 loop
> call p1(r, x);
> end loop;
> end;
> $$;
>
> about 2.2GB RAM and 10 sec.
>
> When I rewrite same to functions then
>
> create or replace function p1func2(inout r int, inout v int) as $$
> begin v := random() * r; end
> $$ language plpgsql;
>
> do $$
> declare r int default 100; x int; re record;
> begin
> for i in 1..30 loop
> re := p1func2(r, x);
> end loop;
> end;
> $$;
>
> Then execution is about 1 sec, and memory requirements are +/- zero.
>
> Minimally it looks so CALL statements has a memory issue.
>
The problem is in plpgsql implementation of CALL statement
In non atomic case - case of using procedures from DO block, the
expression plan is not cached, and plan is generating any time. This is
reason why it is slow.
Unfortunately, generated plans are not released until SPI_finish. Attached
patch fixed this issue.
Regards
Pavel
> Regards
>
> Pavel
>
>
diff --git a/src/pl/plpgsql/src/pl_exec.c b/src/pl/plpgsql/src/pl_exec.c
index aeb6c8fefc..bb08037a94 100644
--- a/src/pl/plpgsql/src/pl_exec.c
+++ b/src/pl/plpgsql/src/pl_exec.c
@@ -2309,13 +2309,19 @@ exec_stmt_call(PLpgSQL_execstate *estate, PLpgSQL_stmt_call *stmt)
* could have been unset already, in case of a recursive call.
*/
if (expr->plan && !expr->plan->saved)
+ {
+ SPI_freeplan(expr->plan);
expr->plan = NULL;
+ }
PG_RE_THROW();
}
PG_END_TRY();
if (expr->plan && !expr->plan->saved)
+ {
+ SPI_freeplan(expr->plan);
expr->plan = NULL;
+ }
if (rc < 0)
elog(ERROR, "SPI_execute_plan_with_paramlist failed executing query \"%s\": %s",
Re: calling procedures is slow and consumes extra much memory against calling function
On Sun, May 10, 2020 at 10:20:53PM +0200, Pavel Stehule wrote: > When I rewrite same to functions then > > create or replace function p1func2(inout r int, inout v int) as $$ > begin v := random() * r; end > $$ language plpgsql; > > Then execution is about 1 sec, and memory requirements are +/- zero. > > Minimally it looks so CALL statements has a memory issue. Behavior not limited to plpgsql. A plain SQL function shows the same leak patterns: create or replace procedure p1_sql(in r int, in v int) as $$ SELECT r + v; $$ language sql; And I cannot get valgrind to complain about lost references, so this looks like some missing memory context handling. Also, I actually don't quite get why the context created by CreateExprContext() cannot be freed before the procedure returns. A short test shows no problems in calling FreeExprContext() at the end of ExecuteCallStmt(), but that does not address everything. Perhaps a lack of tests with pass-by-reference expressions and procedures? Peter? -- Michael signature.asc Description: PGP signature
Re: calling procedures is slow and consumes extra much memory against calling function
po 11. 5. 2020 v 7:25 odesílatel Pavel Stehule napsal: > Hi > > ne 10. 5. 2020 v 22:20 odesílatel Pavel Stehule > napsal: > >> Hi >> >> I try to use procedures in Orafce package, and I did some easy >> performance tests. I found some hard problems: >> >> 1. test case >> >> create or replace procedure p1(inout r int, inout v int) as $$ >> begin v := random() * r; end >> $$ language plpgsql; >> >> This command requires >> >> do $$ >> declare r int default 100; x int; >> begin >> for i in 1..30 loop >> call p1(r, x); >> end loop; >> end; >> $$; >> >> about 2.2GB RAM and 10 sec. >> >> When I rewrite same to functions then >> >> create or replace function p1func2(inout r int, inout v int) as $$ >> begin v := random() * r; end >> $$ language plpgsql; >> >> do $$ >> declare r int default 100; x int; re record; >> begin >> for i in 1..30 loop >> re := p1func2(r, x); >> end loop; >> end; >> $$; >> >> Then execution is about 1 sec, and memory requirements are +/- zero. >> >> Minimally it looks so CALL statements has a memory issue. >> > > The problem is in plpgsql implementation of CALL statement > > In non atomic case - case of using procedures from DO block, the > expression plan is not cached, and plan is generating any time. This is > reason why it is slow. > > Unfortunately, generated plans are not released until SPI_finish. Attached > patch fixed this issue. > But now, recursive calling doesn't work :-(. So this patch is not enough > Regards > > Pavel > > >> Regards >> >> Pavel >> >>
Re: should INSERT SELECT use a BulkInsertState?
On Fri, May 08, 2020 at 02:25:45AM -0500, Justin Pryzby wrote: > Seems to me it should, at least conditionally. At least if there's a function > scan or a relation or .. > > I mentioned a bit about our use-case here: > https://www.postgresql.org/message-id/20200219173742.GA30939%40telsasoft.com > => I'd prefer our loaders to write their own data rather than dirtying large > fractions of buffer cache and leaving it around for other backends to clean > up. Does it matter in terms of performance and for which cases does it actually matter? > diff --git a/src/include/nodes/execnodes.h b/src/include/nodes/execnodes.h > index 4fee043bb2..daf365f181 100644 > --- a/src/include/nodes/execnodes.h > +++ b/src/include/nodes/execnodes.h > @@ -14,6 +14,7 @@ > #ifndef EXECNODES_H > #define EXECNODES_H > > +#include "access/heapam.h" > #include "access/tupconvert.h" > #include "executor/instrument.h" > #include "fmgr.h" > @@ -1177,6 +1178,7 @@ typedef struct ModifyTableState > List **mt_arowmarks; /* per-subplan ExecAuxRowMark lists */ > EPQStatemt_epqstate;/* for evaluating EvalPlanQual rechecks > */ > boolfireBSTriggers; /* do we need to fire stmt triggers? */ > + BulkInsertState bistate;/* State for bulk insert like INSERT > SELECT */ I think that this needs more thoughts. You are introducing a dependency between some generic execution-related nodes and heap, a table AM. -- Michael signature.asc Description: PGP signature
Re: PG 13 release notes, first draft
Hi Bruce, On 2020/05/05 12:16, Bruce Momjian wrote: I have committed the first draft of the PG 13 release notes. You can see them here: https://momjian.us/pgsql_docs/release-13.html It still needs markup, word wrap, and indenting. The community doc build should happen in a few hours. Thanks for working on this! :-D Could you add "Vinayak Pokale" as a co-author of the following feature since I sometimes read his old patch to create a patch [1] ? === E.1.3.1.6. System Views - Add system view pg_stat_progress_analyze to report analyze progress (Álvaro Herrera, Tatsuro Yamada) + Add system view pg_stat_progress_analyze to report analyze progress (Álvaro Herrera, Tatsuro Yamada, Vinayak Pokale) === [1] https://www.postgresql.org/message-id/20190813140127.GA4933%40alvherre.pgsql Thanks, Tatsuro Yamada
Re: Strange decreasing value of pg_last_wal_receive_lsn()
On Sun, May 10, 2020 at 06:58:50PM +0500, godjan • wrote: > synchronous_standby_names=ANY 1(host1, host2) > synchronous_commit=on Thanks for the details. I was not sure based on your previous messages. > So to understand which standby wrote last data to disk I should know > receive_lsn or write_lsn. If you have just an access to the standbys, using pg_last_wal_replay_lsn() should be enough, no? One tricky point is to make sure that each standby does not have more WAL to replay, though you can do that by looking at the wait event called RecoveryRetrieveRetryInterval for the startup process. Note that when a standby starts and has primary_conninfo set, it would request streaming to start again at the beginning of the segment as mentioned, but it does not change the point up to which the startup process replays the WAL available locally, as that's what takes priority as WAL source (second choice is a WAL archive and third is streaming if all options are set in the recovery configuration). There are several HA solutions floating around in the community, and I got to wonder as well if some of them don't just scan the local pg_wal/ of each standby in this case, even if that's more simple to let the nodes start and replay up to their latest point available. -- Michael signature.asc Description: PGP signature
