Re: extended query protcol violation?
> Tatsuo Ishii writes: >> While looking into an issue of Pgpool-II, I found an interesting >> behavior of a PostgreSQL client. >> Below is a trace from pgproto to reproduce the client's behavior. > >> It starts a transacton. > >> FE=> Parse(stmt="S1", query="BEGIN") >> FE=> Bind(stmt="S1", portal="") >> FE=> Execute(portal="") >> : >> Commit the transaction > >> FE=> Parse(stmt="S1", query="COMMIT") >> FE=> Bind(stmt="S1", portal="") >> FE=> Execute(portal="") > > Hm, I'd expect the second Parse to generate a "prepared statement already > exists" error due to reuse of the "S1" statement name. Is there more > in this trace than you're showing us? Oh, yes. Actually the S1 statement was closed before the parse message. I should have mentioned it in the previous email. >> Send sync message. > >> FE=> Sync > >> Now the interesting part. After sync it a close message is sent. > >> FE=> Close(stmt="S1") > > That part seems fine to me. > >> I thought all extended query protocol messages are finished by a sync >> message. But in the example above, a close message is issued, followed >> by a simple query without a sync message. Should PostgreSQL regard >> this as a protocol violation? > > No, although it looks like buggy behavior on the client's part, because > it's unlikely to work well if there's any error in the Close operation. > The protocol definition is that an error causes the backend to discard > messages until Sync, so that if that Close fails, the following simple > Query will just be ignored. Most likely, that's not the behavior the > client wanted ... but it's not a protocol violation, IMO. Yes, you are right. I actually tried with errornouse close message (instead of giving 'S' to indicate that I want to close a statement, I gave 'T'). Ssubsequent simple query "BEGIN" was ignored. BTW, I also noticed that after the last "BEGIN" simple query, backend responded with CloseComplete message before a CommandComplete message. Accoring to our docs: https://www.postgresql.org/docs/11/protocol-flow.html#id-1.10.5.7.4 responses of a simple query do not include CloseComplete (or any other extended protocol message responses). So it seems current behavior of the backend does not follow the protocol defined in the doc. Probably we should either fix the doc (stats that resoponses from a simple query are not limited to those messages) or fix the behavior of the backend. Here is the whole message exchanging log: FE=> Parse(stmt="S1", query="BEGIN") FE=> Bind(stmt="S1", portal="") FE=> Execute(portal="") FE=> Close(stmt="S1") FE=> Parse(stmt="S2", query="SELECT 1") FE=> Bind(stmt="S2", portal="") FE=> Execute(portal="") FE=> Parse(stmt="S1", query="COMMIT") FE=> Bind(stmt="S1", portal="") FE=> Execute(portal="") FE=> Sync <= BE ParseComplete <= BE BindComplete <= BE CommandComplete(BEGIN) <= BE CloseComplete <= BE ParseComplete <= BE BindComplete <= BE DataRow <= BE CommandComplete(SELECT 1) <= BE ParseComplete <= BE BindComplete <= BE CommandComplete(COMMIT) <= BE ReadyForQuery(I) FE=> Close(stmt="S2") FE=> Close(stmt="S1") FE=> Query (query="BEGIN") <= BE CloseComplete <= BE CloseComplete <= BE CommandComplete(BEGIN) <= BE ReadyForQuery(T) FE=> Terminate Best regards, -- Tatsuo Ishii SRA OSS, Inc. Japan English: http://www.sraoss.co.jp/index_en.php Japanese:http://www.sraoss.co.jp
Re: extended query protcol violation?
Tatsuo>responses of a simple query do not include CloseComplete Tatsuo, where do you get the logs from? I guess you are just confused by the PRINTED order of the messages in the log. Note: wire order do not have to be exactly the same as the order in the log since messages are buffered, then might be read in batches. In other words, an application might just batch (send all three) close(s2), close(s1), query(begin) messages, then read the responses. How does it break protocol? Vladimir
Re: Connections hang indefinitely while taking a gin index's LWLock buffer_content lock
Hi! Thanks, Alexander! > 8 дек. 2018 г., в 6:54, Alexander Korotkov написал(а): > > Yep, please find attached draft patch. Patch seems good to me, I'll check it in more detail. The patch gets posting item at FirstOffsetNumber instead of btree->getLeftMostChild(). This seem OK, since dataGetLeftMostPage() is doing just the same, but with few Assert()s. > BTW, it seems that I've another bug in GIN. README says that > > "However, posting trees are only > fully searched from left to right, starting from the leftmost leaf. (The > tree-structure is only needed by insertions, to quickly find the correct > insert location). So as long as we don't delete the leftmost page on each > level, a search can never follow a downlink to page that's about to be > deleted." > > But that's not really true once we teach GIN to skip parts of posting > trees in PostgreSQL 9.4. So, there might be a risk to visit page to > be deleted using downlink. But in order to get real problem, vacuum > should past cleanup stage and someone else should reuse this page, > before we traverse downlink. Thus, the risk of real problem is very > low. But it still should be addressed. There's a patch above in this thread 0001-Use-correct-locking-protocol-during-GIN-posting-tree.patch where I propose stamping every deleted page with GinPageSetDeleteXid(page, ReadNewTransactionId()); and avoid reusing the page before TransactionIdPrecedes(GinPageGetDeleteXid(page), RecentGlobalDataXmin). Should we leave alone this bug for future fixes to keep current fix noninvasive? Best regards, Andrey Borodin.
Re: extended query protcol violation?
> Tatsuo>responses of a simple query do not include CloseComplete > > Tatsuo, where do you get the logs from? As I said, pgproto. https://github.com/tatsuo-ishii/pgproto > I guess you are just confused by the PRINTED order of the messages in the > log. > Note: wire order do not have to be exactly the same as the order in the log > since messages are buffered, then might be read in batches. pgproto directly reads from socket using read system call. There's no buffer here. > In other words, an application might just batch (send all three) close(s2), > close(s1), query(begin) messages, then read the responses. > How does it break protocol? Again as I said before, the doc says in extended query protocol a sequence of extended messages (parse, bind. describe, execute, closes) should be followed by a sync message. ie. close close sync query(begin) Maybe close close query(begin) is not a violation of protocol, but still I would say this is buggy because of the reason Tom said, and I agree with him. Best regards, -- Tatsuo Ishii SRA OSS, Inc. Japan English: http://www.sraoss.co.jp/index_en.php Japanese:http://www.sraoss.co.jp
Re: Undo logs
On Tue, Dec 4, 2018 at 3:00 PM Dilip Kumar wrote:
>
> On Sat, Dec 1, 2018 at 12:58 PM Amit Kapila wrote:
> >
> >
> > 13.
> > PrepareUndoInsert()
> > {
> > ..
> > if (!UndoRecPtrIsValid(prepared_urec_ptr))
> > + urecptr = UndoRecordAllocate(urec, 1, upersistence, txid);
> > + else
> > + urecptr = prepared_urec_ptr;
> > +
> > + size = UndoRecordExpectedSize(urec);
> > ..
> >
> > I think we should make above code a bit more bulletproof. As it is
> > written, there is no guarantee the size we have allocated is same as
> > we are using in this function.
> I agree
> How about if we take 'size' as output
> > parameter from UndoRecordAllocate and then use it in this function?
> > Additionally, we can have an Assert that the size returned by
> > UndoRecordAllocate is same as UndoRecordExpectedSize.
> With this change we will be able to guarantee when we are allocating
> single undo record
> but multi prepare will still be a problem. I haven't fix this as of
> now. I will think on how
> to handle both the cases when we have to prepare one time or when we
> have to allocate
> once and prepare multiple time.
>
Yeah, this appears tricky. I think we can leave it as it is unless we
get some better idea.
1.
+void
+UndoRecordOnUndoLogChange(UndoPersistence persistence)
+{
+ prev_txid[persistence] = InvalidTransactionId;
+}
Are we using this API in this or any other patch or in zheap? I see
this can be useful API, but unless we have a plan to use it, I don't
think we should retain it.
2.
+ * Handling multi log:
+ *
+ * It is possible that the undo record of a transaction can be spread across
+ * multiple undo log. And, we need some special handling while inserting the
+ * undo for discard and rollback to work sanely.
+ *
+ * If the undorecord goes to next log then we insert a transaction header for
+ * the first record in the new log and update the transaction header with this
+ * new log's location. This will allow us to connect transactions across logs
+ * when the same transaction span across log (for this we keep track of the
+ * previous logno in undo log meta) which is required to find the latest undo
+ * record pointer of the aborted transaction for executing the undo actions
+ * before discard. If the next log get processed first in that case we
+ * don't need to trace back the actual start pointer of the transaction,
+ * in such case we can only execute the undo actions from the current log
+ * because the undo pointer in the slot will be rewound and that
will be enough
+ * to avoid executing same actions. However, there is possibility that after
+ * executing the undo actions the undo pointer got discarded, now in later
+ * stage while processing the previous log it might try to fetch the undo
+ * record in the discarded log while chasing the transaction header chain.
+ * To avoid this situation we first check if the next_urec of the transaction
+ * is already discarded then no need to access that and start executing from
+ * the last undo record in the current log.
I think I see the problem in the discard mechanism when the log is
spread across multiple logs. Basically, if the second log contains
undo of some transaction prior to the transaction which has just
decided to spread it's undo in the chosen undo log, then we might
discard the undo log of some transaction(s) inadvertently. Am, I
missing something? If not, then I guess we need to ensure that we
don't immediately discard the undo in the second log when a single
transactions undo is spreaded across two logs
Before choosing a new undo log to span the undo for a transaction, do
we ensure that it is not already linked with some other undo log for a
similar reason?
One more thing in this regard:
+UndoRecordAllocate(UnpackedUndoRecord *undorecords, int nrecords,
+TransactionId txid, UndoPersistence upersistence)
{
..
..
+ if (InRecovery)
+ urecptr = UndoLogAllocateInRecovery(txid, size, upersistence);
+ else
+ urecptr = UndoLogAllocate(size, upersistence);
+
+ log = UndoLogGet(UndoRecPtrGetLogNo(urecptr), false);
+
+ /*
+ * By now, we must be attached to some undo log unless we are in recovery.
+ */
+ Assert(AmAttachedToUndoLog(log) || InRecovery);
+
+ /*
+ * We can consider the log as switched if this is the first record of the
+ * log and not the first record of the transaction i.e. same transaction
+ * continued from the previous log.
+ */
+ if ((UndoRecPtrGetOffset(urecptr) == UndoLogBlockHeaderSize) &&
+ log->meta.prevlogno != InvalidUndoLogNumber)
+ log_switched = true;
..
..
}
Isn't there a hidden assumption in the above code that you will always
get a fresh undo log if the undo doesn't fit in the currently attached
log? What is the guarantee of same?
3.
+/*
+ * Call PrepareUndoInsert to tell the undo subsystem about the undo record you
+ * intended to insert. Upon return, the necessary undo buffers are pinned and
+ * locked.
+ * This should be done before any critical section is established, since it
+ * c
Re: extended query protcol violation?
On Sat, 8 Dec 2018 at 05:16, Tatsuo Ishii wrote: > > Tatsuo>responses of a simple query do not include CloseComplete > > > > Tatsuo, where do you get the logs from? > > As I said, pgproto. > > https://github.com/tatsuo-ishii/pgproto > > > I guess you are just confused by the PRINTED order of the messages in the > > log. > > Note: wire order do not have to be exactly the same as the order in the > log > > since messages are buffered, then might be read in batches. > > pgproto directly reads from socket using read system call. There's no > buffer here. > > > In other words, an application might just batch (send all three) > close(s2), > > close(s1), query(begin) messages, then read the responses. > > How does it break protocol? > > Again as I said before, the doc says in extended query protocol a > sequence of extended messages (parse, bind. describe, execute, closes) > should be followed by a sync message. ie. > > close > close > sync > query(begin) > > Maybe > > close > close > query(begin) > > is not a violation of protocol, but still I would say this is buggy > because of the reason Tom said, and I agree with him. > Curious what client is this that is violating the protocol. Dave Cramer da...@postgresintl.com www.postgresintl.com > >
Re: extended query protcol violation?
>Curious what client is this that is violating the protocol. I stand corrected: that is not a violation, however client might get unexpected failure of query(begin) in a rare case of close(s1) or close(s2) fail. Vladimir
docs: outdated reference to recursive expression evaluation
In confg.sgml, in the section about max_stack_depth, there's this sentence: "The safety margin is needed because the stack depth is not checked in every routine in the server, but only in key potentially-recursive routines such as expression evaluation." Since the change in expression evaluation in v10, there's probably a better example of recursive routines, but I'm not sure what that would be. -John Naylor
Re: extended query protcol violation?
> Curious what client is this that is violating the protocol. I heard it was a Java program. Best regards, -- Tatsuo Ishii SRA OSS, Inc. Japan English: http://www.sraoss.co.jp/index_en.php Japanese:http://www.sraoss.co.jp
Re: extended query protcol violation?
On Sat, 8 Dec 2018 at 07:50, Tatsuo Ishii wrote: > > Curious what client is this that is violating the protocol. > > I heard it was a Java program. > This is not surprising there are a proliferation of non-blocking implementations, probably approaching 10 different implementations now. Dave Cramer da...@postgresintl.com www.postgresintl.com
Re: WIP: Avoid creation of the free space map for small tables
On Fri, Dec 7, 2018 at 7:25 PM John Naylor wrote: > > On 12/6/18, Amit Kapila wrote: > > On Thu, Dec 6, 2018 at 10:53 PM John Naylor wrote: > >> > >> I've added an additional regression test for finding the right block > >> and removed a test I thought was redundant. I've kept the test file in > >> its own schedule. > >> > > > > +# -- > > +# fsm does a vacuum, and running it in parallel seems to prevent heap > > truncation. > > +# -- > > +test: fsm > > + > > > > It is not clear to me from the comment why running it in parallel > > prevents heap truncation, can you explain what behavior are you seeing > > and what makes you think that running it in parallel caused it? > > One of the tests deletes all records from the relation and vacuums. In > serial schedule, the heap and FSM are truncated; in parallel they are > not. Make check fails, since since the tests measure relation size. > Taking a closer look, I'm even more alarmed to discover that vacuum > doesn't even seem to remove deleted rows in parallel schedule (that > was in the last test I added), which makes no sense and causes that > test to fail. I looked in vacuum.sql for possible clues, but didn't > see any. > I couldn't resist the temptation to figure out what's going on here. The newly added tests have deletes followed by vacuum and then you check whether the vacuum has removed the data by checking heap and or FSM size. Now, when you run such a test in parallel, the vacuum can sometimes skip removing the rows because there are parallel transactions open which can see the deleted rows. You can easily verify this phenomenon by running the newly added tests in one session in psql when there is another parallel session which has an open transaction. For example: Session-1 Begin; Insert into foo values(1); Session-2 \i fsm.sql Now, you should see the results similar to what you are seeing when you ran the fsm test by adding it to one of the parallel group. Can you test this at your end and confirm whether my analysis is correct or not. So, you can keep the test as you have in parallel_schedule, but comment needs to be changed. Also, you need to add the new test in serial_schedule. I have done both the changes in the attached patch, kindly confirm if this looks correct to you. -- With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.com v11-0001-Avoid-creation-of-the-free-space-map-for-small-table.patch Description: Binary data
Re: extended query protcol violation?
>> > Curious what client is this that is violating the protocol. >> >> I heard it was a Java program. >> > > This is not surprising there are a proliferation of non-blocking > implementations, > probably approaching 10 different implementations now. Do you think some of the implementations may not be appropriate if they behave like that discussed in this thread? If so, maybe it is worth to add a warning to the backend. Best regards, -- Tatsuo Ishii SRA OSS, Inc. Japan English: http://www.sraoss.co.jp/index_en.php Japanese:http://www.sraoss.co.jp
Re: COPY FROM WHEN condition
On 12/6/18 4:52 PM, Robert Haas wrote: > On Wed, Nov 28, 2018 at 6:17 PM Tomas Vondra > wrote: >>> Comparing with overhead of setting snapshot before evaluating every row >>> and considering this >>> >>> kind of usage is not frequent it seems to me the behavior is acceptable >> >> I'm not really buying the argument that this behavior is acceptable >> simply because using the feature like this will be uncommon. That seems >> like a rather weak reason to accept it. >> >> I however agree we don't want to make COPY less efficient, at least not >> unless absolutely necessary. But I think we can handle this simply by >> restricting what's allowed to appear the FILTER clause. >> >> It should be fine to allow IMMUTABLE and STABLE functions, but not >> VOLATILE ones. That should fix the example I shared, because f() would >> not be allowed. > > I don't think that's a very good solution. It's perfectly sensible > for someone to want to do WHERE/FILTER random() < 0.01 to load a > smattering of rows, and this would rule that out for no very good > reason. > Good point. I agree that's a much more plausible use case for this feature, and forbidding volatile functions would break it. > I think it would be fine to just document that if the filter condition > examines the state of the database, it will not see the results of the > COPY which is in progress. I'm pretty sure there are other cases - > for example with triggers - where you can get code to run that can't > see the results of the command currently in progress, so I don't > really buy the idea that having a feature which works that way is > categorically unacceptable. > > I agree that we can't justify flagrantly wrong behavior on the grounds > that correct behavior is expensive or on the grounds that the > incorrect cases will be rare. However, when there's more than one > sensible behavior, it's not unreasonable to pick one that is easier to > implement or cheaper or whatever, and that's the category into which I > would put this decision. > OK, makes sense. I withdraw my objections to the original behavior, and agree it's acceptable if it's reasonably documented. regards -- Tomas Vondra http://www.2ndQuadrant.com PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Re: extended query protcol violation?
On Sat, 8 Dec 2018 at 08:18, Tatsuo Ishii wrote: > >> > Curious what client is this that is violating the protocol. > >> > >> I heard it was a Java program. > >> > > > > This is not surprising there are a proliferation of non-blocking > > implementations, > > probably approaching 10 different implementations now. > > Do you think some of the implementations may not be appropriate if > they behave like that discussed in this thread? If so, maybe it is > worth to add a warning to the backend. > Given that java code is notorious for not reading warnings, I'd say no That said I'd probably be in favour of a DEBUG mode that did warn. Dave Cramer da...@postgresintl.com www.postgresintl.com
Re: pg_partition_tree crashes for a non-defined relation
Greetings,
* Michael Paquier (mich...@paquier.xyz) wrote:
> On Fri, Dec 07, 2018 at 11:33:32PM -0500, Tom Lane wrote:
> > How about cases where the relation OID exists but it's the wrong
> > kind of relation?
>
> Such cases already return an error:
> =# create sequence popo;
> CREATE SEQUENCE
> =# select pg_partition_tree('popo');
> ERROR: 42809: "popo" is not a table, a foreign table, or an index
> LOCATION: pg_partition_tree, partitionfuncs.c:54
>
> I think that's a sensible choice, because it makes no sense to look for
> the inheritors of unsupported relkinds. Perhaps you have a different
> view on the matter?
We should really have a more clearly defined policy around this, but my
recollection is that we often prefer to return NULL rather than throwing
an error for the convenience of people doing things like querying
pg_class using similar functions.
I wonder if we maybe should have a regression test for every such
function which just queries the catalog in a way to force the function
to be called for every relation defined in the regression tests, to
ensure that it doesn't segfault or throw an error..
Thanks!
Stephen
signature.asc
Description: PGP signature
Re: WIP: Avoid creation of the free space map for small tables
On 12/8/18, Amit Kapila wrote: > On Fri, Dec 7, 2018 at 7:25 PM John Naylor wrote: > I couldn't resist the temptation to figure out what's going on here. > The newly added tests have deletes followed by vacuum and then you > check whether the vacuum has removed the data by checking heap and or > FSM size. Now, when you run such a test in parallel, the vacuum can > sometimes skip removing the rows because there are parallel > transactions open which can see the deleted rows. Ah yes, of course. > You can easily > verify this phenomenon by running the newly added tests in one session > in psql when there is another parallel session which has an open > transaction. For example: > > Session-1 > Begin; > Insert into foo values(1); > > Session-2 > \i fsm.sql > > Now, you should see the results similar to what you are seeing when > you ran the fsm test by adding it to one of the parallel group. Can > you test this at your end and confirm whether my analysis is correct > or not. Yes, I see the same behavior. > So, you can keep the test as you have in parallel_schedule, but > comment needs to be changed. Also, you need to add the new test in > serial_schedule. I have done both the changes in the attached patch, > kindly confirm if this looks correct to you. Looks good to me. I'll just note that the new line in the serial schedule has an extra space at the end. Thanks for looking into this. -John Naylor
Re: [PROPOSAL]a new data type 'bytea' for ECPG
Matsumura-san, > > I do think, though, we should change the debug output for > > ecpg_free_params(). > > I try to change about it. Next patch will print binary in hex-format. Thank you. > > > > The patch does not support ECPG.bytea in sqltype of "struct > > > > sqlvar_struct" > > > > because of compatibility. > > > > Sorry I do not really understand what you mean. Could you please > > explain? > > I meaned that existing applications that receive data of bytea column > with using sqlda will encounter their unknown type(=ECPG.bytea) in > sqlvar_struct.sqltype. You mean if they are not recompiled? If so, yes, how else could that be handled? Michael -- Michael Meskes Michael at Fam-Meskes dot De, Michael at Meskes dot (De|Com|Net|Org) Meskes at (Debian|Postgresql) dot Org Jabber: michael at xmpp dot meskes dot org VfL Borussia! Força Barça! SF 49ers! Use Debian GNU/Linux, PostgreSQL
Re: docs: outdated reference to recursive expression evaluation
John Naylor writes: > In confg.sgml, in the section about max_stack_depth, there's this sentence: > "The safety margin is needed because the stack depth is not checked in > every routine in the server, but only in key potentially-recursive > routines such as expression evaluation." > Since the change in expression evaluation in v10, there's probably a > better example of recursive routines, but I'm not sure what that would > be. We could say "expression compilation" and it'd still be valid. Or just drop the last four words altogether. I don't think we want to expend the verbiage to be more precise here, since it's only a tangential point. BTW, while looking at this I noted that copyfuncs.c has a check_stack_depth call but outfuncs, readfuncs, equalfuncs don't. Surely that's not good. regards, tom lane
No such file or directory in pg_replslot
I don't know if this applies only to pglogical or logical decoding in
general. This is on a 9.6.10 provider running pglogical 2.2.0. Subscriber
has same versions. We had a replication delay situation this morning,
which I think may have been due to a really long transaction but I've yet
to verify that.
I disabled and re-enabled replication and at one point, this created an
error on start_replication_slot that the pid was already active.
Somehow replication got wedged and now even though replication appears to
be working, strace shows these kinds of errors continually:
open("pg_replslot/pgl_foo_providerb97b25d_foo336ddc1/xid-1248981532-lsn-C940-F400.snap",
O_RDONLY) = -1 ENOENT (No such file or directory)
open("pg_replslot/pgl_foo_providerb97b25d_foo336ddc1/xid-1248981532-lsn-C940-F500.snap",
O_RDONLY) = -1 ENOENT (No such file or directory)
open("pg_replslot/pgl_foo_providerb97b25d_foo336ddc1/xid-1248981532-lsn-C940-F600.snap",
O_RDONLY) = -1 ENOENT (No such file or directory)
open("pg_replslot/pgl_foo_providerb97b25d_foo336ddc1/xid-1248981532-lsn-C940-F700.snap",
O_RDONLY) = -1 ENOENT (No such file or directory)
open("pg_replslot/pgl_foo_providerb97b25d_foo336ddc1/xid-1248981532-lsn-C940-F800.snap",
O_RDONLY) = -1 ENOENT (No such file or directory)
open("pg_replslot/pgl_foo_providerb97b25d_foo336ddc1/xid-1248981532-lsn-C940-F900.snap",
O_RDONLY) = -1 ENOENT (No such file or directory)
open("pg_replslot/pgl_foo_providerb97b25d_foo336ddc1/xid-1248981532-lsn-C940-FA00.snap",
O_RDONLY) = -1 ENOENT (No such file or directory)
open("pg_replslot/pgl_foo_providerb97b25d_foo336ddc1/xid-1248981532-lsn-C940-FB00.snap",
O_RDONLY) = -1 ENOENT (No such file or directory)
open("pg_replslot/pgl_foo_providerb97b25d_foo336ddc1/xid-1248981532-lsn-C940-FC00.snap",
O_RDONLY) = -1 ENOENT (No such file or directory)
open("pg_replslot/pgl_foo_providerb97b25d_foo336ddc1/xid-1248981532-lsn-C940-FD00.snap",
O_RDONLY) = -1 ENOENT (No such file or directory)
open("pg_replslot/pgl_foo_providerb97b25d_foo336ddc1/xid-1248981532-lsn-C940-FE00.snap",
O_RDONLY) = -1 ENOENT (No such file or directory)
Any suggestions? This is a showstopper for us.
Thank you,
Jeremy
Re: No such file or directory in pg_replslot
On December 8, 2018 9:08:09 AM PST, Jeremy Finzel wrote:
>I don't know if this applies only to pglogical or logical decoding in
>general. This is on a 9.6.10 provider running pglogical 2.2.0.
>Subscriber
>has same versions. We had a replication delay situation this morning,
>which I think may have been due to a really long transaction but I've
>yet
>to verify that.
>
>I disabled and re-enabled replication and at one point, this created an
>error on start_replication_slot that the pid was already active.
>
>Somehow replication got wedged and now even though replication appears
>to
>be working, strace shows these kinds of errors continually:
>open("pg_replslot/pgl_foo_providerb97b25d_foo336ddc1/xid-1248981532-lsn-C940-F400.snap",
>O_RDONLY) = -1 ENOENT (No such file or directory)
>open("pg_replslot/pgl_foo_providerb97b25d_foo336ddc1/xid-1248981532-lsn-C940-F500.snap",
>O_RDONLY) = -1 ENOENT (No such file or directory)
>open("pg_replslot/pgl_foo_providerb97b25d_foo336ddc1/xid-1248981532-lsn-C940-F600.snap",
>O_RDONLY) = -1 ENOENT (No such file or directory)
>open("pg_replslot/pgl_foo_providerb97b25d_foo336ddc1/xid-1248981532-lsn-C940-F700.snap",
>O_RDONLY) = -1 ENOENT (No such file or directory)
>open("pg_replslot/pgl_foo_providerb97b25d_foo336ddc1/xid-1248981532-lsn-C940-F800.snap",
>O_RDONLY) = -1 ENOENT (No such file or directory)
>open("pg_replslot/pgl_foo_providerb97b25d_foo336ddc1/xid-1248981532-lsn-C940-F900.snap",
>O_RDONLY) = -1 ENOENT (No such file or directory)
>open("pg_replslot/pgl_foo_providerb97b25d_foo336ddc1/xid-1248981532-lsn-C940-FA00.snap",
>O_RDONLY) = -1 ENOENT (No such file or directory)
>open("pg_replslot/pgl_foo_providerb97b25d_foo336ddc1/xid-1248981532-lsn-C940-FB00.snap",
>O_RDONLY) = -1 ENOENT (No such file or directory)
>open("pg_replslot/pgl_foo_providerb97b25d_foo336ddc1/xid-1248981532-lsn-C940-FC00.snap",
>O_RDONLY) = -1 ENOENT (No such file or directory)
>open("pg_replslot/pgl_foo_providerb97b25d_foo336ddc1/xid-1248981532-lsn-C940-FD00.snap",
>O_RDONLY) = -1 ENOENT (No such file or directory)
>open("pg_replslot/pgl_foo_providerb97b25d_foo336ddc1/xid-1248981532-lsn-C940-FE00.snap",
>O_RDONLY) = -1 ENOENT (No such file or directory)
>
>Any suggestions? This is a showstopper for us.
That doesn't indicate an error. You need to provide more details what made you
consider things wedged...
Andres
--
Sent from my Android device with K-9 Mail. Please excuse my brevity.
Re: No such file or directory in pg_replslot
> > That doesn't indicate an error. You need to provide more details what > made you consider things wedged... > > Andres > Thank you very much for the reply. We typically see no visible replication delay over 5 minutes ever. Today we saw a delay of over 3 hours, and no obvious increase in workload either on the provider or the subscriber. I also did not see the LSN advancing whatsoever in terms of applying changes. I first checked for long-running transactions on the master but there was nothing too unusual except an ANALYZE which I promptly killed, but with no improvement to the situation. I found the messages above using strace after canceling the subscription and finding that the process was taking extremely long to cancel. There are 2.1 million files in pg_replslot which I don't think is normal? Any ideas as to where I should be looking or what could cause this? Thanks, Jeremy
proposal: new polymorphic types - commontype and commontypearray
Hi,
the possibility to use polymorphic types is a specific interesting
PostgreSQL feature. The polymorphic types allows to use almost all types,
but when some type is selected, then this type is required strictly without
possibility to use some implicit casting.
So if I have a fx(anyelement, anyelement), then I can call function fx with
parameters (int, int), (numeric, numeric), but I cannot to use parameters
(int, numeric). The strict design has sense, but for few important cases is
too restrictive. We are not able to implement (with plpgsql) functions like
coalesce, greatest, least where all numeric types can be used.
Alternative solution can be based on usage "any" type. But we can work with
this type only from "C" extensions, and there is some performance
penalization due dynamic casting inside function.
Four years ago I proposed implicit casting to common type of arguments with
anyelement type.
https://www.postgresql.org/message-id/CAFj8pRCZVo_xoW0cfxt%3DmmgjXKBgr3Gm1VMGL_zx9wDRHmm6Cw%40mail.gmail.com
My proposal was rejected, because it introduce compatibility issues.
Now I have a solution that doesn't break anything. With two new polymorphic
types: commontype and commontypearray we can write functions like coalesce,
greatest, ..
More, these types are independent on current polymorphic types - and can be
used with current polymorphic types together to cover some new use cases.
CREATE OR REPLACE FUNCTION fx(anyelement, commontype, anyelement,
commontype)
RETURNS commontype
or
CREATE OR REPLACE FUNCTION fx(anyelement, commontype, anyelement,
commontype)
RETURNS anyelement
and commontype and anyelement types can be really independent.
Comments, notes?
Regards
Pavel
diff --git a/doc/src/sgml/datatype.sgml b/doc/src/sgml/datatype.sgml
index ed0ee584c9..10876837e2 100644
--- a/doc/src/sgml/datatype.sgml
+++ b/doc/src/sgml/datatype.sgml
@@ -4761,6 +4761,14 @@ SELECT * FROM pg_attribute
anyrange
+
+commontype
+
+
+
+commontypearray
+
+
void
@@ -4870,6 +4878,20 @@ SELECT * FROM pg_attribute
).
+
+commontype
+Indicates that a function accepts any data type. Values
+are converted to real common type.
+(see ).
+
+
+
+commontypearray
+Indicates that a function accepts any array data type. The
+elements of array are converted to common type of these values.
+(see ).
+
+
cstring
Indicates that a function accepts or returns a null-terminated C string.
diff --git a/doc/src/sgml/extend.sgml b/doc/src/sgml/extend.sgml
index a6b77c1cfe..dece346c03 100644
--- a/doc/src/sgml/extend.sgml
+++ b/doc/src/sgml/extend.sgml
@@ -231,7 +231,7 @@
Five pseudo-types of special interest are anyelement,
anyarray, anynonarray, anyenum,
- and anyrange,
+ anyrange, commontype and commontypearray.
which are collectively called polymorphic types.
Any function declared using these types is said to be
a polymorphic function. A polymorphic function can
@@ -267,6 +267,15 @@
be an enum type.
+
+ Second family of polymorphic types are types commontype and
+ commontypearray. These types are similar to types
+ anyelement and anyarray. The arguments declared
+ as anyelement requires same real type of passed values. For
+ commontype's arguments is selected common type, and later
+ all these arguments are casted to this common type.
+
+
Thus, when more than one argument position is declared with a polymorphic
type, the net effect is that only certain combinations of actual argument
diff --git a/src/backend/catalog/pg_aggregate.c b/src/backend/catalog/pg_aggregate.c
index 4b12e9f274..d31658af53 100644
--- a/src/backend/catalog/pg_aggregate.c
+++ b/src/backend/catalog/pg_aggregate.c
@@ -133,7 +133,7 @@ AggregateCreate(const char *aggName,
hasInternalArg = false;
for (i = 0; i < numArgs; i++)
{
- if (IsPolymorphicType(aggArgTypes[i]))
+ if (IsPolymorphicTypeAny(aggArgTypes[i]))
hasPolyArg = true;
else if (aggArgTypes[i] == INTERNALOID)
hasInternalArg = true;
@@ -143,7 +143,7 @@ AggregateCreate(const char *aggName,
* If transtype is polymorphic, must have polymorphic argument also; else
* we will have no way to deduce the actual transtype.
*/
- if (IsPolymorphicType(aggTransType) && !hasPolyArg)
+ if (IsPolymorphicTypeAny(aggTransType) && !hasPolyArg)
ereport(ERROR,
(errcode(ERRCODE_INVALID_FUNCTION_DEFINITION),
errmsg("cannot determine transition data type"),
@@ -153,7 +153,7 @@ AggregateCreate(const char *aggName,
* Likewise for moving-aggregate transtype, if any
*/
if (OidIsValid(aggmTransType) &&
- IsPolymorphicType(aggmTransType) && !hasPolyArg)
+ IsPolymorphicTypeAny(aggmTransType) && !hasPolyArg)
ereport(ERROR,
(errcode(ERRCODE_INVALID_FUNCTION_DEFINITION),
errmsg("cannot determi
Re: Statement-level rollback
On 2018-Dec-07, Andres Freund wrote: > I think it could partially be addressed by not allowing to set it on the > commandline, server config, etc. So the user would have to set it on a > per-connection basis, potentially via the connection string. This is what patch 0001 does -- it's only allowed in the connection string, or on ALTER USER / ALTER DATABASE. Setting it in postgresql.conf is forbidden, as well as changing from transaction to statement in SET (the opposite is allowed, though.) -- Álvaro Herrerahttps://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Re: Statement-level rollback
Hi, On 2018-12-08 17:10:27 -0300, Alvaro Herrera wrote: > On 2018-Dec-07, Andres Freund wrote: > > > I think it could partially be addressed by not allowing to set it on the > > commandline, server config, etc. So the user would have to set it on a > > per-connection basis, potentially via the connection string. > > This is what patch 0001 does -- it's only allowed in the connection > string, or on ALTER USER / ALTER DATABASE. Setting it in > postgresql.conf is forbidden, as well as changing from transaction to > statement in SET (the opposite is allowed, though.) I don't think allowing to set it on a per-user basis is acceptable either, it still leaves the client in a state where they'll potentially be confused about it. Do you have a proposal to address the issue that this makes it just about impossible to write UDFs in a safe way? Greetings, Andres Freund
Re: Statement-level rollback
On 2018-Dec-08, Andres Freund wrote: > On 2018-12-08 17:10:27 -0300, Alvaro Herrera wrote: > > This is what patch 0001 does -- it's only allowed in the connection > > string, or on ALTER USER / ALTER DATABASE. Setting it in > > postgresql.conf is forbidden, as well as changing from transaction to > > statement in SET (the opposite is allowed, though.) > > I don't think allowing to set it on a per-user basis is acceptable > either, it still leaves the client in a state where they'll potentially > be confused about it. Hmm, true. > Do you have a proposal to address the issue that this makes it just > about impossible to write UDFs in a safe way? Not yet, but I'll give it a think next week. -- Álvaro Herrerahttps://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Re: pg_partition_tree crashes for a non-defined relation
On Sat, Dec 08, 2018 at 08:46:08AM -0500, Stephen Frost wrote: > We should really have a more clearly defined policy around this, but my > recollection is that we often prefer to return NULL rather than throwing > an error for the convenience of people doing things like querying > pg_class using similar functions. Yes, that's visibly right. At least that's what I can see from the various pg_get_*def and pg_*_is_visible. Returning NULL would indeed be more consistent. > I wonder if we maybe should have a regression test for every such > function which just queries the catalog in a way to force the function > to be called for every relation defined in the regression tests, to > ensure that it doesn't segfault or throw an error.. Like sqlsmith? It looks hard to me to make something like that part of the main regression test suite, as that's going to be costly and hard to scale with. -- Michael signature.asc Description: PGP signature
Re: No such file or directory in pg_replslot
On Sat, Dec 8, 2018 at 1:21 PM Jeremy Finzel wrote: > That doesn't indicate an error. You need to provide more details what >> made you consider things wedged... >> >> Andres >> > > Thank you very much for the reply. We typically see no visible > replication delay over 5 minutes ever. Today we saw a delay of over 3 > hours, and no obvious increase in workload either on the provider or the > subscriber. I also did not see the LSN advancing whatsoever in terms of > applying changes. > > I first checked for long-running transactions on the master but there was > nothing too unusual except an ANALYZE which I promptly killed, but with no > improvement to the situation. > > I found the messages above using strace after canceling the subscription > and finding that the process was taking extremely long to cancel. There > are 2.1 million files in pg_replslot which I don't think is normal? Any > ideas as to where I should be looking or what could cause this? > > Thanks, > Jeremy > I have very good news in that waiting it out for several hours, it resolved itself. Thank you, your input steered us in the right direction! Jeremy
min_parallel_table_size and inheritence
The docs say: https://www.postgresql.org/docs/current/runtime-config-query.html |min_parallel_table_scan_size Sets the minimum amount of table data that must be scanned in order for a parallel scan to be considered. [...] I'd like to set parallel_min_table_size=32MB, but it looks like that won't do what I intend for at least one of our tables using inheritence. It seems to me that an individual table should not be scanned in parallel if its size is below the threshold, even if it's a child and has siblings which are larger and scanned in parallel. I found that the current behavior seems to be more or less deliberate, but maybe should be revisited following implementation of "parallel append" node, as previously discussed. commit 2609e91fcf9dcf36af40cd0c5b755dccf6057df6 Author: Robert Haas Date: Tue Mar 14 14:33:14 2017 -0400 |Fix regression in parallel planning against inheritance tables. | |Commit 51ee6f3160d2e1515ed6197594bda67eb99dc2cc accidentally changed |the behavior around inheritance hierarchies; before, we always |considered parallel paths even for very small inheritance children, |because otherwise an inheritance hierarchy with even one small child |wouldn't be eligible for parallelism. That exception was inadverently |removed; put it back. [...] |Discussion: http://postgr.es/m/cae9k0pmgsoohrd60shu09arvthrss8s6pmyhjkwhxww9c_x...@mail.gmail.com postgres=# CREATE TABLE x(i int); postgres=# CREATE TABLE x1() INHERITS(x); postgres=# ANALYZE x,x1; postgres=# INSERT INTO x1 SELECT * FROM generate_series(1,99); postgres=# SET min_parallel_table_scan_size='99MB'; postgres=# explain (COSTS OFF) SELECT * FROM x a NATURAL JOIN x b WHERE a.i<99; | Gather | Workers Planned: 2 | -> Parallel Hash Join | Hash Cond: (b_1.i = a_1.i) | -> Parallel Append | -> Parallel Seq Scan on x1 b_1 | -> Parallel Seq Scan on x b | -> Parallel Hash | -> Parallel Append | -> Parallel Seq Scan on x1 a_1 | Filter: (i < 99) | -> Parallel Seq Scan on x a | Filter: (i < 99) Does parallel seq scan on table which is less than half the threshold. | public | x1 | table | pryzbyj | 35 MB | Similar if x is a partitioned/relkind=p: |postgres=# explain (COSTS OFF) SELECT * FROM x a NATURAL JOIN x b WHERE a.i<99; | Gather | Workers Planned: 1 | -> Parallel Append | -> Parallel Hash Join | Hash Cond: (b.i = a.i) | -> Parallel Seq Scan on x1 b | -> Parallel Hash | -> Parallel Seq Scan on x1 a | Filter: (i < 99) But not parallel if I join x1 directly: postgres=# explain (COSTS OFF) SELECT * FROM x1 a NATURAL JOIN x1 b WHERE a.i<99; | Hash Join | Hash Cond: (b.i = a.i) | -> Seq Scan on x1 b | -> Hash | -> Seq Scan on x1 a | Filter: (i < 99) ..unless I lower threshold: postgres=# SET min_parallel_table_scan_size='34MB'; postgres=# explain (COSTS OFF) SELECT * FROM x1 a NATURAL JOIN x1 b WHERE a.i<99; | Gather | Workers Planned: 1 | -> Parallel Hash Join | Hash Cond: (b.i = a.i) | -> Parallel Seq Scan on x1 b | -> Parallel Hash | -> Parallel Seq Scan on x1 a | Filter: (i < 99) Justin
automatically assigning catalog toast oids
Commit 96cdeae07 added toast tables to most catalogs. One disadvantage
is that the toast declarations require hard-coded oids, even though
only shared catalogs actually need stable oids. Now that we can assign
oids on the fly, it makes sense to do so for toast tables as well, as
in the attached.
-John Naylor
src/backend/catalog/genbki.pl | 26 ++
src/include/catalog/toasting.h | 32 ++--
2 files changed, 28 insertions(+), 30 deletions(-)
diff --git a/src/backend/catalog/genbki.pl b/src/backend/catalog/genbki.pl
index edc8ea9f53..f4d7160c6a 100644
--- a/src/backend/catalog/genbki.pl
+++ b/src/backend/catalog/genbki.pl
@@ -285,6 +285,7 @@ my @tables_needing_macros;
foreach my $catname (@catnames)
{
my $catalog = $catalogs{$catname};
+ my $has_toastable_type = 0;
# Create one definition header with macro definitions for each catalog.
my $def_file = $output_path . $catname . '_d.h';
@@ -345,6 +346,12 @@ EOM
# Build hash of column names for use later
$attnames{$attname} = 1;
+ # Flag catalogs with toastable datatypes.
+ if ($types{$atttype}->{typstorage} eq 'x')
+ {
+ $has_toastable_type = 1;
+ }
+
# Emit column definitions
if (!$first)
{
@@ -506,6 +513,25 @@ EOM
}
}
+ # Create toast declarations for normal catalogs. To prevent
+ # circular dependencies and to avoid adding complexity to VACUUM
+ # FULL logic, exclude pg_class, pg_attribute, and pg_index. Also,
+ # to prevent pg_upgrade from seeing a non-empty new cluster, exclude
+ # pg_largeobject and pg_largeobject_metadata from the set as large
+ # object data is handled as user data. Those relations have no reason
+ # to use a toast table anyway. Toast tables and indexes for shared
+ # catalogs need stable oids, so must be specified in toasting.h.
+ if ($has_toastable_type and !$catalog->{shared_relation}
+ and $catname ne 'pg_class'
+ and $catname ne 'pg_attribute'
+ and $catname ne 'pg_index'
+ and $catname ne 'pg_largeobject'
+ and $catname ne 'pg_largeobject_metadata')
+ {
+ push @toast_decls, sprintf "declare toast %s %s on %s\n",
+ $maxoid++, $maxoid++, $catname;
+ }
+
print $bki "close $catname\n";
printf $def "\n#endif\t\t\t\t\t\t\t/* %s_D_H */\n", uc $catname;
diff --git a/src/include/catalog/toasting.h b/src/include/catalog/toasting.h
index f259890e43..1c389c685d 100644
--- a/src/include/catalog/toasting.h
+++ b/src/include/catalog/toasting.h
@@ -42,37 +42,9 @@ extern void BootstrapToastTable(char *relName,
* the OID to assign to the toast table, and the OID to assign to the
* toast table's index. The reason we hard-wire these OIDs is that we
* need stable OIDs for shared relations, and that includes toast tables
- * of shared relations.
+ * of shared relations. Toast table commands for normal catalogs are
+ * generated by genbki.pl, and oids for those are assigned on the fly.
*/
-
-/* normal catalogs */
-DECLARE_TOAST(pg_aggregate, 4159, 4160);
-DECLARE_TOAST(pg_attrdef, 2830, 2831);
-DECLARE_TOAST(pg_collation, 4161, 4162);
-DECLARE_TOAST(pg_constraint, 2832, 2833);
-DECLARE_TOAST(pg_default_acl, 4143, 4144);
-DECLARE_TOAST(pg_description, 2834, 2835);
-DECLARE_TOAST(pg_event_trigger, 4145, 4146);
-DECLARE_TOAST(pg_extension, 4147, 4148);
-DECLARE_TOAST(pg_foreign_data_wrapper, 4149, 4150);
-DECLARE_TOAST(pg_foreign_server, 4151, 4152);
-DECLARE_TOAST(pg_foreign_table, 4153, 4154);
-DECLARE_TOAST(pg_init_privs, 4155, 4156);
-DECLARE_TOAST(pg_language, 4157, 4158);
-DECLARE_TOAST(pg_namespace, 4163, 4164);
-DECLARE_TOAST(pg_partitioned_table, 4165, 4166);
-DECLARE_TOAST(pg_policy, 4167, 4168);
-DECLARE_TOAST(pg_proc, 2836, 2837);
-DECLARE_TOAST(pg_rewrite, 2838, 2839);
-DECLARE_TOAST(pg_seclabel, 3598, 3599);
-DECLARE_TOAST(pg_statistic, 2840, 2841);
-DECLARE_TOAST(pg_statistic_ext, 3439, 3440);
-DECLARE_TOAST(pg_trigger, 2336, 2337);
-DECLARE_TOAST(pg_ts_dict, 4169, 4170);
-DECLARE_TOAST(pg_type, 4171, 4172);
-DECLARE_TOAST(pg_user_mapping, 4173, 4174);
-
-/* shared catalogs */
DECLARE_TOAST(pg_authid, 4175, 4176);
#define PgAuthidToastTable 4175
#define PgAuthidToastIndex 4176
Re: Too-short timeouts in test code
On Sat, Dec 08, 2018 at 04:16:01PM -0800, Noah Misch wrote:
> The 180s timeout in poll_query_until has been trouble-free since 2a0f89c
> introduced it two years ago. I plan to raise the timeouts in question to
> 180s, as attached.
That's annoying, thanks for tracking those down.
> @@ -72,7 +72,7 @@ my $endpos = $node_master->safe_psql('postgres',
> print "waiting to replay $endpos\n";
>
> my $stdout_recv = $node_master->pg_recvlogical_upto(
> - 'postgres', 'test_slot', $endpos, 10,
> + 'postgres', 'test_slot', $endpos, 180,
> 'include-xids' => '0',
> 'skip-empty-xacts' => '1');
Instead of allowing callers of pg_recvlogical_upto() to define the
timeout they want, perhaps it would be better to hardcode the timeout
limit within the routine?
--
Michael
signature.asc
Description: PGP signature
Re: Too-short timeouts in test code
On Sun, Dec 09, 2018 at 11:14:12AM +0900, Michael Paquier wrote:
> On Sat, Dec 08, 2018 at 04:16:01PM -0800, Noah Misch wrote:
> > @@ -72,7 +72,7 @@ my $endpos = $node_master->safe_psql('postgres',
> > print "waiting to replay $endpos\n";
> >
> > my $stdout_recv = $node_master->pg_recvlogical_upto(
> > - 'postgres', 'test_slot', $endpos, 10,
> > + 'postgres', 'test_slot', $endpos, 180,
> > 'include-xids' => '0',
> > 'skip-empty-xacts' => '1');
>
> Instead of allowing callers of pg_recvlogical_upto() to define the
> timeout they want, perhaps it would be better to hardcode the timeout
> limit within the routine?
pg_recvlogical_upto() has the ability to make timeout non-fatal, by calling it
in an array context. No in-tree test uses that. Out-of-tree code using that
feature would likely benefit from the ability to set timeout duration. Hence,
I'm inclined not remove the timeout duration parameter.
Re: Too-short timeouts in test code
On Sat, Dec 08, 2018 at 06:24:35PM -0800, Noah Misch wrote: > pg_recvlogical_upto() has the ability to make timeout non-fatal, by calling it > in an array context. No in-tree test uses that. Out-of-tree code using that > feature would likely benefit from the ability to set timeout duration. Hence, > I'm inclined not remove the timeout duration parameter. No objections with this line of thoughts. The rest of the patch looks fine to me. -- Michael signature.asc Description: PGP signature
Re: [Todo item] Add entry creation timestamp column to pg_stat_replication
On Fri, Dec 07, 2018 at 03:45:59PM +0900, myungkyu.lim wrote: > Thank you for your attention! And committed. There was one thing that the patch was doing wrong: there is no point to convert the timestamp to a string if no log message is generated, so I have added a check on log_min_messages to avoid the extra processing for normal workloads, and pushed. Thanks for the patch, TODO_item--. -- Michael signature.asc Description: PGP signature
Re: pg_partition_tree crashes for a non-defined relation
On Sun, Dec 09, 2018 at 08:15:07AM +0900, Michael Paquier wrote:
> On Sat, Dec 08, 2018 at 08:46:08AM -0500, Stephen Frost wrote:
>> We should really have a more clearly defined policy around this, but my
>> recollection is that we often prefer to return NULL rather than throwing
>> an error for the convenience of people doing things like querying
>> pg_class using similar functions.
>
> Yes, that's visibly right. At least that's what I can see from the
> various pg_get_*def and pg_*_is_visible. Returning NULL would indeed
> be more consistent.
Thinking more about your argument, scanning fully pg_class is quite
sensible as well because there is no need to apply an extra qual on
relkind, so let's change the function as you suggest, by returning NULL
on invalid relation type. Any opinions about the attached then which
does the switch?
--
Michael
diff --git a/src/backend/utils/adt/partitionfuncs.c b/src/backend/utils/adt/partitionfuncs.c
index 78dd2b542b..87c5a2f4d5 100644
--- a/src/backend/utils/adt/partitionfuncs.c
+++ b/src/backend/utils/adt/partitionfuncs.c
@@ -23,6 +23,7 @@
#include "funcapi.h"
#include "utils/fmgrprotos.h"
#include "utils/lsyscache.h"
+#include "utils/syscache.h"
/*
@@ -42,16 +43,16 @@ pg_partition_tree(PG_FUNCTION_ARGS)
FuncCallContext *funcctx;
ListCell **next;
+ if (!SearchSysCacheExists1(RELOID, ObjectIdGetDatum(rootrelid)))
+ PG_RETURN_NULL();
+
/* Only allow relation types that can appear in partition trees. */
if (relkind != RELKIND_RELATION &&
relkind != RELKIND_FOREIGN_TABLE &&
relkind != RELKIND_INDEX &&
relkind != RELKIND_PARTITIONED_TABLE &&
relkind != RELKIND_PARTITIONED_INDEX)
- ereport(ERROR,
-(errcode(ERRCODE_WRONG_OBJECT_TYPE),
- errmsg("\"%s\" is not a table, a foreign table, or an index",
- get_rel_name(rootrelid;
+ PG_RETURN_NULL();
/* stuff done only on the first call of the function */
if (SRF_IS_FIRSTCALL())
diff --git a/src/test/regress/expected/partition_info.out b/src/test/regress/expected/partition_info.out
index 6b116125e6..202d820827 100644
--- a/src/test/regress/expected/partition_info.out
+++ b/src/test/regress/expected/partition_info.out
@@ -6,6 +6,12 @@ SELECT * FROM pg_partition_tree(NULL);
---+-++---
(0 rows)
+SELECT * FROM pg_partition_tree(0);
+ relid | parentrelid | isleaf | level
+---+-++---
+ | ||
+(1 row)
+
-- Test table partition trees
CREATE TABLE ptif_test (a int, b int) PARTITION BY range (a);
CREATE TABLE ptif_test0 PARTITION OF ptif_test
@@ -107,8 +113,16 @@ DROP TABLE ptif_normal_table;
CREATE VIEW ptif_test_view AS SELECT 1;
CREATE MATERIALIZED VIEW ptif_test_matview AS SELECT 1;
SELECT * FROM pg_partition_tree('ptif_test_view');
-ERROR: "ptif_test_view" is not a table, a foreign table, or an index
+ relid | parentrelid | isleaf | level
+---+-++---
+ | ||
+(1 row)
+
SELECT * FROM pg_partition_tree('ptif_test_matview');
-ERROR: "ptif_test_matview" is not a table, a foreign table, or an index
+ relid | parentrelid | isleaf | level
+---+-++---
+ | ||
+(1 row)
+
DROP VIEW ptif_test_view;
DROP MATERIALIZED VIEW ptif_test_matview;
diff --git a/src/test/regress/sql/partition_info.sql b/src/test/regress/sql/partition_info.sql
index 5a76f22b05..9b55a7fe5c 100644
--- a/src/test/regress/sql/partition_info.sql
+++ b/src/test/regress/sql/partition_info.sql
@@ -2,6 +2,7 @@
-- Tests for pg_partition_tree
--
SELECT * FROM pg_partition_tree(NULL);
+SELECT * FROM pg_partition_tree(0);
-- Test table partition trees
CREATE TABLE ptif_test (a int, b int) PARTITION BY range (a);
signature.asc
Description: PGP signature
Too-short timeouts in test code
We've had more buildfarm failures due to hard-coded, short timeouts:
http://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=chipmunk&dt=2018-10-13%2021%3A06%3A58
10s timeout while running pg_recvlogical
http://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=hornet&dt=2018-12-03%2005%3A52%3A12
30s timeout while running pg_recvlogical
https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=mandrill&dt=2018-11-30%2014%3A31%3A18
60s timeout in isolationtester try_complete_step()
The 180s timeout in poll_query_until has been trouble-free since 2a0f89c
introduced it two years ago. I plan to raise the timeouts in question to
180s, as attached.
diff --git a/src/test/isolation/README b/src/test/isolation/README
index bea278a..780b6dc 100644
--- a/src/test/isolation/README
+++ b/src/test/isolation/README
@@ -108,8 +108,8 @@ Each step may contain commands that block until further
action has been taken
deadlock). A test that uses this ability must manually specify valid
permutations, i.e. those that would not expect a blocked session to execute a
command. If a test fails to follow that rule, isolationtester will cancel it
-after 60 seconds. If the cancel doesn't work, isolationtester will exit
-uncleanly after a total of 75 seconds of wait time. Testing invalid
+after 180 seconds. If the cancel doesn't work, isolationtester will exit
+uncleanly after a total of 200 seconds of wait time. Testing invalid
permutations should be avoided because they can make the isolation tests take
a very long time to run, and they serve no useful testing purpose.
diff --git a/src/test/isolation/isolationtester.c
b/src/test/isolation/isolationtester.c
index 7df67da..9134b05 100644
--- a/src/test/isolation/isolationtester.c
+++ b/src/test/isolation/isolationtester.c
@@ -783,15 +783,15 @@ try_complete_step(Step *step, int flags)
td += (int64) current_time.tv_usec - (int64)
start_time.tv_usec;
/*
-* After 60 seconds, try to cancel the query.
+* After 180 seconds, try to cancel the query.
*
* If the user tries to test an invalid permutation, we
don't want
* to hang forever, especially when this is running in
the
-* buildfarm. So try to cancel it after a minute.
This will
-* presumably lead to this permutation failing, but
remaining
-* permutations and tests should still be OK.
+* buildfarm. This will presumably lead to this
permutation
+* failing, but remaining permutations and tests should
still be
+* OK.
*/
- if (td > 60 * USECS_PER_SEC && !canceled)
+ if (td > 180 * USECS_PER_SEC && !canceled)
{
PGcancel *cancel = PQgetCancel(conn);
@@ -808,15 +808,15 @@ try_complete_step(Step *step, int flags)
}
/*
-* After 75 seconds, just give up and die.
+* After 200 seconds, just give up and die.
*
* Since cleanup steps won't be run in this case, this
may cause
* later tests to fail. That stinks, but it's better
than waiting
* forever for the server to respond to the cancel.
*/
- if (td > 75 * USECS_PER_SEC)
+ if (td > 200 * USECS_PER_SEC)
{
- fprintf(stderr, "step %s timed out after 75
seconds\n",
+ fprintf(stderr, "step %s timed out after 200
seconds\n",
step->name);
exit_nicely();
}
diff --git a/src/test/recovery/t/006_logical_decoding.pl
b/src/test/recovery/t/006_logical_decoding.pl
index 884b0ae..c23cc4d 100644
--- a/src/test/recovery/t/006_logical_decoding.pl
+++ b/src/test/recovery/t/006_logical_decoding.pl
@@ -72,7 +72,7 @@ my $endpos = $node_master->safe_psql('postgres',
print "waiting to replay $endpos\n";
my $stdout_recv = $node_master->pg_recvlogical_upto(
- 'postgres', 'test_slot', $endpos, 10,
+ 'postgres', 'test_slot', $endpos, 180,
'include-xids' => '0',
'skip-empty-xacts' => '1');
chomp($stdout_recv);
@@ -84,7 +84,7 @@ $node_master->poll_query_until('postgres',
) or die "slot never became inactive";
$stdout_recv = $node_master->pg_recvlogical_upto(
- 'postgres', 'test_slot', $endpos, 10,
+ 'postgres', 'test_slot', $endpos, 180,
'include-xids' => '0',
'skip-empty-xacts' => '1');
chomp($stdout_recv);
diff --git a/src/test/recovery/t/010_log
