Re: More issues with pg_verify_checksums and checksum verification in base backups
On Sun, Oct 21, 2018 at 08:56:32PM -0400, Stephen Frost wrote: > All of this pie-in-the-sky about what pluggable storage might have is > just hand-waving, in my opinion, and not worth much more than that. I > hope (and suspect..) that the actual pluggable storage that's being > worked on doesn't do any of this "just drop a file somewhere" because > there's a lot of downsides to it- and if it did, it wouldn't be much > more than what we can do with an FDW, so why go through and add it? Well, there is no point in enforcing a rule that something is forbidden if if was never implied and never documented (the rule here is to be able to drop custom files into global/, base/ or pg_tblspc.). Postgres is highly-customizable, I would prefer if features in core are designed so as we keep things extensible, the checksum verification for base backup on the contrary restricts things. So, do we have other opinions about this thread? -- Michael signature.asc Description: PGP signature
Re: Unordered wait event ClogGroupUpdate
On Wed, Oct 24, 2018 at 10:48 AM Michael Paquier wrote: > > That's a valid argument. Additionally, I've found > > WAIT_EVENT_HASH_GROW_BUCKETS_ALLOCATING and > > WAIT_EVENT_HASH_GROW_BATCHES_ALLOCATING are added in a > > non-alphabetical order in WaitEventIPC enum. > And those ones are also incorrect after another lookup: > - WAIT_EVENT_PARALLEL_FINISH > - WAIT_EVENT_HASH_GROW_BATCHES_DECIDING > - WAIT_EVENT_LOGICAL_APPLY_MAIN > I don't see more of them.. Nice. Same here. -- Thanks & Regards, Kuntal Ghosh EnterpriseDB: http://www.enterprisedb.com
Re: Unordered wait event ClogGroupUpdate
On Wed, Oct 24, 2018 at 10:25:37AM +0530, Kuntal Ghosh wrote: > That's a valid argument. Additionally, I've found > WAIT_EVENT_HASH_GROW_BUCKETS_ALLOCATING and > WAIT_EVENT_HASH_GROW_BATCHES_ALLOCATING are added in a > non-alphabetical order in WaitEventIPC enum. Indeed, thanks for double-checking. And those ones are also incorrect after another lookup: - WAIT_EVENT_PARALLEL_FINISH - WAIT_EVENT_HASH_GROW_BATCHES_DECIDING - WAIT_EVENT_LOGICAL_APPLY_MAIN I don't see more of them.. -- Michael signature.asc Description: PGP signature
Re: Restore CurrentUserId only if 'prevUser' is valid when abort transaction
On Tue, Oct 23, 2018 at 02:40:30PM +0900, Michael Paquier wrote:
> Actually, as StartSubTransaction also switches to TRANS_START for a
> savepoint, if there is an error until the state is switched to
> TRANS_INPROGRESS then the code would fail to switch back to
> CurrentUserId even if it is set, and it should be switched. So that
> solution is not correct either as AtSubStart_ResourceOwner() or such
> could fail on memory allocation. That's unlikely going to happen, but
> it could.
So, I have spent a couple of hours today looking a bit more at the
problem, and I have hacked the attached patch that I am pretty happy
with:
- Normal transactions can rely on TRANS_START to decide if the security
context can be reset or not.
- When defining a savepoint, the subtransaction state is initialized by
PushTransaction() before being switched to its sub-in-progress state
when the query which created the savepoint commits. In this case, we
should call GetUserIdAndSecContext() just before switching the
transaction context.
The patch includes a set tweaks I used to inject some errors in specific
code paths and trigger failures, checking if a security context which
has been set is correctly reset:
- /tmp/error_start for the end of StartTransaction
- /tmp/error_sub for the end of StartSubTransaction
- /tmp/error_push for the end of PushTransaction.
Like on HEAD, this patch still triggers the following WARNING if
injecting an error in PushTransaction as StartSubTransaction has not
switched the status of the transaction yet:
AbortSubTransaction while in DEFAULT state
Another WARNING which can be reached is the following if injecting an
error in StartSubTransaction:
AbortSubTransaction while in START state
Per the set of routines called when starting the subtransaction, I think
that we ought to do as main transactions and silence this warning
equally. I am attaching the patch for review by others. Please note
that this includes the error injections to ease tests.
--
Michael
diff --git a/src/backend/access/transam/xact.c b/src/backend/access/transam/xact.c
index 8c1621d949..92913fbb94 100644
--- a/src/backend/access/transam/xact.c
+++ b/src/backend/access/transam/xact.c
@@ -1823,6 +1823,12 @@ StartTransaction(void)
s->state = TRANS_START;
s->transactionId = InvalidTransactionId; /* until assigned */
+ {
+ struct stat statbuf;
+ if (stat("/tmp/error_start", ) == 0)
+ elog(ERROR, "error injected in StartTransaction");
+ }
+
/*
* Make sure we've reset xact state variables
*
@@ -1919,9 +1925,6 @@ StartTransaction(void)
s->childXids = NULL;
s->nChildXids = 0;
s->maxChildXids = 0;
- GetUserIdAndSecContext(>prevUser, >prevSecContext);
- /* SecurityRestrictionContext should never be set outside a transaction */
- Assert(s->prevSecContext == 0);
/*
* initialize other subsystems for new transaction
@@ -1930,6 +1933,15 @@ StartTransaction(void)
AtStart_Cache();
AfterTriggerBeginXact();
+ /*
+ * Security context initialization needs to happen just before switching
+ * the transaction state as resetting it to its previous state depends
+ * on the transaction status being TRANS_START.
+ */
+ GetUserIdAndSecContext(>prevUser, >prevSecContext);
+ /* SecurityRestrictionContext should never be set outside a transaction */
+ Assert(s->prevSecContext == 0);
+
/*
* done with start processing, set current transaction state to "in
* progress"
@@ -2520,28 +2532,34 @@ AbortTransaction(void)
* check the current transaction state
*/
is_parallel_worker = (s->blockState == TBLOCK_PARALLEL_INPROGRESS);
- if (s->state != TRANS_INPROGRESS && s->state != TRANS_PREPARE)
+ if (s->state != TRANS_START &&
+ s->state != TRANS_INPROGRESS &&
+ s->state != TRANS_PREPARE)
elog(WARNING, "AbortTransaction while in %s state",
TransStateAsString(s->state));
Assert(s->parent == NULL);
- /*
- * set the current transaction state information appropriately during the
- * abort processing
- */
- s->state = TRANS_ABORT;
-
/*
* Reset user ID which might have been changed transiently. We need this
* to clean up in case control escaped out of a SECURITY DEFINER function
* or other local change of CurrentUserId; therefore, the prior value of
* SecurityRestrictionContext also needs to be restored.
*
+ * If the transaction is aborted when it has been partially started, then
+ * the user ID does not need to be set to its previous value.
+ *
* (Note: it is not necessary to restore session authorization or role
* settings here because those can only be changed via GUC, and GUC will
* take care of rolling them back if need be.)
*/
- SetUserIdAndSecContext(s->prevUser, s->prevSecContext);
+ if (s->state != TRANS_START)
+ SetUserIdAndSecContext(s->prevUser, s->prevSecContext);
+
+ /*
+ * set the current transaction state information appropriately during the
+ * abort processing
+ */
+ s->state = TRANS_ABORT;
/* If in parallel mode, clean up workers and exit parallel
Re: Unordered wait event ClogGroupUpdate
On Wed, Oct 24, 2018 at 5:56 AM Michael Paquier wrote: > baaf272 has added support for group updates in clog, however it has > added the wait event WAIT_EVENT_CLOG_GROUP_UPDATE in a non-alphabetical > order. There are many events, so keeping things in order helps users in > finding them. That's a valid argument. Additionally, I've found WAIT_EVENT_HASH_GROW_BUCKETS_ALLOCATING and WAIT_EVENT_HASH_GROW_BATCHES_ALLOCATING are added in a non-alphabetical order in WaitEventIPC enum. > Are there any objections to the attached, which reorders things > properly? This is a patch for HEAD, for v11 I propose to only fix the > documentation side of things to avoid an ABI breakage. +1 -- Thanks & Regards, Kuntal Ghosh EnterpriseDB: http://www.enterprisedb.com
Re: [PATCH] Tab complete EXECUTE FUNCTION for CREATE (EVENT) TRIGGER
On Wed, Oct 24, 2018 at 08:43:05AM +0900, Michael Paquier wrote:
> On Tue, Oct 23, 2018 at 12:26:35PM +0100, Dagfinn Ilmari Mannsåker wrote:
> > The last-minute change for CREATE (EVENT) TRIGGER to accept EXECUTE
> > FUNCTION as well as EXECUTE PROCEDURE did not update the tab completion
> > in psql to match. Please find attached two patches that do this for
> > CREATE TRIGGER and CREATE EVENT TRIGGER, respectively.
>
> Yes, it would be nice to fix that.
>
> > To keep the duplication minimal, I've changed it from completing EXECUTE
> > PROCEDURE as a single thing to just EXECUTE, and then completing
> > FUNCTION or FUNCTION and PROCEDURE after that depending on the server
> > version.
>
> + else if (HeadMatches("CREATE", "EVENT", "TRIGGER") &&
> TailMatches("EXECUTE"))
> + if (pset.sversion >= 11)
> + COMPLETE_WITH("FUNCTION", "PROCEDURE");
> + else
> + COMPLETE_WITH("PROCEDURE");
>
> PROCEDURE is documented as deprecated as of v11 for CREATE TRIGGER
> and CREATE EVENT TRIGGER. Wouldn't it be better to just complete
> with FUNCTION for a v11 or newer server? I think that we want to
> encourage users to use EXECUTE FUNCTION if possible.
+1 for not completing with syntax we've just deprecated.
Best,
David.
--
David Fetter http://fetter.org/
Phone: +1 415 235 3778
Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate
Re: Log timestamps at higher resolution
On Wed, Oct 24, 2018 at 08:46:47AM +0900, Michael Paquier wrote:
> On Wed, Oct 24, 2018 at 12:11:18AM +0200, David Fetter wrote:
> > That's an interesting point. pgbadger is the only one I recall using
> > that's still maintained. Which others would it be useful to test?
>
> There could be private solutions as well. My take is that we should use
> separate letters and not change the existing ones or we'll get
> complains.
I believe that the utility of having finer time resolution outweighs
the inconvenience of changing processing systems, to the extent that
that's a consideration.
Adding yet more random letter options to log_line_prefix isn't free
either.
For one thing, there are a limited number of single-letter options we
can use, and we need to make sure they can continue to make sense, or
at least have reasonably consistent meanings.
For another, having separate letter rather than number modifiers as
printf("%03d") does, is just lousy API design. Baroque is OK for
music if you're in the mood, but not so great for log line prefix
format codes. If we do go the number option route, we can only not
break people's fussy parsers by having a default format of length 3, a
decision which will get harder and harder to justify as time goes on.
I'm happy to code up a number option if that's what people want, but
that'll break things the same way the current patch does, modulo The
Inexplicable Decision To Default To Three Decimal Places, which code
archaeologists will love and system implementers will hate.
> > Also, do we have tests--or at least ideas of how to
> > test--functionality relating to logging? I was a little bit taken
> > aback by the fact that `make check-world` passed after the change.
>
> This requires server-level changes where a TAP test is usually adapted,
> and there is no test for logging yet.
Is that worth a separate patch?
I suspect that as things Cloud™ (a.k.a. Renting Other People's Data
Centers) get even more popular, we'll want to improve our logging
game, and that will sooner rather than later have features complicated
enough to be worth testing.
Best,
David.
--
David Fetter http://fetter.org/
Phone: +1 415 235 3778
Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate
Re: Optimze usage of immutable functions as relation
Hello.
# It might be better that you provided self-contained test case.
As the discussion between Heikki and Tom just upthread, it would
be doable but that usage isn't typical. The query would be
normally written as followings and they are transformed as
desired.
select '|'||subject||'|', ts_rank_cd(body_tsvector,q) from
messages where body_tsvector @@ to_tsquery('tuple');
or (this doesn't look normal, thought..)
select '|'||subject||'|', ts_rank_cd(body_tsvector,q) from
messages, (select to_tsquery('tuple') as q) q
where body_tsvector @@ q;
This means that the wanted pull up logic is almost already
there. You should look on how it is handled.
At Sat, 20 Oct 2018 01:31:04 +0700, Aleksandr Parfenov wrote
in
> Hi,
>
> Thank you for the review.
> I fixed a typo and some comments. Please find new version attached.
I had the following error with the following query.
=# explain select * from pg_stat_get_activity(NULL) a join log(10.0) p on
a.pid = p.p;
ERROR: no relation entry for relid 2
As Andrew pointed, you are missing many things to do.
regards.
--
Kyotaro Horiguchi
NTT Open Source Software Center
Unordered wait event ClogGroupUpdate
Hi all, baaf272 has added support for group updates in clog, however it has added the wait event WAIT_EVENT_CLOG_GROUP_UPDATE in a non-alphabetical order. There are many events, so keeping things in order helps users in finding them. Are there any objections to the attached, which reorders things properly? This is a patch for HEAD, for v11 I propose to only fix the documentation side of things to avoid an ABI breakage. I checked the other wait events and things are in order. Thanks, -- Michael diff --git a/doc/src/sgml/monitoring.sgml b/doc/src/sgml/monitoring.sgml index 0484cfa77a..a173ebf5c3 100644 --- a/doc/src/sgml/monitoring.sgml +++ b/doc/src/sgml/monitoring.sgml @@ -1280,6 +1280,10 @@ postgres 27093 0.0 0.0 30096 2752 ?Ss 11:34 0:00 postgres: ser BtreePage Waiting for the page number needed to continue a parallel B-tree scan to become available. + + ClogGroupUpdate + Waiting for group leader to update transaction status at transaction end. + ExecuteGather Waiting for activity from child process when executing Gather node. @@ -1384,10 +1388,6 @@ postgres 27093 0.0 0.0 30096 2752 ?Ss 11:34 0:00 postgres: ser ProcArrayGroupUpdate Waiting for group leader to clear transaction id at transaction end. - - ClogGroupUpdate - Waiting for group leader to update transaction status at transaction end. - ReplicationOriginDrop Waiting for a replication origin to become inactive to be dropped. diff --git a/src/backend/postmaster/pgstat.c b/src/backend/postmaster/pgstat.c index 8de603d193..9c75e250bf 100644 --- a/src/backend/postmaster/pgstat.c +++ b/src/backend/postmaster/pgstat.c @@ -3582,6 +3582,9 @@ pgstat_get_wait_ipc(WaitEventIPC w) case WAIT_EVENT_BTREE_PAGE: event_name = "BtreePage"; break; + case WAIT_EVENT_CLOG_GROUP_UPDATE: + event_name = "ClogGroupUpdate"; + break; case WAIT_EVENT_EXECUTE_GATHER: event_name = "ExecuteGather"; break; @@ -3660,9 +3663,6 @@ pgstat_get_wait_ipc(WaitEventIPC w) case WAIT_EVENT_PROCARRAY_GROUP_UPDATE: event_name = "ProcArrayGroupUpdate"; break; - case WAIT_EVENT_CLOG_GROUP_UPDATE: - event_name = "ClogGroupUpdate"; - break; case WAIT_EVENT_REPLICATION_ORIGIN_DROP: event_name = "ReplicationOriginDrop"; break; diff --git a/src/include/pgstat.h b/src/include/pgstat.h index d59c24ae23..cdb3f6da04 100644 --- a/src/include/pgstat.h +++ b/src/include/pgstat.h @@ -802,6 +802,7 @@ typedef enum WAIT_EVENT_BGWORKER_SHUTDOWN = PG_WAIT_IPC, WAIT_EVENT_BGWORKER_STARTUP, WAIT_EVENT_BTREE_PAGE, + WAIT_EVENT_CLOG_GROUP_UPDATE, WAIT_EVENT_EXECUTE_GATHER, WAIT_EVENT_HASH_BATCH_ALLOCATING, WAIT_EVENT_HASH_BATCH_ELECTING, @@ -828,7 +829,6 @@ typedef enum WAIT_EVENT_PARALLEL_BITMAP_SCAN, WAIT_EVENT_PARALLEL_CREATE_INDEX_SCAN, WAIT_EVENT_PROCARRAY_GROUP_UPDATE, - WAIT_EVENT_CLOG_GROUP_UPDATE, WAIT_EVENT_REPLICATION_ORIGIN_DROP, WAIT_EVENT_REPLICATION_SLOT_DROP, WAIT_EVENT_SAFE_SNAPSHOT, signature.asc Description: PGP signature
Re: [HACKERS] Transactions involving multiple postgres foreign servers, take 2
On Tue, Oct 23, 2018 at 12:54 PM Kyotaro HORIGUCHI
wrote:
>
> Hello.
>
> # It took a long time to come here..
>
> At Fri, 19 Oct 2018 21:38:35 +0900, Masahiko Sawada
> wrote in
> > On Wed, Oct 10, 2018 at 1:34 PM Masahiko Sawada
> > wrote:
> ...
> > * Updated docs, added the new section "Distributed Transaction" at
> > Chapter 33 to explain the concept to users
> >
> > * Moved atomic commit codes into src/backend/access/fdwxact directory.
> >
> > * Some bug fixes.
> >
> > Please reivew them.
>
> I have some comments, with apologize in advance for possible
> duplicate or conflict with others' comments so far.
Thank youf so much for reviewing this patch!
>
> 0001:
>
> This sets XACT_FLAG_WROTENONTEMPREL when RELPERSISTENT_PERMANENT
> relation is modified. Isn't it needed when UNLOGGED tables are
> modified? It may be better that we have dedicated classification
> macro or function.
I think even if we do atomic commit for modifying the an UNLOGGED
table and a remote table the data will get inconsistent if the local
server crashes. For example, if the local server crashes after
prepared the transaction on foreign server but before the local commit
and, we will lose the all data of the local UNLOGGED table whereas the
modification of remote table is rollbacked. In case of persistent
tables, the data consistency is left. So I think the keeping data
consistency between remote data and local unlogged table is difficult
and want to leave it as a restriction for now. Am I missing something?
>
> The flag is handled in heapam.c. I suppose that it should be done
> in the upper layer considering coming pluggable storage.
> (X_F_ACCESSEDTEMPREL is set in heapam, but..)
>
Yeah, or we can set the flag after heap_insert in ExecInsert.
>
> 0002:
>
> The name FdwXactParticipantsForAC doesn't sound good for me. How
> about FdwXactAtomicCommitPartitcipants?
+1, will fix it.
>
> Well, as the file comment of fdwxact.c,
> FdwXactRegisterTransaction is called from FDW driver and
> F_X_MarkForeignTransactionModified is called from executor. I
> think that we should clarify who is responsible to the whole
> sequence. Since the state of local tables affects, I suppose
> executor is that. Couldn't we do the whole thing within executor
> side? I'm not sure but I feel that
> F_X_RegisterForeignTransaction can be a part of
> F_X_MarkForeignTransactionModified. The callers of
> MarkForeignTransactionModified can find whether the table is
> involved in 2pc by IsTwoPhaseCommitEnabled interface.
Indeed. We can register foreign servers by executor while FDWs don't
need to register anything. I will remove the registration function so
that FDW developers don't need to call the register function but only
need to provide atomic commit APIs.
>
>
> > if (foreign_twophase_commit == true &&
> > ((MyXactFlags & XACT_FLAGS_FDWNOPREPARE) != 0) )
> > ereport(ERROR,
> > (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
> >errmsg("cannot COMMIT a distributed
> > transaction that has operated on foreign server that doesn't support atomic
> > commit")));
>
> The error is emitted when a the GUC is turned off in the
> trasaction where MarkTransactionModify'ed. I think that the
> number of the variables' possible states should be reduced for
> simplicity. For example in the case, once foreign_twopase_commit
> is checked in a transaction, subsequent changes in the
> transaction should be ignored during the transaction.
>
I might have not gotten your comment correctly but since the
foreign_twophase_commit is a PGC_USERSET parameter I think we need to
check it at commit time. Also we need to keep participant servers even
when foreign_twophase_commit is off if both max_prepared_foreign_xacts
and max_foreign_xact_resolvers are > 0.
I will post the updated patch in this week.
Regards,
--
Masahiko Sawada
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center
Re: Function to promote standby servers
On Tue, Oct 23, 2018 at 09:42:16AM +0200, Laurenz Albe wrote: > No objections from me; on the contrary, I would like to thank you for > your effort here. I appreciate that you probably spent more time > tutoring me than it would have taken you to write this yourself. You're welcome. Happy that it helped. -- Michael signature.asc Description: PGP signature
Re: Buildfarm failures for hash indexes: buffer leaks
On Tue, Oct 23, 2018 at 07:50:57AM -0700, Andres Freund wrote: > FWIW, my animal 'serinus', which runs debian's gcc-snapshot shows the same > problem: > https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=serinus=2018-10-22%2006%3A34%3A02 > > So it seems much more likely to be 1). Thanks all for looking at it. Indeed it looks like a compiler issue here. -- Michael signature.asc Description: PGP signature
Re: Log timestamps at higher resolution
On Wed, Oct 24, 2018 at 12:11:18AM +0200, David Fetter wrote: > That's an interesting point. pgbadger is the only one I recall using > that's still maintained. Which others would it be useful to test? There could be private solutions as well. My take is that we should use separate letters and not change the existing ones or we'll get complains. > Also, do we have tests--or at least ideas of how to > test--functionality relating to logging? I was a little bit taken > aback by the fact that `make check-world` passed after the change. This requires server-level changes where a TAP test is usually adapted, and there is no test for logging yet. -- Michael signature.asc Description: PGP signature
Re: [PATCH] Tab complete EXECUTE FUNCTION for CREATE (EVENT) TRIGGER
On Tue, Oct 23, 2018 at 12:26:35PM +0100, Dagfinn Ilmari Mannsåker wrote:
> The last-minute change for CREATE (EVENT) TRIGGER to accept EXECUTE
> FUNCTION as well as EXECUTE PROCEDURE did not update the tab completion
> in psql to match. Please find attached two patches that do this for
> CREATE TRIGGER and CREATE EVENT TRIGGER, respectively.
Yes, it would be nice to fix that.
> To keep the duplication minimal, I've changed it from completing EXECUTE
> PROCEDURE as a single thing to just EXECUTE, and then completing
> FUNCTION or FUNCTION and PROCEDURE after that depending on the server
> version.
+ else if (HeadMatches("CREATE", "EVENT", "TRIGGER") &&
TailMatches("EXECUTE"))
+ if (pset.sversion >= 11)
+ COMPLETE_WITH("FUNCTION", "PROCEDURE");
+ else
+ COMPLETE_WITH("PROCEDURE");
PROCEDURE is documented as deprecated as of v11 for CREATE TRIGGER and
CREATE EVENT TRIGGER. Wouldn't it be better to just complete with
FUNCTION for a v11 or newer server? I think that we want to encourage
users to use EXECUTE FUNCTION if possible.
--
Michael
signature.asc
Description: PGP signature
Re: Log timestamps at higher resolution
On Tue, Oct 23, 2018 at 04:14:50PM -0300, Alvaro Herrera wrote:
> On 2018-Oct-23, David Fetter wrote:
>
> > On Wed, Oct 24, 2018 at 08:00:24AM +1300, Thomas Munro wrote:
> > > On Wed, Oct 24, 2018 at 7:51 AM David Fetter wrote:
> > > > Per gripes I've been hearing with increasing frequency, please find
> > > > attached a patch that implements $Subject. It's microsecond resolution
> > > > because at least at the moment, nanosecond resolution doesn't appear
> > > > to be helpful in this context.
> > >
> > > Wouldn't you want to choose a new letter or some other way to make
> > > existing format control strings do what they always did?
> >
> > I hadn't because I'd looked at the existing format as merely buggy in
> > lacking precision, although I guess with really fussy log processors,
> > this change could break things.
> >
> > Have you seen processors like that in the wild?
>
> pgbadger does this:
> '%m' => [('t_mtimestamp', '(\d{4}-\d{2}-\d{2}
> \d{2}:\d{2}:\d{2})\.\d+(?: [A-Z\+\-\d]{3,6})?')], # timestamp with
> milliseconds
>
> which should cope with however many digits there are (\d+).
> But I would expect others to be less forgiving ...
That's an interesting point. pgbadger is the only one I recall using
that's still maintained. Which others would it be useful to test?
Also, do we have tests--or at least ideas of how to
test--functionality relating to logging? I was a little bit taken
aback by the fact that `make check-world` passed after the change.
Best,
David.
--
David Fetter http://fetter.org/
Phone: +1 415 235 3778
Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate
Re: Regarding query minimizer (simplifier)
*Order is reversed. * *1.sql* 9.4.19: 20ms 10.5 : 1,227ms *4.sql* 9.4.19: 13ms 10.5 : 88,721ms *20.sql* 9.4.19: 271ms 10.5 : 6,104ms *22.sql* 9.4.19: 8ms 10.5 : 105ms On Tue, Oct 23, 2018 at 3:15 PM Jinho Jung wrote: > Hello, > > We appreciate you taking time for test! When we do more evaluation, we > noticed that the previously attached query made regression only on DBs that > we installed from APT manager (i.e., apt-get command) not on DBs that we > built from the source code. But we also confirmed that there are many cases > that cause regression to all DBs (installed from APT and build from source > code) > > Hope you can also test these queries too. These are the execution time on > our machine. > > *1.sql* > 10.5 : 20ms > 9.4.19: 1,227ms > > *4.sql* > 10.5 : 13ms > 9.4.19: 88,721ms > > *20.sql* > 10.5 : 271ms > 9.4.19: 6,104ms > > *22.sql* > 10.5 : 8ms > 9.4.19: 105ms > > Jinho Jung > > On Tue, Oct 23, 2018 at 9:52 AM Jung, Jinho wrote: > >> >> Hello Tom, >> >> >> Sorry for the misleading. Could you try these two queries? I made the >> query even slower in latest version of postgres. These are information >> about how we set up evaluation environment and query result. >> >> >> Thanks, >> >> Jinho Jung >> >> >> Install Multiple version of DBs in one machine >> == >> # Install 10.5 >> $ wget --quiet -O - >> https://www.postgresql.org/media/keys/ACCC4CF8.asc | sudo apt-key add - >> >> $ sudo sh -c 'echo "deb http://apt.postgresql.org/pub/repos/apt/ >> xenial-pgdg main" > /etc/apt/sources.list.d/pgdg_xenial.list' >> $ sudo apt update >> $ sudo apt-get install postgresql-10 >> >> # Install 9.6 >> $ sudo apt-get install postgresql-9.6 >> >> # Install 9.5 >> $ sudo apt-get install postgresql-9.5 >> >> # Install 9.4 >> $ sudo apt-get install postgresql-9.4 postgresql-contrib-9.4 >> libpq-dev postgresql-server-dev-9.4 >> >> # check >> $ pg_lsclusters >> >> >> Original regression query >> == >> explain analyze >> select >> 1 >> from >> information_schema.role_usage_grants as ref_2, >> lateral ( >> select >> max((null)) over (partition by ref_3.amopfamily) as c8 >> from >> pg_catalog.pg_amop as ref_3 >> ) as subq_0 >> ; >> >> ORIGINAL querying time >> on old version(9.4/9.5): 5.7ms >> on latest version(10): 91.76ms >> >> >> >> CORRELATED query to maximize error >> === >> explain analyze >> select * >> from information_schema.role_usage_grants f1 >> where grantor = >> ( select max(ref_2.grantor) >> from >>information_schema.role_usage_grants as ref_2, >>lateral ( >> select >>max((null)) over (partition by ref_3.amopfamily) as c8 >> from >> pg_catalog.pg_amop as ref_3 >> ) as subq_0 >> where ref_2.object_catalog = f1.object_catalog >> ) >> ; >> >> >> CORRELATED querying time >> on old version(9.4/9.5): 0.6s >> on latest version(10): 113s >> 188 times slower >> >> >> >> -- >> *From:* Tom Lane >> *Sent:* Saturday, October 13, 2018 5:59:06 PM >> *To:* Jung, Jinho >> *Cc:* pgsql-hackers@lists.postgresql.org >> *Subject:* Re: Regarding query minimizer (simplifier) >> >> "Jung, Jinho" writes: >> > Hello, I am Jinho Jung, Phd Student from GeorgiaTech, and try to find >> any SQL queries that cause performance regression. While conducting >> evaluation, I found an interesting query which makes x80 times slower >> execution in version 10.5 than version 9.4. Please see the attached files, >> if you are interested. >> >> Hm, testing this in the regression database, it seems pretty speedy >> across all supported branches, and indeed slower in 9.4 than later >> branches (~25 ms vs ~10 ms). >> >> It seems likely that you're testing in a very different database, >> perhaps one with many more tables ... but if you don't explain the >> test scenario, we aren't going to have much luck investigating. >> >> regards, tom lane >> >
Re: Regarding query minimizer (simplifier)
Hello, We appreciate you taking time for test! When we do more evaluation, we noticed that the previously attached query made regression only on DBs that we installed from APT manager (i.e., apt-get command) not on DBs that we built from the source code. But we also confirmed that there are many cases that cause regression to all DBs (installed from APT and build from source code) Hope you can also test these queries too. These are the execution time on our machine. *1.sql* 10.5 : 20ms 9.4.19: 1,227ms *4.sql* 10.5 : 13ms 9.4.19: 88,721ms *20.sql* 10.5 : 271ms 9.4.19: 6,104ms *22.sql* 10.5 : 8ms 9.4.19: 105ms Jinho Jung On Tue, Oct 23, 2018 at 9:52 AM Jung, Jinho wrote: > > Hello Tom, > > > Sorry for the misleading. Could you try these two queries? I made the > query even slower in latest version of postgres. These are information > about how we set up evaluation environment and query result. > > > Thanks, > > Jinho Jung > > > Install Multiple version of DBs in one machine > == > # Install 10.5 > $ wget --quiet -O - https://www.postgresql.org/media/keys/ACCC4CF8.asc > | sudo apt-key add - > $ sudo sh -c 'echo "deb http://apt.postgresql.org/pub/repos/apt/ > xenial-pgdg main" > /etc/apt/sources.list.d/pgdg_xenial.list' > $ sudo apt update > $ sudo apt-get install postgresql-10 > > # Install 9.6 > $ sudo apt-get install postgresql-9.6 > > # Install 9.5 > $ sudo apt-get install postgresql-9.5 > > # Install 9.4 > $ sudo apt-get install postgresql-9.4 postgresql-contrib-9.4 libpq-dev > postgresql-server-dev-9.4 > > # check > $ pg_lsclusters > > > Original regression query > == > explain analyze > select > 1 > from > information_schema.role_usage_grants as ref_2, > lateral ( > select > max((null)) over (partition by ref_3.amopfamily) as c8 > from > pg_catalog.pg_amop as ref_3 > ) as subq_0 > ; > > ORIGINAL querying time > on old version(9.4/9.5): 5.7ms > on latest version(10): 91.76ms > > > > CORRELATED query to maximize error > === > explain analyze > select * > from information_schema.role_usage_grants f1 > where grantor = > ( select max(ref_2.grantor) > from >information_schema.role_usage_grants as ref_2, >lateral ( > select >max((null)) over (partition by ref_3.amopfamily) as c8 > from > pg_catalog.pg_amop as ref_3 > ) as subq_0 > where ref_2.object_catalog = f1.object_catalog > ) > ; > > > CORRELATED querying time > on old version(9.4/9.5): 0.6s > on latest version(10): 113s > 188 times slower > > > > -- > *From:* Tom Lane > *Sent:* Saturday, October 13, 2018 5:59:06 PM > *To:* Jung, Jinho > *Cc:* pgsql-hackers@lists.postgresql.org > *Subject:* Re: Regarding query minimizer (simplifier) > > "Jung, Jinho" writes: > > Hello, I am Jinho Jung, Phd Student from GeorgiaTech, and try to find > any SQL queries that cause performance regression. While conducting > evaluation, I found an interesting query which makes x80 times slower > execution in version 10.5 than version 9.4. Please see the attached files, > if you are interested. > > Hm, testing this in the regression database, it seems pretty speedy > across all supported branches, and indeed slower in 9.4 than later > branches (~25 ms vs ~10 ms). > > It seems likely that you're testing in a very different database, > perhaps one with many more tables ... but if you don't explain the > test scenario, we aren't going to have much luck investigating. > > regards, tom lane > 22.sql Description: application/sql 20.sql Description: application/sql 4.sql Description: application/sql 1.sql Description: application/sql
Re: Log timestamps at higher resolution
On 2018-Oct-23, David Fetter wrote:
> On Wed, Oct 24, 2018 at 08:00:24AM +1300, Thomas Munro wrote:
> > On Wed, Oct 24, 2018 at 7:51 AM David Fetter wrote:
> > > Per gripes I've been hearing with increasing frequency, please find
> > > attached a patch that implements $Subject. It's microsecond resolution
> > > because at least at the moment, nanosecond resolution doesn't appear
> > > to be helpful in this context.
> >
> > Wouldn't you want to choose a new letter or some other way to make
> > existing format control strings do what they always did?
>
> I hadn't because I'd looked at the existing format as merely buggy in
> lacking precision, although I guess with really fussy log processors,
> this change could break things.
>
> Have you seen processors like that in the wild?
pgbadger does this:
'%m' => [('t_mtimestamp', '(\d{4}-\d{2}-\d{2}
\d{2}:\d{2}:\d{2})\.\d+(?: [A-Z\+\-\d]{3,6})?')], # timestamp with milliseconds
which should cope with however many digits there are (\d+).
But I would expect others to be less forgiving ...
--
Álvaro Herrerahttps://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Re: Log timestamps at higher resolution
On Wed, Oct 24, 2018 at 08:00:24AM +1300, Thomas Munro wrote: > On Wed, Oct 24, 2018 at 7:51 AM David Fetter wrote: > > Per gripes I've been hearing with increasing frequency, please find > > attached a patch that implements $Subject. It's microsecond resolution > > because at least at the moment, nanosecond resolution doesn't appear > > to be helpful in this context. > > Wouldn't you want to choose a new letter or some other way to make > existing format control strings do what they always did? I hadn't because I'd looked at the existing format as merely buggy in lacking precision, although I guess with really fussy log processors, this change could break things. Have you seen processors like that in the wild? Best, David. -- David Fetter http://fetter.org/ Phone: +1 415 235 3778 Remember to vote! Consider donating to Postgres: http://www.postgresql.org/about/donate
Re: Log timestamps at higher resolution
On Wed, Oct 24, 2018 at 7:51 AM David Fetter wrote: > Per gripes I've been hearing with increasing frequency, please find > attached a patch that implements $Subject. It's microsecond resolution > because at least at the moment, nanosecond resolution doesn't appear > to be helpful in this context. Wouldn't you want to choose a new letter or some other way to make existing format control strings do what they always did? -- Thomas Munro http://www.enterprisedb.com
Log timestamps at higher resolution
Folks,
Per gripes I've been hearing with increasing frequency, please find
attached a patch that implements $Subject. It's microsecond resolution
because at least at the moment, nanosecond resolution doesn't appear
to be helpful in this context.
Best,
David.
--
David Fetter http://fetter.org/
Phone: +1 415 235 3778
Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate
>From 29e620bbae9d458e5789350ed19d63c48b8459ff Mon Sep 17 00:00:00 2001
From: David Fetter
Date: Tue, 23 Oct 2018 11:35:34 -0700
Subject: [PATCH] log_line_prefix was milliseconds: now microseconds
To: pgsql-hack...@postgresql.org
---
doc/src/sgml/config.sgml | 6 +++---
src/backend/utils/error/elog.c| 14 +++---
src/backend/utils/misc/postgresql.conf.sample | 6 +++---
3 files changed, 13 insertions(+), 13 deletions(-)
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index 7554cba3f9..8faaa35b86 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -5593,17 +5593,17 @@ local0.*/var/log/postgresql
%t
- Time stamp without milliseconds
+ Time stamp without microseconds
no
%m
- Time stamp with milliseconds
+ Time stamp with microseconds
no
%n
- Time stamp with milliseconds (as a Unix epoch)
+ Time stamp with microseconds (as a Unix epoch)
no
diff --git a/src/backend/utils/error/elog.c b/src/backend/utils/error/elog.c
index b9c11301ca..70a61fde4c 100644
--- a/src/backend/utils/error/elog.c
+++ b/src/backend/utils/error/elog.c
@@ -,7 +,7 @@ static void
setup_formatted_log_time(void)
{
pg_time_t stamp_time;
- char msbuf[13];
+ char msbuf[16];
if (!saved_timeval_set)
{
@@ -2238,13 +2238,13 @@ setup_formatted_log_time(void)
* nonempty or CSV mode can be selected.
*/
pg_strftime(formatted_log_time, FORMATTED_TS_LEN,
- /* leave room for milliseconds... */
-"%Y-%m-%d %H:%M:%S %Z",
+ /* leave room for microseconds... */
+"%Y-%m-%d %H:%M:%S%Z",
pg_localtime(_time, log_timezone));
- /* 'paste' milliseconds into place... */
- sprintf(msbuf, ".%03d", (int) (saved_timeval.tv_usec / 1000));
- memcpy(formatted_log_time + 19, msbuf, 4);
+ /* 'paste' microseconds into place... */
+ sprintf(msbuf, ".%06d", saved_timeval.tv_usec);
+ memcpy(formatted_log_time + 19, msbuf, 7);
}
/*
@@ -2665,7 +2665,7 @@ write_csvlog(ErrorData *edata)
initStringInfo();
/*
- * timestamp with milliseconds
+ * timestamp with microseconds
*
* Check if the timestamp is already calculated for the syslog message,
* and use it if so. Otherwise, get the current timestamp. This is done
diff --git a/src/backend/utils/misc/postgresql.conf.sample b/src/backend/utils/misc/postgresql.conf.sample
index 4e61bc6521..dcea840b8f 100644
--- a/src/backend/utils/misc/postgresql.conf.sample
+++ b/src/backend/utils/misc/postgresql.conf.sample
@@ -470,9 +470,9 @@
# %r = remote host and port
# %h = remote host
# %p = process ID
- # %t = timestamp without milliseconds
- # %m = timestamp with milliseconds
- # %n = timestamp with milliseconds (as a Unix epoch)
+ # %t = timestamp without microseconds
+ # %m = timestamp with microseconds
+ # %n = timestamp with microseconds (as a Unix epoch)
# %i = command tag
# %e = SQL state
# %c = session ID
--
2.17.2
GOOGLE CODE-IN 2018
Hello, I am extremely excited to be part of PostgreSQL... Every task seems to be exciting! I have basic knowledge of SQL and am thinking of being part of your organization's tasks. Thankyou. Yours faithfully, PMS
RE: Postgres older version 8.3.7 on ubuntu 14
Thank you so much for your prompt reply, Background on this, We have postgres same version for the past 12 years with 8.3.7, We are moving to cloud GCP To start with testing the 8.3.7 on ubuntu 14.04 LTS version, I am finding difficulty in installing the 8.3.7 version, Here is the latest error, checking build system type... x86_64-unknown-linux-gnu checking host system type... x86_64-unknown-linux-gnu checking which template to use... linux checking whether to build with 64-bit integer date/time support... no checking whether NLS is wanted... no checking for default port number... 5432 checking for gcc... gcc checking for C compiler default output file name... a.out checking whether the C compiler works... yes checking whether we are cross compiling... no checking for suffix of executables... checking for suffix of object files... o checking whether we are using the GNU C compiler... yes checking whether gcc accepts -g... yes checking for gcc option to accept ANSI C... none needed checking if gcc supports -Wdeclaration-after-statement... yes checking if gcc supports -Wendif-labels... yes checking if gcc supports -fno-strict-aliasing... yes checking if gcc supports -fwrapv... yes configure: using CFLAGS=-O2 -Wall -Wmissing-prototypes -Wpointer-arith -Winline -Wdeclaration-after-statement -Wendif-labels -fno-strict-aliasing -fwrapv checking whether the C compiler still works... yes checking how to run the C preprocessor... gcc -E checking allow thread-safe client libraries... no checking whether to build with Tcl... no checking whether to build Perl modules... no checking whether to build Python modules... no checking whether to build with GSSAPI support... no checking whether to build with Kerberos 5 support... no checking whether to build with PAM support... no checking whether to build with LDAP support... no checking whether to build with Bonjour support... no checking whether to build with OpenSSL support... no checking for egrep... grep -E configure: using CPPFLAGS= -D_GNU_SOURCE configure: using LDFLAGS= checking for ld used by GCC... /usr/bin/ld checking if the linker (/usr/bin/ld) is GNU ld... yes checking for ranlib... ranlib checking for strip... strip checking whether it is possible to strip libraries... yes checking for tar... /bin/tar checking whether ln -s works... yes checking for gawk... gawk checking for bison... no configure: WARNING: *** Without Bison you will not be able to build PostgreSQL from CVS nor *** change any of the parser definition files. You can obtain Bison from *** a GNU mirror site. (If you are using the official distribution of *** PostgreSQL then you do not need to worry about this, because the Bison *** output is pre-generated.) To use a different yacc program (possible, *** but not recommended), set the environment variable YACC before running *** 'configure'. checking for flex... no configure: WARNING: *** Without Flex you will not be able to build PostgreSQL from CVS or *** change any of the scanner definition files. You can obtain Flex from *** a GNU mirror site. (If you are using the official distribution of *** PostgreSQL then you do not need to worry about this because the Flex *** output is pre-generated.) checking for perl... /usr/bin/perl checking for main in -lm... yes checking for library containing setproctitle... no checking for library containing dlopen... -ldl checking for library containing socket... none required checking for library containing shl_load... no checking for library containing getopt_long... none required checking for library containing crypt... -lcrypt checking for library containing fdatasync... none required checking for library containing shmget... none required checking for -lreadline... no checking for -ledit... no configure: error: readline library not found If you have readline already installed, see config.log for details on the failure. It is possible the compiler isn't looking in the proper directory. Use --without-readline to disable readline support. root@postgres83ubuntu:/home/rsa-key-20181011/postgresql-8.3.7# CFLAGS=-fno-aggressive-loop-optimizations ./configure --prefix /usr/local/pgsql checking build system type... x86_64-unknown-linux-gnu checking host system type... x86_64-unknown-linux-gnu checking which template to use... linux checking whether to build with 64-bit integer date/time support... no checking whether NLS is wanted... no checking for default port number... 5432 checking for gcc... gcc checking for C compiler default output file name... a.out checking whether the C compiler works... yes checking whether we are cross compiling... no checking for suffix of executables... checking for suffix of object files... o checking whether we are using the GNU C compiler... yes checking whether gcc accepts -g... yes checking for gcc option to accept ANSI C... none needed checking if gcc supports -Wdeclaration-after-statement... yes checking if gcc supports -Wendif-labels... yes checking if gcc supports
Re: Remove obsolete pg_attrdef.adsrc column
> On 23 Oct 2018, at 15:17, Peter Eisentraut > wrote: > > I propose the attached patch to remove the long-unused catalog column > pg_attrdef.adsrc. +1, I ran into a bug in an app as recently as today where adsrc was used instead of pg_get_expr(). Patch looks good. I probably would’ve opted for mentioning how to get a human readable version on the page, along the lines of the attached version, but I may be biased from having dealt with apps that need just that. cheers ./daniel petere-attrdef_adsrc_remove.diff Description: Binary data
Re: Postgres older version 8.3.7 on ubuntu 14
On 10/23/18 1:51 PM, Vaidyanathaswamy, Anandsaikrishnan wrote: > configure: error: readline library not found > If you have readline already installed, see config.log for details on the > failure. It is possible the compiler isn't looking in the proper directory. > Use --without-readline to disable readline support. The missing library provides features like previous command recall and editing in psql. If those features are valuable to you, you can install the readline library; otherwise, you can add --without-readline on the configure command, and build PostgreSQL without it. -Chap
Re: Remove obsolete pg_attrdef.adsrc column
On 2018-Oct-23, Peter Eisentraut wrote: > I propose the attached patch to remove the long-unused catalog column > pg_attrdef.adsrc. +1, looks good. I think this change has been waiting for a very long time -- documented as useless by 81c41e3d0ed3 (Jan 2005, general doc copy-edit, a paragraph you're now removing). Interestingly, it seems pg_dump stopped relying on that column as a side-effect of 9f0ae0c82060 (May 2002, "First pass at schema-fying pg_dump/pg_restore"); adsrc remained used for 7.1 and older only, which was removed afterwards. -- Álvaro Herrerahttps://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Re: [HACKERS] proposal: schema variables
Hi út 23. 10. 2018 v 14:50 odesílatel Erik Rijkers napsal: > > [schema-variables-20181007-01.patch.gz] > > Hi, > > I tried to test your schema-variables patch but got stuck here instead > (after applying succesfully on top of e6f5d1acc): > > make[2]: *** No rule to make target > '../../../src/include/catalog/pg_variable.h', needed by 'bki-stamp'. > Stop. > make[1]: *** [submake-catalog-headers] Error 2 > make[1]: *** Waiting for unfinished jobs > make: *** [submake-generated-headers] Error 2 > Makefile:141: recipe for target 'submake-catalog-headers' failed > src/Makefile.global:370: recipe for target 'submake-generated-headers' > failed > > Unfortunately previous patch was completely broken. I am sorry Please, check this patch. Regards Pavel > thanks, > > Erik Rijkers > schema-variables-20181023-01.patch.gz Description: application/gzip
Re: Postgres older version 8.3.7 on ubuntu 14
On 10/23/18 13:15, Steve Crawford wrote: > On Tue, Oct 23, 2018 at 10:08 AM Vaidyanathaswamy, Anandsaikrishnan < > avaidyanathasw...@corelogic.com> wrote: >> We are not able proceed with the installation manually. I am wondering >> whether ubuntu 14 is compatible with version 8.3.7 ... > First, I would *strongly* suggest upgrading to a newer version for > security, performance and support. > > But if you insist on compiling, the answer is in your output - it isn't > finding a compiler. Be sure to install gcc or other compiler. It would certainly be interesting to know why such an old version is needed. Also, even if it has to be 8.3 for some reason, does it have to be 8.3.7 and not the last maintenance release 8.3.23 ? I have found that to build 8.3 or 8.2 using modern gcc, it is necessary to add a C flag -fno-aggressive-loop-optimizations as in $ CFLAGS=-fno-aggressive-loop-optimizations ./configure But of course, that comes after the first problem, installing a C compiler in the first place. :) Alternatively, you can build from the git commit exactly one later than the one tagged REL8_3_23. That commit just adds the extra C flag. -Chap
Re: Postgres older version 8.3.7 on ubuntu 14
On Tue, Oct 23, 2018 at 10:08 AM Vaidyanathaswamy, Anandsaikrishnan < avaidyanathasw...@corelogic.com> wrote: > Good Morning, > > > > We have older version of postgres 8.3.7, I am trying to install postgres > on ubuntu 14.04 LTS under GCP instance, Im having difficulty in > installating the postgres 8.3.7 . > > > > We are not able proceed with the installation manually. I am wondering > whether ubuntu 14 is compatible with version 8.3.7 > > > > root@postgres83ubuntu:/home/rsa-key-20181011/postgresql-8.3.7# ./configure > > checking build system type... x86_64-unknown-linux-gnu > > checking host system type... x86_64-unknown-linux-gnu > > checking which template to use... linux > > checking whether to build with 64-bit integer date/time support... no > > checking whether NLS is wanted... no > > checking for default port number... 5432 > > checking for gcc... no > > checking for cc... no > > configure: error: no acceptable C compiler found in $PATH > > > > > First, I would *strongly* suggest upgrading to a newer version for security, performance and support. But if you insist on compiling, the answer is in your output - it isn't finding a compiler. Be sure to install gcc or other compiler. Cheers, Steve
Postgres older version 8.3.7 on ubuntu 14
Good Morning, We have older version of postgres 8.3.7, I am trying to install postgres on ubuntu 14.04 LTS under GCP instance, Im having difficulty in installating the postgres 8.3.7 . We are not able proceed with the installation manually. I am wondering whether ubuntu 14 is compatible with version 8.3.7 root@postgres83ubuntu:/home/rsa-key-20181011/postgresql-8.3.7# ./configure checking build system type... x86_64-unknown-linux-gnu checking host system type... x86_64-unknown-linux-gnu checking which template to use... linux checking whether to build with 64-bit integer date/time support... no checking whether NLS is wanted... no checking for default port number... 5432 checking for gcc... no checking for cc... no configure: error: no acceptable C compiler found in $PATH See `config.log' for more details. I am able to perform the 8.4 version with the following documents. https://erpnani.blogspot.com/2016/03/install-postgresql-84-from-its-official.html Do you recommend any such document, Your help is much appreciated. Thanks ! sai ** This message may contain confidential or proprietary information intended only for the use of the addressee(s) named above or may contain information that is legally privileged. If you are not the intended addressee, or the person responsible for delivering it to the intended addressee, you are hereby notified that reading, disseminating, distributing or copying this message is strictly prohibited. If you have received this message by mistake, please immediately notify us by replying to the message and delete the original message and any copies immediately thereafter. Thank you. ** CLLD
Re: Pull up sublink of type 'NOT NOT (expr)'
Hello Richard, Currently for quals in the form of "NOT NOT (SubLink)", this SubLink would not be considered when pulling up sublinks. For instance: gpadmin=# explain select * from a where NOT NOT (a.i in (select b.i from b)); QUERY PLAN - Seq Scan on a (cost=51.50..85.62 rows=1005 width=8) Filter: (hashed SubPlan 1) SubPlan 1 -> Seq Scan on b (cost=0.00..44.00 rows=3000 width=4) (4 rows) Should we give it a chance, like the attached does? Sometimes hashed subplan is faster than hash join and than all the other options, as it preserves the order. Using NOT NOT, one can control whether to use it or not: https://pgblog.bashtanov.com/2017/12/08/double-negative-and-query-performance/ (test case and results in the bottom of the page). Surely dirty tricks should not be the way to control the planner, but when breaking them we should probably provide a way to achieve the same result, ideally making the planner choose the best plan without hints. Best, Alex
Re: Buildfarm failures for hash indexes: buffer leaks
On 2018-10-23 13:54:31 +0200, Fabien COELHO wrote: > > Hello Tom & Amit, > > > > > Both animals use gcc experimental versions, which may rather underline a > > > > new bug in gcc head rather than an existing issue in pg. Or not. > > > > > It is possible, but what could be the possible theory? > > > > It seems like the two feasible theories are (1) gcc bug, or (2) buffer > > leak that only occurs in very narrow circumstances, perhaps from a race > > condition. Given that the hash index code hasn't changed meaningfully > > in several months, I thought (1) seemed more probable. > > Yep, that is my thought as well. FWIW, my animal 'serinus', which runs debian's gcc-snapshot shows the same problem: https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=serinus=2018-10-22%2006%3A34%3A02 So it seems much more likely to be 1). > The problem is that this kind of issue is not simple to wrap-up as a gcc bug > report, unlike other earlier instances that I forwarded to clang & gcc dev > teams. > > I'm in favor in waiting before trying to report it, to check whether the > probable underlying gcc problem is detected, reported by someone else, and > fixed in gcc head. If it persists, then we'll see. I suspect the easiest thing to narrow it down would be to bisect the problem in gcc :( Greetings, Andres Freund
None-reentrant function call in signal handler startup_die()
Hi, I found that in the PG source code function BackendInitialize(), handler for SIGTERM was set to be startup_die(). And in startup_die(), we simply call proc_exit to exit the process. What's more, early in BackendInitialize() function, we called pq_init to setup socket_close() as a process exit callback. The problem is: in socket_close, we have some none-reentrant calls like free(). It may cause deadlock of this backend if we are excueting malloc/free right before we step into the signal hander startup_die(). And I experienced that on my local server :) Similar to the problem in this thread: https://www.postgresql-archive.org/PG-signal-handler-and-non-reentrant-malloc-free-calls-td3403162.html. Is there a way to avoid this deadlock in startup_die()? Thanks.. -- Sent from: http://www.postgresql-archive.org/PostgreSQL-hackers-f1928748.html
Re: Regarding query minimizer (simplifier)
Hello Tom, Sorry for the misleading. Could you try these two queries? I made the query even slower in latest version of postgres. These are information about how we set up evaluation environment and query result. Thanks, Jinho Jung Install Multiple version of DBs in one machine == # Install 10.5 $ wget --quiet -O - https://www.postgresql.org/media/keys/ACCC4CF8.asc | sudo apt-key add - $ sudo sh -c 'echo "deb http://apt.postgresql.org/pub/repos/apt/ xenial-pgdg main" > /etc/apt/sources.list.d/pgdg_xenial.list' $ sudo apt update $ sudo apt-get install postgresql-10 # Install 9.6 $ sudo apt-get install postgresql-9.6 # Install 9.5 $ sudo apt-get install postgresql-9.5 # Install 9.4 $ sudo apt-get install postgresql-9.4 postgresql-contrib-9.4 libpq-dev postgresql-server-dev-9.4 # check $ pg_lsclusters Original regression query == explain analyze select 1 from information_schema.role_usage_grants as ref_2, lateral ( select max((null)) over (partition by ref_3.amopfamily) as c8 from pg_catalog.pg_amop as ref_3 ) as subq_0 ; ORIGINAL querying time on old version(9.4/9.5): 5.7ms on latest version(10): 91.76ms CORRELATED query to maximize error === explain analyze select * from information_schema.role_usage_grants f1 where grantor = ( select max(ref_2.grantor) from information_schema.role_usage_grants as ref_2, lateral ( select max((null)) over (partition by ref_3.amopfamily) as c8 from pg_catalog.pg_amop as ref_3 ) as subq_0 where ref_2.object_catalog = f1.object_catalog ) ; CORRELATED querying time on old version(9.4/9.5): 0.6s on latest version(10): 113s 188 times slower From: Tom Lane Sent: Saturday, October 13, 2018 5:59:06 PM To: Jung, Jinho Cc: pgsql-hackers@lists.postgresql.org Subject: Re: Regarding query minimizer (simplifier) "Jung, Jinho" writes: > Hello, I am Jinho Jung, Phd Student from GeorgiaTech, and try to find any SQL > queries that cause performance regression. While conducting evaluation, I > found an interesting query which makes x80 times slower execution in version > 10.5 than version 9.4. Please see the attached files, if you are interested. Hm, testing this in the regression database, it seems pretty speedy across all supported branches, and indeed slower in 9.4 than later branches (~25 ms vs ~10 ms). It seems likely that you're testing in a very different database, perhaps one with many more tables ... but if you don't explain the test scenario, we aren't going to have much luck investigating. regards, tom lane query_regression Description: query_regression query_much_regression Description: query_much_regression
Re: BUG #15448: server process (PID 22656) was terminated by exception 0xC0000005
Hi, On Tue, Oct 23, 2018 at 8:46 PM Andrew Dunstan wrote: > On 10/22/2018 10:00 PM, Amit Langote wrote: > > After observing the test case in the provided log, I managed to reproduce > > it with the following: > > > > create table foo (a int primary key, b int); > > create table bar (a int references foo on delete cascade, b int); > > insert into foo values (1, 1); > > insert into foo values (2, 2); > > alter table foo add c int; > > alter table foo drop c; > > delete from foo; > > server closed the connection unexpectedly > > This probably means the server terminated abnormally > > before or while processing the request. > > The connection to the server was lost. Attempting reset: Failed. > > > > Analyzing this crash, I located the bug down to GetTupleForTrigger(), but > > perhaps it's really in heap_expand_tuple() / expand_tuple(), where the > > value of trigger tuple's t_self is being switched from a valid one to an > > invalid value. > > > > In heaptuple.c: expand_tuple() > > > > > > ItemPointerSetInvalid(&((*targetHeapTuple)->t_self)); > > > > > > FWIW, attached patch fixes this for me. Adding Andrew whose recent commit > > 7636e5c60f [1] seems to have introduced the heap_expan_tuple call in > > GetTupleForTrigger. Maybe, he can better judge a fix for this. > > Thanks. I think the line in expand_tuple is a thinko and we should > change it, rather than change GetTupleForTrigger(). Agreed. > Here is a patch that does that and also adds your test case to the > regression tests. Looks good. Thanks, Amit
Re: WIP: Avoid creation of the free space map for small tables
I wrote: > Once this is in shape, I'll do some performance testing. On second thought, there's no point in waiting, especially if a regression points to a design flaw. I compiled patched postgres with HEAP_FSM_CREATION_THRESHOLD set to 32, then ran the attached script which populates 100 tables with varying numbers of blocks. I wanted a test that created pages eagerly and wrote to disk as little as possible. Config was stock, except for fsync = off. I took the average of 10 runs after removing the slowest and fastest run: # blocksmaster patch 4 36.4ms 33.9ms 8 50.6ms 48.9ms 12 58.6ms 66.3ms 16 65.5ms 81.4ms It seems under these circumstances a threshold of up to 8 performs comparably to the master branch, with small block numbers possibly faster than with the FSM, provided they're in shared buffers already. I didn't bother testing higher values because it's clear there's a regression starting around 10 or so, beyond which it helps to have the FSM. A case could be made for setting the threshold to 4, since not having 3 blocks of FSM in shared buffers exactly makes up for the 3 other blocks of heap that are checked when free space runs out. I can run additional tests if there's interest. -John Naylor fsm-copy-test.sql Description: application/sql
Remove obsolete pg_attrdef.adsrc column
I propose the attached patch to remove the long-unused catalog column
pg_attrdef.adsrc.
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
From df811a76e026be395a4309a1f0de75540dae5b11 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut
Date: Tue, 23 Oct 2018 14:58:40 +0200
Subject: [PATCH] Remove obsolete pg_attrdef.adsrc column
This has been deprecated and effectively unused for a long time.
---
doc/src/sgml/catalogs.sgml | 16
src/backend/catalog/heap.c | 12
src/include/catalog/pg_attrdef.h | 1 -
3 files changed, 29 deletions(-)
diff --git a/doc/src/sgml/catalogs.sgml b/doc/src/sgml/catalogs.sgml
index 9edba96fab..2ae3070287 100644
--- a/doc/src/sgml/catalogs.sgml
+++ b/doc/src/sgml/catalogs.sgml
@@ -949,25 +949,9 @@ pg_attrdef Columns
The internal representation of the column default value
-
-
- adsrc
- text
-
- A human-readable representation of the default value
-
-
-
-The adsrc field is historical, and is best
-not used, because it does not track outside changes that might affect
-the representation of the default value. Reverse-compiling the
-adbin field (with
pg_get_expr for
-example) is a better way to display the default value.
-
-
diff --git a/src/backend/catalog/heap.c b/src/backend/catalog/heap.c
index 3c9c03c997..640e283688 100644
--- a/src/backend/catalog/heap.c
+++ b/src/backend/catalog/heap.c
@@ -2152,7 +2152,6 @@ StoreAttrDefault(Relation rel, AttrNumber attnum,
Node *expr, bool is_internal, bool
add_column_mode)
{
char *adbin;
- char *adsrc;
Relationadrel;
HeapTuple tuple;
Datum values[4];
@@ -2169,21 +2168,12 @@ StoreAttrDefault(Relation rel, AttrNumber attnum,
*/
adbin = nodeToString(expr);
- /*
-* Also deparse it to form the mostly-obsolete adsrc field.
-*/
- adsrc = deparse_expression(expr,
-
deparse_context_for(RelationGetRelationName(rel),
-
RelationGetRelid(rel)),
- false, false);
-
/*
* Make the pg_attrdef entry.
*/
values[Anum_pg_attrdef_adrelid - 1] = RelationGetRelid(rel);
values[Anum_pg_attrdef_adnum - 1] = attnum;
values[Anum_pg_attrdef_adbin - 1] = CStringGetTextDatum(adbin);
- values[Anum_pg_attrdef_adsrc - 1] = CStringGetTextDatum(adsrc);
adrel = heap_open(AttrDefaultRelationId, RowExclusiveLock);
@@ -2198,10 +2188,8 @@ StoreAttrDefault(Relation rel, AttrNumber attnum,
/* now can free some of the stuff allocated above */
pfree(DatumGetPointer(values[Anum_pg_attrdef_adbin - 1]));
- pfree(DatumGetPointer(values[Anum_pg_attrdef_adsrc - 1]));
heap_freetuple(tuple);
pfree(adbin);
- pfree(adsrc);
/*
* Update the pg_attribute entry for the column to show that a default
diff --git a/src/include/catalog/pg_attrdef.h b/src/include/catalog/pg_attrdef.h
index e4a37ec326..a9a2351efd 100644
--- a/src/include/catalog/pg_attrdef.h
+++ b/src/include/catalog/pg_attrdef.h
@@ -33,7 +33,6 @@ CATALOG(pg_attrdef,2604,AttrDefaultRelationId)
#ifdef CATALOG_VARLEN /* variable-length fields start here */
pg_node_tree adbin BKI_FORCE_NOT_NULL; /* nodeToString
representation of default */
- textadsrc BKI_FORCE_NOT_NULL; /*
human-readable representation of default */
#endif
} FormData_pg_attrdef;
base-commit: 55853d666ce5d0024c50dc3d223346df28abfa70
--
2.19.1
Re: Making all nbtree entries unique by having heap TIDs participate in comparisons
On Tue, Oct 23, 2018 at 11:35 AM Andrey Lepikhov wrote: > I have same problem with background heap & index cleaner (based on your > patch). In this case the bottleneck is WAL-record which I need to write > for each cleaned block and locks which are held during the WAL-record > writing process. Part of the problem here is that v6 uses up to 25 candidate split points, even during regularly calls to _bt_findsplitloc(). That was based on some synthetic test-cases. I've found that I can get most of the benefit in index size with far fewer spilt points, though. The extra work done with an exclusive buffer lock held will be considerably reduced in v7. I'll probably post that in a couple of weeks, since I'm in Europe for pgConf.EU. I don't fully understand the problems here, but even still I know that what you were testing wasn't very well optimized for write-heavy workloads. It would be especially bad with pgbench, since there isn't much opportunity to reduce the size of indexes there. > Maybe you will do a test without writing any data to disk? Yeah, I should test that on its own. I'm particularly interested in TPC-C, because it's a particularly good target for my patch. I can find a way of only executing the read TPC-C queries, to see where they are on their own. TPC-C is particularly write-heavy, especially compared to the much more recent though less influential TPC-E benchmark. -- Peter Geoghegan
Re: removing unnecessary get_att*() lsyscache functions
On 23/10/2018 02:11, Michael Paquier wrote: > On Mon, Oct 22, 2018 at 07:12:28PM +0200, Peter Eisentraut wrote: >> OK, slightly reworked version attached. > > + attTup = (Form_pg_attribute) GETSTRUCT(tuple); > attnum = ((Form_pg_attribute) GETSTRUCT(tuple))->attnum; > > No need to call twice GETSTRUCT here.. The rest looks fine. Fixed that, and committed, thanks. -- Peter Eisentraut http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Re: [HACKERS] proposal: schema variables
[schema-variables-20181007-01.patch.gz] Hi, I tried to test your schema-variables patch but got stuck here instead (after applying succesfully on top of e6f5d1acc): make[2]: *** No rule to make target '../../../src/include/catalog/pg_variable.h', needed by 'bki-stamp'. Stop. make[1]: *** [submake-catalog-headers] Error 2 make[1]: *** Waiting for unfinished jobs make: *** [submake-generated-headers] Error 2 Makefile:141: recipe for target 'submake-catalog-headers' failed src/Makefile.global:370: recipe for target 'submake-generated-headers' failed thanks, Erik Rijkers
Re: WAL archive (archive_mode = always) ?
On Mon, Oct 22, 2018 at 5:06 AM Adelino Silva < adelino.j.si...@googlemail.com> wrote: > Hello Takayuki, > > Sorry can you explain how we can same network bandwidth by not sending the > WAL archive from the primary to the standby(s). > I possible scenario is have to multiple standby servers in same host for > same master. or other scenarios exists ? > Before archive_mode = always became available, we've had to stream the WAL twice, once to the hot standby for immediate application, and once to pg_receivexlog for archival purposes. So it doubled the bandwidth usage. Cheers, Jeff
Re: BUG #15449: file_fdw using program cause exit code error when using LIMIT
I wrote:
> =?utf-8?q?PG_Bug_reporting_form?= writes:
>> SELECT * FROM test_file_fdw_program_limit LIMIT 0;
>> /*
>> [38000] ERROR: program "echo "test"" failed Detail: child process exited
>> with exit code 1
>> */
> Yeah, I can reproduce this on macOS as well as Linux. Capturing stderr
> shows something pretty unsurprising:
> sh: line 1: echo: write error: Broken pipe
> So the called program is behaving in a somewhat reasonable way: it's
> detecting EPIPE on its stdout (after we close the pipe), reporting that,
> and doing exit(1).
> Unfortunately, it's not clear what we could do about that, short of
> always reading the whole program output, which is likely to be a
> cure worse than the disease. If the program were failing thanks to
> SIGPIPE, we could recognize that as a case we can ignore ... but with
> behavior like this, I don't see a reliable way to tell it apart from
> a generic program failure, which surely we'd better report.
After a bit of thought, the problem here is blindingly obvious:
we generally run the backend with SIGPIPE handing set to SIG_IGN,
and evidently popen() allows the called program to inherit that,
at least on these platforms.
So what we need to do is make sure the called program inherits SIG_DFL
handling for SIGPIPE, and then special-case that result as not being
a failure. The attached POC patch does that and successfully makes
the file_fdw problem go away for me.
It's just a POC because there are some things that need more thought
than I've given them:
1. Is it OK to revert SIGPIPE to default processing for *all* programs
launched through OpenPipeStream? If not, what infrastructure do we
need to add to control that? In particular, is it sane to revert
SIGPIPE for a pipe program that we will write to not read from?
(I thought probably yes, because that is the normal Unix behavior,
but it could be debated.)
2. Likewise, is it always OK for ClosePipeToProgram to ignore a
SIGPIPE failure? (For ordinary COPY, maybe it shouldn't, since
we don't intend to terminate that early.)
3. Maybe this should be implemented at some higher level?
4. Are there any other signals we ought to be reverting to default
behavior before launching a COPY TO/FROM PROGRAM?
regards, tom lane
diff --git a/src/backend/commands/copy.c b/src/backend/commands/copy.c
index b58a74f4e3db9d8e8f30d35e08ca84814742faa3..c4c6c875ec8102df2756d8ea1ba79791d63fed25 100644
*** a/src/backend/commands/copy.c
--- b/src/backend/commands/copy.c
*** ClosePipeToProgram(CopyState cstate)
*** 1727,1737
--- 1727,1746
(errcode_for_file_access(),
errmsg("could not close pipe to external command: %m")));
else if (pclose_rc != 0)
+ {
+ /*
+ * If the called program terminated on SIGPIPE, assume it's OK; we
+ * must have chosen to stop reading its output early.
+ */
+ if (WIFSIGNALED(pclose_rc) && WTERMSIG(pclose_rc) == SIGPIPE)
+ return;
+
ereport(ERROR,
(errcode(ERRCODE_EXTERNAL_ROUTINE_EXCEPTION),
errmsg("program \"%s\" failed",
cstate->filename),
errdetail_internal("%s", wait_result_to_str(pclose_rc;
+ }
}
/*
diff --git a/src/backend/storage/file/fd.c b/src/backend/storage/file/fd.c
index 8dd51f176749c5d1b2adc9993b5a9a3571ee566c..6cec85e2c1dc7dea23c8b941e5080714ea65e57a 100644
*** a/src/backend/storage/file/fd.c
--- b/src/backend/storage/file/fd.c
*** OpenTransientFilePerm(const char *fileNa
*** 2430,2440
--- 2430,2445
* Routines that want to initiate a pipe stream should use OpenPipeStream
* rather than plain popen(). This lets fd.c deal with freeing FDs if
* necessary. When done, call ClosePipeStream rather than pclose.
+ *
+ * This function also ensures that the popen'd program is run with default
+ * SIGPIPE processing, rather than the SIG_IGN setting the backend normally
+ * uses. This ensures desirable response to, eg, closing a read pipe early.
*/
FILE *
OpenPipeStream(const char *command, const char *mode)
{
FILE *file;
+ int save_errno;
DO_DB(elog(LOG, "OpenPipeStream: Allocated %d (%s)",
numAllocatedDescs, command));
*** OpenPipeStream(const char *command, cons
*** 2452,2459
TryAgain:
fflush(stdout);
fflush(stderr);
errno = 0;
! if ((file = popen(command, mode)) != NULL)
{
AllocateDesc *desc = [numAllocatedDescs];
--- 2457,2469
TryAgain:
fflush(stdout);
fflush(stderr);
+ pqsignal(SIGPIPE, SIG_DFL);
errno = 0;
! file = popen(command, mode);
! save_errno = errno;
! pqsignal(SIGPIPE, SIG_IGN);
! errno = save_errno;
! if (file != NULL)
{
AllocateDesc *desc = [numAllocatedDescs];
*** TryAgain:
*** 2466,2473
if (errno == EMFILE || errno == ENFILE)
{
- int save_errno = errno;
-
ereport(LOG,
(errcode(ERRCODE_INSUFFICIENT_RESOURCES),
errmsg("out of file descriptors: %m; release and retry")));
--- 2476,2481
Re: [PATCH] Tab complete EXECUTE FUNCTION for CREATE (EVENT) TRIGGER
ilm...@ilmari.org (Dagfinn Ilmari Mannsåker) writes: > Hi hackers, > > The last-minute change for CREATE (EVENT) TRIGGER to accept EXECUTE > FUNCTION as well as EXECUTE PROCEDURE did not update the tab completion > in psql to match. Please find attached two patches that do this for > CREATE TRIGGER and CREATE EVENT TRIGGER, respectively. Added to the current commitfest: https://commitfest.postgresql.org/20/1837/ - ilmari -- "The surreality of the universe tends towards a maximum" -- Skud's Law "Never formulate a law or axiom that you're not prepared to live with the consequences of." -- Skud's Meta-Law
Re: Buildfarm failures for hash indexes: buffer leaks
Hello Tom & Amit, Both animals use gcc experimental versions, which may rather underline a new bug in gcc head rather than an existing issue in pg. Or not. It is possible, but what could be the possible theory? It seems like the two feasible theories are (1) gcc bug, or (2) buffer leak that only occurs in very narrow circumstances, perhaps from a race condition. Given that the hash index code hasn't changed meaningfully in several months, I thought (1) seemed more probable. Yep, that is my thought as well. The problem is that this kind of issue is not simple to wrap-up as a gcc bug report, unlike other earlier instances that I forwarded to clang & gcc dev teams. I'm in favor in waiting before trying to report it, to check whether the probable underlying gcc problem is detected, reported by someone else, and fixed in gcc head. If it persists, then we'll see. -- Fabien.
Re: BUG #15448: server process (PID 22656) was terminated by exception 0xC0000005
On 10/22/2018 10:00 PM, Amit Langote wrote: After observing the test case in the provided log, I managed to reproduce it with the following: create table foo (a int primary key, b int); create table bar (a int references foo on delete cascade, b int); insert into foo values (1, 1); insert into foo values (2, 2); alter table foo add c int; alter table foo drop c; delete from foo; server closed the connection unexpectedly This probably means the server terminated abnormally before or while processing the request. The connection to the server was lost. Attempting reset: Failed. Analyzing this crash, I located the bug down to GetTupleForTrigger(), but perhaps it's really in heap_expand_tuple() / expand_tuple(), where the value of trigger tuple's t_self is being switched from a valid one to an invalid value. In heaptuple.c: expand_tuple() ItemPointerSetInvalid(&((*targetHeapTuple)->t_self)); FWIW, attached patch fixes this for me. Adding Andrew whose recent commit 7636e5c60f [1] seems to have introduced the heap_expan_tuple call in GetTupleForTrigger. Maybe, he can better judge a fix for this. Thanks. I think the line in expand_tuple is a thinko and we should change it, rather than change GetTupleForTrigger(). Here is a patch that does that and also adds your test case to the regression tests. cheers andrew -- Andrew Dunstanhttps://www.2ndQuadrant.com PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services diff --git a/src/backend/access/common/heaptuple.c b/src/backend/access/common/heaptuple.c index 15444cf..28127b3 100644 --- a/src/backend/access/common/heaptuple.c +++ b/src/backend/access/common/heaptuple.c @@ -856,7 +856,7 @@ expand_tuple(HeapTuple *targetHeapTuple, = (HeapTupleHeader) ((char *) *targetHeapTuple + HEAPTUPLESIZE); (*targetHeapTuple)->t_len = len; (*targetHeapTuple)->t_tableOid = sourceTuple->t_tableOid; - ItemPointerSetInvalid(&((*targetHeapTuple)->t_self)); + (*targetHeapTuple)->t_self = sourceTuple->t_self; targetTHeader->t_infomask = sourceTHeader->t_infomask; targetTHeader->t_hoff = hoff; diff --git a/src/test/regress/expected/fast_default.out b/src/test/regress/expected/fast_default.out index 48bd360..0797e11 100644 --- a/src/test/regress/expected/fast_default.out +++ b/src/test/regress/expected/fast_default.out @@ -727,7 +727,17 @@ SELECT * FROM t; (1 row) DROP TABLE t; +-- make sure expanded tuple has correct self pointer +-- it will be required by the RI tigger doing the cascading delete +CREATE TABLE leader (a int PRIMARY KEY, b int); +CREATE TABLE follower (a int REFERENCES leader ON DELETE CASCADE, b int); +INSERT INTO leader VALUES (1, 1), (2, 2); +ALTER TABLE leader ADD c int; +ALTER TABLE leader DROP c; +DELETE FROM leader; -- cleanup +DROP TABLE follower; +DROP TABLE leader; DROP FUNCTION test_trigger(); DROP TABLE t1; DROP FUNCTION set(name); diff --git a/src/test/regress/sql/fast_default.sql b/src/test/regress/sql/fast_default.sql index 06205cb..eefcd49 100644 --- a/src/test/regress/sql/fast_default.sql +++ b/src/test/regress/sql/fast_default.sql @@ -471,7 +471,19 @@ UPDATE t SET y = 2; SELECT * FROM t; DROP TABLE t; +-- make sure expanded tuple has correct self pointer +-- it will be required by the RI tigger doing the cascading delete + +CREATE TABLE leader (a int PRIMARY KEY, b int); +CREATE TABLE follower (a int REFERENCES leader ON DELETE CASCADE, b int); +INSERT INTO leader VALUES (1, 1), (2, 2); +ALTER TABLE leader ADD c int; +ALTER TABLE leader DROP c; +DELETE FROM leader; + -- cleanup +DROP TABLE follower; +DROP TABLE leader; DROP FUNCTION test_trigger(); DROP TABLE t1; DROP FUNCTION set(name);
[PATCH] Tab complete EXECUTE FUNCTION for CREATE (EVENT) TRIGGER
Hi hackers,
The last-minute change for CREATE (EVENT) TRIGGER to accept EXECUTE
FUNCTION as well as EXECUTE PROCEDURE did not update the tab completion
in psql to match. Please find attached two patches that do this for
CREATE TRIGGER and CREATE EVENT TRIGGER, respectively.
To keep the duplication minimal, I've changed it from completing EXECUTE
PROCEDURE as a single thing to just EXECUTE, and then completing
FUNCTION or FUNCTION and PROCEDURE after that depending on the server
version.
- ilmari
--
- Twitter seems more influential [than blogs] in the 'gets reported in
the mainstream press' sense at least. - Matt McLeod
- That'd be because the content of a tweet is easier to condense down
to a mainstream media article. - Calle Dybedahl
>From 61d6fcc4f979f31b40fb54d3a7481e27d62eb772 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Dagfinn=20Ilmari=20Manns=C3=A5ker?=
Date: Fri, 19 Oct 2018 17:13:05 +0100
Subject: [PATCH 1/2] Tab complete EXECUTE FUNCTION for CREATE TRIGGER
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
The change to accept EXECUTE FUNCTION as well as EXECUTE PROCEDURE in
CREATE TRIGGER (commit 0a63f996e018ac508c858e87fa39cc254a5db49f)
didn't tell psql's tab completion system about this.
Change the completion from EXECUTE PROCEDURE to just EXECUTE, then
complete any CREATE TRIGGER … EXECUTE with FUNCTION as well as
PROCEDURE if we're connected to a version 11 or newer server.
In passing, add tab completion of EXECUTE FUNCTION/PROCEDURE after a
complete WHEN ( … ) clause.
The FUNCTION alternative for completing function names isn't strictly
necessary, because words_after_create makes it complete function names
after FUNCTION, but having all the completion logic for a single
command in one place seems neater to me.
---
src/bin/psql/tab-complete.c | 22 +-
1 file changed, 13 insertions(+), 9 deletions(-)
diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
index 299600652f..70aa629a3b 100644
--- a/src/bin/psql/tab-complete.c
+++ b/src/bin/psql/tab-complete.c
@@ -2474,10 +2474,10 @@ psql_completion(const char *text, int start, int end)
COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_views, NULL);
else if (HeadMatches("CREATE", "TRIGGER") && TailMatches("ON", MatchAny))
COMPLETE_WITH("NOT DEFERRABLE", "DEFERRABLE", "INITIALLY",
- "REFERENCING", "FOR", "WHEN (", "EXECUTE PROCEDURE");
+ "REFERENCING", "FOR", "WHEN (", "EXECUTE");
else if (HeadMatches("CREATE", "TRIGGER") &&
(TailMatches("DEFERRABLE") || TailMatches("INITIALLY", "IMMEDIATE|DEFERRED")))
- COMPLETE_WITH("REFERENCING", "FOR", "WHEN (", "EXECUTE PROCEDURE");
+ COMPLETE_WITH("REFERENCING", "FOR", "WHEN (", "EXECUTE");
else if (HeadMatches("CREATE", "TRIGGER") && TailMatches("REFERENCING"))
COMPLETE_WITH("OLD TABLE", "NEW TABLE");
else if (HeadMatches("CREATE", "TRIGGER") && TailMatches("OLD|NEW", "TABLE"))
@@ -2485,17 +2485,17 @@ psql_completion(const char *text, int start, int end)
else if (HeadMatches("CREATE", "TRIGGER") &&
(TailMatches("REFERENCING", "OLD", "TABLE", "AS", MatchAny) ||
TailMatches("REFERENCING", "OLD", "TABLE", MatchAny)))
- COMPLETE_WITH("NEW TABLE", "FOR", "WHEN (", "EXECUTE PROCEDURE");
+ COMPLETE_WITH("NEW TABLE", "FOR", "WHEN (", "EXECUTE");
else if (HeadMatches("CREATE", "TRIGGER") &&
(TailMatches("REFERENCING", "NEW", "TABLE", "AS", MatchAny) ||
TailMatches("REFERENCING", "NEW", "TABLE", MatchAny)))
- COMPLETE_WITH("OLD TABLE", "FOR", "WHEN (", "EXECUTE PROCEDURE");
+ COMPLETE_WITH("OLD TABLE", "FOR", "WHEN (", "EXECUTE");
else if (HeadMatches("CREATE", "TRIGGER") &&
(TailMatches("REFERENCING", "OLD|NEW", "TABLE", "AS", MatchAny, "OLD|NEW", "TABLE", "AS", MatchAny) ||
TailMatches("REFERENCING", "OLD|NEW", "TABLE", MatchAny, "OLD|NEW", "TABLE", "AS", MatchAny) ||
TailMatches("REFERENCING", "OLD|NEW", "TABLE", "AS", MatchAny, "OLD|NEW", "TABLE", MatchAny) ||
TailMatches("REFERENCING", "OLD|NEW", "TABLE", MatchAny, "OLD|NEW", "TABLE", MatchAny)))
- COMPLETE_WITH("FOR", "WHEN (", "EXECUTE PROCEDURE");
+ COMPLETE_WITH("FOR", "WHEN (", "EXECUTE");
else if (HeadMatches("CREATE", "TRIGGER") && TailMatches("FOR"))
COMPLETE_WITH("EACH", "ROW", "STATEMENT");
else if (HeadMatches("CREATE", "TRIGGER") && TailMatches("FOR", "EACH"))
@@ -2503,11 +2503,15 @@ psql_completion(const char *text, int start, int end)
else if (HeadMatches("CREATE", "TRIGGER") &&
(TailMatches("FOR", "EACH", "ROW|STATEMENT") ||
TailMatches("FOR", "ROW|STATEMENT")))
- COMPLETE_WITH("WHEN (", "EXECUTE PROCEDURE");
- /* complete CREATE TRIGGER ... EXECUTE with PROCEDURE */
+ COMPLETE_WITH("WHEN (", "EXECUTE");
+ else if (HeadMatches("CREATE", "TRIGGER") && TailMatches("WHEN", "(*)"))
+ COMPLETE_WITH("EXECUTE");
else if (HeadMatches("CREATE", "TRIGGER") && TailMatches("EXECUTE"))
- COMPLETE_WITH("PROCEDURE");
- else if
Re: Buildfarm failures for hash indexes: buffer leaks
Amit Kapila writes: > On Mon, Oct 22, 2018 at 1:42 PM Fabien COELHO wrote: >> Both animals use gcc experimental versions, which may rather underline a >> new bug in gcc head rather than an existing issue in pg. Or not. > It is possible, but what could be the possible theory? It seems like the two feasible theories are (1) gcc bug, or (2) buffer leak that only occurs in very narrow circumstances, perhaps from a race condition. Given that the hash index code hasn't changed meaningfully in several months, I thought (1) seemed more probable. regards, tom lane
Re: Buildfarm failures for hash indexes: buffer leaks
On Mon, Oct 22, 2018 at 1:42 PM Fabien COELHO wrote: > > Hello Michaël, > > > The first failure is unrelated to the involved commits, as they touched > > completely different areas of the code: > > INSERT INTO hash_split_heap SELECT a/2 FROM generate_series(1, 25000) a; > > + WARNING: buffer refcount leak: [6481] (rel=base/16384/32349, > > blockNum=156, flags=0x9380, refcount=1 1) > > > > And versions older than HEAD do not complain. > > > > Any thoughts? > > Both animals use gcc experimental versions, which may rather underline a > new bug in gcc head rather than an existing issue in pg. Or not. > It is possible, but what could be the possible theory? The warning indicates that somewhere we forgot to call ReleaseBuffer. Today, I had reviewed at the hash index code related to test case that is failing but didn't find any obvious problem. What should we our next step? Do we want to change gcc version and see? -- With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.com
Re: Making all nbtree entries unique by having heap TIDs participate in comparisons
On 19.10.2018 0:54, Peter Geoghegan wrote: I would welcome any theories as to what could be the problem here. I'm think that this is fixable, since the picture for the patch is very positive, provided you only focus on bgwriter/checkpoint activity and on-disk sizes. It seems likely that there is a very specific gap in my understanding of how the patch affects buffer cleaning. I have same problem with background heap & index cleaner (based on your patch). In this case the bottleneck is WAL-record which I need to write for each cleaned block and locks which are held during the WAL-record writing process. Maybe you will do a test without writing any data to disk? -- Andrey Lepikhov Postgres Professional https://postgrespro.com The Russian Postgres Company
Re: pgbench - add pseudo-random permutation function
Hello,
> Also, the alternate implementation should not change the result, so
> something looks amiss in your version. Moreover, I'm unclear how to
> implement an overflow multiply with the safe no overflow version.
(snip)
I made an honest mistake. I had assumed the modulo number of Knuth's LCG
is (2 ^ 64 - 1).
BTW, I found other overflow issue.
In pseudorandom_perm(), `modular_multiply() + (key >> LCG_SHIFT)` may
overflow if the result of modular_multiply() is large. Therefore, I've
improved it.
Also, I've simplified Step 5 in modular_multiply().
I attached pgbench-prp-func-9.patch.
Best regards,
diff --git a/config/c-compiler.m4 b/config/c-compiler.m4
index af2dea1..5b09f73 100644
--- a/config/c-compiler.m4
+++ b/config/c-compiler.m4
@@ -327,6 +327,26 @@ fi])# PGAC_C_BUILTIN_BSWAP64
+# PGAC_C_BUILTIN_CLZLL
+# -
+# Check if the C compiler understands __builtin_clzll(),
+# and define HAVE__BUILTIN_CLZLL if so.
+# Both GCC & CLANG seem to have one.
+AC_DEFUN([PGAC_C_BUILTIN_CLZLL],
+[AC_CACHE_CHECK(for __builtin_clzll, pgac_cv__builtin_clzll,
+[AC_COMPILE_IFELSE([AC_LANG_SOURCE(
+[static unsigned long int x = __builtin_clzll(0xaabbccddeeff0011);]
+)],
+[pgac_cv__builtin_clzll=yes],
+[pgac_cv__builtin_clzll=no])])
+if test x"$pgac_cv__builtin_clzll" = xyes ; then
+AC_DEFINE(HAVE__BUILTIN_CLZLL, 1,
+ [Define to 1 if your compiler understands __builtin_clzll.])
+fi])# PGAC_C_BUILTIN_CLZLL
+
+
+
+
# PGAC_C_BUILTIN_CONSTANT_P
# -
# Check if the C compiler understands __builtin_constant_p(),
diff --git a/configure b/configure
index 43ae8c8..c455505 100755
--- a/configure
+++ b/configure
@@ -13952,6 +13952,30 @@ if test x"$pgac_cv__builtin_bswap64" = xyes ; then
$as_echo "#define HAVE__BUILTIN_BSWAP64 1" >>confdefs.h
fi
+{ $as_echo "$as_me:${as_lineno-$LINENO}: checking for __builtin_clzll" >&5
+$as_echo_n "checking for __builtin_clzll... " >&6; }
+if ${pgac_cv__builtin_clzll+:} false; then :
+ $as_echo_n "(cached) " >&6
+else
+ cat confdefs.h - <<_ACEOF >conftest.$ac_ext
+/* end confdefs.h. */
+static unsigned long int x = __builtin_clzll(0xaabbccddeeff0011);
+
+_ACEOF
+if ac_fn_c_try_compile "$LINENO"; then :
+ pgac_cv__builtin_clzll=yes
+else
+ pgac_cv__builtin_clzll=no
+fi
+rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext
+fi
+{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $pgac_cv__builtin_clzll" >&5
+$as_echo "$pgac_cv__builtin_clzll" >&6; }
+if test x"$pgac_cv__builtin_clzll" = xyes ; then
+
+$as_echo "#define HAVE__BUILTIN_CLZLL 1" >>confdefs.h
+
+fi
{ $as_echo "$as_me:${as_lineno-$LINENO}: checking for __builtin_constant_p" >&5
$as_echo_n "checking for __builtin_constant_p... " >&6; }
if ${pgac_cv__builtin_constant_p+:} false; then :
diff --git a/configure.in b/configure.in
index 519ecd5..f7eb78a 100644
--- a/configure.in
+++ b/configure.in
@@ -1486,6 +1486,7 @@ PGAC_C_TYPES_COMPATIBLE
PGAC_C_BUILTIN_BSWAP16
PGAC_C_BUILTIN_BSWAP32
PGAC_C_BUILTIN_BSWAP64
+PGAC_C_BUILTIN_CLZLL
PGAC_C_BUILTIN_CONSTANT_P
PGAC_C_BUILTIN_UNREACHABLE
PGAC_C_COMPUTED_GOTO
diff --git a/doc/src/sgml/ref/pgbench.sgml b/doc/src/sgml/ref/pgbench.sgml
index b5e3a62..77cbfcd 100644
--- a/doc/src/sgml/ref/pgbench.sgml
+++ b/doc/src/sgml/ref/pgbench.sgml
@@ -929,7 +929,7 @@ pgbench options
d
default_seed
- seed used in hash functions by default
+ seed used in hash and pseudo-random permutation functions by
default
@@ -1390,6 +1390,13 @@ pgbench options
d
1024.0
+ pr_perm(i,
size [, seed ]
)
+ integer
+ pseudo-random permutation in [0,size)
+ pr_perm(0, 4)
+ 0, 1, 2
or 3
+
+
random(lb,
ub)
integer
uniformly-distributed random integer in [lb,
ub]
@@ -1551,6 +1558,24 @@ f(x) = PHI(2.0 * parameter * (x - mu) / (max - min + 1))
/
+Function pr_perm implements a pseudo-random permutation.
+It allows to mix the output of non uniform random functions so that
+values drawn more often are not trivially correlated.
+It permutes integers in [0, size) using a seed by applying rounds of
+simple invertible functions, similarly to an encryption function,
+although beware that it is not at all cryptographically secure.
+Compared to hash functions discussed above, the function
+ensures that a perfect permutation is applied: there are no collisions
+nor holes in the output values.
+Values outside the interval are interpreted modulo the size.
+The function errors if size is not positive.
+If no seed is provided, :default_seed is used.
+For a given size and seed, the function is fully deterministic: if two
+permutations on the same size must not be correlated, use distinct seeds
+as outlined in the previous example about hash functions.
+
+
+
As an example, the full definition of the built-in TPC-B-like
transaction is:
Re: Function to promote standby servers
Michael Paquier wrote: > If there are no > objections, I'll look at this patch again by the end of this week in > order to get it committed. No objections from me; on the contrary, I would like to thank you for your effort here. I appreciate that you probably spent more time tutoring me than it would have taken you to write this yourself. Yours, Laurenz Albe
Re: libpq host/hostaddr/conninfo inconsistencies
Hello Arthur, sh> psql "host=127.0.0.2 hostaddr=127.0.0.1" I'm not sure that is is the issue. User defined the host name and psql show it. The issue is that "host" is an ip, "\conninfo" will inform wrongly that you are connected to "127.0.0.2", but the actual connection is really to "127.0.0.1", this is plain misleading, and I consider this level of unhelpfullness more a bug than a feature. I didn't think that this is an issue, because I determined "host" as just a host's display name when "hostaddr" is defined. When I type "\conninfo", I do not expect to have false clues that must be interpreted depending on a fine knowledge of the documentation and the connection parameters possibly typed hours earlier, I would just expect to have a direct answer describing in a self contained way what the connection actually is. So user may determine 127.0.0.1 (hostaddr) as "happy_host", for example. It shouldn't be a real host. They may determine it if they can access the initial connection information, which means an careful inquest because \conninfo does not say what it is... If they just read what is said, they just get wrong informations. I searched for another use cases of PQhost(). In PostgreSQL source code I found that it is used in pg_dump and psql to connect to some instance. There is the next issue with PQhost() and psql (pg_dump could have it too, see CloneArchive() in pg_backup_archiver.c and _connectDB() in pg_backup_db.c): $ psql "host=host_1,host_2 hostaddr=127.0.0.1,127.0.0.3 dbname=postgres" =# \conninfo You are connected to database "postgres" as user "artur" on host "host_1" at port "5432". =# \connect test could not translate host name "host_1" to address: Неизвестное имя или служба Previous connection kept So in the example above you cannot reuse connection string with \connect. What do you think? I think that this is another connection related "feature", aka bug, that should be fixed as well:-( I cannot agree with you. When I've learned libpq before I found host/hostaddr rules description useful. And I disagree that it is good to remove it (as the patch does). Of course it is only my point of view and others may have another opinion. I'm not sure I understand your concern. Do you mean that you would prefer the document to keep describing that host/hostaddr/port accepts one value, and then have in some other place or at the end of the option documentation a line that say, "by the way, we really accept lists, and they must be somehow consistent between host/hostaddr/port"? I wrote about the following part of the documentation: - Using hostaddr instead of host allows the - application to avoid a host name look-up, which might be important - in applications with time constraints. However, a host name is - required for GSSAPI or SSPI authentication - methods, as well as for verify-full SSL - certificate verification. The following rules are used: - ... So I think description of these rules is useful here and shouldn't be removed. Ok, I have put back a summary description of which rules apply, which are somehow simpler & saner, at least this is the aim of this patch. Your patch removes it and maybe it shouldn't do that. But now I realised that the patch breaks this behavior and backward compatibility is broken. Indeed. The incompatible changes are that "host" must always be provided, instead of letting the user providing an IP either in host or hostaddr (currently both work although undocumented), and that "hostaddr" can only be provided for a host name, not for an IP or socket. Patch gives me an error if I specified only hostaddr: psql -d "hostaddr=127.0.0.1" psql: host "/tmp" cannot have an hostaddr "127.0.0.1" This is the expected modified behavior: hostaddr can only be specified on a host when it is a name, which is not the case here. See the comment above about backward compatibility. psql without the patch can connect to an instance if I specify only hostaddr. Yes, that is intentional and is the purpose of this patch: to provide a simple connection model for the user: use "host" to connect to a target server, and "hostaddr" as a lookup shortcut only. For a reminder, my main issues with the current status are: (1) the documentation is inconsistent with the implementation: "host" can be given an IP, but this is not documented. "hostaddr" can be provided for anything, and overshadows the initial specification, but: (2) "\conninfo" does not give a clue about what the connection really is in such cases. Moreover, you found another issue with psql's "\connect" which does not work properly when both "host" & "hostaddr" are given. In the attached patch, I tried to clarify the documentation further and fix some rebase issues I had. ISTM that all relevant informations provided in the previous version are still there. The backward incompatibility is
Re: Pluggable Storage - Andres's take
On Tue, Oct 23, 2018 at 5:49 PM Haribabu Kommi wrote: > I am able to generate the simple test and found the problem. The issue > with the following > SQL. > > SELECT * >INTO TABLE xacttest >FROM aggtest; > > During the processing of the above query, the tuple that is selected from > the aggtest is > sent to the intorel_receive() function, and the same tuple is used for the > insert, because > of this reason, the tuple xmin is updated and it leads to failure of > selecting the data from > another query. I fixed this issue by materializing the slot. > Wrong patch attached in the earlier mail, sorry for the inconvenience. Attached proper fix patch. I will look into isolation test failures. Regards, Haribabu Kommi Fujitsu Australia 0002-Materialize-the-slot-before-they-are-processed-using.patch Description: Binary data
Re: Pluggable Storage - Andres's take
On Mon, Oct 22, 2018 at 6:16 PM Haribabu Kommi wrote: > On Thu, Oct 18, 2018 at 1:04 PM Haribabu Kommi > wrote: > >> On Tue, Oct 9, 2018 at 1:46 PM Haribabu Kommi >> wrote: >> >>> >>> I also observed the failure of aggregates.sql, will look into it. >>> >> >> The random failure of aggregates.sql is as follows >> >> SELECT avg(a) AS avg_32 FROM aggtest WHERE a < 100; >> !avg_32 >> ! - >> ! 32.6667 >> (1 row) >> >> -- In 7.1, avg(float4) is computed using float8 arithmetic. >> --- 8,16 >> (1 row) >> >> SELECT avg(a) AS avg_32 FROM aggtest WHERE a < 100; >> ! avg_32 >> ! >> ! >> (1 row) >> >> Same NULL result for another aggregate query on column b. >> >> The aggtest table is accessed by two tests that are running in parallel. >> i.e aggregates.sql and transactions.sql, In transactions.sql, inside a >> transaction >> all the records in the aggtest table are deleted and aborted the >> transaction, >> I suspect that some visibility checks are having some race conditions >> that leads >> to no records on the table aggtest, thus it returns NULL result. >> >> If I try the scenario manually by opening a transaction and deleting the >> records, the >> issue is not occurring. >> >> I am yet to find the cause for this problem. >> > > I am not yet able to generate a test case where the above issue can occur > easily for > debugging, it is happening randomly. I will try to add some logs to find > out the problem. > I am able to generate the simple test and found the problem. The issue with the following SQL. SELECT * INTO TABLE xacttest FROM aggtest; During the processing of the above query, the tuple that is selected from the aggtest is sent to the intorel_receive() function, and the same tuple is used for the insert, because of this reason, the tuple xmin is updated and it leads to failure of selecting the data from another query. I fixed this issue by materializing the slot. During the above test run, I found another issue during analyze, that is trying to access the invalid offset data. Attached a fix patch. Regards, Haribabu Kommi Fujitsu Australia 0001-scan-start-offset-fix-during-analyze.patch Description: Binary data 0002-Materialize-all-the-slots-before-they-are-processed-.patch Description: Binary data
Re: Side effect of CVE-2017-7484 fix?
On Mon, Oct 22, 2018 at 7:16 PM Tom Lane wrote: > > Dilip Kumar writes: > > As part of the security fix > > (e2d4ef8de869c57e3bf270a30c12d48c2ce4e00c), we have restricted the > > users from accessing the statistics of the table if the user doesn't > > have privileges on the table and the function is not leakproof. Now, > > as a side effect of this, if the user has the privileges on the root > > partitioned table but does not have privilege on the child tables, the > > user will be able to access the data of the child table but it won't > > be able to access the statistics of the child table. This may result > > in a bad plan. > > This was complained of already, > https://www.postgresql.org/message-id/flat/3876.1531261875%40sss.pgh.pa.us > > regards, tom lane Ok, I see. Thanks. -- Regards, Dilip Kumar EnterpriseDB: http://www.enterprisedb.com
