Re: truncating timestamps on arbitrary intervals

[David Fetter]

On Wed, Feb 26, 2020 at 10:50:19AM +0800, John Naylor wrote:
> Hi,
> 
> When analyzing time-series data, it's useful to be able to bin
> timestamps into equally spaced ranges. date_trunc() is only able to
> bin on a specified whole unit.

Thanks for adding this very handy feature!

> In the attached patch for the March
> commitfest, I propose a new function date_trunc_interval(), which can
> truncate to arbitrary intervals, e.g.:
> 
> select date_trunc_interval('15 minutes', timestamp '2020-02-16
> 20:48:40'); date_trunc_interval
> -
>  2020-02-16 20:45:00
> (1 row)

I believe the following should error out, but doesn't.

# SELECT date_trunc_interval('1 year 1 ms', TIMESTAMP '2001-02-16 20:38:40');
 date_trunc_interval 
═
 2001-01-01 00:00:00
(1 row)

> With this addition, it might be possible to turn the existing
> date_trunc() functions into wrappers. I haven't done that here because
> it didn't seem practical at this point. For one, the existing
> functions have special treatment for weeks, centuries, and millennia.

I agree that turning it into a wrapper would be separate work.

> Note: I've only written the implementation for the type timestamp
> without timezone. Adding timezone support would be pretty simple,
> but I wanted to get feedback on the basic idea first before making
> it complete. I've also written tests and very basic documentation.

Please find attached an update that I believe fixes the bug I found in
a principled way.

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 5e36c4c888c65e358d2f87d84b64bc14d52f2b39 Mon Sep 17 00:00:00 2001
From: David Fetter 
Date: Tue, 25 Feb 2020 23:49:35 -0800
Subject: [PATCH v2] Add date_trunc_interval(interval, timestamp)
To: hackers
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="2.24.1"

This is a multi-part message in MIME format.
--2.24.1
Content-Type: text/plain; charset=UTF-8; format=fixed
Content-Transfer-Encoding: 8bit


per John Naylor

diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
index ceda48e0fc..3863c222a2 100644
--- a/doc/src/sgml/func.sgml
+++ b/doc/src/sgml/func.sgml
@@ -6949,6 +6949,15 @@ SELECT regexp_match('abc01234xyz', '(?:(.*?)(\d+)(.*)){1,1}');
 2 days 03:00:00

 
+   
+date_trunc_interval(interval, timestamp)
+timestamp
+Truncate to specified precision; see 
+
+date_trunc_interval('15 minutes', timestamp '2001-02-16 20:38:40')
+2001-02-16 20:30:00
+   
+

 
  
@@ -7818,7 +7827,7 @@ SELECT date_part('hour', INTERVAL '4 hours 3 minutes');
   
 
   
-   date_trunc
+   date_trunc, date_trunc_interval
 

 date_trunc
@@ -7902,6 +7911,21 @@ SELECT date_trunc('hour', INTERVAL '3 days 02:47:33');
 Result: 3 days 02:00:00
 

+
+   
+The function date_trunc_interval is 
+similar to the date_trunc, except that it
+truncates to an arbitrary interval.
+   
+
+   
+Example:
+
+SELECT date_trunc_interval('5 minutes', TIMESTAMP '2001-02-16 20:38:40');
+Result: 2001-02-16 20:35:00
+
+   
+
   
 
   
diff --git a/src/backend/utils/adt/timestamp.c b/src/backend/utils/adt/timestamp.c
index 0b6c9d5ea8..ed742592af 100644
--- a/src/backend/utils/adt/timestamp.c
+++ b/src/backend/utils/adt/timestamp.c
@@ -30,6 +30,7 @@
 #include "nodes/nodeFuncs.h"
 #include "nodes/supportnodes.h"
 #include "parser/scansup.h"
+#include "port/pg_bitutils.h"
 #include "utils/array.h"
 #include "utils/builtins.h"
 #include "utils/datetime.h"
@@ -3804,6 +3805,144 @@ timestamptz_age(PG_FUNCTION_ARGS)
  *-*/
 
 
+/* timestamp_trunc_interval()
+ * Truncate timestamp to specified interval.
+ */
+Datum
+timestamp_trunc_interval(PG_FUNCTION_ARGS)
+{
+	Interval   *interval = PG_GETARG_INTERVAL_P(0);
+	Timestamp	timestamp = PG_GETARG_TIMESTAMP(1);
+	Timestamp	result;
+	fsec_t		ifsec,
+tfsec;
+	uint32_t	unit = 0,
+popcount = 0;
+	enum		TimeUnit {
+	us = 1 << 0,
+	ms = 1 << 1,
+	second = 1 << 2,
+	minute = 1 << 3,
+	hour   = 1 << 4,
+	day= 1 << 5,
+	month  = 1 << 6,
+	year   = 1 << 7
+};
+
+	struct pg_tm it;
+	struct pg_tm tt;
+
+	if (TIMESTAMP_NOT_FINITE(timestamp))
+		PG_RETURN_TIMESTAMP(timestamp);
+
+	if (interval2tm(*interval, , ) != 0)
+		elog(ERROR, "could not convert interval to tm");
+
+	if (timestamp2tm(timestamp, NULL, , , NULL, NULL) != 0)
+		ereport(ERROR,
+(errcode(ERRCODE_DATETIME_VALUE_OUT_OF_RANGE),
+ errmsg("timestamp out of range")));
+
+	if (it.tm_year != 0)
+	{
+		tt.tm_year = it.tm_year * (tt.tm_year / it.tm_year);
+		unit |= year;
+	}
+	if (it.tm_mon != 0)
+	{
+		tt.tm_mon = it.tm_mon * (tt.tm_mon / it.tm_mon);
+		unit |= month;
+	}
+	if (it.tm_mday != 0)
+	{
+		tt.tm_mday = it.tm_mday * (tt.tm_mday / it.tm_mday);
+		

Re: Identifying user-created objects

[Masahiko Sawada]

On Thu, 13 Feb 2020 at 17:13, Julien Rouhaud  wrote:
>
> On Thu, Feb 13, 2020 at 8:32 AM Amit Langote  wrote:
> >
> > On Thu, Feb 13, 2020 at 10:30 AM Kyotaro Horiguchi
> >  wrote:
> > > At Mon, 10 Feb 2020 14:32:44 +0900, Amit Langote 
> > >  wrote in
> > > > Agree that ObjectIsUserObject(oid) is easier to read than oid >=
> > > > FirstNormalObject.  I would have not bothered, for example, if it was
> > > > something like oid >= FirstUserObjectId to begin with.
> > >
> > > Aside from the naming, I'm not sure it's sensible to use
> > > FirstNormalObjectId since I don't see a clear definition or required
> > > characteristics for "user created objects" is.  If we did CREATE
> > > TABLE, FUNCTION or maybe any objects during single-user mode before
> > > the first object is created during normal multiuser operation, the
> > > "user-created(or not?)" object has an OID less than
> > > FirstNormalObjectId. If such objects are the "user created object", we
> > > need FirstUserObjectId defferent from FirstNormalObjectId.
> >
> > Interesting observation.  Connecting to database in --single mode,
> > whether done using initdb or directly, is always considered
> > "bootstrapping", so the OIDs from the bootstrapping range are
> > consumed.
> >
> > $ postgres --single -D pgdata postgres
> >
> > PostgreSQL stand-alone backend 13devel
> > backend> create table a (a int);
> > backend> select 'a'::regclass::oid;
> >  1: oid (typeid = 26, len = 4, typmod = -1, byval = t)
> > 
> >  1: oid = "14168"   (typeid = 26, len = 4, typmod = -1, byval = 
> > t)
> >
> > Here, FirstBootstrapObjectId < 14168 < FirstNormalObjectId
>
> FTR it's also possible to get the same result using binary mode and
> binary_upgrade_set_next_XXX functions.
>
> > Maybe we could document that pg_is_user_object() and its internal
> > counterpart returns true only for objects that are created during
> > "normal" multi-user database operation.
>
> +1

Agreed.

Attached updated version patch.

Regards,

-- 
Masahiko Sawadahttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


identify_user_object_v3.patch
Description: Binary data

Re: Resolving the python 2 -> python 3 mess

[Andrew Dunstan]

On 2/25/20 8:24 PM, Andrew Dunstan wrote:
> On 2/25/20 7:08 PM, Tom Lane wrote:
>> Andrew Dunstan  writes:
>>> On 2/25/20 5:06 PM, Tom Lane wrote:
 No joy there --- now that I look closer, it seems the cfbot doesn't
 build any of the external-language PLs on Windows.  I'll have to
 wait for some reviewer to try it.
>>> What are the requirements for testing? bowerbird builds with python 2.7,
>>> although I guess I should really try to upgrade it  3.x.
>> Has to be python 3, unfortunately; the patch has no effect on a
>> python 2 build.
>>
>>  
>
>
> Yeah, I have python3 working on drongo, I'll test there.
>

It's almost there, you need to add something like this to Mkvcbuild.pm:


    if ($solution->{options}->{python2_stub})
    {
        my $plpython2_stub =
          $solution->AddProject('plpython2', 'dll', 'PLs',
'src/pl/stub_plpython2');
        $plpython2_stub->AddReference($postgres);
    }

cheers


andrew

-- 
Andrew Dunstanhttps://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Commit fest manager for 2020-03

[Michael Paquier]

Hi all,

The next commit fets is going to begin in a couple of days, and there
is a total of 207 patches registered as of today.  We don't have any
manager yet, so is there any volunteer for taking the lead this time?

Thanks,
--
Michael


signature.asc
Description: PGP signature

Re: Some problems of recovery conflict wait events

[Masahiko Sawada]

On Tue, 18 Feb 2020 at 17:58, Masahiko Sawada
 wrote:
>
> Hi all,
>
> When recovery conflicts happen on the streaming replication standby,
> the wait event of startup process is null when
> max_standby_streaming_delay = 0 (to be exact, when the limit time
> calculated by max_standby_streaming_delay is behind the last WAL data
> receipt time is behind). Moreover the process title of waiting startup
> process looks odd in the case of lock conflicts.
>
> 1. When max_standby_streaming_delay > 0 and the startup process
> conflicts with a lock,
>
> * wait event
>  backend_type | wait_event_type | wait_event
> --+-+
>  startup  | Lock| relation
> (1 row)
>
> * ps
> 42513   ??  Ss 0:00.05 postgres: startup   recovering
> 00010003 waiting
>
> Looks good.
>
> 2. When max_standby_streaming_delay > 0 and the startup process
> conflicts with a snapshot,
>
> * wait event
>  backend_type | wait_event_type | wait_event
> --+-+
>  startup  | |
> (1 row)
>
> * ps
> 44299   ??  Ss 0:00.05 postgres: startup   recovering
> 00010003 waiting
>
> wait_event_type and wait_event are null in spite of waiting for
> conflict resolution.
>
> 3. When max_standby_streaming_delay > 0 and the startup process
> conflicts with a lock,
>
> * wait event
>  backend_type | wait_event_type | wait_event
> --+-+
>  startup  | |
> (1 row)
>
> * ps
> 46510   ??  Ss 0:00.05 postgres: startup   recovering
> 00010003 waiting waiting
>
> wait_event_type and wait_event are null and the process title is
> wrong; "waiting" appears twice.
>
> The cause of the first problem, wait_event_type and wait_event are not
> set, is that WaitExceedsMaxStandbyDelay which is called by
> ResolveRecoveryConflictWithVirtualXIDs waits for other transactions
> using pg_usleep rather than WaitLatch. I think we can change it so
> that it uses WaitLatch and those caller passes wait event information.
>
> For the second problem, wrong process title, the cause is also
> relevant with ResolveRecoveryConflictWithVirtualXIDs; in case of lock
> conflicts we add "waiting" to the process title in WaitOnLock but we
> add it again in ResolveRecoveryConflictWithVirtualXIDs. I think we can
> have WaitOnLock not set process title in recovery case.
>
> This problem exists on 12, 11 and 10. I'll submit the patch.
>

I've attached patches that fix the above two issues.

0001 patch fixes the first problem. Currently there are 5 types of
recovery conflict resolution: snapshot, tablespace, lock, database and
buffer pin, and we set wait events to only 2 events out of 5: lock
(only when doing ProcWaitForSignal) and buffer pin. Therefore, users
cannot know that the startup process is waiting or not, and what
waiting for. This patch sets wait events to more 3 events: snapshot,
tablespace and lock. For wait events of those 3 events, I thought that
we can create a new more appropriate wait event type, say
RecoveryConflict, and set it for them. However, considering
back-patching to existing versions, adding new wait event type would
not be acceptable. So this patch sets existing wait events such as
PG_WAIT_LOCK to those 3 places and doesn't not set a wait event for
conflict resolution on dropping database because there is not an
appropriate existing one. I'll start a separate thread about
improvement on wait events of recovery conflict resolution for PG13 if
necessary.

0002 patch fixes the second problem. With this patch, the process
title is updated properly in all recovery conflict resolution cases.

Regards,

-- 
Masahiko Sawadahttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


0001-Set-wait-events-for-recovery-conflict-resolution.patch
Description: Binary data


0002-Fix-process-title-update-during-recovery-conflicts.patch
Description: Binary data

Re: [HACKERS] WAL logging problem in 9.4.3?

[Noah Misch]

On Tue, Feb 25, 2020 at 10:01:51AM +0900, Kyotaro Horiguchi wrote:
> At Sat, 22 Feb 2020 21:12:20 -0800, Noah Misch  wrote in 
> > On Fri, Feb 21, 2020 at 04:49:59PM +0900, Kyotaro Horiguchi wrote:
> > > At Wed, 19 Feb 2020 17:29:08 +0900 (JST), Kyotaro Horiguchi 
> > >  wrote in 
> > > > At Tue, 18 Feb 2020 23:44:52 -0800, Noah Misch  
> > > > wrote in 

> > > In swap_relation_files, we can remove rel2-related code when #ifndef
> > > USE_ASSERT_CHECKING.
> > 
> > When state is visible to many compilation units, we should avoid making that
> > state depend on --enable-cassert.  That would be a recipe for a Heisenbug.  
> > In
> > a hot code path, it might be worth the risk.
> 
> I aggree that the new #ifdef can invite a Heisenbug. I thought that
> you didn't want that because it doesn't make substantial difference.

v35nm added swap_relation_files() code so AssertPendingSyncs_RelationCache()
could check rd_droppedSubid relations.  v30nm, which did not have
rd_droppedSubid, removed swap_relation_files() code that wasn't making a
difference.

> If we decide to keep the consistency there, I would like to describe
> the code is there for consistency, not for the benefit of a specific
> assertion.
> 
> (cluster.c:1116)
> -* new. The next step for rel2 is deletion, but copy rd_*Subid for the
> -* benefit of AssertPendingSyncs_RelationCache().
> +* new. The next step for rel2 is deletion, but copy rd_*Subid for the
> +* consistency of the fieles. It is checked later by
> +* AssertPendingSyncs_RelationCache().

I think the word "consistency" is too vague for "consistency of the fields" to
convey information.  May I just remove the last sentence of the comment
(everything after "* new.")?

> > > config.sgml:
> > > +When wal_level is minimal 
> > > and a
> > > +transaction commits after creating or rewriting a permanent 
> > > table,
> > > +materialized view, or index, this setting determines how to 
> > > persist
> > > 
> > > "creating or truncation" a permanent table?  and maybe "refreshing
> > > matview and reindex". I'm not sure that they can be merged that way.
> ...
> > I like mentioning truncation, but I dislike how this implies that CREATE
> > INDEX, CREATE MATERIALIZED VIEW, and ALTER INDEX SET TABLESPACE aren't in
> > scope.  While I usually avoid the word "relation" in documentation, I can
> > justify it here to make the sentence less complex.  How about the following?
> > 
> > --- a/doc/src/sgml/config.sgml
> > +++ b/doc/src/sgml/config.sgml
> > @@ -2484,9 +2484,9 @@ include_dir 'conf.d'
> >  In minimal level, no information is logged for
> > -tables or indexes for the remainder of a transaction that creates 
> > or
> > -truncates them.  This can make bulk operations much faster (see
> > -).  But minimal WAL does not contain
> > -enough information to reconstruct the data from a base backup and 
> > the
> > -WAL logs, so replica or higher must be used to
> > -enable WAL archiving () and
> > -streaming replication.
> > +permanent relations for the remainder of a transaction that 
> > creates,
> > +rewrites, or truncates them.  This can make bulk operations much
> > +faster (see ).  But minimal WAL does
> > +not contain enough information to reconstruct the data from a base
> > +backup and the WAL logs, so replica or higher 
> > must
> > +be used to enable WAL archiving ( > linkend="guc-archive-mode"/>)
> > +and streaming replication.
> > 
> > @@ -2891,9 +2891,9 @@ include_dir 'conf.d'
> >  When wal_level is minimal 
> > and a
> > -transaction commits after creating or rewriting a permanent table,
> > -materialized view, or index, this setting determines how to persist
> > -the new data.  If the data is smaller than this setting, write it 
> > to
> > -the WAL log; otherwise, use an fsync of the data file.  Depending 
> > on
> > -the properties of your storage, raising or lowering this value 
> > might
> > -help if such commits are slowing concurrent transactions.  The 
> > default
> > -is two megabytes (2MB).
> > +transaction commits after creating, rewriting, or truncating a
> > +permanent relation, this setting determines how to persist the new
> > +data.  If the data is smaller than this setting, write it to the 
> > WAL
> > +log; otherwise, use an fsync of the data file.  Depending on the
> > +properties of your storage, raising or lowering this value might 
> > help
> > +if such commits are slowing concurrent transactions.  The default 
> > is
> > +two megabytes (2MB).
> > 
> 
> I agree that relation works as the generic name of table-like
> objects. Addition to that, doesn't using the word "storage file" make
> it more clearly?  I'm not confident on the wording itself, but it will
> look like the 

truncating timestamps on arbitrary intervals

[John Naylor]

Hi,

When analyzing time-series data, it's useful to be able to bin
timestamps into equally spaced ranges. date_trunc() is only able to
bin on a specified whole unit. In the attached patch for the March
commitfest, I propose a new function date_trunc_interval(), which can
truncate to arbitrary intervals, e.g.:

select date_trunc_interval('15 minutes', timestamp '2020-02-16
20:48:40'); date_trunc_interval
-
 2020-02-16 20:45:00
(1 row)

With this addition, it might be possible to turn the existing
date_trunc() functions into wrappers. I haven't done that here because
it didn't seem practical at this point. For one, the existing
functions have special treatment for weeks, centuries, and millennia.

Note: I've only written the implementation for the type timestamp
without timezone. Adding timezone support would be pretty simple, but
I wanted to get feedback on the basic idea first before making it
complete. I've also written tests and very basic documentation.

-- 
John Naylorhttps://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


v1-datetrunc_interval.patch
Description: Binary data

Re: Autovacuum on partitioned table

[yuzuko]

Hi,

Thanks for reviewing the patch.

> > We can make it work correctly but I think perhaps we can skip updating
> > statistics values of partitioned tables other than n_mod_since_analyze
> > as the first step. Because if we support also n_live_tup and
> > n_dead_tup, user might get confused that other statistics values such
> > as seq_scan, seq_tup_read however are not supported.
>
> +1, that makes sense.
>
Yes, Indeed.  I modified it not to update statistics other than
n_mod_since_analyze.
Attach the v5 patch.  In this patch, pgstat_report_analyze() always reports 0 as
msg.m_live_tuples and m_dead_tuples when the relation is partitioned.

-- 
Best regards,
Yuzuko Hosoya
NTT Open Source Software Center


v5_autovacuum_on_partitioned_table.patch
Description: Binary data

Re: [PATCH] pg_upgrade: report the reason for failing to open the cluster version file

[Michael Paquier]

On Tue, Feb 25, 2020 at 11:55:06PM +, Dagfinn Ilmari Mannsåker wrote:
> @@ -164,11 +164,11 @@ get_major_server_version(ClusterInfo *cluster)
>   snprintf(ver_filename, sizeof(ver_filename), "%s/PG_VERSION",
>cluster->pgdata);
>   if ((version_fd = fopen(ver_filename, "r")) == NULL)
> - pg_fatal("could not open version file: %s\n", ver_filename);
> + pg_fatal("could not open version file \"%s\": %m\n", 
> ver_filename);

Here I think that it would be better to just use "could not open
file" as we know that we are dealing with a version file already
thanks to ver_filename.

>   if (fscanf(version_fd, "%63s", cluster->major_version_str) == 0 ||
>   sscanf(cluster->major_version_str, "%d.%d", , ) < 1)
> - pg_fatal("could not parse PG_VERSION file from %s\n", 
> cluster->pgdata);
> + pg_fatal("could not parse PG_VERSION file from \"%s\"\n", 
> cluster->pgdata);
>  
>   fclose(version_fd);

No objection to this one.
--
Michael


signature.asc
Description: PGP signature

Re: [Proposal] Add accumulated statistics for wait event

[Kyotaro Horiguchi]

Hello.  I had a brief look on this and have some comments on this.

At Tue, 25 Feb 2020 07:53:26 +, "imai.yoshik...@fujitsu.com" 
 wrote in 
> Thanks for Wang's mail, I noticed my 0002 patch was wrong from v3.
> 
> Here, I attach correct patches.
> 
> Also I will begin to do some benchmark with higher scale and higher number of
> users and try to change stats reporting implementation to not affect
> performance, which I couldn't have not started because of another tasks.

It uses its own hash implement. Aside from the appropriateness of
having another implement of existing tool, in the first place hash
works well for wide, sparse and uncertain set of keys. But they are in
rather a dense and narrow set with certain and fixed members. It
registers more than 200 entries but bucket size is 461. It would be
needed to avoid colliisions, but seems a bit too wasting.

It seems trying to avoid doing needless work when the feature is not
activated by checking "if (wa_hash==NULL)", but the hash is created
being filled with possible entries regardless of whether
track_wait_timing is on or off. As the result
pgstat_report_waitaccum_end calls pgstat_get_wa_entry tremendously
frequently. This should be the reason for the recent benchmark result.
I'm not sure such frequency of hash-searching is acceptable even if
the feature is turned on.

I think we need a smarter mapping scheme of events to entries.


regards.

-- 
Kyotaro Horiguchi
NTT Open Source Software Center

Re: Resolving the python 2 -> python 3 mess

[Andrew Dunstan]

On 2/25/20 7:08 PM, Tom Lane wrote:
> Andrew Dunstan  writes:
>> On 2/25/20 5:06 PM, Tom Lane wrote:
>>> No joy there --- now that I look closer, it seems the cfbot doesn't
>>> build any of the external-language PLs on Windows.  I'll have to
>>> wait for some reviewer to try it.
>> What are the requirements for testing? bowerbird builds with python 2.7,
>> although I guess I should really try to upgrade it  3.x.
> Has to be python 3, unfortunately; the patch has no effect on a
> python 2 build.
>
>   



Yeah, I have python3 working on drongo, I'll test there.


cheers


andrew


-- 
Andrew Dunstanhttps://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Re: Resolving the python 2 -> python 3 mess

[Tom Lane]

Andrew Dunstan  writes:
> On 2/25/20 5:06 PM, Tom Lane wrote:
>> No joy there --- now that I look closer, it seems the cfbot doesn't
>> build any of the external-language PLs on Windows.  I'll have to
>> wait for some reviewer to try it.

> What are the requirements for testing? bowerbird builds with python 2.7,
> although I guess I should really try to upgrade it  3.x.

Has to be python 3, unfortunately; the patch has no effect on a
python 2 build.

regards, tom lane

Re: [PATCH] pg_upgrade: report the reason for failing to open the cluster version file

[Dagfinn Ilmari Mannsåker]

Daniel Gustafsson  writes:

> A few lines further down from this we report an error in case we are unable to
> parse the file in question:
>
> pg_fatal("could not parse PG_VERSION file from %s\n", 
> cluster->pgdata);
>
> Should the pgdata argument be quoted there as well, like \"%s\", to make it
> consistent for how we report filenames and directories in pg_upgrade?

Good point, I agree we should. Updated patch attached.

- ilmari
-- 
"A disappointingly low fraction of the human race is,
 at any given time, on fire." - Stig Sandbeck Mathisen

>From a503ffdb1499e73bfb75373a9af2e766e279beeb Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Dagfinn=20Ilmari=20Manns=C3=A5ker?= 
Date: Tue, 25 Feb 2020 18:07:14 +
Subject: [PATCH v2] pg_upgrade: add %m to version file open failure message

Also quote the file name in the error messages.
---
 src/bin/pg_upgrade/server.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/bin/pg_upgrade/server.c b/src/bin/pg_upgrade/server.c
index e244256501..be604d3351 100644
--- a/src/bin/pg_upgrade/server.c
+++ b/src/bin/pg_upgrade/server.c
@@ -164,11 +164,11 @@ get_major_server_version(ClusterInfo *cluster)
 	snprintf(ver_filename, sizeof(ver_filename), "%s/PG_VERSION",
 			 cluster->pgdata);
 	if ((version_fd = fopen(ver_filename, "r")) == NULL)
-		pg_fatal("could not open version file: %s\n", ver_filename);
+		pg_fatal("could not open version file \"%s\": %m\n", ver_filename);
 
 	if (fscanf(version_fd, "%63s", cluster->major_version_str) == 0 ||
 		sscanf(cluster->major_version_str, "%d.%d", , ) < 1)
-		pg_fatal("could not parse PG_VERSION file from %s\n", cluster->pgdata);
+		pg_fatal("could not parse PG_VERSION file from \"%s\"\n", cluster->pgdata);
 
 	fclose(version_fd);
 
-- 
2.22.0

Re: [PATCH] pg_upgrade: report the reason for failing to open the cluster version file

[Daniel Gustafsson]

> On 26 Feb 2020, at 00:14, Dagfinn Ilmari Mannsåker  wrote:

> It would have saved some minutes of debugging time if that had included
> the reason why the open failed, so here's a patch to do so.

+1 on the attached patch.  A quick skim across the similar error reportings in
pg_upgrade doesn't turn up any others which lack the more detailed information.

> - pg_fatal("could not open version file: %s\n", ver_filename);
> + pg_fatal("could not open version file \"%s\": %m\n", 
> ver_filename);

A few lines further down from this we report an error in case we are unable to
parse the file in question:

pg_fatal("could not parse PG_VERSION file from %s\n", cluster->pgdata);

Should the pgdata argument be quoted there as well, like \"%s\", to make it
consistent for how we report filenames and directories in pg_upgrade?

cheers ./daniel

[PATCH] pg_upgrade: report the reason for failing to open the cluster version file

[Dagfinn Ilmari Mannsåker]

Hi Hackers,

The other day I was helping someone with pg_upgrade on IRC, and they got
a rather unhelpful error message:

  ERROR: could not open version file /path/to/new/cluster/PG_VERSION

It would have saved some minutes of debugging time if that had included
the reason why the open failed, so here's a patch to do so.

- 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 84b99582a3447213f666c2f6f52c22ef518d82d5 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Dagfinn=20Ilmari=20Manns=C3=A5ker?= 
Date: Tue, 25 Feb 2020 18:07:14 +
Subject: [PATCH] pg_upgrade: add %m to version file open failure message

---
 src/bin/pg_upgrade/server.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/bin/pg_upgrade/server.c b/src/bin/pg_upgrade/server.c
index e244256501..c9b56ae175 100644
--- a/src/bin/pg_upgrade/server.c
+++ b/src/bin/pg_upgrade/server.c
@@ -164,7 +164,7 @@ get_major_server_version(ClusterInfo *cluster)
 	snprintf(ver_filename, sizeof(ver_filename), "%s/PG_VERSION",
 			 cluster->pgdata);
 	if ((version_fd = fopen(ver_filename, "r")) == NULL)
-		pg_fatal("could not open version file: %s\n", ver_filename);
+		pg_fatal("could not open version file \"%s\": %m\n", ver_filename);
 
 	if (fscanf(version_fd, "%63s", cluster->major_version_str) == 0 ||
 		sscanf(cluster->major_version_str, "%d.%d", , ) < 1)
-- 
2.22.0

Re: Resolving the python 2 -> python 3 mess

[Andrew Dunstan]

On 2/25/20 5:06 PM, Tom Lane wrote:
> I wrote:
>> I set up the MSVC scripts to default to building the stub extension.
>> I don't know if we really want to commit it that way, but the idea
>> for the moment is to try to get the cfbot to test it on Windows.
> No joy there --- now that I look closer, it seems the cfbot doesn't
> build any of the external-language PLs on Windows.  I'll have to
> wait for some reviewer to try it.
>
>   



What are the requirements for testing? bowerbird builds with python 2.7,
although I guess I should really try to upgrade it  3.x.


cheers


andrew


-- 
Andrew Dunstanhttps://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Re: Resolving the python 2 -> python 3 mess

[Tom Lane]

I wrote:
> I set up the MSVC scripts to default to building the stub extension.
> I don't know if we really want to commit it that way, but the idea
> for the moment is to try to get the cfbot to test it on Windows.

No joy there --- now that I look closer, it seems the cfbot doesn't
build any of the external-language PLs on Windows.  I'll have to
wait for some reviewer to try it.

regards, tom lane

Re: Ought to use heap_multi_insert() for pg_attribute/depend insertions?

[Daniel Gustafsson]

> On 25 Feb 2020, at 00:29, Andres Freund  wrote:
> On 2020-02-23 16:27:57 +0900, Michael Paquier wrote:

>> Good catch.  I would not backpatch that as it is not a live bug
>> because heap_multi_insert() is not used for catalogs yet.  With your
>> patch, that would be the case though..
> 
> Thanks for pushing.

+1, thanks to the both of you for helping with the patch.  Attached is a v8 of
the patchset to make the cfbot happier, as the previous no longer applies.

In doing that I realized that there is another hunk in this patch for fixing up
logical decoding multi-insert support, which is independent of the patch in
question here so I split it off.  It's another issue which cause no harm at all
today, but fails as soon as someone tries to perform multi inserts into the
catalog.

AFAICT, the current coding assumes that the multi-insert isn't from a catalog,
and makes little attempts at surviving in case it is.  The attached patch fixes
that by taking a fast-path exit in case it is a catalog multi-insert, as there
is nothing to decode.

cheers ./daniel



catalog_multi_insert-v8.patch
Description: Binary data


decodemultiinsert_tupledata.patch
Description: Binary data

[PATCH v1] Allow COPY "test" to output a header and add header matching mode to COPY FROM

[Rémi Lapeyre]

Hi, here's a new version of the patch with the header matching feature.
I should apply cleanly on master, let me know if anything's wrong.

---
 contrib/file_fdw/input/file_fdw.source  |  7 +-
 contrib/file_fdw/output/file_fdw.source | 13 ++--
 doc/src/sgml/ref/copy.sgml  |  9 ++-
 src/backend/commands/copy.c | 93 ++---
 src/test/regress/input/copy.source  | 71 ++-
 src/test/regress/output/copy.source | 58 ++-
 6 files changed, 202 insertions(+), 49 deletions(-)

diff --git a/contrib/file_fdw/input/file_fdw.source b/contrib/file_fdw/input/file_fdw.source
index 45b728eeb3..7a3983c785 100644
--- a/contrib/file_fdw/input/file_fdw.source
+++ b/contrib/file_fdw/input/file_fdw.source
@@ -37,7 +37,6 @@ CREATE USER MAPPING FOR regress_no_priv_user SERVER file_server;

 -- validator tests
 CREATE FOREIGN TABLE tbl () SERVER file_server OPTIONS (format 'xml');  -- ERROR
-CREATE FOREIGN TABLE tbl () SERVER file_server OPTIONS (format 'text', header 'true');  -- ERROR
 CREATE FOREIGN TABLE tbl () SERVER file_server OPTIONS (format 'text', quote ':');  -- ERROR
 CREATE FOREIGN TABLE tbl () SERVER file_server OPTIONS (format 'text', escape ':'); -- ERROR
 CREATE FOREIGN TABLE tbl () SERVER file_server OPTIONS (format 'binary', header 'true');-- ERROR
@@ -80,6 +79,12 @@ CREATE FOREIGN TABLE agg_bad (
 OPTIONS (format 'csv', filename '@abs_srcdir@/data/agg.bad', header 'true', delimiter ';', quote '@', escape '"', null '');
 ALTER FOREIGN TABLE agg_bad ADD CHECK (a >= 0);

+-- test header matching
+CREATE FOREIGN TABLE header_match ("1" int, foo text) SERVER file_server
+OPTIONS (format 'csv', filename '@abs_srcdir@/data/list1.csv', delimiter ',', header 'match');
+CREATE FOREIGN TABLE header_dont_match (a int, foo text) SERVER file_server
+OPTIONS (format 'csv', filename '@abs_srcdir@/data/list1.csv', delimiter ',', header 'match');	-- ERROR
+
 -- per-column options tests
 CREATE FOREIGN TABLE text_csv (
 word1 text OPTIONS (force_not_null 'true'),
diff --git a/contrib/file_fdw/output/file_fdw.source b/contrib/file_fdw/output/file_fdw.source
index 52b4d5f1df..d76a3dc6f8 100644
--- a/contrib/file_fdw/output/file_fdw.source
+++ b/contrib/file_fdw/output/file_fdw.source
@@ -33,14 +33,12 @@ CREATE USER MAPPING FOR regress_no_priv_user SERVER file_server;
 -- validator tests
 CREATE FOREIGN TABLE tbl () SERVER file_server OPTIONS (format 'xml');  -- ERROR
 ERROR:  COPY format "xml" not recognized
-CREATE FOREIGN TABLE tbl () SERVER file_server OPTIONS (format 'text', header 'true');  -- ERROR
-ERROR:  COPY HEADER available only in CSV mode
 CREATE FOREIGN TABLE tbl () SERVER file_server OPTIONS (format 'text', quote ':');  -- ERROR
 ERROR:  COPY quote available only in CSV mode
 CREATE FOREIGN TABLE tbl () SERVER file_server OPTIONS (format 'text', escape ':'); -- ERROR
 ERROR:  COPY escape available only in CSV mode
 CREATE FOREIGN TABLE tbl () SERVER file_server OPTIONS (format 'binary', header 'true');-- ERROR
-ERROR:  COPY HEADER available only in CSV mode
+ERROR:  COPY HEADER available only in CSV and text mode
 CREATE FOREIGN TABLE tbl () SERVER file_server OPTIONS (format 'binary', quote ':');-- ERROR
 ERROR:  COPY quote available only in CSV mode
 CREATE FOREIGN TABLE tbl () SERVER file_server OPTIONS (format 'binary', escape ':');   -- ERROR
@@ -95,6 +93,11 @@ CREATE FOREIGN TABLE agg_bad (
 ) SERVER file_server
 OPTIONS (format 'csv', filename '@abs_srcdir@/data/agg.bad', header 'true', delimiter ';', quote '@', escape '"', null '');
 ALTER FOREIGN TABLE agg_bad ADD CHECK (a >= 0);
+-- test header matching
+CREATE FOREIGN TABLE header_match ("1" int, foo text) SERVER file_server
+OPTIONS (format 'csv', filename '/Users/remi/src/postgresql/contrib/file_fdw/data/list1.csv', delimiter ',', header 'match');
+CREATE FOREIGN TABLE header_dont_match (a int, foo text) SERVER file_server
+OPTIONS (format 'csv', filename '/Users/remi/src/postgresql/contrib/file_fdw/data/list1.csv', delimiter ',', header 'match');	-- ERROR
 -- per-column options tests
 CREATE FOREIGN TABLE text_csv (
 word1 text OPTIONS (force_not_null 'true'),
@@ -441,12 +444,14 @@ SET ROLE regress_file_fdw_superuser;
 -- cleanup
 RESET ROLE;
 DROP EXTENSION file_fdw CASCADE;
-NOTICE:  drop cascades to 7 other objects
+NOTICE:  drop cascades to 9 other objects
 DETAIL:  drop cascades to server file_server
 drop cascades to user mapping for regress_file_fdw_superuser on server file_server
 drop cascades to user mapping for regress_no_priv_user on server file_server
 drop cascades to foreign table agg_text
 drop cascades to foreign table agg_csv
 drop cascades to foreign table agg_bad
+drop cascades to foreign table header_match
+drop cascades to foreign table header_dont_match
 drop cascades to foreign table text_csv
 DROP ROLE regress_file_fdw_superuser, regress_file_fdw_user, regress_no_priv_user;
diff --git 

Re: [postgis-devel] About EXTENSION from UNPACKAGED on PostgreSQL 13

[Tom Lane]

=?UTF-8?Q?Darafei_=22Kom=D1=8Fpa=22_Praliaskouski?=  writes:
> Removal of FROM UNPACKAGED breaks PostGIS 2.5 -> 3.0 upgrade path, and
> we haven't yet found a proper replacement since such removal wasn't
> something we were expecting.

I'd agree with Stephen's comment:

> On Tue, Feb 25, 2020 at 11:37 PM Stephen Frost  wrote:
>> Why is it impacting the PostGIS upgrade path?  The FROM UNPACKAGED was
>> never intended to be used as an upgrade path..

This seems like a serious abuse of the FROM option, not to mention being
fundamentally unsafe --- the whole problem with FROM is that you can't
be entirely sure what the starting state is.  So unless you can make a
pretty strong case as to why you need to do it like that, and that there's
no other way to handle it in the many months before v13 ships, I'm not
going to have a lot of sympathy.

regards, tom lane

Re: [postgis-devel] About EXTENSION from UNPACKAGED on PostgreSQL 13

[Komяpa]

Hi,

PostGIS 2.5 had raster and vector blended together in single extension.
In PostGIS 3, they were split out into postgis and postgis_raster extensions.
To upgrade, there is now postgis_extensions_upgrade() function, that
unpackages the raster part out of postgis extensions, upgrades it, and
packages raster functions back into postgis_raster by utilizing FROM
UNPACKAGED.
Removal of FROM UNPACKAGED breaks PostGIS 2.5 -> 3.0 upgrade path, and
we haven't yet found a proper replacement since such removal wasn't
something we were expecting.

On Tue, Feb 25, 2020 at 11:37 PM Stephen Frost  wrote:
>
> Greetings,
>
> * Darafei "Komяpa" Praliaskouski (m...@komzpa.net) wrote:
> > can it be raised on pgsql-hackers as a thing impacting PostGIS upgrade path?
>
> Why is it impacting the PostGIS upgrade path?  The FROM UNPACKAGED was
> never intended to be used as an upgrade path..
>
> Thanks,
>
> Stephen
> ___
> postgis-devel mailing list
> postgis-de...@lists.osgeo.org
> https://lists.osgeo.org/mailman/listinfo/postgis-devel



-- 
Darafei Praliaskouski
Support me: http://patreon.com/komzpa

Re: Internal key management system

[Cary Huang]

Hi 

I would like to share with you a front end patch based on kms_v4.patch that you 
have shared, called "kms_v4_fe.patch". 



The patch integrates front end tool pg_waldump with the KMSv4 and obtain 
encryption and decryption cipher contexts from the KMS backend. These cipher 
contexts can then be used in subsequent encryption and decryption operations 
provided by cipher.h when TDE is enabled. I added two common functions in your 
kmgr_utils that other front end tools affected by TDE can also use to obtain 
the cipher contexts. Do let me know if this is how you would envision KMS APIs 
to be used by a front end. 



cheers





Cary Huang

-

HighGo Software Inc. (Canada)

mailto:cary.hu...@highgo.ca

http://www.highgo.ca




 On Mon, 24 Feb 2020 17:55:09 -0800 Masahiko Sawada 
 wrote 



On Thu, 20 Feb 2020 at 16:16, Masahiko Sawada 
 wrote: 
> 
> On Wed, 19 Feb 2020 at 09:29, Cary Huang  wrote: 
> > 
> > Hi 
> > 
> > I have tried the attached kms_v3 patch and have some comments: 
> > 
> > 1) In the comments, I think you meant hmac + iv + encrypted data instead of 
> > iv + hmac + encrypted data? 
> > 
> > ---> in kmgr_wrap_key( ): 
> > +   /* 
> > +* Assemble the wrapped key. The order of the wrapped key is iv, 
> > hmac and 
> > +* encrypted data. 
> > +*/ 
> 
> Right, will fix. 
> 
> > 
> > 
> > 2) I see that create_keywrap_ctx function in kmgr_utils.c and regular 
> > cipher context init will both call ossl_aes256_encrypt_init to initialise 
> > context for encryption and key wrapping. In ossl_aes256_encrypt_init, the 
> > cipher method always initialises to aes-256-cbc, which is ok for keywrap 
> > because under CBC block cipher mode, we only had to supply one unique IV as 
> > initial value. But for actual WAL and buffer encryption that will come in 
> > later, I think the discussion is to use CTR block cipher mode, which 
> > requires one unique IV for each block, and the sequence id from WAL and 
> > buffer can be used to derive unique IV for each block for better security? 
> > I think it would be better to allow caller to decide which EVP_CIPHER to 
> > initialize? EVP_aes_256_cbc(), EVP_aes_256_ctr() or others? 
> > 
> > +ossl_aes256_encrypt_init(pg_cipher_ctx *ctx, uint8 *key) 
> > +{ 
> > +   if (!EVP_EncryptInit_ex(ctx, EVP_aes_256_cbc(), NULL, NULL, NULL)) 
> > +   return false; 
> > +   if (!EVP_CIPHER_CTX_set_key_length(ctx, PG_AES256_KEY_LEN)) 
> > +   return false; 
> > +   if (!EVP_EncryptInit_ex(ctx, NULL, NULL, key, NULL)) 
> > +   return false; 
> > + 
> > +   /* 
> > +* Always enable padding. We don't need to check the return 
> > +* value as EVP_CIPHER_CTX_set_padding always returns 1. 
> > +*/ 
> > +   EVP_CIPHER_CTX_set_padding(ctx, 1); 
> > + 
> > +   return true; 
> > +} 
> 
> It seems good. We can expand it to make caller decide the block cipher 
> mode of operation and key length. I removed such code from the 
> previous patch to make it simple since currently we support only 
> AES-256 CBC. 
> 
> > 
> > 3) Following up point 2), I think we should enhance the enum to include not 
> > only the Encryption algorithm and key size, but also the block cipher mode 
> > as well because having all 3 pieces of information can describe exactly how 
> > KMS is performing the encryption and decryption. So when we call 
> > "ossl_aes256_encrypt_init", we can include the new enum as input parameter 
> > and it will initialise the EVP_CIPHER_CTX with either EVP_aes_256_cbc() or 
> > EVP_aes_256_ctr() for different purposes (key wrapping, or WAL 
> > encryption..etc). 
> > 
> > ---> kmgr.h 
> > +/* Value of key_management_cipher */ 
> > +enum 
> > +{ 
> > +   KMGR_CIPHER_OFF = 0, 
> > +   KMGR_CIPHER_AES256 
> > +}; 
> > + 
> > 
> > so it would become 
> > +enum 
> > +{ 
> > +   KMGR_CIPHER_OFF = 0, 
> > +   KMGR_CIPHER_AES256_CBC = 1, 
> > +   KMGR_CIPHER_AES256_CTR = 2 
> > +}; 
> > 
> > If you agree with this change, several other places will need to be changed 
> > as well, such as "kmgr_cipher_string", "kmgr_cipher_value" and the initdb 
> > code 
> 
> KMGR_CIPHER_XXX is relevant with cipher mode used by KMS and KMS would 
> still use AES256 CBC even if we had TDE which would use AES256 CTR. 
> 
> After more thoughts, I think currently we can specify -e aes-256 to 
> initdb but actually this is not necessary. When 
> --cluster-passphrase-command specified, we enable the internal KMS and 
> always use AES256 CBC. Something like -e option will be needed after 
> supporting TDE with several cipher options. Thoughts? 
> 
> > 
> > 4) the pg_wrap_key and pg_unwrap_key SQL functions defined in kmgr.c 
> > I tried these new SQL functions and found that the pg_unwrap_key will 
> > produce the original key with 4 bytes less. This is because the result 
> > length is not 

Re: Resolving the python 2 -> python 3 mess

[Tom Lane]

I wrote:
> Here's an updated pair of patches that attempt to fix the MSVC
> scripts (pretty blindly) and provide a very simple regression test.

A little *too* blindly, evidently.  Try again ...

regards, tom lane

diff --git a/doc/src/sgml/plpython.sgml b/doc/src/sgml/plpython.sgml
index 1921915..ac989a3 100644
--- a/doc/src/sgml/plpython.sgml
+++ b/doc/src/sgml/plpython.sgml
@@ -164,13 +164,6 @@
   
 
   
-   See also the
-   document https://docs.python.org/3/whatsnew/3.0.html;>What's
-   New In Python 3.0 for more information about porting to
-   Python 3.
-  
-
-  
It is not allowed to use PL/Python based on Python 2 and PL/Python
based on Python 3 in the same session, because the symbols in the
dynamic modules would clash, which could result in crashes of the
@@ -179,6 +172,90 @@
a mismatch is detected.  It is possible, however, to use both
PL/Python variants in the same database, from separate sessions.
   
+
+  
+   Converting from Python 2 to Python 3
+
+   
+See the
+document https://docs.python.org/3/whatsnew/3.0.html;>What's
+New In Python 3.0 for the Python community's information and
+recommendations about porting to Python 3.
+   
+
+   
+PostgreSQL provides some support for helping
+you to convert existing Python 2 routines to Python 3.  In an
+installation built with Python 3, there is an
+extension convert_python3 that changes functions
+and procedures from the plpythonu
+and plpython2u languages to
+the plpython3u language.  While doing so, it applies
+the 2to3 tool described in the above document to
+the body of each such routine.
+   
+
+   
+Using convert_python3 can be as simple as:
+
+CREATE EXTENSION convert_python3;
+CALL convert_python3_all();
+
+This must be done as database superuser.  If you wish, you can drop the
+extension once you're done converting everything.
+   
+
+   
+Since convert_python3 is Python 3 code, be careful
+not to install or run it in a session that has previously executed any
+Python 2 code.  As explained above, that won't work.
+   
+
+   
+convert_python3_all has two optional arguments: the
+name of the conversion tool to use (by default 2to3,
+but you might for instance need to provide a full path name) and any
+special command-line options to provide to it.  You might for example
+want to adjust the set of fixer rules
+that 2to3 applies:
+
+CALL convert_python3_all(options = '-f idioms -x apply');
+
+See 2to3's
+https://docs.python.org/3/library/2to3.html;>documentation
+for more information.
+   
+
+   
+The convert_python3 extension also provides a
+procedure that converts just one Python 2 function at a time:
+
+CALL convert_python3_one('myfunc(int)');
+
+The argument is the target function's OID, which can be written as
+a regprocedure constant (see
+).  The main reason to use this would be
+if you need to use different options for different functions.  It has
+the same optional arguments as convert_python3_all:
+
+CALL convert_python3_one('otherfunc(text)', tool = '/usr/bin/2to3',
+ options = '-f idioms');
+
+   
+
+   
+If you have needs that go beyond this, consult the source code for
+the convert_python3 extension (it's just a
+couple of plpython3u procedures) and adapt those
+procedures as necessary.
+   
+
+   
+Keep in mind that if you've constructed any DO blocks
+that use Python 2 code, those will have to be fixed up manually,
+wherever the source code for them exists.
+   
+  
  
 
  
diff --git a/src/pl/plpython/Makefile b/src/pl/plpython/Makefile
index 9e95285..03f858b 100644
--- a/src/pl/plpython/Makefile
+++ b/src/pl/plpython/Makefile
@@ -38,6 +38,9 @@ DATA = $(NAME)u.control $(NAME)u--1.0.sql
 ifeq ($(python_majorversion),2)
 DATA += plpythonu.control plpythonu--1.0.sql
 endif
+ifeq ($(python_majorversion),3)
+DATA += convert_python3.control convert_python3--1.0.sql
+endif
 
 # header files to install - it's not clear which of these might be needed
 # so install them all.
diff --git a/src/pl/plpython/convert_python3--1.0.sql b/src/pl/plpython/convert_python3--1.0.sql
new file mode 100644
index 000..3444ac0
--- /dev/null
+++ b/src/pl/plpython/convert_python3--1.0.sql
@@ -0,0 +1,149 @@
+/* convert_python3--1.0.sql */
+
+-- complain if script is sourced in psql, rather than via CREATE EXTENSION
+\echo Use "CREATE EXTENSION convert_python3" to load this file. \quit
+
+-- This module provides two procedures, one to convert all python2
+-- functions and one to do just one.  They're nearly identical, and
+-- in principle convert_python3_all() could be written as a loop
+-- around convert_python3_one().  It's not done that way since
+-- creating a temp directory for each function in a bulk conversion
+-- could get expensive.
+
+-- For some benighted reason, lib2to3 has exactly no documented API,
+-- so 

Re: Resolving the python 2 -> python 3 mess

[Tom Lane]

Here's an updated pair of patches that attempt to fix the MSVC
scripts (pretty blindly) and provide a very simple regression test.
I'm not too sure whether the regression test will really prove
workable or not: for starters, it'll fail if "2to3" isn't available
in the PATH.  Perhaps there's reason to object to even trying to
test that, on security grounds.

I set up the MSVC scripts to default to building the stub extension.
I don't know if we really want to commit it that way, but the idea
for the moment is to try to get the cfbot to test it on Windows.

regards, tom lane

diff --git a/doc/src/sgml/plpython.sgml b/doc/src/sgml/plpython.sgml
index 1921915..ac989a3 100644
--- a/doc/src/sgml/plpython.sgml
+++ b/doc/src/sgml/plpython.sgml
@@ -164,13 +164,6 @@
   
 
   
-   See also the
-   document https://docs.python.org/3/whatsnew/3.0.html;>What's
-   New In Python 3.0 for more information about porting to
-   Python 3.
-  
-
-  
It is not allowed to use PL/Python based on Python 2 and PL/Python
based on Python 3 in the same session, because the symbols in the
dynamic modules would clash, which could result in crashes of the
@@ -179,6 +172,90 @@
a mismatch is detected.  It is possible, however, to use both
PL/Python variants in the same database, from separate sessions.
   
+
+  
+   Converting from Python 2 to Python 3
+
+   
+See the
+document https://docs.python.org/3/whatsnew/3.0.html;>What's
+New In Python 3.0 for the Python community's information and
+recommendations about porting to Python 3.
+   
+
+   
+PostgreSQL provides some support for helping
+you to convert existing Python 2 routines to Python 3.  In an
+installation built with Python 3, there is an
+extension convert_python3 that changes functions
+and procedures from the plpythonu
+and plpython2u languages to
+the plpython3u language.  While doing so, it applies
+the 2to3 tool described in the above document to
+the body of each such routine.
+   
+
+   
+Using convert_python3 can be as simple as:
+
+CREATE EXTENSION convert_python3;
+CALL convert_python3_all();
+
+This must be done as database superuser.  If you wish, you can drop the
+extension once you're done converting everything.
+   
+
+   
+Since convert_python3 is Python 3 code, be careful
+not to install or run it in a session that has previously executed any
+Python 2 code.  As explained above, that won't work.
+   
+
+   
+convert_python3_all has two optional arguments: the
+name of the conversion tool to use (by default 2to3,
+but you might for instance need to provide a full path name) and any
+special command-line options to provide to it.  You might for example
+want to adjust the set of fixer rules
+that 2to3 applies:
+
+CALL convert_python3_all(options = '-f idioms -x apply');
+
+See 2to3's
+https://docs.python.org/3/library/2to3.html;>documentation
+for more information.
+   
+
+   
+The convert_python3 extension also provides a
+procedure that converts just one Python 2 function at a time:
+
+CALL convert_python3_one('myfunc(int)');
+
+The argument is the target function's OID, which can be written as
+a regprocedure constant (see
+).  The main reason to use this would be
+if you need to use different options for different functions.  It has
+the same optional arguments as convert_python3_all:
+
+CALL convert_python3_one('otherfunc(text)', tool = '/usr/bin/2to3',
+ options = '-f idioms');
+
+   
+
+   
+If you have needs that go beyond this, consult the source code for
+the convert_python3 extension (it's just a
+couple of plpython3u procedures) and adapt those
+procedures as necessary.
+   
+
+   
+Keep in mind that if you've constructed any DO blocks
+that use Python 2 code, those will have to be fixed up manually,
+wherever the source code for them exists.
+   
+  
  
 
  
diff --git a/src/pl/plpython/Makefile b/src/pl/plpython/Makefile
index 9e95285..03f858b 100644
--- a/src/pl/plpython/Makefile
+++ b/src/pl/plpython/Makefile
@@ -38,6 +38,9 @@ DATA = $(NAME)u.control $(NAME)u--1.0.sql
 ifeq ($(python_majorversion),2)
 DATA += plpythonu.control plpythonu--1.0.sql
 endif
+ifeq ($(python_majorversion),3)
+DATA += convert_python3.control convert_python3--1.0.sql
+endif
 
 # header files to install - it's not clear which of these might be needed
 # so install them all.
diff --git a/src/pl/plpython/convert_python3--1.0.sql b/src/pl/plpython/convert_python3--1.0.sql
new file mode 100644
index 000..3444ac0
--- /dev/null
+++ b/src/pl/plpython/convert_python3--1.0.sql
@@ -0,0 +1,149 @@
+/* convert_python3--1.0.sql */
+
+-- complain if script is sourced in psql, rather than via CREATE EXTENSION
+\echo Use "CREATE EXTENSION convert_python3" to load this file. \quit
+
+-- This module provides two procedures, one to convert all python2
+-- 

Re: [Patch] Base backups and random or zero pageheaders

[Michael Banck]

Hi,

Am Dienstag, den 25.02.2020, 19:34 +0500 schrieb Asif Rehman:
> On Fri, Oct 18, 2019 at 2:06 PM Michael Banck  
> wrote:
> > Here is finally a rebased patch for the (IMO) more important issue in
> > pg_basebackup. I've added a commitfest entry for this now: 
> > https://commitfest.postgresql.org/25/2308/
> 
> The patch does not seem to apply anymore, can you rebase it?

Thanks for letting me know, please find attached a rebased version. I
hope the StaticAssertDecl() is still correct in bufpage.h.


Michael

-- 
Michael Banck
Projektleiter / Senior Berater
Tel.: +49 2166 9901-171
Fax:  +49 2166 9901-100
Email: michael.ba...@credativ.de

credativ GmbH, HRB Mönchengladbach 12080
USt-ID-Nummer: DE204566209
Trompeterallee 108, 41189 Mönchengladbach
Geschäftsführung: Dr. Michael Meskes, Jörg Folz, Sascha Heuer

Unser Umgang mit personenbezogenen Daten unterliegt
folgenden Bestimmungen: https://www.credativ.de/datenschutz
From 084a2fe968a3ee9c0bb4f18fa9fbc8a582f38b3c Mon Sep 17 00:00:00 2001
From: Michael Banck 
Date: Fri, 18 Oct 2019 10:48:22 +0200
Subject: [PATCH] Fix checksum verification in base backups for random or zero
 page headers

We currently do not checksum a page if it is considered new by PageIsNew() or
if its pd_lsn page header field is larger than the LSN at the beginning of the
base backup. However, this means that if the whole page header is zero,
PageIsNew() will consider that page new due to the pd_upper field being zero
and no subsequent checksum verification is done. Also, a random value in the
pd_lsn field will very likely be larger than the LSN at backup start, again
resulting in the page not getting checksummed.

To fix, factor out the all-zeroes check from PageIsVerified() into a new method
PageIsZero() and call that for pages considered new by PageIsNew(). If a page
is all zero, consider that a checksum failure. Also check the pd_lsn field
against both the backup start pointer and the current pointer from
GetInsertRecPtr().

Add two more tests to the pg_basebackup TAP tests to check those errors.

Reported-By: Andres Freund
Discussion: https://postgr.es/m/20190319200050.ncuxejradurja...@alap3.anarazel.de
diff --git a/src/backend/replication/basebackup.c b/src/backend/replication/basebackup.c
index ca8bebf432..5cefc218a0 100644
--- a/src/backend/replication/basebackup.c
+++ b/src/backend/replication/basebackup.c
@@ -1468,87 +1468,28 @@ sendFile(const char *readfilename, const char *tarfilename, struct stat *statbuf
 page = buf + BLCKSZ * i;
 
 /*
- * Only check pages which have not been modified since the
- * start of the base backup. Otherwise, they might have been
- * written only halfway and the checksum would not be valid.
- * However, replaying WAL would reinstate the correct page in
- * this case. We also skip completely new pages, since they
- * don't have a checksum yet.
+ * We skip completely new pages after checking they are
+ * all-zero, since they don't have a checksum yet.
  */
-if (!PageIsNew(page) && PageGetLSN(page) < startptr)
+if (PageIsNew(page))
 {
-	checksum = pg_checksum_page((char *) page, blkno + segmentno * RELSEG_SIZE);
-	phdr = (PageHeader) page;
-	if (phdr->pd_checksum != checksum)
+	if (!PageIsZero(page))
 	{
 		/*
-		 * Retry the block on the first failure.  It's
-		 * possible that we read the first 4K page of the
-		 * block just before postgres updated the entire block
-		 * so it ends up looking torn to us.  We only need to
-		 * retry once because the LSN should be updated to
-		 * something we can ignore on the next pass.  If the
-		 * error happens again then it is a true validation
-		 * failure.
+		 * pd_upper is zero, but the page is not all zero.  We
+		 * cannot run pg_checksum_page() on the page as it
+		 * would throw an assertion failure.  Consider this a
+		 * checksum failure.
 		 */
-		if (block_retry == false)
-		{
-			/* Reread the failed block */
-			if (fseek(fp, -(cnt - BLCKSZ * i), SEEK_CUR) == -1)
-			{
-ereport(ERROR,
-		(errcode_for_file_access(),
-		 errmsg("could not fseek in file \"%s\": %m",
-readfilename)));
-			}
-
-			if (fread(buf + BLCKSZ * i, 1, BLCKSZ, fp) != BLCKSZ)
-			{
-/*
- * If we hit end-of-file, a concurrent
- * truncation must have occurred, so break out
- * of this loop just as if the initial fread()
- * returned 0. We'll drop through to the same
- * code that handles that case. (We must fix
- * up cnt first, though.)
- */
-if (feof(fp))
-{
-	cnt = BLCKSZ * i;
-	break;
-}
-
-ereport(ERROR,
-		(errcode_for_file_access(),
-		 errmsg("could not reread block %d of file \"%s\": %m",
-blkno, readfilename)));
-			}
-
-			if (fseek(fp, cnt - BLCKSZ * i - BLCKSZ, SEEK_CUR) 

Re: Yet another vectorized engine

[Konstantin Knizhnik]

I have ported vectorize_engine  for zedstore (vertical table AM).
Results of TPCH-10G/Q1 are the following:

par.workers
PG9_6
vectorize=off
PG9_6
vectorize=on
master
vectorize=off
jit=on
master
vectorize=off
jit=off master
vectorize=on
jit=on  master
vectorize=on
jit=off zedstore vectorize=off
jit=on
zedstore vectorize=off
jit=off zedstore vectorize=on
jit=on  zedstore vectorize=on
jit=off
0
36
20
16
25.5
15
17.5
18
26
17
19
4
10
-
5   7
-
-   5
7
-
-


As you can see from this table time of query execution without 
vectorization is almost the same for zedstore as for standard heap.
If means that expression execution overhead is dominated in this case 
despite to the underlying storage.
Enabling vectorize engine increases speed of zedstore as well as of 
standard heap.

But still standard heap is faster.

May be my implementation of extracting data from zedstore is not optimal 
- I just calling in the loop  zsbt_tid_scan_next + zsbt_attr_fetch.
I attached my implementation of zedstoream_getnexttile (I have added 
scan_getnexttile to tableAM interface).


Also I noticed that currently zedstore doesn't correctly calculate set 
of used attributes and so is extract useless data.
For example query like "select sum(x) from foo" cause fetching of all 
attributes from foo although we need just "x".


--
Konstantin Knizhnik
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company

/*-
 *
 * zedstoream_handler.c
 *	  ZedStore table access method code
 *
 * Portions Copyright (c) 1996-2019, PostgreSQL Global Development Group
 * Portions Copyright (c) 1994, Regents of the University of California
 *
 *
 * IDENTIFICATION
 *	  src/backend/access/zedstore/zedstoream_handler.c
 *-
 */
#include "postgres.h"

#include 

#include "access/genam.h"
#include "access/heapam.h"
#include "access/multixact.h"
#include "access/relscan.h"
#include "access/tableam.h"
#include "access/tsmapi.h"
#include "access/tupdesc_details.h"
#include "access/xact.h"
#include "access/zedstore_internal.h"
#include "access/zedstore_undorec.h"
#include "catalog/catalog.h"
#include "catalog/index.h"
#include "catalog/storage.h"
#include "catalog/storage_xlog.h"
#include "commands/progress.h"
#include "commands/vacuum.h"
#include "executor/executor.h"
#include "miscadmin.h"
#include "optimizer/plancat.h"
#include "pgstat.h"
#include "parser/parse_relation.h"
#include "storage/lmgr.h"
#include "storage/predicate.h"
#include "storage/procarray.h"
#include "utils/builtins.h"
#include "utils/rel.h"

typedef struct ZedStoreProjectData
{
	int			num_proj_atts;
	Bitmapset   *project_columns;
	int		   *proj_atts;
	ZSTidTreeScan tid_scan;
	ZSAttrTreeScan *attr_scans;
	MemoryContext context;
}  ZedStoreProjectData;

typedef struct ZedStoreDescData
{
	/* scan parameters */
	TableScanDescData rs_scan;  /* */
	ZedStoreProjectData proj_data;

	bool		started;
	zstid		cur_range_start;
	zstid		cur_range_end;

	/* These fields are used for bitmap scans, to hold a "block's" worth of data */
#define	MAX_ITEMS_PER_LOGICAL_BLOCK		MaxHeapTuplesPerPage
	int			bmscan_ntuples;
	zstid	   *bmscan_tids;
	int			bmscan_nexttuple;

	/* These fields are use for TABLESAMPLE scans */
	zstid   min_tid_to_scan;
	zstid   max_tid_to_scan;
	zstid   next_tid_to_scan;

} ZedStoreDescData;

typedef struct ZedStoreDescData *ZedStoreDesc;

typedef struct ZedStoreIndexFetchData
{
	IndexFetchTableData idx_fetch_data;
	ZedStoreProjectData proj_data;
} ZedStoreIndexFetchData;

typedef struct ZedStoreIndexFetchData *ZedStoreIndexFetch;

typedef struct ParallelZSScanDescData *ParallelZSScanDesc;

static IndexFetchTableData *zedstoream_begin_index_fetch(Relation rel);
static void zedstoream_end_index_fetch(IndexFetchTableData *scan);
static bool zedstoream_fetch_row(ZedStoreIndexFetchData *fetch,
 ItemPointer tid_p,
 Snapshot snapshot,
 TupleTableSlot *slot);
static bool zs_acquire_tuplock(Relation relation, ItemPointer tid, LockTupleMode mode,
			   LockWaitPolicy wait_policy, bool *have_tuple_lock);

static bool zs_blkscan_next_block(TableScanDesc sscan,
  BlockNumber blkno, OffsetNumber *offsets, int noffsets,
  bool predicatelocks);
static bool zs_blkscan_next_tuple(TableScanDesc sscan, TupleTableSlot *slot);

static Size zs_parallelscan_estimate(Relation rel);
static Size zs_parallelscan_initialize(Relation rel, ParallelTableScanDesc pscan);
static void zs_parallelscan_reinitialize(Relation rel, ParallelTableScanDesc pscan);
static bool zs_parallelscan_nextrange(Relation rel, ParallelZSScanDesc pzscan,
	  zstid *start, zstid *end);
static void 

Connections dropping while using Postgres backend DB with Ejabberd

[Dipanjan Ganguly]

Greetings,

I was trying to use postgresql database as a backend with Ejabberd XMPP
server for load test (Using TSUNG).

Noticed, while using Mnesia the  “simultaneous users and open TCP/UDP
connections”  graph in Tsung report is showing consistency, but while using
Postgres, we see drop in connections during 100 to 500 seconds of runtime,
and then recovering and staying consistent.

I have been trying to figure out what the issue could be without any
success. I am kind of a noob in this technology, and hoping for some help
from the good people from the community to understand the problem and how
to fix this. Below are some details..

· Postgres server utilization is low ( Avg load 1, Highest Cpu
utilization 26%, lowest freemem  9000)



Tsung  graph:
[image: image.png]
   Graph 1: Postgres 12 Backen
[image: image.png]

  Graph 2: Mnesia backend


· Ejabberd Server: Ubuntu 16.04, 16 GB ram, 4 core CPU.

· Postgres on remote server: same config

· Errors encountered during the same time:  error_connect_etimedout
(same outcome for other 2 tests)

· *Tsung Load:  *512 Bytes message size, user arrival rate 50/s,
80k registered users.

· Postgres server utilization is low ( Avg load 1, Highest Cpu
utilization 26%, lowest freemem  9000)

· Same tsung.xm and userlist used for the tests in Mnesia and
Postgres.

*Postgres Configuration used:*
shared_buffers = 4GB
effective_cache_size = 12GB
maintenance_work_mem = 1GB
checkpoint_completion_target = 0.9
wal_buffers = 16MB
default_statistics_target = 100
random_page_cost = 4
effective_io_concurrency = 2
work_mem = 256MB
min_wal_size = 1GB
max_wal_size = 2GB
max_worker_processes = 4
max_parallel_workers_per_gather = 2
max_parallel_workers = 4
max_parallel_maintenance_workers = 2
max_connections=5


Kindly help understanding this behavior.  Some advice on how to fix this
will be a big help .



Thanks,

Dipanjan

Re: Parallel copy

[Tomas Vondra]

On Sun, Feb 23, 2020 at 05:09:51PM -0800, Andres Freund wrote:

Hi,

On 2020-02-19 11:38:45 +0100, Tomas Vondra wrote:

I generally agree with the impression that parsing CSV is tricky and
unlikely to benefit from parallelism in general. There may be cases with
restrictions making it easier (e.g. restrictions on the format) but that
might be a bit too complex to start with.

For example, I had an idea to parallelise the planning by splitting it
into two phases:


FWIW, I think we ought to rewrite our COPY parsers before we go for
complex schemes. They're way slower than a decent green-field
CSV/... parser.



Yep, that's quite possible.




The one piece of information I'm missing here is at least a very rough
quantification of the individual steps of CSV processing - for example
if parsing takes only 10% of the time, it's pretty pointless to start by
parallelising this part and we should focus on the rest. If it's 50% it
might be a different story. Has anyone done any measurements?


Not recently, but I'm pretty sure that I've observed CSV parsing to be
way more than 10%.



Perhaps. I guess it'll depend on the CSV file (number of fields, ...),
so I still think we need to do some measurements first. I'm willing to
do that, but (a) I doubt I'll have time for that until after 2020-03,
and (b) it'd be good to agree on some set of typical CSV files.

regards

--
Tomas Vondra  http://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

[GsoC] Read/write transaction-level routing in Odyssey Project Idea

[Kostas Chasialis]

Greetings,

I am a Computer Science student at National and Kapodistrian University of
Athens, and I would like to be a part of this year's GsoC program.
During my academic courses, I developed an interest on databases (back-end)
and the main language I used during my academic career is C.

A project I recently developed was In-Memory SUM SQL Query Execution on
RDBMS-like queries using SortMerge Join. As input data we used the data
provided from SIGMOD 2018 contest.
(github link : https://github.com/kchasialis/Query-Compiler-Executor)

Therefore, the project idea "Read/Write transaction-level routing in
Odyssea" highly intrigued me and I would like to be a part of it.
I believe that I have the necessary background to live up to your
expectations and I would like to learn more things about this project and
ways I can contribute to its development.

Thanks in advance, Kostas.

Re: [Proposal] Global temporary tables

[tushar]

Hi,

I have created two  global temporary tables like this -

Case 1-
postgres=# create global  temp table foo(n int) *with 
(on_c*ommit_delete_rows='true');

CREATE TABLE

Case 2-
postgres=# create global  temp table bar1(n int) *on c*ommit delete rows;
CREATE TABLE


but   if i try to do the same having only 'temp' keyword , Case 2 is 
working fine but getting this error  for case 1 -


postgres=# create   temp table foo1(n int) with 
(on_commit_delete_rows='true');

ERROR:  regular table cannot specifie on_commit_delete_rows
postgres=#

postgres=#  create   temp table bar1(n int) on commit delete rows;
CREATE TABLE

i think this error message need to be more clear .

regards,
tushar

On 2/25/20 7:19 PM, Pavel Stehule wrote/:



út 25. 2. 2020 v 14:36 odesílatel Prabhat Sahu 
mailto:prabhat.s...@enterprisedb.com>> 
napsal:


Hi All,

Please check the below findings on GTT.
_-- Scenario 1:_
Under "information_schema", We are not allowed to create
"temporary table", whereas we can CREATE/DROP "Global Temporary
Table", is it expected ?


It is ok for me. temporary tables should be created only in 
proprietary schema. For GTT there is not risk of collision, so it can 
be created in any schema where are necessary access rights.


Pavel


postgres=# create temporary table information_schema.temp1(c1 int);
ERROR:  cannot create temporary relation in non-temporary schema
LINE 1: create temporary table information_schema.temp1(c1 int);
                               ^

postgres=# create global temporary table
information_schema.temp1(c1 int);
CREATE TABLE

postgres=# drop table information_schema.temp1 ;
DROP TABLE

_-- Scenario 2:_
Here I am getting the same error message in both the below cases.
We may add a "global" keyword with GTT related error message.

postgres=# create global temporary table gtt1 (c1 int unique);
CREATE TABLE
postgres=# create temporary table tmp1 (c1 int unique);
CREATE TABLE

postgres=# create temporary table tmp2 (c1 int references gtt1(c1) );
ERROR:  constraints on temporary tables may reference only
temporary tables

postgres=# create global temporary table gtt2 (c1 int references
tmp1(c1) );
ERROR:  constraints on temporary tables may reference only
temporary tables

Thanks,
Prabhat Sahu

On Tue, Feb 25, 2020 at 2:25 PM 曾文旌(义从)
mailto:wenjing@alibaba-inc.com>>
wrote:




2020年2月24日 下午5:44，Prabhat Sahu
mailto:prabhat.s...@enterprisedb.com>> 写道：

On Fri, Feb 21, 2020 at 9:10 PM 曾文旌(义从)
mailto:wenjing@alibaba-inc.com>> wrote:

Hi,
I have started testing the "Global temporary table" feature,
That's great, I see hope.
from "gtt_v11-pg13.patch". Below is my findings:

-- session 1:
postgres=# create global temporary table gtt1(a int);
CREATE TABLE

-- seeeion 2:
postgres=# truncate gtt1 ;
ERROR:  could not open file "base/13585/t3_16384": No
such file or directory

is it expected?

Oh ,this is a bug, I fixed it.

Thanks for the patch.
I have verified the same, Now the issue is resolved with v12
patch.

Kindly confirm the below scenario:

postgres=# create global temporary table gtt1 (c1 int unique);
CREATE TABLE

postgres=# create global temporary table gtt2 (c1 int
references gtt1(c1) );
ERROR:  referenced relation "gtt1" is not a global temp table

postgres=# create table tab2 (c1 int references gtt1(c1) );
ERROR:  referenced relation "gtt1" is not a global temp table

Thanks,
Prabhat Sahu


GTT supports foreign key constraints
in global_temporary_table_v13-pg13.patch


Wenjing





-- 


With Regards,
Prabhat Kumar Sahu
EnterpriseDB: http://www.enterprisedb.com




--
regards,tushar
EnterpriseDB  https://www.enterprisedb.com/
The Enterprise PostgreSQL Company

Re: pg_trigger.tgparentid

[Amit Langote]

On Tue, Feb 25, 2020 at 11:01 PM Alvaro Herrera
 wrote:
> On 2020-Feb-25, Amit Langote wrote:
> > On Tue, Feb 25, 2020 at 3:58 AM Alvaro Herrera  
> > wrote:
> > > Thanks for pointing this out -- I agree it needed rewording.  I slightly
> > > adjusted your text like this:
> > >
> > >  * Internal triggers require careful examination.  
> > > Ideally, we don't
> > >  * clone them.  However, if our parent is itself a 
> > > partition, there
> > >  * might be internal triggers that must not be skipped; 
> > > for example,
> > >  * triggers on our parent that are in turn clones from 
> > > its parent (our
> > >  * grandparent) are marked internal, yet they are to be 
> > > cloned.
> > >
> > > Is this okay for you?
> >
> > Thanks.  Your revised text looks good, except there is a typo:
> >
> > in turn clones -> in turn cloned
>
> Actually, that was on purpose ...  (I also changed "were" to "are" to match.)

Ah, got it.

Thanks,
Amit

Re: [Patch] Base backups and random or zero pageheaders

[Asif Rehman]

On Fri, Oct 18, 2019 at 2:06 PM Michael Banck 
wrote:

> Hi,
>
> Am Samstag, den 04.05.2019, 21:50 +0900 schrieb Michael Paquier:
> > On Tue, Apr 30, 2019 at 03:07:43PM +0200, Michael Banck wrote:
> > > This is still an open item for the back branches I guess, i.e. zero
> page
> > > header for pg_verify_checksums and additionally random page header for
> > > pg_basebackup's base backup.
> >
> > I may be missing something, but could you add an entry in the future
> > commit fest about the stuff discussed here?  I have not looked at your
> > patch closely..  Sorry.
>
> Here is finally a rebased patch for the (IMO) more important issue in
> pg_basebackup. I've added a commitfest entry for this now:
> https://commitfest.postgresql.org/25/2308/
>
>
>
Hi Michael,

The patch does not seem to apply anymore, can you rebase it?

--
Asif Rehman
Highgo Software (Canada/China/Pakistan)
URL : www.highgo.ca

Re: WIP/PoC for parallel backup

[Asif Rehman]

Hi,

I have created a commitfest entry.
https://commitfest.postgresql.org/27/2472/


On Mon, Feb 17, 2020 at 1:39 PM Asif Rehman  wrote:

> Thanks Jeevan. Here is the documentation patch.
>
> On Mon, Feb 10, 2020 at 6:49 PM Jeevan Chalke <
> jeevan.cha...@enterprisedb.com> wrote:
>
>> Hi Asif,
>>
>> On Thu, Jan 30, 2020 at 7:10 PM Asif Rehman 
>> wrote:
>>
>>>
>>> Here are the the updated patches, taking care of the issues pointed
>>> earlier. This patch adds the following commands (with specified option):
>>>
>>> START_BACKUP [LABEL ''] [FAST]
>>> STOP_BACKUP [NOWAIT]
>>> LIST_TABLESPACES [PROGRESS]
>>> LIST_FILES [TABLESPACE]
>>> LIST_WAL_FILES [START_WAL_LOCATION 'X/X'] [END_WAL_LOCATION 'X/X']
>>> SEND_FILES '(' FILE, FILE... ')' [START_WAL_LOCATION 'X/X']
>>> [NOVERIFY_CHECKSUMS]
>>>
>>>
>>> Parallel backup is not making any use of tablespace map, so I have
>>> removed that option from the above commands. There is a patch pending
>>> to remove the exclusive backup; we can further refactor the
>>> do_pg_start_backup
>>> function at that time, to remove the tablespace information and move the
>>> creation of tablespace_map file to the client.
>>>
>>>
>>> I have disabled the maxrate option for parallel backup. I intend to send
>>> out a separate patch for it. Robert previously suggested to implement
>>> throttling on the client-side. I found the original email thread [1]
>>> where throttling was proposed and added to the server. In that thread,
>>> it was originally implemented on the client-side, but per many
>>> suggestions,
>>> it was moved to server-side.
>>>
>>> So, I have a few suggestions on how we can implement this:
>>>
>>> 1- have another option for pg_basebackup (i.e. per-worker-maxrate) where
>>> the user could choose the bandwidth allocation for each worker. This
>>> approach
>>> can be implemented on the client-side as well as on the server-side.
>>>
>>> 2- have the maxrate, be divided among workers equally at first. and the
>>> let the main thread keep adjusting it whenever one of the workers
>>> finishes.
>>> I believe this would only be possible if we handle throttling on the
>>> client.
>>> Also, as I understand it, implementing this will introduce additional
>>> mutex
>>> for handling of bandwidth consumption data so that rate may be adjusted
>>> according to data received by threads.
>>>
>>> [1]
>>> https://www.postgresql.org/message-id/flat/521B4B29.20009%402ndquadrant.com#189bf840c87de5908c0b4467d31b50af
>>>
>>> --
>>> Asif Rehman
>>> Highgo Software (Canada/China/Pakistan)
>>> URL : www.highgo.ca
>>>
>>>
>>
>> The latest changes look good to me. However, the patch set is missing the
>> documentation.
>> Please add those.
>>
>> Thanks
>>
>> --
>> Jeevan Chalke
>> Associate Database Architect & Team Lead, Product Development
>> EnterpriseDB Corporation
>> The Enterprise PostgreSQL Company
>>
>>
>
> --
> --
> Asif Rehman
> Highgo Software (Canada/China/Pakistan)
> URL : www.highgo.ca
>
>

-- 
--
Asif Rehman
Highgo Software (Canada/China/Pakistan)
URL : www.highgo.ca

Re: pg_trigger.tgparentid

[Alvaro Herrera]

On 2020-Feb-25, Amit Langote wrote:

> On Tue, Feb 25, 2020 at 3:58 AM Alvaro Herrera  
> wrote:

> > Thanks for pointing this out -- I agree it needed rewording.  I slightly
> > adjusted your text like this:
> >
> >  * Internal triggers require careful examination.  Ideally, 
> > we don't
> >  * clone them.  However, if our parent is itself a 
> > partition, there
> >  * might be internal triggers that must not be skipped; for 
> > example,
> >  * triggers on our parent that are in turn clones from its 
> > parent (our
> >  * grandparent) are marked internal, yet they are to be 
> > cloned.
> >
> > Is this okay for you?
> 
> Thanks.  Your revised text looks good, except there is a typo:
> 
> in turn clones -> in turn cloned

Actually, that was on purpose ...  (I also changed "were" to "are" to match.)

-- 
Álvaro Herrerahttps://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Re: [Proposal] Global temporary tables

[tushar]

Hi ,

pg_upgrade  scenario is failing if database is containing  global 
temporary table


=
centos@tushar-ldap-docker bin]$ ./psql postgres
psql (13devel)
Type "help" for help.

postgres=# create global temporary table  t(n int);
CREATE TABLE
postgres=# \q
===

run pg_upgrade -

[centos@tushar-ldap-docker bin]$ ./pg_upgrade -d /tmp/t1/ -D /tmp/t2 -b 
. -B .

Performing Consistency Checks
-
Checking cluster versions    ok
Checking database user is the install user   ok
Checking database connection settings   ok
Checking for prepared transactions ok
Checking for reg* data types in user tables ok
--
--
If pg_upgrade fails after this point, you must re-initdb the
new cluster before continuing.

Performing Upgrade
--
Analyzing all rows in the new cluster    ok
Freezing all rows in the new cluster  ok
Deleting files from new pg_xact    ok
--
--
Restoring database schemas in the new cluster
ok
Copying user relation files
  /tmp/t1/base/13585/16384
error while copying relation "public.t": could not open file 
"/tmp/t1/base/13585/16384": No such file or directory

Failure, exiting

regards,

On 2/25/20 7:06 PM, Prabhat Sahu wrote:

Hi All,

Please check the below findings on GTT.
_-- Scenario 1:_
Under "information_schema", We are not allowed to create "temporary 
table", whereas we can CREATE/DROP "Global Temporary Table", is it 
expected ?


postgres=# create temporary table information_schema.temp1(c1 int);
ERROR:  cannot create temporary relation in non-temporary schema
LINE 1: create temporary table information_schema.temp1(c1 int);
                               ^

postgres=# create global temporary table information_schema.temp1(c1 int);
CREATE TABLE

postgres=# drop table information_schema.temp1 ;
DROP TABLE

_-- Scenario 2:_
Here I am getting the same error message in both the below cases.
We may add a "global" keyword with GTT related error message.

postgres=# create global temporary table gtt1 (c1 int unique);
CREATE TABLE
postgres=# create temporary table tmp1 (c1 int unique);
CREATE TABLE

postgres=# create temporary table tmp2 (c1 int references gtt1(c1) );
ERROR:  constraints on temporary tables may reference only temporary 
tables


postgres=# create global temporary table gtt2 (c1 int references 
tmp1(c1) );
ERROR:  constraints on temporary tables may reference only temporary 
tables


Thanks,
Prabhat Sahu

On Tue, Feb 25, 2020 at 2:25 PM 曾文旌(义从) > wrote:





2020年2月24日 下午5:44，Prabhat Sahu mailto:prabhat.s...@enterprisedb.com>> 写道：

On Fri, Feb 21, 2020 at 9:10 PM 曾文旌(义从)
mailto:wenjing@alibaba-inc.com>> wrote:

Hi,
I have started testing the "Global temporary table" feature,
That's great, I see hope.
from "gtt_v11-pg13.patch". Below is my findings:

-- session 1:
postgres=# create global temporary table gtt1(a int);
CREATE TABLE

-- seeeion 2:
postgres=# truncate gtt1 ;
ERROR:  could not open file "base/13585/t3_16384": No such
file or directory

is it expected?

Oh ,this is a bug, I fixed it.

Thanks for the patch.
I have verified the same, Now the issue is resolved with v12 patch.

Kindly confirm the below scenario:

postgres=# create global temporary table gtt1 (c1 int unique);
CREATE TABLE

postgres=# create global temporary table gtt2 (c1 int references
gtt1(c1) );
ERROR:  referenced relation "gtt1" is not a global temp table

postgres=# create table tab2 (c1 int references gtt1(c1) );
ERROR:  referenced relation "gtt1" is not a global temp table

Thanks,
Prabhat Sahu


GTT supports foreign key constraints
in global_temporary_table_v13-pg13.patch


Wenjing





--

With Regards,
Prabhat Kumar Sahu
EnterpriseDB: http://www.enterprisedb.com 



--
regards,tushar
EnterpriseDB  https://www.enterprisedb.com/
The Enterprise PostgreSQL Company

Re: [Proposal] Global temporary tables

[Pavel Stehule]

út 25. 2. 2020 v 14:36 odesílatel Prabhat Sahu <
prabhat.s...@enterprisedb.com> napsal:

> Hi All,
>
> Please check the below findings on GTT.
> *-- Scenario 1:*
> Under "information_schema", We are not allowed to create "temporary
> table", whereas we can CREATE/DROP "Global Temporary Table", is it expected
> ?
>

It is ok for me. temporary tables should be created only in proprietary
schema. For GTT there is not risk of collision, so it can be created in any
schema where are necessary access rights.

Pavel


> postgres=# create temporary table information_schema.temp1(c1 int);
> ERROR:  cannot create temporary relation in non-temporary schema
> LINE 1: create temporary table information_schema.temp1(c1 int);
>^
>
> postgres=# create global temporary table information_schema.temp1(c1 int);
> CREATE TABLE
>
> postgres=# drop table information_schema.temp1 ;
> DROP TABLE
>
> *-- Scenario 2:*
> Here I am getting the same error message in both the below cases.
> We may add a "global" keyword with GTT related error message.
>
> postgres=# create global temporary table gtt1 (c1 int unique);
> CREATE TABLE
> postgres=# create temporary table tmp1 (c1 int unique);
> CREATE TABLE
>
> postgres=# create temporary table tmp2 (c1 int references gtt1(c1) );
> ERROR:  constraints on temporary tables may reference only temporary tables
>
> postgres=# create global temporary table gtt2 (c1 int references tmp1(c1)
> );
> ERROR:  constraints on temporary tables may reference only temporary tables
>
> Thanks,
> Prabhat Sahu
>
> On Tue, Feb 25, 2020 at 2:25 PM 曾文旌(义从) 
> wrote:
>
>>
>>
>> 2020年2月24日 下午5:44，Prabhat Sahu  写道：
>>
>> On Fri, Feb 21, 2020 at 9:10 PM 曾文旌(义从) 
>> wrote:
>>
>>> Hi,
>>> I have started testing the "Global temporary table" feature,
>>> That's great, I see hope.
>>> from "gtt_v11-pg13.patch". Below is my findings:
>>>
>>> -- session 1:
>>> postgres=# create global temporary table gtt1(a int);
>>> CREATE TABLE
>>>
>>> -- seeeion 2:
>>> postgres=# truncate gtt1 ;
>>> ERROR:  could not open file "base/13585/t3_16384": No such file or
>>> directory
>>>
>>> is it expected?
>>>
>>> Oh ,this is a bug, I fixed it.
>>>
>> Thanks for the patch.
>> I have verified the same, Now the issue is resolved with v12 patch.
>>
>> Kindly confirm the below scenario:
>>
>> postgres=# create global temporary table gtt1 (c1 int unique);
>> CREATE TABLE
>>
>> postgres=# create global temporary table gtt2 (c1 int references gtt1(c1)
>> );
>> ERROR:  referenced relation "gtt1" is not a global temp table
>>
>> postgres=# create table tab2 (c1 int references gtt1(c1) );
>> ERROR:  referenced relation "gtt1" is not a global temp table
>>
>> Thanks,
>> Prabhat Sahu
>>
>>
>> GTT supports foreign key constraints
>> in global_temporary_table_v13-pg13.patch
>>
>>
>> Wenjing
>>
>>
>>
>>
>
> --
>
> With Regards,
> Prabhat Kumar Sahu
> EnterpriseDB: http://www.enterprisedb.com
>

Re: [Proposal] Global temporary tables

[Prabhat Sahu]

Hi All,

Please check the below findings on GTT.
*-- Scenario 1:*
Under "information_schema", We are not allowed to create "temporary table",
whereas we can CREATE/DROP "Global Temporary Table", is it expected ?

postgres=# create temporary table information_schema.temp1(c1 int);
ERROR:  cannot create temporary relation in non-temporary schema
LINE 1: create temporary table information_schema.temp1(c1 int);
   ^

postgres=# create global temporary table information_schema.temp1(c1 int);
CREATE TABLE

postgres=# drop table information_schema.temp1 ;
DROP TABLE

*-- Scenario 2:*
Here I am getting the same error message in both the below cases.
We may add a "global" keyword with GTT related error message.

postgres=# create global temporary table gtt1 (c1 int unique);
CREATE TABLE
postgres=# create temporary table tmp1 (c1 int unique);
CREATE TABLE

postgres=# create temporary table tmp2 (c1 int references gtt1(c1) );
ERROR:  constraints on temporary tables may reference only temporary tables

postgres=# create global temporary table gtt2 (c1 int references tmp1(c1) );
ERROR:  constraints on temporary tables may reference only temporary tables

Thanks,
Prabhat Sahu

On Tue, Feb 25, 2020 at 2:25 PM 曾文旌(义从)  wrote:

>
>
> 2020年2月24日 下午5:44，Prabhat Sahu  写道：
>
> On Fri, Feb 21, 2020 at 9:10 PM 曾文旌(义从) 
> wrote:
>
>> Hi,
>> I have started testing the "Global temporary table" feature,
>> That's great, I see hope.
>> from "gtt_v11-pg13.patch". Below is my findings:
>>
>> -- session 1:
>> postgres=# create global temporary table gtt1(a int);
>> CREATE TABLE
>>
>> -- seeeion 2:
>> postgres=# truncate gtt1 ;
>> ERROR:  could not open file "base/13585/t3_16384": No such file or
>> directory
>>
>> is it expected?
>>
>> Oh ,this is a bug, I fixed it.
>>
> Thanks for the patch.
> I have verified the same, Now the issue is resolved with v12 patch.
>
> Kindly confirm the below scenario:
>
> postgres=# create global temporary table gtt1 (c1 int unique);
> CREATE TABLE
>
> postgres=# create global temporary table gtt2 (c1 int references gtt1(c1)
> );
> ERROR:  referenced relation "gtt1" is not a global temp table
>
> postgres=# create table tab2 (c1 int references gtt1(c1) );
> ERROR:  referenced relation "gtt1" is not a global temp table
>
> Thanks,
> Prabhat Sahu
>
>
> GTT supports foreign key constraints
> in global_temporary_table_v13-pg13.patch
>
>
> Wenjing
>
>
>
>

-- 

With Regards,
Prabhat Kumar Sahu
EnterpriseDB: http://www.enterprisedb.com

Re: ALTER TABLE ... SET STORAGE does not propagate to indexes

[Kuntal Ghosh]

On Tue, Feb 25, 2020 at 1:09 PM Peter Eisentraut
 wrote:
>
> On 2020-02-24 12:21, Kuntal Ghosh wrote:
> > I've reproduced the issue on head. And, the patch seems to solve the
> > problem. The patch looks good to me. But, I've a small doubt regarding
> > the changes in test_decoding regression file.
> >
> > diff --git a/contrib/test_decoding/sql/toast.sql
> > b/contrib/test_decoding/sql/toast.sql
> > ..
> > -INSERT INTO toasted_several(toasted_key) VALUES(repeat('9876543210', 
> > 1));
> > -SELECT pg_column_size(toasted_key) > 2^16 FROM toasted_several;
> > +INSERT INTO toasted_several(toasted_key) VALUES(repeat('9876543210', 269));
> > +SELECT pg_column_size(toasted_key) > 2^11 FROM toasted_several;
> >
> > This actually tests whether we can decode "old" tuples bigger than the
> > max heap tuple size correctly which is around 8KB. But, the above
> > changes will make the tuple size around 3KB. So, it'll not be able to
> > test that particular scenario.Thoughts?
>
> OK, this is interesting.  The details of this are somewhat unfamiliar to
> me, but it appears that due to TOAST_INDEX_HACK in indextuple.c, an
> index tuple cannot be larger than 8191 bytes when untoasted (but not
> uncompressed).
>
> What the test case above is testing is a situation where the heap tuple
> is stored toasted uncompressed (storage external) but the index tuple is
> not (probably compressed inline).  This is exactly the situation that I
> was contending should not be possible, because it cannot be dumped or
> restored.
>
Yeah. If we only commit this patch to fix the issue, we're going to
put some restriction for the above situation, i.e., the index for an
external attribute has to be stored as an external (i.e. uncompressed)
value. So, a lot of existing workload might start failing after an
upgrade. I think there should be an option to store the index of an
external attribute as a compressed inline value.

> An alternative would be that we make this situation fully supported.
> Then we'd probably need at least ALTER INDEX ... ALTER COLUMN ... SET
> STORAGE, and some pg_dump support.
>
> Thoughts?
Yes. We need the support for this syntax along with the bug fix patch.

-- 
Thanks & Regards,
Kuntal Ghosh
EnterpriseDB: http://www.enterprisedb.com

Re: RS_EPHEMERAL vs RS_TEMPORARY

[Antonin Houska]

Peter Eisentraut  wrote:

> On 2020-02-25 08:30, Antonin Houska wrote:
> > I'm trying to figure out what's specific about RS_EPHEMERAL and RS_TEMPORARY
> > slot kinds. The following comment (see definition of the
> > ReplicationSlotPersistency enumeration) tells when each kind is dropped
> 
> The general idea is that an "ephemeral" slot is a future persistent slot that
> is not completely initialized yet.  If there is a crash and you find an
> ephemeral slot, you can clean it up.  The name is perhaps a bit odd, you can
> think of it as an uninitialized one.  A temporary slot is one that behaves
> like a temporary table: It is removed at the end of a session.
> 
> Perhaps the implementation differences are not big or are none, but it's
> relevant for reporting.  For example, the pg_replication_slots view shows
> which slots are temporary.  You wouldn't want to show an ephemeral slot as
> temporary.

ok, so only comments seem to be the problem.

Anyway, the reason I started to think about it was that I noticed an Assert()
statement I considered inaccurate. Does this patch make sense?

-- 
Antonin Houska
Web: https://www.cybertec-postgresql.com

@@ -682,7 +682,7 @@ ReplicationSlotPersist(void)
ReplicationSlot *slot = MyReplicationSlot;
 
Assert(slot != NULL);
-   Assert(slot->data.persistency != RS_PERSISTENT);
+   Assert(slot->data.persistency == RS_EPHEMERAL);
 
SpinLockAcquire(>mutex);
slot->data.persistency = RS_PERSISTENT;

Re: Error on failed COMMIT

[Laurenz Albe]

On Tue, 2020-02-25 at 13:25 +0530, Robert Haas wrote:
> On Tue, Feb 25, 2020 at 12:47 PM Vladimir Sitnikov
>  wrote:
> > Noone suggested that "commit leaves the session in a transaction state".
> > Of course, every commit should terminate the transaction.
> > However, if a commit fails (for any reason), it should produce the relevant 
> > ERROR that explains what went wrong rather than silently doing a rollback.
> 
> OK, I guess I misinterpreted the proposal. That would be much less
> problematic -- any driver or application that can't handle ERROR in
> response to an attempted COMMIT would be broken already.

I agree with that.

There is always some chance that someone relies on COMMIT not
throwing an error when it rolls back, but I think that throwing an
error is actually less astonishing than *not* throwing one.

So, +1 for the proposal from me.

Yours,
Laurenz Albe

Re: [Patch] pg_rewind: options to use restore_command from recovery.conf or command line

[Alexander Korotkov]

On Sat, Feb 1, 2020 at 2:16 AM Alexey Kondratov
 wrote:
> New version is attached. Do you have any other comments or objections?

Now patch looks much better.  Thanks to Michael Paquier for review.
I've just revised commit message reflecting we've removed one of the
new options.

But I have following question.

+   # Move all old master WAL files to the archive.
+   RecursiveCopy::copypath(
+   $node_master->data_dir . "/pg_wal",
+   $node_master->archive_dir
+   );
+   chmod(0700, $node_master->archive_dir);
+
+   # Fast way to remove entire directory content
+   rmtree($node_master->data_dir . "/pg_wal");
+   mkdir($node_master->data_dir . "/pg_wal");
+   chmod(0700, $node_master->data_dir . "/pg_wal");

I think usage of chmod() deserves comment.  As I get default
permissions are sufficient for work, but we need to set them to
satisfy 'check PGDATA permissions' test.

--
Alexander Korotkov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company


v15-0001-pg_rewind-Add-options-to-restore-WAL-files.patch
Description: Binary data

Re: Yet another vectorized engine

[Konstantin Knizhnik]

On 25.02.2020 11:06, Hubert Zhang wrote:

Hi Konstantin,

I checkout your branch pg13 in repo 
https://github.com/zhangh43/vectorize_engine

After I fixed some compile error, I tested Q1 on TPCH-10G
The result is different from yours and vectorize version is too slow. 
Note that I disable parallel worker by default.

no JIT no Vectorize:  36 secs
with JIT only:             23 secs
with Vectorize only:   33 secs
JIT + Vectorize:         29 secs

My config option is `CFLAGS='-O3 -g -march=native' 
--prefix=/usr/local/pgsql/ --disable-cassert --enable-debug --with-llvm`
I will do some spike on why vectorized is so slow. Could you please 
provide your compile option and the TPCH dataset size and your 
queries(standard Q1?) to help me to debug on it.





Hi, Hubert

Sorry, looks like I have used slightly deteriorated snapshot of master 
so I have not noticed some problems.

Fixes are committed.

Most of the time is spent in unpacking heap tuple 
(tts_buffer_heap_getsomeattrs):


  24.66%  postgres  postgres [.] tts_buffer_heap_getsomeattrs
   8.28%  postgres  vectorize_engine.so  [.] VExecStoreColumns
   5.94%  postgres  postgres [.] HeapTupleSatisfiesVisibility
   4.21%  postgres  postgres [.] bpchareq
   4.12%  postgres  vectorize_engine.so  [.] vfloat8_accum


In my version of nodeSeqscan I do not keep all fetched 1024 heap tuples 
but stored there attribute values in vector columns immediately.
But to avoid extraction of useless data it is necessary to know list of 
used columns.
The same problem is solved in zedstore, but unfortunately there is no 
existed method in Postgres to get list
of used attributes. I have done it but my last implementation contains 
error which cause loading of all columns.

Fixed version is committed.

Now profile without JIT is:

 15.52%  postgres  postgres [.] tts_buffer_heap_getsomeattrs
  10.25%  postgres  postgres [.] ExecInterpExpr
   6.54%  postgres  postgres [.] HeapTupleSatisfiesVisibility
   5.12%  postgres  vectorize_engine.so  [.] VExecStoreColumns
   4.86%  postgres  postgres [.] bpchareq
   4.80%  postgres  vectorize_engine.so  [.] vfloat8_accum
   3.78%  postgres  postgres [.] tts_minimal_getsomeattrs
   3.66%  postgres  vectorize_engine.so  [.] VExecAgg
   3.38%  postgres  postgres [.] hashbpchar

and with JIT:

 13.88%  postgres  postgres [.] tts_buffer_heap_getsomeattrs
   7.15%  postgres  vectorize_engine.so  [.] vfloat8_accum
   6.03%  postgres  postgres [.] HeapTupleSatisfiesVisibility
   5.55%  postgres  postgres [.] bpchareq
   4.42%  postgres  vectorize_engine.so  [.] VExecStoreColumns
   4.19%  postgres  postgres [.] hashbpchar
   4.09%  postgres  vectorize_engine.so  [.] vfloat8pl

On Mon, Feb 24, 2020 at 8:43 PM Hubert Zhang > wrote:


Hi Konstantin,
I have added you as a collaborator on github. Please accepted and
try again.
I think non collaborator could also open pull requests.

On Mon, Feb 24, 2020 at 8:02 PM Konstantin Knizhnik
mailto:k.knizh...@postgrespro.ru>> wrote:



On 24.02.2020 05:08, Hubert Zhang wrote:

Hi

On Sat, Feb 22, 2020 at 12:58 AM Konstantin Knizhnik
mailto:k.knizh...@postgrespro.ru>> wrote:



On 12.02.2020 13:12, Hubert Zhang wrote:

On Tue, Feb 11, 2020 at 1:20 AM Konstantin Knizhnik
mailto:k.knizh...@postgrespro.ru>> wrote:


So looks like PG-13 provides significant advantages
in OLAP queries comparing with 9.6!
Definitely it doesn't mean that vectorized executor
is not needed for new version of Postgres.
Once been ported, I expect that it should provide
comparable improvement of performance.

But in any case I think that vectorized executor
makes sense only been combine with columnar store.


Thanks for the test. +1 on vectorize should be combine
with columnar store. I think when we support this extension
on master, we could try the new zedstore.
I'm not active on this work now, but will continue when
I have time. Feel free to join bring vops's feature into
this extension.
Thanks

Hubert Zhang


I have ported vectorize_engine to the master.
It takes longer than I expected: a lot of things were
changed in executor.

Results are the following:


par.warkers
PG9_6
vectorize=off
PG9_6
vectorize=on
master
vectorize=off
jit=on
master
vectorize=off
jit=off master
vectorize=on
jit=ofn master
vectorize=on
 

Re: SPI Concurrency Precautions? Problems with Parallel Execution of Multiple CREATE TABLE statements

[Robert Haas]

On Sat, Feb 22, 2020 at 5:50 AM Tom Mercha  wrote:
> I am spawning a number of dynamic background workers to execute each
> statement. When I spawn 4 workers on a quad-core machine, the resutling
> execution time per statement is {0.158s, 0.216s, 0.399s, 0.396s}.
> However, when I have just one worker, the results are {0.151s, 0.141s,
> 0.146s, 0.136s}.
>
> The way I am executing my statements is through SPI in each worker
> (using a PG extension) as follows:
>   SPI_connect();
>   SPI_exec(queryString, 0);
>   SPI_finish();
> In both test cases, SPI_connect/finish are executed 4 times.
>
> What I expect is that with 4 workers, each statements will take approx
> 0.15s to execute since they are independent from each other. This would
> result in approx a 4x speedup. Despite seeing concurrency, I am seeing
> that each invdividual statement will take longer to execute. I am
> struggling to understand this behavior, what this suggests to me is that
> there is a lock somewhere which completely defeats my purpose.
>
> I was wondering how I could execute my CREATE TABLE statements in a
> parallel fashion given that they are independent from each other. If the
> lock is the problem, what steps could I take to relax it? I would
> greatly appreciate any guidance or insights on this topic.

Well, I'm not altogether sure that your expectations are realistic.
Rarely do things parallelize perfectly. In a case like this, some time
is probably being spent doing disk I/O. When multiple processes do CPU
work at the same time, you should be able to see near-linear speedup,
but when multiple processes do disk I/O at the same time, you may see
no speedup at all, or even a slowdown, because of the way that disks
work. This seems especially likely given how short the queries are and
the fact that they create a new table, which involves an fsync()
operation.

It's possible that if you run a query to select the wait events from
pg_stat_activity, maybe using psql's \watch with a fractional value,
you might be able to see something about what those queries are
actually spending time on. It's also possible that you might get more
interesting results if you have things that run for longer than a few
hundred milliseconds. But in general I would question the assumption
that this ought to scale well.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Re: [Proposal] Add accumulated statistics for wait event

[王胜利]

Thank you for this update! I will try it.

Best regards,
Victor wang


| |
王胜利
|
|
邮箱：atti...@126.com
|

签名由 网易邮箱大师 定制

On 02/25/2020 15:53, imai.yoshik...@fujitsu.com wrote:
On Fri, Feb 14, 2020 at 11:59 AM, 王胜利 wrote:
>I am glad to know you are working on PG accumulated statistics 
> feature, and I am interested on it.
>I see these two patch file you made, can you let me know which branch 
> of PG code based?
>
>when I use this:  https://github.com/postgres/postgres/commits/master, 
> and apply these patches, report some error.

Thanks for Wang's mail, I noticed my 0002 patch was wrong from v3.

Here, I attach correct patches.

Also I will begin to do some benchmark with higher scale and higher number of
users and try to change stats reporting implementation to not affect
performance, which I couldn't have not started because of another tasks.

--
Yoshikazu Imai

Re: progress reporting views and TimestampTz fields

[Vik Fearing]

On 25/02/2020 09:13, Fujii Masao wrote:
> Hi,
> 
> I'm thinking to change the progress reporting views like
> pg_stat_progress_vacuum so that they also report the time when
> the target command was started and the time when the phase was
> last changed. IMO this is very helpful to estimate the remaining
> time required to complete the current phase. For example,
> if pg_stat_progress_vacuum reports that the current phase
> "scanning heap" started 1 hour before and the progress percentage
> is 50%, we can imagine the remaining time of this phase would be
> approximately 1 hour. Of course, this is not the exact estimation,
> but would be helpful as a hint for operations. Thought?
> 
> ProgressCommandType st_progress_command;
> Oid    st_progress_command_target;
> int64    st_progress_param[PGSTAT_NUM_PROGRESS_PARAM];
> 
> We cannnot add those timestamp fields simply in the progress
> reporting views because the type of the fields in PgBackendStatus
> struct is only int64 for now, as the above. So I'm thinking to add
> new TimestampTz fields (maybe four fields are enough even for
> future usager?) into PgBackendStatus and make pg_stat_get_progress_info()
> report those fields as timestamp. This change leads to increase
> in the size of PgBackendStatus, as demerit. But I like this approach
> because it's simple and intuitive.
> 
> Another idea is to store TimestampTz values in int64 fields
> (for example, always store TimestampTz values in the last two int64
> fields) and make pg_stat_get_progress_info() report not only int64
> but also those TimestampTz fields. This approach doesn't increase
> the struct size, but is a bit tricky. Also int64 fields that TimestampTz
> values will be stored into might be already used to store int64 values
> in some existing extensions. If we need to handle this case, further
> tricky way might need to be implemented. That sounds not good.
> 
> Therefore, I'd like to implement the first idea that I described, to
> add the timestamp fields in the progress reporting view. Thought?

+1 on the idea.  No opinion on the implementation.
-- 
Vik Fearing

Re: Error on failed COMMIT

[Vladimir Sitnikov]

Tom>I think we still end up concluding that altering this behavior has more
Tom>downside than upside.

What is the downside?

Applications, drivers, and poolers already expect that commit might produce
an error and terminate the transaction at the same time.

"The data is successfully committed to the database if and only if commit
returns without error".
^^^ the above is way easier to reason about than "user must check multiple
unrelated outcomes to tell if the changes are committed or not".

Vladimir

Re: plan cache overhead on plpgsql expression

[Amit Langote]

Hi Pavel,

On Tue, Feb 25, 2020 at 4:16 PM Pavel Stehule  wrote:
>
> Hi
>
> I added this patch to a commitfest
>
> https://commitfest.postgresql.org/27/2467/
>
> It is very interesting speedup and it is in good direction to JIT expressions

Thank you.  I was planning to do that myself.

I will take a look at your other comments in a day or two.

Thanks,
Amit

Define variables in the approprieate scope

[Antonin Houska]

I've noticed that two variables in RelationCopyStorage() are defined in a
scope higher than necessary. Please see the patch.

-- 
Antonin Houska
Web: https://www.cybertec-postgresql.com

diff --git a/src/backend/catalog/storage.c b/src/backend/catalog/storage.c
index fddfbf1d8c..14d170823f 100644
--- a/src/backend/catalog/storage.c
+++ b/src/backend/catalog/storage.c
@@ -336,15 +336,11 @@ void
 RelationCopyStorage(SMgrRelation src, SMgrRelation dst,
 	ForkNumber forkNum, char relpersistence)
 {
-	PGAlignedBlock buf;
-	Page		page;
 	bool		use_wal;
 	bool		copying_initfork;
 	BlockNumber nblocks;
 	BlockNumber blkno;
 
-	page = (Page) buf.data;
-
 	/*
 	 * The init fork for an unlogged relation in many respects has to be
 	 * treated the same as normal relation, changes need to be WAL logged and
@@ -364,6 +360,9 @@ RelationCopyStorage(SMgrRelation src, SMgrRelation dst,
 
 	for (blkno = 0; blkno < nblocks; blkno++)
 	{
+		PGAlignedBlock buf;
+		Page	page = (Page) buf.data;
+
 		/* If we got a cancel signal during the copy of the data, quit */
 		CHECK_FOR_INTERRUPTS();
 

progress reporting views and TimestampTz fields

[Fujii Masao]

Hi,

I'm thinking to change the progress reporting views like
pg_stat_progress_vacuum so that they also report the time when
the target command was started and the time when the phase was
last changed. IMO this is very helpful to estimate the remaining
time required to complete the current phase. For example,
if pg_stat_progress_vacuum reports that the current phase
"scanning heap" started 1 hour before and the progress percentage
is 50%, we can imagine the remaining time of this phase would be
approximately 1 hour. Of course, this is not the exact estimation,
but would be helpful as a hint for operations. Thought?

ProgressCommandType st_progress_command;
Oid st_progress_command_target;
int64   st_progress_param[PGSTAT_NUM_PROGRESS_PARAM];

We cannnot add those timestamp fields simply in the progress
reporting views because the type of the fields in PgBackendStatus
struct is only int64 for now, as the above. So I'm thinking to add
new TimestampTz fields (maybe four fields are enough even for
future usager?) into PgBackendStatus and make pg_stat_get_progress_info()
report those fields as timestamp. This change leads to increase
in the size of PgBackendStatus, as demerit. But I like this approach
because it's simple and intuitive.

Another idea is to store TimestampTz values in int64 fields
(for example, always store TimestampTz values in the last two int64
fields) and make pg_stat_get_progress_info() report not only int64
but also those TimestampTz fields. This approach doesn't increase
the struct size, but is a bit tricky. Also int64 fields that TimestampTz
values will be stored into might be already used to store int64 values
in some existing extensions. If we need to handle this case, further
tricky way might need to be implemented. That sounds not good.

Therefore, I'd like to implement the first idea that I described, to
add the timestamp fields in the progress reporting view. Thought?

Regards,

--
Fujii Masao
NTT DATA CORPORATION
Advanced Platform Technology Group
Research and Development Headquarters

Re: Yet another vectorized engine

[Hubert Zhang]

Hi Konstantin,

I checkout your branch pg13 in repo
https://github.com/zhangh43/vectorize_engine
After I fixed some compile error, I tested Q1 on TPCH-10G
The result is different from yours and vectorize version is too slow. Note
that I disable parallel worker by default.
no JIT no Vectorize:  36 secs
with JIT only: 23 secs
with Vectorize only:   33 secs
JIT + Vectorize: 29 secs

My config option is `CFLAGS='-O3 -g -march=native'
--prefix=/usr/local/pgsql/ --disable-cassert --enable-debug --with-llvm`
I will do some spike on why vectorized is so slow. Could you please provide
your compile option and the TPCH dataset size and your queries(standard
Q1?) to help me to debug on it.

On Mon, Feb 24, 2020 at 8:43 PM Hubert Zhang  wrote:

> Hi Konstantin,
> I have added you as a collaborator on github. Please accepted and try
> again.
> I think non collaborator could also open pull requests.
>
> On Mon, Feb 24, 2020 at 8:02 PM Konstantin Knizhnik <
> k.knizh...@postgrespro.ru> wrote:
>
>>
>>
>> On 24.02.2020 05:08, Hubert Zhang wrote:
>>
>> Hi
>>
>> On Sat, Feb 22, 2020 at 12:58 AM Konstantin Knizhnik <
>> k.knizh...@postgrespro.ru> wrote:
>>
>>>
>>>
>>> On 12.02.2020 13:12, Hubert Zhang wrote:
>>>
>>> On Tue, Feb 11, 2020 at 1:20 AM Konstantin Knizhnik <
>>> k.knizh...@postgrespro.ru> wrote:
>>>

 So looks like PG-13 provides significant advantages in OLAP queries
 comparing with 9.6!
 Definitely it doesn't mean that vectorized executor is not needed for
 new version of Postgres.
 Once been ported, I expect that it should provide comparable
 improvement of performance.

 But in any case I think that vectorized executor makes sense only been
 combine with columnar store.

>>>
>>> Thanks for the test. +1 on vectorize should be combine with columnar
>>> store. I think when we support this extension
>>> on master, we could try the new zedstore.
>>> I'm not active on this work now, but will continue when I have time.
>>> Feel free to join bring vops's feature into this extension.
>>>
>>> Thanks
>>>
>>> Hubert Zhang
>>>
>>>
>>> I have ported vectorize_engine to the master.
>>> It takes longer than I expected: a lot of things were changed in
>>> executor.
>>>
>>> Results are the following:
>>>
>>>
>>> par.warkers
>>> PG9_6
>>> vectorize=off
>>> PG9_6
>>> vectorize=on
>>> master
>>> vectorize=off
>>> jit=on
>>> master
>>> vectorize=off
>>> jit=off master
>>> vectorize=on
>>> jit=ofn master
>>> vectorize=on
>>> jit=off
>>> 0
>>> 36
>>> 20
>>> 16
>>> 25.5
>>> 15
>>> 17.5
>>> 4
>>> 10
>>> -
>>> 5 7
>>> -
>>> -
>>>
>>> So it proves the theory that JIT provides almost the same speedup as
>>> vector executor (both eliminates interpretation overhead but in different
>>> way).
>>> I still not sure that we need vectorized executor: because with standard
>>> heap it provides almost no improvements comparing with current JIT version.
>>> But in any case I am going to test it with vertical storage (zedstore or
>>> cstore).
>>>
>>>
>> Thanks for the porting and testing.
>> Yes, PG master and 9.6 have many changes, not only executor, but also
>> tupletableslot interface.
>>
>> What matters the performance of JIT and Vectorization is its
>> implementation. This is just the beginning of vectorization work, just as
>> your vops extension reported, vectorization could run 10 times faster in
>> PG. With the overhead of row storage(heap), we may not reach that speedup,
>> but I think we could do better. Also +1 on vertical storage.
>>
>> BTW, welcome to submit your PR for the PG master version.
>>
>>
>>
>> Sorry, but I have no permissions to push changes to your repository.
>> I can certainly create my own fork of vectorize_engine, but I think it
>> will be beter if I push pg13 branch in your repository.
>>
>>
>>
>
> --
> Thanks
>
> Hubert Zhang
>


-- 
Thanks

Hubert Zhang

Re: RS_EPHEMERAL vs RS_TEMPORARY

[Peter Eisentraut]

On 2020-02-25 08:30, Antonin Houska wrote:

I'm trying to figure out what's specific about RS_EPHEMERAL and RS_TEMPORARY
slot kinds. The following comment (see definition of the
ReplicationSlotPersistency enumeration) tells when each kind is dropped


The general idea is that an "ephemeral" slot is a future persistent slot 
that is not completely initialized yet.  If there is a crash and you 
find an ephemeral slot, you can clean it up.  The name is perhaps a bit 
odd, you can think of it as an uninitialized one.  A temporary slot is 
one that behaves like a temporary table: It is removed at the end of a 
session.


Perhaps the implementation differences are not big or are none, but it's 
relevant for reporting.  For example, the pg_replication_slots view 
shows which slots are temporary.  You wouldn't want to show an ephemeral 
slot as temporary.


--
Peter Eisentraut  http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services