Re: Patch to add hook to copydir()
Should I forget about getting such a patch in? I am up for implementing alternative solutions if the current one is considered unacceptable. As I tried to demonstrate in my last email, previous attempts also failed. On Tue, Sep 3, 2019 at 12:14 PM Swen Kooij wrote: > > I read two previous proposals for something similar. > > First one is from 2013 [0]. It proposed a OS and filesystem specific > implementation. Which was then changed to a patch that adds a > config option to specify shell commands that postgres should use > for copying files and dirs. This was after commentary that such a > feature is better suited for an extension. Which is what I am trying to do. > > What was true in 2013 (when the other patch was proposed) is still > true. There's no good way at the moment to hook into how postgres > copies files and directories. > > The second thread I found from 2018 [1] proposes filesystem specific > changes (BTRFS, APFS and XFS) that operate on a file level. Really > nice patch actually, but much more invasive than what I am proposing. > > > Both patches make rather invasive and significant changes specific > to the features they're trying to build. The general sentiment I got > from reading those two threads is that building in support for these > kind of features is hard and is probably better first done as an extension. > > I understand that the patch I proposed is not an ideal interface, > but it gets the job done without having to built this kind of > functionality into postgres. I am not proposing any filesystem > or OS specific hooks/changes in postgres to make this work. > > I tried to override the copydir() function without any hooks > and I haven't gotten it to work yet. If it does, I'd still prefer the > hook. It's more predictable and probably more portable. > > [0] https://www.postgresql.org/message-id/511b5d11.4040...@socialserve.com > [1] > https://www.postgresql.org/message-id/bc9ca382-b98d-0446-f699-8c5de2307ca7%402ndquadrant.com > > On Tue, Sep 3, 2019 at 9:48 AM Peter Eisentraut > wrote: > > > > On 2019-09-02 22:16, Swen Kooij wrote: > > > Is there anything that I am missing? My early experiments have been > > > very promising but my experience with Postgres internals is limited. Any > > > help or feedback would be much appreciated. > > > > You might want to review several previous threads that were > > contemplating doing some reflink stuff with btrfs during database > > creation. Not exactly what you are doing, but related. > > > > Also, wouldn't it work if your extension just defined its own copydir() > > in the .so? That should then be used over the built-in one -- maybe. > > > > -- > > Peter Eisentraut http://www.2ndQuadrant.com/ > > PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Re: BUG #15977: Inconsistent behavior in chained transactions
> Looking at the latest patch, the comment blocks on top of TBLOCK_STARTED > and TBLOCK_IMPLICIT_INPROGRESS in EndTransactionBlock() need an update > to mention the difference of behavior with chained transactions. And > the same comment rework should be done in UserAbortTransactionBlock() > for TBLOCK_IMPLICIT_INPROGRESS/TBLOCK_STARTED? I made another patch for that. I don't have much confidence with my English spelling so further improvements may be needed. > If it is too much a change and potential regression, then I think that the > "and chain" variants should be consistent and just raise warnings. We don't have an exact answer for implicit transaction chaining behavior yet. So I think it's better to disable this feature until someone discovers the use cases for this. Permitting AND CHAIN without a detailed specification might cause troubles in future. disable-implicit-transaction-chaining-v5.patch Description: Binary data
Re: BUG #15977: Inconsistent behavior in chained transactions
Hi, On 2019-09-06 16:54:15 +0900, Michael Paquier wrote: > On Thu, Sep 05, 2019 at 02:11:35PM -0700, Andres Freund wrote: > > On 2019-09-05 14:16:11 +0200, Peter Eisentraut wrote: > >> I'm content with this patch. > > > > Would need tests. > > The latest patch sends adds coverage for all the new code paths > added. Do you have something else in mind? Missed them somehow. But I don't think they're quite sufficient. I think at least we also need tests that test things like multi-statement exec_simple-query() *with* explicit transactions and chaining. > >> Better disable questionable cases now and maybe re-enable them later > >> if someone wants to make a case for it. > > > > I do think the fact that COMMIT in multi-statement implicit transaction > > has some usecase, is an argument for just implementing it properly... > > Like Peter, I would also keep an ERROR for now, as we could always > relax that later on. I mean, I agree it's better to err that way, but it still seems unnecessary to design things in a way that prevents legit cases, that we then may have to allow later. Error -> no error is a behavioural change too, even if obviously less likely to cause problems. Greetings, Andres Freund
Re: BUG #15977: Inconsistent behavior in chained transactions
I made another patch for that. I don't have much confidence with my English spelling so further improvements may be needed. If it is too much a change and potential regression, then I think that the "and chain" variants should be consistent and just raise warnings. We don't have an exact answer for implicit transaction chaining behavior yet. So I think it's better to disable this feature until someone discovers the use cases for this. Permitting AND CHAIN without a detailed specification might cause troubles in future. I think that it would be too bad to remove this feature for a small implementation-dependent corner case. Documentation says that COMMIT/ROLLBACK [AND CHAIN] apply to the "current transaction", and "BEGIN initiates a transaction block". If there is no BEGIN, there is no "current transaction", so basically the behavior is unspecified, whether AND CHAIN or not, and we are free somehow. In such case, I'm simply arguing for consistency: whatever the behavior, the chain and no chain variants should behave the same. Now, I'd prefer error in all cases, no doubt about that, which might be considered a regression. A way around that could be to have a GUC decide between a strict behavior (error) and the old behavior (warning). -- Fabien.
Re: BUG #15977: Inconsistent behavior in chained transactions
> Missed them somehow. But I don't think they're quite sufficient. I think > at least we also need tests that test things like multi-statement > exec_simple-query() *with* explicit transactions and chaining. Added a few more tests for that. > Now, I'd prefer error in all cases, no doubt about that, which might be > considered a regression. A way around that could be to have a GUC decide > between a strict behavior (error) and the old behavior (warning). I think it's more better to have a GUC to disable implicit transaction 'block' feature, because that's probably the root of all issues. 2019年9月7日(土) 22:23 Fabien COELHO : > > > I made another patch for that. > > I don't have much confidence with my English spelling so further > > improvements may be needed. > > > >> If it is too much a change and potential regression, then I think that > the > >> "and chain" variants should be consistent and just raise warnings. > > > We don't have an exact answer for implicit transaction chaining behavior > > yet. > > > So I think it's better to disable this feature until someone discovers > the > > use cases for this. > > > Permitting AND CHAIN without a detailed specification might cause > troubles > > in future. > > I think that it would be too bad to remove this feature for a small > implementation-dependent corner case. > > Documentation says that COMMIT/ROLLBACK [AND CHAIN] apply to the "current > transaction", and "BEGIN initiates a transaction block". > > If there is no BEGIN, there is no "current transaction", so basically the > behavior is unspecified, whether AND CHAIN or not, and we are free > somehow. > > In such case, I'm simply arguing for consistency: whatever the behavior, > the chain and no chain variants should behave the same. > > Now, I'd prefer error in all cases, no doubt about that, which might be > considered a regression. A way around that could be to have a GUC decide > between a strict behavior (error) and the old behavior (warning). > > -- > Fabien. > disable-implicit-transaction-chaining-v6.patch Description: Binary data
Re: BUG #15977: Inconsistent behavior in chained transactions
Now, I'd prefer error in all cases, no doubt about that, which might be considered a regression. A way around that could be to have a GUC decide between a strict behavior (error) and the old behavior (warning). I think it's more better to have a GUC to disable implicit transaction 'block' feature, because that's probably the root of all issues. Hmmm… I'm not sure that erroring out on "SELECT 1" because there is no explicit "BEGIN" is sellable, even under some GUC. -- Fabien.
Re: BUG #15977: Inconsistent behavior in chained transactions
No, but instead always do an implicit COMMIT after each statement, like SELECT 1; SELECT 2; (not \;) in psql. The PostgreSQL document even states that 'Issuing COMMIT when not inside a transaction does no harm, but it will provoke a warning message.' for a long time, but in fact it have side-effect when used in an implicit transactions. If we can ensure that the COMMIT/ROLLBACK really does nothing, we don't have to distinguish CHAIN and NO CHAIN errors anymore. 2019年9月8日(日) 2:04 Fabien COELHO : > > >> Now, I'd prefer error in all cases, no doubt about that, which might be > >> considered a regression. A way around that could be to have a GUC decide > >> between a strict behavior (error) and the old behavior (warning). > > > > I think it's more better to have a GUC to disable implicit transaction > > 'block' feature, because that's probably the root of all issues. > > Hmmm… I'm not sure that erroring out on "SELECT 1" because there is no > explicit "BEGIN" is sellable, even under some GUC. > > -- > Fabien.
Re: SQL-spec incompatibilities in similar_escape() and related stuff
Alvaro Herrera from 2ndQuadrant writes: > Marked ready for committer. Thanks for reviewing. I adopted your doc change suggestions and pushed it. regards, tom lane
Re: Change ereport level for QueuePartitionConstraintValidation
I wrote: > Or, of course, we could forget the whole thing and switch the output > level for these messages to NOTICE instead. I'm not for that, but > now that we see what it'll cost us to have them better hidden, we can > at least have an informed debate. > Thoughts? Hearing no comments, I've pushed that patch, and marked the v12 open item closed. regards, tom lane
Re: Change ereport level for QueuePartitionConstraintValidation
On 2019-Sep-07, Tom Lane wrote: > I wrote: > > Or, of course, we could forget the whole thing and switch the output > > level for these messages to NOTICE instead. I'm not for that, but > > now that we see what it'll cost us to have them better hidden, we can > > at least have an informed debate. > > Thoughts? > > Hearing no comments, I've pushed that patch, and marked the v12 > open item closed. I've marked https://commitfest.postgresql.org/24/2076/ committed also. Thanks, -- Álvaro Herrerahttps://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Re: Change ereport level for QueuePartitionConstraintValidation
Alvaro Herrera from 2ndQuadrant writes: > I've marked https://commitfest.postgresql.org/24/2076/ committed also. Yeah, I just remembered about doing that, and saw you'd beat me to it. regards, tom lane
Re: [PATCH] Connection time for \conninfo
On Thu, Sep 5, 2019 at 2:22 PM Tom Lane wrote:
>
> =?UTF-8?Q?Rodrigo_Ram=C3=ADrez_Norambuena?= writes:
> > On Tue, Sep 3, 2019 at 11:06 PM Michael Paquier wrote:
> >> You can do basically the same thing by looking at
> >> pg_stat_activity.backend_start and compare it with the clock
> >> timestamp. Doing it at SQL level the way you want has also the
> >> advantage to offer you a modular format output.
>
> > What are you say seams little trick to what I propose in this patch
> > because you'll need filter what is your connection, and if the
> > connection is through the connection pooler could be more.
> > The main goal this is only getting information from the client side
> > (psql) as frontend.
>
> A couple of thoughts on this ---
> [...]
Hi Tom, about your concerns and feedback I send a new proposal of
patch related with the original idea.
Regards!
--
Rodrigo Ramírez Norambuena
http://www.rodrigoramirez.com/
From c71a88d67bfa7f3c7fe2d4a684057f364027e1ec Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Rodrigo=20Ram=C3=ADrez=20Norambuena?=
Date: Tue, 3 Sep 2019 17:15:39 -0400
Subject: [PATCH] Add to the \conninfo start time current connection:
Add extra information for \conninfo psql command to show the start time
of the connection.
The time will be reestablished again if the connection is reset and
reconnected successfully with the backend.
diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c
index 93953fc8e7..1758e20ad7 100644
--- a/src/bin/psql/command.c
+++ b/src/bin/psql/command.c
@@ -263,6 +263,19 @@ HandleSlashCmds(PsqlScanState scan_state,
}
+static void
+printTimeConnected(void)
+{
+
+ const char *strftime_fmt;
+ char timebuf[128];
+
+ strftime_fmt = "%c";
+ strftime(timebuf, sizeof(timebuf), strftime_fmt, localtime(&pset.connection_start));
+
+ printf(_("The connection started at %s\n"), timebuf);
+}
+
/*
* Subroutine to actually try to execute a backslash command.
*
@@ -624,6 +637,7 @@ exec_command_conninfo(PsqlScanState scan_state, bool active_branch)
}
printSSLInfo();
printGSSInfo();
+ printTimeConnected();
}
}
@@ -3326,6 +3340,7 @@ SyncVariables(void)
pset.encoding = PQclientEncoding(pset.db);
pset.popt.topt.encoding = pset.encoding;
pset.sversion = PQserverVersion(pset.db);
+ pset.connection_start = time(NULL);
SetVariable(pset.vars, "DBNAME", PQdb(pset.db));
SetVariable(pset.vars, "USER", PQuser(pset.db));
diff --git a/src/bin/psql/settings.h b/src/bin/psql/settings.h
index 5be5091f0e..955e696d8e 100644
--- a/src/bin/psql/settings.h
+++ b/src/bin/psql/settings.h
@@ -141,6 +141,7 @@ typedef struct _psqlSettings
const char *prompt3;
PGVerbosity verbosity; /* current error verbosity level */
PGContextVisibility show_context; /* current context display level */
+ time_t connection_start; /* unixtime for start time connection */
} PsqlSettings;
extern PsqlSettings pset;
--
2.21.0
Re: moonjelly vs tsearch
News from Fabien's bleeding edge compiler zoo: apparently GCC 10 20190831 thinks the tsearch test should produce different output than 20190824 did. It looks like the parsing of question marks change... Indeed, that looks pretty strange. I can't wait for next week's installment. I'll relaunch gcc trunk compilation before the end of the week. It did not do anything then, but this Saturday update fixed the issue. -- Fabien.
