Re: [HACKERS] Parallel Aggregate

[David Rowley]

On 7 March 2016 at 18:19, Haribabu Kommi  wrote:
> Here I attached update patch with further changes,
> 1. Explain plan changes for parallel grouping

Perhaps someone might disagree with me, but I'm not all that sure I
really get the need for that. With nodeAgg.c we're doing something
fundamentally different in Partial mode as we are in Finalize mode,
that's why I wanted to give an indication of that in the explain.c
originally. A query with no Aggregate functions using nodeGroup.c
there is no special handling in the executor for partial and final
stages, so I really don't see why we need to give the impression that
there is in EXPLAIN.

-- 
 David Rowley   http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Support for N synchronous standby servers - take 2

[Masahiko Sawada]

Reply to multiple hackers.
Thank you for reviewing this patch.

> +used.  Priority is given to servers in the order that the appear
> in the list.
>
> s/the appear/they appear/
>
> -The minimum wait time is the roundtrip time between primary to standby.
> +The minimum wait time is the roundtrip time between the primary and the
> +almost synchronous standby.
>
> s/almost/slowest/

Will fix this typo. Thanks!

On Fri, Mar 4, 2016 at 5:22 PM, Kyotaro HORIGUCHI
 wrote:
> Hello,
>
> Sorry for long, hard-to-read writings in advance..
>
> At Thu, 3 Mar 2016 23:30:49 +0900, Masahiko Sawada  
> wrote in 
>> Hi,
>>
>> Thank you so much for reviewing this patch!
>>
>> All review comments regarding document and comment are fixed.
>> Attached latest v14 patch.
>>
>> > This accepts 'abc^Id' as a name, which is wrong behavior (but
>> > such appliction names are not allowed anyway. If you assume so,
>> > I'd like to see a comment for that.).
>>
>> 'abc^Id' is accepted as application_name, no?
>> postgres(1)=# set application_name to 'abc^Id';
>> SET
>> postgres(1)=# show application_name ;
>>  application_name
>> --
>>  abc^Id
>> (1 row)
>
> Sorry, I implicitly used "^" in the meaning of "ctrl key". So
> "^I" is so-called Ctrl-I, that is horizontal tab or 0x09. So the
> following in psql shows that.
>
> =# set application_name to E'abc\td';
> =# show application_name ;
>  application_name
> --
>  ab?d
> (1 row)
>
> The  is replaced with '?' (literally) at the time of
> guc assinment.

Oh, I see.
I will comment for that.

>> > addlit_xd_string(char *ytext) and addlitchar_xd_string(unsigned
>> > char ychar) requires differnt character types. Is there any reason
>> > for that?
>>
>> Because addlit_xd_string() is for adding string(char *) to xd_string,
>> OTOH addlit_xd_char() is for adding just one character to xd_string.
>
> Umm. My qustion might have been a bit out of the point.
>
> The addlitchar_xd_string(str,unsigned char c) does
> appendStringInfoChar(, c). On the other hand, the signature of
> the function of stringinfo is the following.
>
> AppendStringInfoChar(StringInfo str, char ch);
>
> Of course "char" is equivalent of "signed char" as
> default. addlitchar_xd_string assigns the given character in
> "unsigned char" to the parameter of AppendStringInfoChar of
> "signed char".
>
> These two are incompatible types. Imagine the
> following codelet,
>
> #include 
>
> void hoge(signed char c){
>   int ch = c;
>   fprintf(stderr, "char = %d\n", ch);
> }
>
> int main(void)
> {
>   unsigned char u;
>
>   u = 200;
>   hoge(u);
>   return 0;
> }
>
> The result is -56. So we generally should get rid of such type of
> mixture of signedness for no particular reason.
>
> In this case, the domain of the variable is 0x20-0x7e so no
> problem won't be actualized but also there's no reason for the
> signedness mixture.

Thank you for explanation.
I will fix this.

>> > I personally don't like addlit*string() things for such simple
>> > syntax but itself is acceptble enough for me. However it uses
>> > StringInfo to hold double-quoted names, which pallocs 1024 bytes
>> > of memory chunk for every double-quoted name. The chunks are
>> > finally stacked up left uncollected until the current
>> > memorycontext is deleted or reset (It is deleted just after
>> > finishing config file processing). Addition to that, setting
>> > s_s_names runs the parser twice. It seems to me too greedy and
>> > seems that static char [NAMEDATALEN] is enough using the v12 way
>> > without palloc/repalloc.
>>
>> I though that length of group name could be more than NAMEDATALEN, so
>> I use StringInfo.
>> Is it not necessary?
>
> Such long names doesn't seem to necessary. Too long identifiers
> no longer act as identifier for human eyeballs. We are limiting
> the length of identifiers of the whole database system to
> NAMEDATALEN-1, which seems to have been enough so I don't see any
> reason to have a group name longer than that.
>

I see. I will fix this.

>> > I found that the name SyncGroupName.wait_num is not
>> > instinctive. How about sync_num, sync_member_num or
>> > sync_standby_num? If the last is preferable, .members also should
>> > be .standbys .
>>
>> Thanks, sync_num is preferable to me.
>>
>> ===
>> > I am quite uncomfortable with the existence of
>> > WanSnd.sync_standby_priority. It represented the pirority in the
>> > old linear s_s_names format but nested groups or even
>> > single-level quarum list obviously doesn't fit it. Can we get rid
>> > of sync_standby_priority, even though we realize atmost
>> > n-priority for now?
>>
>> We could get rid of sync_standby_priority.
>> But if so, we will not be able to see the next sync standby in
>> pg_stat_replication system view.
>> Regarding each node priority, I was thinking that standbys in quorum
>> list have same 

Re: [HACKERS] [PROPOSAL] VACUUM Progress Checker.

[Amit Langote]

Horiguchi-san,

Thanks a lot for taking a look!

On 2016/03/07 13:02, Kyotaro HORIGUCHI wrote:
> At Sat, 5 Mar 2016 16:41:29 +0900, Amit Langote wrote:
>> On Sat, Mar 5, 2016 at 4:24 PM, Amit Langote  wrote:
>>> So, I took the Vinayak's latest patch and rewrote it a little
>> ...
>>> I broke it into two:
>>>
>>> 0001-Provide-a-way-for-utility-commands-to-report-progres.patch
>>> 0002-Implement-progress-reporting-for-VACUUM-command.patch
>>
>> Oops, unamended commit messages in those patches are misleading.  So,
>> please find attached corrected versions.
> 
> The 0001-P.. adds the following interface functions.
> 
> +extern void pgstat_progress_set_command(BackendCommandType cmdtype);
> +extern void pgstat_progress_set_command_target(Oid objid);
> +extern void pgstat_progress_update_param(int index, uint32 val);
> +extern void pgstat_reset_local_progress(void);
> +extern int   pgstat_progress_get_num_param(BackendCommandType cmdtype);
> 
> I don't like to treat the target object id differently from other
> parameters. It could not be needed at all, or could be needed two
> or more in contrast. Although oids are not guaranteed to fit
> uint32, we have already stored BlockNumber there.

I thought giving cmdtype and objid each its own slot would make things a
little bit clearer than stuffing them into st_progress_param[0] and
st_progress_param[1], respectively.  Is that what you are suggesting?
Although as I've don, a separate field st_command_objid may be a bit too much.

If they are not special fields, I think we don't need special interface
functions *set_command() and *set_command_target().  But I am still
inclined toward keeping the former.

> # I think that integer arrays might be needed to be passed as a
> # parameter, but it would be the another issue.

Didn't really think about it.  Maybe we should consider a scenario that
would require it.

> pg_stat_get_progress_info returns a tuple with 10 integer columns
> (plus an object id). The reason why I suggested use of an integer
> array is that it allows the API to serve arbitrary number of
> parmeters without a modification of API, and array indexes are
> coloreless than any concrete names. Howerver I don't stick to
> that if we agree that it is ok to have fixed number of paremters.

I think the fixed number of parameters in the form of a fixed-size array
is because st_progress_param[] is part of a shared memory structure as
discussed before.  Although such interface has been roughly modeled on how
pg_statistic catalog and pg_stats view or get_attstatsslot() function
work, shared memory structures take the place of the catalog, so there are
some restrictions (fixed size array being one).

Regarding index into st_progress_param[], pgstat.c/pgstatfuncs.c should
not bother what it is.  As exemplified in patch 0002, individual index
numbers can be defined as macros by individual command modules (suggested
by Robert recently) with certain convention for readability such as the
following in lazyvacuum.c:

#define PROG_PAR_VAC_RELID 0
#define PROG_PAR_VAC_PHASE_ID  1
#define PROG_PAR_VAC_HEAP_BLKS 2
#define PROG_PAR_VAC_CUR_HEAP_BLK  3
... so on.

Then, to report a changed parameter:

pgstat_progress_update_param(PROG_PAR_VAC_PHASE_ID, LV_PHASE_SCAN_HEAP);
...
pgstat_progress_update_param(PROG_PAR_VAC_CUR_HEAP_BLK, blkno);

by the way, following is proargnames[] for pg_stat_get_progress_info():

cmdtype integer,
OUT pid integer,
OUT param1 integer,
OUT param2 integer,
...
OUT param10 integer

So, it is a responsibility of a command specific progress view definition
that it interprets values of param1..param10 appropriately.  In fact, the
implementer of the progress reporting for a command determines what goes
into which slot of st_progress_param[], to begin with.

> pgstat_progress_get_num_param looks not good in the aspect of
> genericity. I'd like to define it as an integer array by idexed
> by the command type if it is needed. However it seems to me to be
> enough that pg_stat_get_progress_info always returns 10 integers
> regardless of what the numbers are for. The user sql function,
> pg_stat_vacuum_progress as the first user, knows how many numbers
> should be read for its work. It reads zeroes safely even if it
> reads more than what the producer side offered (unless it tries
> to divide something with it).

Thinking a bit, perhaps we don't need num_param(cmdtpye) function or array
at all as you seem to suggest.  It serves no useful purpose now that I see
it. pg_stat_get_progress_info() should simply copy
st_progress_param[0...PG_STAT_GET_PROGRESS_COLS-1] to the result and view
definer knows what's what.

Attached updated patches which incorporate above mentioned changes.  If
Vinayak has something else in mind about anything, he can weigh in.

Thanks,
Amit
>From 52a398f5104cd50f8bfcc4fd1fbb5bb102eddbf5 Mon Sep 17 00:00:00 2001
From: Amit 

Re: [HACKERS] silent data loss with ext4 / all current versions

[Andres Freund]

Hi,

On 2016-03-05 19:54:05 -0800, Andres Freund wrote:
> I started working on this; delayed by taking longer than planned on the
> logical decoding stuff (quite a bit complicated by
> e1a11d93111ff3fba7a91f3f2ac0b0aca16909a8).  I'm not very happy with the
> error handling as it is right now.  For one, you have rename_safe return
> error codes, and act on them in the callers, but on the other hand you
> call fsync_fname which always errors out in case of failure.  I also
> don't like the new messages much.
> 
> Will continue working on this tomorrow.

So, here's my current version of this. I've not performed any testing
yet, and it's hot of the press. There's some comment smithing
needed. But otherwise I'm starting to like this.

Changes:
* renamed rename_safe to durable_rename
* renamed replace_safe to durable_link_or_rename (there was never any
  replacing going on)
* pass through elevel to the underlying routines, otherwise we could
  error out with ERROR when we don't want to. That's particularly
  important in case of things like InstallXLogFileSegment().
* made fsync_fname use fsync_fname_ext, add 'accept permission errors'
  param
* have walkdir call a wrapper, to add ignore_perms param

What do you think?

I sure wish we had the recovery testing et al. in all the back
branches...

- Andres
>From e60caf094f68496658e969cdd4df919fd66e9d29 Mon Sep 17 00:00:00 2001
From: Andres Freund 
Date: Sun, 6 Mar 2016 22:20:17 -0800
Subject: [PATCH 1/2] durable-rename-andres-v8

---
 src/backend/access/transam/timeline.c|  40 +
 src/backend/access/transam/xlog.c|  64 ++--
 src/backend/access/transam/xlogarchive.c |  21 +--
 src/backend/postmaster/pgarch.c  |   6 +-
 src/backend/replication/logical/origin.c |  23 +--
 src/backend/replication/slot.c   |   2 +-
 src/backend/storage/file/fd.c| 267 +++
 src/include/storage/fd.h |   4 +-
 8 files changed, 232 insertions(+), 195 deletions(-)

diff --git a/src/backend/access/transam/timeline.c b/src/backend/access/transam/timeline.c
index f6da673..bd91573 100644
--- a/src/backend/access/transam/timeline.c
+++ b/src/backend/access/transam/timeline.c
@@ -418,24 +418,10 @@ writeTimeLineHistory(TimeLineID newTLI, TimeLineID parentTLI,
 	TLHistoryFilePath(path, newTLI);
 
 	/*
-	 * Prefer link() to rename() here just to be really sure that we don't
-	 * overwrite an existing file.  However, there shouldn't be one, so
-	 * rename() is an acceptable substitute except for the truly paranoid.
+	 * Perform the rename using link if available, paranoidly trying to avoid
+	 * overwriting an existing file (there shouldn't be one).
 	 */
-#if HAVE_WORKING_LINK
-	if (link(tmppath, path) < 0)
-		ereport(ERROR,
-(errcode_for_file_access(),
- errmsg("could not link file \"%s\" to \"%s\": %m",
-		tmppath, path)));
-	unlink(tmppath);
-#else
-	if (rename(tmppath, path) < 0)
-		ereport(ERROR,
-(errcode_for_file_access(),
- errmsg("could not rename file \"%s\" to \"%s\": %m",
-		tmppath, path)));
-#endif
+	durable_link_or_rename(tmppath, path, ERROR);
 
 	/* The history file can be archived immediately. */
 	if (XLogArchivingActive())
@@ -508,24 +494,10 @@ writeTimeLineHistoryFile(TimeLineID tli, char *content, int size)
 	TLHistoryFilePath(path, tli);
 
 	/*
-	 * Prefer link() to rename() here just to be really sure that we don't
-	 * overwrite an existing logfile.  However, there shouldn't be one, so
-	 * rename() is an acceptable substitute except for the truly paranoid.
+	 * Perform the rename using link if available, paranoidly trying to avoid
+	 * overwriting an existing file (there shouldn't be one).
 	 */
-#if HAVE_WORKING_LINK
-	if (link(tmppath, path) < 0)
-		ereport(ERROR,
-(errcode_for_file_access(),
- errmsg("could not link file \"%s\" to \"%s\": %m",
-		tmppath, path)));
-	unlink(tmppath);
-#else
-	if (rename(tmppath, path) < 0)
-		ereport(ERROR,
-(errcode_for_file_access(),
- errmsg("could not rename file \"%s\" to \"%s\": %m",
-		tmppath, path)));
-#endif
+	durable_link_or_rename(tmppath, path, ERROR);
 }
 
 /*
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index 00f139a..2d63a54 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -3299,34 +3299,16 @@ InstallXLogFileSegment(XLogSegNo *segno, char *tmppath,
 	}
 
 	/*
-	 * Prefer link() to rename() here just to be really sure that we don't
-	 * overwrite an existing logfile.  However, there shouldn't be one, so
-	 * rename() is an acceptable substitute except for the truly paranoid.
+	 * Perform the rename using link if available, paranoidly trying to avoid
+	 * overwriting an existing file (there shouldn't be one).
 	 */
-#if HAVE_WORKING_LINK
-	if (link(tmppath, path) < 0)
+	if (durable_link_or_rename(tmppath, path, LOG) != 0)
 	{
 		if (use_lock)
 			LWLockRelease(ControlFileLock);
-		ereport(LOG,

Re: [HACKERS] How can we expand PostgreSQL ecosystem?

[Tsunakawa, Takayuki]

> From: pgsql-hackers-ow...@postgresql.org
> [mailto:pgsql-hackers-ow...@postgresql.org] On Behalf Of Mark Kirkwood
> For cloud - in particular Openstack (which I am working with ATM), the
> biggest thing would be:
> 
> - multi-master replication
> 
> or failing that:
> 
> - self managing single master failover (voting/quorum etc)
> 
> so that operators can essentially 'set and forget'. We currently use
> Mysql+ Galera (multi master) and Mongodb (self managing single master)
> and the convenience and simplicity is just so important (Openstack is a
> huge complex collection of services - hand holding of any one service is
> pretty much a non starter).

Yes, I was also asked whether PostgreSQL has any optional functionality like 
Galera Cluster for MySQL.  He was planning a scalable PaaS service which 
performs heavy reads and writes.  Demand exists.

Regards
Takayuki Tsunakawa




-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] WIP: Upper planner pathification

[Tom Lane]

Peter Geoghegan  writes:
> In summary, I think it's surprising that a max_parallel_degree of 1
> doesn't disable parallel workers entirely.

Yeah, it's not exactly clear what "1" should mean that's different
from "0", in this case.

regards, tom lane


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] WIP: Upper planner pathification

[Peter Geoghegan]

On Sun, Mar 6, 2016 at 9:59 PM, Tom Lane  wrote:
> Perhaps it was intentional when written, but if Robert's advice is correct
> that the new upper-planner path nodes should copy up parallel_degree from
> their children, then it cannot be the case that parallel_degree>0 in a
> node above the scan level implies that that node type has any special
> behavior for parallelism.
>
> I continue to bemoan the lack of documentation about what these fields
> mean.  As far as I can find, the sum total of the documentation about
> this field is
>
> int parallel_degree; /* desired parallel degree; 0 = not parallel 
> */

While it doesn't particularly relate to parallel joins, I've expressed
a general concern about the max_parallel_degree GUC that I think is
worth considering again:

http://www.postgresql.org/message-id/cam3swzrs1mtvrkkasy1xbshgzxkd6-hnxx3gq7x-p-dz0zt...@mail.gmail.com

In summary, I think it's surprising that a max_parallel_degree of 1
doesn't disable parallel workers entirely.

-- 
Peter Geoghegan


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] WIP: Upper planner pathification

[Tom Lane]

Amit Kapila  writes:
> On Mon, Mar 7, 2016 at 9:13 AM, Tom Lane  wrote:
>> AFAICS, those are about generating partial paths, which is a completely
>> different thing from whether a regular path is parallel-safe or not.

> Okay, but the main point I wanted to convey is that I think setting
> parallel_degree = 0 in mergejoin path is not necessarily a copy-paste
> error.

Perhaps it was intentional when written, but if Robert's advice is correct
that the new upper-planner path nodes should copy up parallel_degree from
their children, then it cannot be the case that parallel_degree>0 in a
node above the scan level implies that that node type has any special
behavior for parallelism.

I continue to bemoan the lack of documentation about what these fields
mean.  As far as I can find, the sum total of the documentation about
this field is

int parallel_degree; /* desired parallel degree; 0 = not parallel */

Last I checked, "degree" meant 1/360'th part of a circle, or some
fraction of the distance between water's freezing and boiling points,
or possibly an award for academic achievement.  So I'm not really
going to hold still for any claim that this is self-explanatory.

regards, tom lane


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Parallel Aggregate

[Tom Lane]

Haribabu Kommi  writes:
> 2. Temporary fix for float aggregate types in _equalAggref because of
> a change in aggtype to trans type, otherwise the parallel aggregation
> plan failure in set_plan_references. whenever the aggtype is not matched,
> it verifies with the trans type also.

That is a completely unacceptable kluge.  Quite aside from being ugly as
sin, it probably breaks more things than it fixes, first because it breaks
the fundamental semantics of equal() across the board, and second because
it puts catalog lookups into equal(), which *will* cause problems.  You
can not expect that this will get committed, not even as a "temporary fix".

regards, tom lane


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Parallel Aggregate

[Haribabu Kommi]

On Sun, Mar 6, 2016 at 10:21 PM, Haribabu Kommi
 wrote:
>
> Pending:
> 1. Explain plan needs to be corrected for parallel grouping similar like
> parallel aggregate.

Here I attached update patch with further changes,
1. Explain plan changes for parallel grouping

2. Temporary fix for float aggregate types in _equalAggref because of
a change in aggtype to trans type, otherwise the parallel aggregation
plan failure in set_plan_references. whenever the aggtype is not matched,
it verifies with the trans type also.

3. Generates parallel path for all partial paths and add it to the path_list,
based on the cheapest_path, the plan is chosen.


To apply this patch, first apply the patch in [1]

[1] - http://www.postgresql.org/message-id/14172.1457228...@sss.pgh.pa.us


Regards,
Hari Babu
Fujitsu Australia


parallelagg_v2.patch
Description: Binary data

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] WIP: Upper planner pathification

[Amit Kapila]

On Mon, Mar 7, 2016 at 9:13 AM, Tom Lane  wrote:
>
> Amit Kapila  writes:
>  Is there some reason why hash and nestloop are safe but merge isn't?
>
> > To make hash and nestloop work in parallel queries, we just push those
> > nodes below gather node.  Refer code
> > paths match_unsorted_outer()->consider_parallel_nestloop()
> > and hash_inner_and_outer()->try_partial_hashjoin_path().
>
> AFAICS, those are about generating partial paths, which is a completely
> different thing from whether a regular path is parallel-safe or not.
>

Okay, but the main point I wanted to convey is that I think setting
parallel_degree = 0 in mergejoin path is not necessarily a copy-paste
error.  If you see the other path generation code like
create_index_path(), create_samplescan_path(), etc. there we set
parallel_degree to zero even though the parallel-safety is determined based
on reloptinfo.  And I don't see any use of setting parallel_degree for path
which can't be pushed beneath gather (aka can be executed by multiple
workers).


With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com

Re: [HACKERS] psql completion for ids in multibyte string

[Kyotaro HORIGUCHI]

At Fri, 4 Mar 2016 12:30:17 -0500, Robert Haas  wrote in 

> >>> I committed this and back-patched this but (1) I avoided changing the
> >>> other functions for now and (2) I gave both the byte length and the
> >>> character length new names to avoid confusion.
> >>
> >> These tweaks appear to have been universally disliked by buildfarm
> >> members.
> >
> > Crap.  Wasn't careful enough, sorry.  Will fix shortly.
> 
> Fix pushed.

Thank you for committing this. I can see only one commit for this
in the repository but I believe it is the fixed one.

regards,

-- 
Kyotaro Horiguchi
NTT Open Source Software Center




-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] [PROPOSAL] VACUUM Progress Checker.

[Kyotaro HORIGUCHI]

Hi, Thank you for the patch.

At Sat, 5 Mar 2016 16:41:29 +0900, Amit Langote  wrote 
in 
> On Sat, Mar 5, 2016 at 4:24 PM, Amit Langote  wrote:
> > So, I took the Vinayak's latest patch and rewrote it a little
> ...
> > I broke it into two:
> >
> > 0001-Provide-a-way-for-utility-commands-to-report-progres.patch
> > 0002-Implement-progress-reporting-for-VACUUM-command.patch
> 
> Oops, unamended commit messages in those patches are misleading.  So,
> please find attached corrected versions.

The 0001-P.. adds the following interface functions.

+extern void pgstat_progress_set_command(BackendCommandType cmdtype);
+extern void pgstat_progress_set_command_target(Oid objid);
+extern void pgstat_progress_update_param(int index, uint32 val);
+extern void pgstat_reset_local_progress(void);
+extern int pgstat_progress_get_num_param(BackendCommandType cmdtype);

I don't like to treat the target object id differently from other
parameters. It could not be needed at all, or could be needed two
or more in contrast. Although oids are not guaranteed to fit
uint32, we have already stored BlockNumber there.

# I think that integer arrays might be needed to be passed as a
# parameter, but it would be the another issue.

pg_stat_get_progress_info returns a tuple with 10 integer columns
(plus an object id). The reason why I suggested use of an integer
array is that it allows the API to serve arbitrary number of
parmeters without a modification of API, and array indexes are
coloreless than any concrete names. Howerver I don't stick to
that if we agree that it is ok to have fixed number of paremters.

pgstat_progress_get_num_param looks not good in the aspect of
genericity. I'd like to define it as an integer array by idexed
by the command type if it is needed. However it seems to me to be
enough that pg_stat_get_progress_info always returns 10 integers
regardless of what the numbers are for. The user sql function,
pg_stat_vacuum_progress as the first user, knows how many numbers
should be read for its work. It reads zeroes safely even if it
reads more than what the producer side offered (unless it tries
to divide something with it).


What do you think about this?

regards,

-- 
Kyotaro Horiguchi
NTT Open Source Software Center




-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] WIP: Upper planner pathification

[Tom Lane]

Amit Kapila  writes:
 Is there some reason why hash and nestloop are safe but merge isn't?

> To make hash and nestloop work in parallel queries, we just push those
> nodes below gather node.  Refer code
> paths match_unsorted_outer()->consider_parallel_nestloop()
> and hash_inner_and_outer()->try_partial_hashjoin_path().

AFAICS, those are about generating partial paths, which is a completely
different thing from whether a regular path is parallel-safe or not.
(I think, anyway.  It would be nice if this stuff were documented better.
It would also likely be a good thing if partial-ness of a path were marked
in the path itself, which does not seem to be the case now.  Or at the
very least, it'd be a good thing if create_foo_path and the underlying
costing functions were told it was a partial path, because how the heck
can they generate sane cost numbers without that knowledge?)

regards, tom lane


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Badly designed error reporting code in controldata_utils.c

[Tom Lane]

Joe Conway  writes:
> On 03/06/2016 05:47 PM, Tom Lane wrote:
>> That's much better, but is there a reason you're using exit(2)
>> and not exit(EXIT_FAILURE) ?

> Only because I was trying to stick with what was originally in
> src/bin/pg_controldata/pg_controldata.c

Meh.  It looks to me like the standard way to handle this
for code in src/common/ is exit(EXIT_FAILURE).

regards, tom lane


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] WIP: Upper planner pathification

[Amit Kapila]

On Sun, Mar 6, 2016 at 9:02 PM, Tom Lane  wrote:
>
> Amit Kapila  writes:
> > On Sat, Mar 5, 2016 at 10:11 PM, Tom Lane  wrote:
> >> Is there some reason why hash and nestloop are safe but merge isn't?
>
> > I think it is because we consider to pushdown hash and nestloop to
workers,
> > but not merge join and the reason for not pushing mergejoin is that
> > currently we don't have executor support for same, more work is needed
> > there.
>
> If that's true, then mergejoin paths ought to be marked parallel-unsafe
> explicitly (with a comment as to why), not just silently reduced to degree
> zero in a manner that looks more like an oversight than anything
> intentional.
>
> I also note that the regression tests pass with this patch and parallel
> mode forced, which seems unlikely if allowing a parallel worker to execute
> a join works for only two out of the three join types.  And checking the
> git history for nodeHashjoin.c, nodeHash.c, and nodeNestloop.c shows no
> evidence that any of those files have been touched for parallel query,
> so it's pretty hard to see a reason why those would work in parallel
> queries but nodeMergejoin.c not.
>

To make hash and nestloop work in parallel queries, we just push those
nodes below gather node.  Refer code
paths match_unsorted_outer()->consider_parallel_nestloop()
and hash_inner_and_outer()->try_partial_hashjoin_path().   Once the partial
path for hash and nestloop gets generated in above code path, we generate
gather path in function add_paths_to_joinrel()->generate_gather_paths().
You won't find the code to generate partial path for merge join.


With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com

Re: [HACKERS] JPUG wants to have a copyright notice on the translated doc

[Tatsuo Ishii]

> Thanks.
> 
> Korean Document is translating currently.

Oh, I see.

> When that be done, I will announce to official site.
> current, translation progress is 50%.
> This work is dependent only voluntary support by 6 peoples.
> so, translating is very slow. :)

Same thing can be said to Japanese translation work. It requires
tremendous work.  The project page is on GitHub.

https://github.com/pgsql-jp/jpug-doc

While we are working, we are facing problem of GitHub's limitation:
when a pull request is submitted, the diff is not shown if the
document based on the pull request is too big. Because some of
PostgreSQL SGML file is quite large, they easily hit the limit.
Moreover, we leave the original English text, and we add Japanese
translation, the size of the file is almost double.

I'm thinking about requesting to split large SGML files into small
pieces. I'm not sure if developers accept it or not, though.

Best regards,
--
Tatsuo Ishii
SRA OSS, Inc. Japan
English: http://www.sraoss.co.jp/index_en.php
Japanese:http://www.sraoss.co.jp

> I also hope that many Korean documents appear on the official site.
> 
> Regards, Ioseph
> 
> 2016-03-07 (월), 11:19 +0900, Tatsuo Ishii:
>> You'd better to ask to place the below above to
>> http://www.postgresql.org/docs/.  Currently only French and Japanese
>> are listed. I'm sure there are more local translations.
>> 
>> Not only people living in Korea are interested in Korean translated
>> docs. There are many Korean speaking people in the world. Same thing
>> can be said to other languages of course.
>> 
>> Best regards,
>> --
>> Tatsuo Ishii
>> SRA OSS, Inc. Japan
>> English: http://www.sraoss.co.jp/index_en.php
>> Japanese:http://www.sraoss.co.jp
>> 
>> > Thanks too!
>> > 
>> > Korean User Group has been translating PGDoc since 2013.
>> > 
>> > I'll append Copyright in PGDoc.kr too. 
>> > http://postgresql.kr/docs/current/
>> > 
>> > Regards, Ioseph.
>> > 
>> > 
>> > 2016-03-04 (금), 17:55 -0800, Joshua D. Drake:
>> >> On 03/04/2016 05:39 PM, Tatsuo Ishii wrote:
>> >> > JPUG (Japan PostgreSQL Users Group) would like to add a copyright
>> >> > ntice to the Japanese translated docs.
>> >> >
>> >> > http://www.postgresql.jp/document/9.5/html/
>> >> >
>> >> > Currently "Copyright 1996-2016 The PostgreSQL Global Development
>> >> > Group" is showed on the translated doc (of course in Japanese). What
>> >> > JPUG is wanting is, adding something like "Copyright 2016 Japan
>> >> > PostgreSQL Users Group" to this.
>> >> 
>> >> As I understand it the translation would be copyrighted by the people 
>> >> that do the translation so it is perfectly reasonable to have JPUG hold 
>> >> the copyright for the .jp translation.
>> >> 
>> >> >
>> >> > The reason for this is, "Copyright 1996-2016 The PostgreSQL Global
>> >> > Development Group" may not be effective from the point of Japan laws
>> >> > because "The PostgreSQL Global Development Group" is not a valid
>> >> > entity to be copyright holder according to Japan's laws. To prevent
>> >> > from abuses of the translated docs, JPUG thinks that JPUG needs to add
>> >> > the copyright notice by JPUG to the Japanese translated docs (note
>> >> > that JPUG is a registered non profit organization and can be a
>> >> > copyright holder).
>> >> 
>> >> Considering they are BSD licensed, I am not sure what abuses could be 
>> >> taken?
>> >> 
>> >> Sincerely,
>> >> 
>> >> JD
>> >> 
>> >> -- 
>> >> Command Prompt, Inc.  http://the.postgres.company/
>> >>  +1-503-667-4564
>> >> PostgreSQL Centered full stack support, consulting and development.
>> >> Everyone appreciates your honesty, until you are honest with them.
>> >> 
>> >> 
>> > 
>> > 
> 
> 


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] JPUG wants to have a copyright notice on the translated doc

[Ioseph Kim]

Thanks.

Korean Document is translating currently.
When that be done, I will announce to official site.
current, translation progress is 50%.
This work is dependent only voluntary support by 6 peoples.
so, translating is very slow. :)

I also hope that many Korean documents appear on the official site.

Regards, Ioseph

2016-03-07 (월), 11:19 +0900, Tatsuo Ishii:
> You'd better to ask to place the below above to
> http://www.postgresql.org/docs/.  Currently only French and Japanese
> are listed. I'm sure there are more local translations.
> 
> Not only people living in Korea are interested in Korean translated
> docs. There are many Korean speaking people in the world. Same thing
> can be said to other languages of course.
> 
> Best regards,
> --
> Tatsuo Ishii
> SRA OSS, Inc. Japan
> English: http://www.sraoss.co.jp/index_en.php
> Japanese:http://www.sraoss.co.jp
> 
> > Thanks too!
> > 
> > Korean User Group has been translating PGDoc since 2013.
> > 
> > I'll append Copyright in PGDoc.kr too. 
> > http://postgresql.kr/docs/current/
> > 
> > Regards, Ioseph.
> > 
> > 
> > 2016-03-04 (금), 17:55 -0800, Joshua D. Drake:
> >> On 03/04/2016 05:39 PM, Tatsuo Ishii wrote:
> >> > JPUG (Japan PostgreSQL Users Group) would like to add a copyright
> >> > ntice to the Japanese translated docs.
> >> >
> >> > http://www.postgresql.jp/document/9.5/html/
> >> >
> >> > Currently "Copyright 1996-2016 The PostgreSQL Global Development
> >> > Group" is showed on the translated doc (of course in Japanese). What
> >> > JPUG is wanting is, adding something like "Copyright 2016 Japan
> >> > PostgreSQL Users Group" to this.
> >> 
> >> As I understand it the translation would be copyrighted by the people 
> >> that do the translation so it is perfectly reasonable to have JPUG hold 
> >> the copyright for the .jp translation.
> >> 
> >> >
> >> > The reason for this is, "Copyright 1996-2016 The PostgreSQL Global
> >> > Development Group" may not be effective from the point of Japan laws
> >> > because "The PostgreSQL Global Development Group" is not a valid
> >> > entity to be copyright holder according to Japan's laws. To prevent
> >> > from abuses of the translated docs, JPUG thinks that JPUG needs to add
> >> > the copyright notice by JPUG to the Japanese translated docs (note
> >> > that JPUG is a registered non profit organization and can be a
> >> > copyright holder).
> >> 
> >> Considering they are BSD licensed, I am not sure what abuses could be 
> >> taken?
> >> 
> >> Sincerely,
> >> 
> >> JD
> >> 
> >> -- 
> >> Command Prompt, Inc.  http://the.postgres.company/
> >>  +1-503-667-4564
> >> PostgreSQL Centered full stack support, consulting and development.
> >> Everyone appreciates your honesty, until you are honest with them.
> >> 
> >> 
> > 
> > 




-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] JPUG wants to have a copyright notice on the translated doc

[Tatsuo Ishii]

You'd better to ask to place the below above to
http://www.postgresql.org/docs/.  Currently only French and Japanese
are listed. I'm sure there are more local translations.

Not only people living in Korea are interested in Korean translated
docs. There are many Korean speaking people in the world. Same thing
can be said to other languages of course.

Best regards,
--
Tatsuo Ishii
SRA OSS, Inc. Japan
English: http://www.sraoss.co.jp/index_en.php
Japanese:http://www.sraoss.co.jp

> Thanks too!
> 
> Korean User Group has been translating PGDoc since 2013.
> 
> I'll append Copyright in PGDoc.kr too. 
> http://postgresql.kr/docs/current/
> 
> Regards, Ioseph.
> 
> 
> 2016-03-04 (금), 17:55 -0800, Joshua D. Drake:
>> On 03/04/2016 05:39 PM, Tatsuo Ishii wrote:
>> > JPUG (Japan PostgreSQL Users Group) would like to add a copyright
>> > ntice to the Japanese translated docs.
>> >
>> > http://www.postgresql.jp/document/9.5/html/
>> >
>> > Currently "Copyright 1996-2016 The PostgreSQL Global Development
>> > Group" is showed on the translated doc (of course in Japanese). What
>> > JPUG is wanting is, adding something like "Copyright 2016 Japan
>> > PostgreSQL Users Group" to this.
>> 
>> As I understand it the translation would be copyrighted by the people 
>> that do the translation so it is perfectly reasonable to have JPUG hold 
>> the copyright for the .jp translation.
>> 
>> >
>> > The reason for this is, "Copyright 1996-2016 The PostgreSQL Global
>> > Development Group" may not be effective from the point of Japan laws
>> > because "The PostgreSQL Global Development Group" is not a valid
>> > entity to be copyright holder according to Japan's laws. To prevent
>> > from abuses of the translated docs, JPUG thinks that JPUG needs to add
>> > the copyright notice by JPUG to the Japanese translated docs (note
>> > that JPUG is a registered non profit organization and can be a
>> > copyright holder).
>> 
>> Considering they are BSD licensed, I am not sure what abuses could be taken?
>> 
>> Sincerely,
>> 
>> JD
>> 
>> -- 
>> Command Prompt, Inc.  http://the.postgres.company/
>>  +1-503-667-4564
>> PostgreSQL Centered full stack support, consulting and development.
>> Everyone appreciates your honesty, until you are honest with them.
>> 
>> 
> 
> 


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Badly designed error reporting code in controldata_utils.c

[Joe Conway]

On 03/06/2016 05:47 PM, Tom Lane wrote:
> Joe Conway  writes:
>> Something like the attached what you had in mind?
> 
> That's much better, but is there a reason you're using exit(2)
> and not exit(EXIT_FAILURE) ?

Only because I was trying to stick with what was originally in
src/bin/pg_controldata/pg_controldata.c

Joe

-- 
Crunchy Data - http://crunchydata.com
PostgreSQL Support for Secure Enterprises
Consulting, Training, & Open Source Development



signature.asc
Description: OpenPGP digital signature

Re: [HACKERS] JPUG wants to have a copyright notice on the translated doc

[Ioseph Kim]

Thanks too!

Korean User Group has been translating PGDoc since 2013.

I'll append Copyright in PGDoc.kr too. 
http://postgresql.kr/docs/current/

Regards, Ioseph.


2016-03-04 (금), 17:55 -0800, Joshua D. Drake:
> On 03/04/2016 05:39 PM, Tatsuo Ishii wrote:
> > JPUG (Japan PostgreSQL Users Group) would like to add a copyright
> > ntice to the Japanese translated docs.
> >
> > http://www.postgresql.jp/document/9.5/html/
> >
> > Currently "Copyright 1996-2016 The PostgreSQL Global Development
> > Group" is showed on the translated doc (of course in Japanese). What
> > JPUG is wanting is, adding something like "Copyright 2016 Japan
> > PostgreSQL Users Group" to this.
> 
> As I understand it the translation would be copyrighted by the people 
> that do the translation so it is perfectly reasonable to have JPUG hold 
> the copyright for the .jp translation.
> 
> >
> > The reason for this is, "Copyright 1996-2016 The PostgreSQL Global
> > Development Group" may not be effective from the point of Japan laws
> > because "The PostgreSQL Global Development Group" is not a valid
> > entity to be copyright holder according to Japan's laws. To prevent
> > from abuses of the translated docs, JPUG thinks that JPUG needs to add
> > the copyright notice by JPUG to the Japanese translated docs (note
> > that JPUG is a registered non profit organization and can be a
> > copyright holder).
> 
> Considering they are BSD licensed, I am not sure what abuses could be taken?
> 
> Sincerely,
> 
> JD
> 
> -- 
> Command Prompt, Inc.  http://the.postgres.company/
>  +1-503-667-4564
> PostgreSQL Centered full stack support, consulting and development.
> Everyone appreciates your honesty, until you are honest with them.
> 
> 




-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Badly designed error reporting code in controldata_utils.c

[Tom Lane]

Joe Conway  writes:
> Something like the attached what you had in mind?

That's much better, but is there a reason you're using exit(2)
and not exit(EXIT_FAILURE) ?

regards, tom lane


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] [PROPOSAL] VACUUM Progress Checker.

[pokurev]

Hi Amit,

Thank you for updating the patch. I am testing it and I will try to improve it.

Regards,
Vinayak
> -Original Message-
> From: Amit Langote [mailto:amitlangot...@gmail.com]
> Sent: Saturday, March 05, 2016 4:41 PM
> To: Robert Haas 
> Cc: SPS ポクレ ヴィナヤック(三技術) ;
> Amit Langote ; Kyotaro HORIGUCHI
> ; pgsql-hackers@postgresql.org; SPS 坂野
> 昌平(三技術) 
> Subject: Re: [HACKERS] [PROPOSAL] VACUUM Progress Checker.
> 
> On Sat, Mar 5, 2016 at 4:24 PM, Amit Langote 
> wrote:
> > So, I took the Vinayak's latest patch and rewrote it a little
> ...
> > I broke it into two:
> >
> > 0001-Provide-a-way-for-utility-commands-to-report-progres.patch
> > 0002-Implement-progress-reporting-for-VACUUM-command.patch
> 
> Oops, unamended commit messages in those patches are misleading.  So,
> please find attached corrected versions.
> 
> Thanks,
> Amit

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] The plan for FDW-based sharding

[Robert Haas]

On Fri, Mar 4, 2016 at 10:54 PM, Craig Ringer  wrote:
> I've got to say that this is somewhat reminicient of the discussions around
> in-core pooling, where argument 1 is applied to justify excluding pooling
> from core/contrib.
>
> I don't have a strong position on whether a DTM should be in core or not as
> I haven't done enough work in the area. I do think it's interesting to
> strongly require that a DTM be in core while we also reject things like
> pooling that are needed by a large proportion of users.

I don't remember this discussion, but I don't think I feel differently
about either of these two issues.  I'm not opposed to having some
hooks in core to make it easier to build a DTM, but I'm not convinced
that these hooks are the right hooks or that the design underlying
those hooks is correct.  And, eventually, I would like to see a DTM in
core or contrib so that it can be accessible to everyone relatively
easily.  Now, on connection pooling, I am similarly not opposed to
having some well-designed hooks, but I also think in the long run it
would be better for some improvements in this area to be part of core.
None of that means I would support any particular hook proposal, of
course.

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


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Badly designed error reporting code in controldata_utils.c

[Andres Freund]

On 2016-03-06 17:16:42 -0800, Joe Conway wrote:
> - #ifndef FRONTEND
> - /* NOTE: caller must provide gettext call around the format string */
> - #define log_error(...)  \
> - elog(ERROR, __VA_ARGS__)
> - #else
> - #define log_error(...)  \
> - do { \
> - char *buf = psprintf(__VA_ARGS__); \
> - fprintf(stderr, "%s: %s\n", progname, buf); \
> - exit(2); \
> - } while (0)
> - #endif
> -

FWIW I'm considering implementing elog/ereport properly for the
frontend.  We've grown several hacks around that not being present, and
I think those by now have a higher aggregate complexity than a proper
implementation would have.

Greetings,

Andres Freund


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] The plan for FDW-based sharding

[Robert Haas]

On Fri, Mar 4, 2016 at 10:23 PM, Craig Ringer  wrote:
> I can imagine that many such hooks would have little use beyond PPAS, but
> I'm somewhat curious as to if any would have wider applications. It's not
> unusual for me to be working on something and think "gee, I wish there was a
> hook here".

Well, on the whole, we've adopted an approach of "hack core and
merge", so to some extent you have to use your imagination to think
about what it would look like if it were all done using hooks.  But
we've also actually added hooks to Advanced Server in some places
where PostgreSQL doesn't have them, and it's not hard to imagine that
somebody else might find those useful, at least.  Whether they'd be
useful enough that the PostgreSQL community would accept them if
EnterpriseDB were to approve open-sourcing them is another
question

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


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Badly designed error reporting code in controldata_utils.c

[Joe Conway]

On 03/06/2016 01:26 PM, Joe Conway wrote:
> On 03/06/2016 01:24 PM, Tom Lane wrote:
>> * It's randomly unlike every single other place we've addressed the
>> same problem.  Everywhere else in src/common does it like this:

[...snip...]

>> and I think that's what this needs to do too, especially in view of the
>> fact that there are only two places that would have to be fixed anyway.
> 
> Ok, will fix.

Something like the attached what you had in mind?

Thanks,

Joe

-- 
Crunchy Data - http://crunchydata.com
PostgreSQL Support for Secure Enterprises
Consulting, Training, & Open Source Development
diff --git a/src/common/controldata_utils.c b/src/common/controldata_utils.c
index b6d0a12..65f2cb9 100644
*** a/src/common/controldata_utils.c
--- b/src/common/controldata_utils.c
***
*** 28,46 
  #include "common/controldata_utils.h"
  #include "port/pg_crc32c.h"
  
- #ifndef FRONTEND
- /* NOTE: caller must provide gettext call around the format string */
- #define log_error(...)	\
- 	elog(ERROR, __VA_ARGS__)
- #else
- #define log_error(...)	\
- 	do { \
- 			char *buf = psprintf(__VA_ARGS__); \
- 			fprintf(stderr, "%s: %s\n", progname, buf); \
- 			exit(2); \
- 	} while (0)
- #endif
- 
  /*
   * get_controlfile(char *DataDir, const char *progname)
   *
--- 28,33 
*** get_controlfile(char *DataDir, const cha
*** 59,70 
  	snprintf(ControlFilePath, MAXPGPATH, "%s/global/pg_control", DataDir);
  
  	if ((fd = open(ControlFilePath, O_RDONLY | PG_BINARY, 0)) == -1)
! 		log_error(_("could not open file \"%s\" for reading: %s"),
!   ControlFilePath, strerror(errno));
  
  	if (read(fd, ControlFile, sizeof(ControlFileData)) != sizeof(ControlFileData))
! 		log_error(_("could not read file \"%s\": %s"),
!   ControlFilePath, strerror(errno));
  
  	close(fd);
  
--- 46,76 
  	snprintf(ControlFilePath, MAXPGPATH, "%s/global/pg_control", DataDir);
  
  	if ((fd = open(ControlFilePath, O_RDONLY | PG_BINARY, 0)) == -1)
! #ifndef FRONTEND
! 		ereport(ERROR,
! (errcode_for_file_access(),
! errmsg("could not open file \"%s\" for reading: %m",
! 	   ControlFilePath)));
! #else
! 	{
! 		fprintf(stderr, _("%s: could not open file \"%s\" for reading: %s\n"),
! progname, ControlFilePath, strerror(errno));
! 		exit(2);
! 	}
! #endif
  
  	if (read(fd, ControlFile, sizeof(ControlFileData)) != sizeof(ControlFileData))
! #ifndef FRONTEND
! 		ereport(ERROR,
! (errcode_for_file_access(),
! errmsg("could not read file \"%s\": %m", ControlFilePath)));
! #else
! 	{
! 		fprintf(stderr, _("%s: could not read file \"%s\": %s\n"),
! progname, ControlFilePath, strerror(errno));
! 		exit(2);
! 	}
! #endif
  
  	close(fd);
  


signature.asc
Description: OpenPGP digital signature

[HACKERS] gzclose don't set properly the errno in pg_restore

[Fabrízio de Royes Mello]

Hi all,

I'm facing with a strange error message during a failed pg_restore:

pg_restore: processing data for table "historicopesquisaitem"
pg_restore: [directory archiver] could not close data file: Success

I got this error trying to restore from a directory dump file.

Looking at the pg_restore code seems that "gzclose" didn't set the 'errno'
because I got a Z_BUF_ERROR [1] from "cfclose" return and the "errno" still
0 (zero).

412 if (cfclose(cfp) !=0)
413 exit_horribly(modulename, "could not close data file: %s\n",
414   strerror(errno));

Should we set the errno using the gzclose return code or just add some
check to "strerror" the "errno" or the "gzclose return code" if they are
different?

Thoughts?


[1] http://www.zlib.net/manual.html

--
Fabrízio de Royes Mello
Consultoria/Coaching PostgreSQL
>> Timbira: http://www.timbira.com.br
>> Blog: http://fabriziomello.github.io
>> Linkedin: http://br.linkedin.com/in/fabriziomello
>> Twitter: http://twitter.com/fabriziomello
>> Github: http://github.com/fabriziomello

Re: [HACKERS] Proposal: "Causal reads" mode for load balancing reads without stale data

[Thomas Munro]

On Thu, Mar 3, 2016 at 7:34 PM, Michael Paquier
 wrote:
> On Tue, Mar 1, 2016 at 11:53 AM, Thomas Munro
>  wrote:
>> On Tue, Mar 1, 2016 at 2:46 PM, Amit Langote
>>  wrote:
>>>
>>> Hi,
>>>
>>> On 2016/02/29 18:05, Thomas Munro wrote:
 On Mon, Feb 29, 2016 at 9:05 PM, Amit Langote wrote:
> + servers.  A transaction that is run with
> causal_reads set
> + to on is guaranteed either to see the effects of all
> + completed transactions run on the primary with the setting on, 
> or to
> + receive an error "standby is not available for causal reads".
>
> "A transaction that is run" means "A transaction that is run on a
> standby", right?

 Well, it could be any server, standby or primary.  Of course standbys
 are the interesting case since it it was already true that if you run
 two sequential transactions run on the primary, the second can see the
 effect of the first, but I like the idea of a general rule that
 applies anywhere, allowing you not to care which server it is.
>>>
>>> I meant actually in context of that sentence only.
>>
>> Ok, here's a new version that includes that change, fixes a conflict
>> with recent commit 10b48522 and removes an accidental duplicate copy
>> of the README file.
>
> Finally I got my eyes on this patch.
>
> To put it short, this patch introduces two different concepts:
> - addition of a new value, remote_apply, for synchronous_commit, which
> is actually where things overlap a bit with the N-sync patch, because
> by combining the addition of remote_apply with the N-sync patch, it is
> possible to ensure that an LSN is applied to multiple targets instead
> of one now. In this case, as the master will wait for the LSN to be
> applied on the sync standby, there is no need for the application to
> have error handling in case a read transaction is happening on the
> standby as the change is already visible on the standby when
> committing it on master. However the cost here is that if the standby
> node lags behind, it puts some extra wait as well on the master side.
> - casual reads, which makes the master able to drop standbys that are
> lagging too much behind and let callers of standbys know that it is
> lagging to much behind and cannot satisfy causal reads. In this case
> error handling is needed by the application in case a standby is
> lagging to much behind.
>
> That's one of my concerns about this patch now: it is trying to do too
> much. I think that there is definitely a need for both things:
> applications may be fine to pay the lagging price when remote_apply is
> used by not having an extra error handling layer, or they cannot
> accept a perhaps large of lag and are ready to have an extra error
> handling layer to manage read failures on standbys. So I would
> recommend a split to begin with:
> 1) Addition of remote_apply in synchronous_commit, which would be
> quite a simple patch, and I am convinced that there are benefits in
> having that. Compared to the previous patch sent, a standby message is
> sent back to the master once COMMIT has been applied, accelerating
> things a bit.
> 2) Patch for causal reads, with all its extra parametrization logic
> and stuff to select standby candidates.

Thanks.  Yes, this makes a lot of sense.  I have done some work on
splitting this up and will post the result soon, along with my
responses to your other feedback.

> Here is as well a more detailed review...
>
> Regression tests are failing, rules.sql is generating diffs because
> pg_stat_replication is changed.
>
> CausalReadsWaitForLSN() should be called for 2PC I think, for PREPARE,
> COMMIT PREPARED and ROLLBACK PREPARED. WAL replay for 2PC should also
> call XLogRequestWalReceiverReply() when needed.
>
> The new parameters are missing from postgresql.conf.sample.
>
> +static bool
> +SyncRepCheckEarlyExit(void)
> +{
> Doing this refactoring would actually make sense as a separate patch I
> think, and that would simplify the core patch for causal reads.
>
> +For this reason we say that the causal reads guarantee only holds as
> +long as the absolute difference between the system clocks of the
> +machines is no more than max_clock_skew.  The theory is that NTP makes
> +it possible to reason about the maximum possible clock difference
> +between machines and choose a value that allows for a much larger
> +difference.  However, we do make a best effort attempt to detect
> +misconfigured systems as described above, to catch the case of servers
> +not running ntp a correctly configured ntp daemon, or with a clock so
> +far out of whack that ntp refuses to fix it.
> Just wondering how this reacts when standby and master are on
> different timezones. I know of two ways to measure WAL lag: one is
> what you are doing, by using a timestamp and rely on the two servers
> to be in sync at clock level. 

Re: [HACKERS] character_not_in_repertoire vs. untranslatable_character

[Tom Lane]

Chapman Flack  writes:
> So there's an ISO error 22021 "character not in repertoire" and
> a PostgreSQL error 22P05 "untranslatable character" that seem
> very similar.

> If I look in backend/utils/mb/wchar.c, it looks as if PostgreSQL
> uses the first for the case of a corrupted encoding (bytes that
> can't be decoded to a character at all), and the second for the
> case of a valid character that isn't available in a conversion's
> destination encoding.

Yeah, that's the intended distinction I believe, though I would not
want to swear that we've been 100% consistent.  22021 means "this
character is bad in isolation", AFAICT, so it didn't seem appropriate
for the conversion scenario.

regards, tom lane


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] The plan for FDW-based sharding

[Thom Brown]

On 6 Mar 2016 8:27 p.m., "Peter Geoghegan"  wrote:
>
> On Fri, Mar 4, 2016 at 4:41 PM, Robert Haas  wrote:
> > Yeah, I agree with that.  I am utterly mystified by why Bruce keeps
> > beating this drum, and am frankly pretty annoyed about it.  In the
> > first place, he seems to think that he invented the idea of using FDWs
> > for sharding in PostgreSQL, but I don't think that's true.  I think it
> > was partly my idea, and partly something that the NTT folks have been
> > working on for years (cf, e.g.,
> > cb1ca4d800621dcae67ca6c799006de99fa4f0a5).  As far as I understand it,
> > Bruce came in near the end of that conversation and now wants to claim
> > credit for something that doesn't really exist yet and, to the extent
> > that it does exist, wasn't even his idea.
>
> I think that it's easy to have the same idea as someone else
> independently. I've had that happen several times myself; ideas that
> other people had that I felt I could have easily had myself, or did in
> fact have. Most of the ideas that I have are fairly heavily based on
> known techniques. I don't think that I've ever creating a PostgreSQL
> feature that was in some way truly original, except perhaps for some
> aspects of how UPSERT works.

Everything is a remix.

Thom

Re: [HACKERS] Badly designed error reporting code in controldata_utils.c

[Joe Conway]

On 03/06/2016 01:24 PM, Tom Lane wrote:
> * It's randomly unlike every single other place we've addressed the
> same problem.  Everywhere else in src/common does it like this:
> 
> #ifndef FRONTEND
> ereport(ERROR,
> (errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED),
>  errmsg("out of memory")));
> #else
> fprintf(stderr, _("out of memory\n"));
> exit(EXIT_FAILURE);
> #endif
> 
> and I think that's what this needs to do too, especially in view of the
> fact that there are only two places that would have to be fixed anyway.

Ok, will fix.

Joe

-- 
Crunchy Data - http://crunchydata.com
PostgreSQL Support for Secure Enterprises
Consulting, Training, & Open Source Development



signature.asc
Description: OpenPGP digital signature

[HACKERS] Badly designed error reporting code in controldata_utils.c

[Tom Lane]

My attention was drawn to the log_error() stuff in controldata_utils.c by
the fact that buildfarm member pademelon spit up on it.  The reason for
that compile failure is that pademelon's dinosaur of a compiler doesn't
support __VA_ARGS__.  I do not feel a need to get into a discussion about
whether we should move our portability goalposts for the convenience of
this commit, because there are other reasons why this is a crummy solution
for error reporting:

* It uses elog() not ereport() for what seems a not-particularly-internal
error, which among other things means that an entirely inappropriate
errcode() will be reported.

* It relies on strerror(errno), not %m, which may not work reliably even
in elog() and certainly won't in ereport() (because of order-of-evaluation
uncertainties).

* Translatability of the error message in the frontend context seems
a bit dubious; generally we let translators work with the whole string
to be printed, not just part of it.

* It's randomly unlike every single other place we've addressed the
same problem.  Everywhere else in src/common does it like this:

#ifndef FRONTEND
ereport(ERROR,
(errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED),
 errmsg("out of memory")));
#else
fprintf(stderr, _("out of memory\n"));
exit(EXIT_FAILURE);
#endif

and I think that's what this needs to do too, especially in view of the
fact that there are only two places that would have to be fixed anyway.

regards, tom lane


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] The plan for FDW-based sharding

[Peter Geoghegan]

On Fri, Mar 4, 2016 at 4:41 PM, Robert Haas  wrote:
> Yeah, I agree with that.  I am utterly mystified by why Bruce keeps
> beating this drum, and am frankly pretty annoyed about it.  In the
> first place, he seems to think that he invented the idea of using FDWs
> for sharding in PostgreSQL, but I don't think that's true.  I think it
> was partly my idea, and partly something that the NTT folks have been
> working on for years (cf, e.g.,
> cb1ca4d800621dcae67ca6c799006de99fa4f0a5).  As far as I understand it,
> Bruce came in near the end of that conversation and now wants to claim
> credit for something that doesn't really exist yet and, to the extent
> that it does exist, wasn't even his idea.

I think that it's easy to have the same idea as someone else
independently. I've had that happen several times myself; ideas that
other people had that I felt I could have easily had myself, or did in
fact have. Most of the ideas that I have are fairly heavily based on
known techniques. I don't think that I've ever creating a PostgreSQL
feature that was in some way truly original, except perhaps for some
aspects of how UPSERT works.

Who cares whose idea FDW sharding was? It matters not a whit. It
probably independently occurred to several people that the FDW
interface could be built to support horizontal sharding more directly.
The idea almost suggests itself.

> EnterpriseDB *does* have a plan to try to continue enhancing foreign
> data wrappers so that you can run queries against foreign tables and
> get reasonable plans, something that currently isn't true.  I haven't
> heard anybody objecting to that, and I don't expect to hear anybody
> objecting to that, because it's hard to imagine why you wouldn't want
> queries against foreign data wrappers to produce better plans than
> they do today.  At worst, you might think it doesn't matter either
> way, but actually, I think there are a substantial number of people
> who are pretty happy about join pushdown and I expect that when and if
> we get aggregate pushdown working there will be even more people who
> are happy about that.

I think that that's Bruce's point, to a large degree.

>> Alternately, you can just work on the individual FDW features, which
>> *everyone* thinks are a good idea, and when most of them are done, FDW-based
>> scaleout will be such an obvious solution that nobody will argue with it.
>
> That's exactly what the people at EnterpriseDB who are actually doing
> work in this area are attempting to do.  Meanwhile, there's also
> Bruce, who is neither doing nor planning to do any work in this area,
> nor advising either EnterpriseDB or the PostgreSQL community to
> undertake any particular project, but who *is* making it sound like
> there is a super sekret plan that nobody else gets to see.

Is he? I didn't get that impression.

I think Bruce is trying to facilitate discussion, which can sometimes
require being a bit provocative. I think you're being quite unfair,
and mischaracterizing his words. I've heard Bruce talk about
horizontal scaling on several occasions, including at a talk in San
Francisco about a year ago, and I just thought it was Bruce being
Bruce -- primarily, a facilitator. I think that he is not especially
motivated by taking credit either here or in general, and not at all
by taking credit for other people's work.

It's not hard to get agreement about something abstract, like the
general idea of a distributed transaction manager. I fear that any
particular detailed interpretation of what that phrase means will be
very hard to get accepted into PostgreSQL.

-- 
Peter Geoghegan


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] pg_basebackup compression TODO item

[Euler Taveira]

On 03-03-2016 14:44, Magnus Hagander wrote:
> On Thu, Mar 3, 2016 at 6:34 PM, Andres Freund  > wrote:
> 
> On 2016-03-03 18:31:03 +0100, Magnus Hagander wrote:
> > I think we want it at protocol level rather than pg_basebackup level.
> 
> I think we may want both eventually, but I do agree that protocol level
> has a lot higher "priority" than that. Something like protocol level
> compression has a bit of different tradeofs than compressing base
> backups, and it's nice not to compress, uncompress, compress again.
> 
> 
> 
> Yeah, good point, we definitely want both. Based on the field experience
> I've had (which might differ from others), having it protocol level
> would help more people tough, so should be higher prio.
> 
Some time ago, I started a thread [1] to implement compression at
protocol level. The use cases are data load over slow links and reduce
bandwidth consumption during replication.

At that time, there wasn't a consensus about which compression algorithm
to choose. After the WAL compression feature, I think we can do some POC
with LZ compression (that is already available in common).

I'll try to update the code and do some benchmarks.


[1] http://www.postgresql.org/message-id/4fd9698f.2090...@timbira.com


-- 
   Euler Taveira   Timbira - http://www.timbira.com.br/
   PostgreSQL: Consultoria, Desenvolvimento, Suporte 24x7 e Treinamento


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

[HACKERS] character_not_in_repertoire vs. untranslatable_character

[Chapman Flack]

So there's an ISO error 22021 "character not in repertoire" and
a PostgreSQL error 22P05 "untranslatable character" that seem
very similar.

If I look in backend/utils/mb/wchar.c, it looks as if PostgreSQL
uses the first for the case of a corrupted encoding (bytes that
can't be decoded to a character at all), and the second for the
case of a valid character that isn't available in a conversion's
destination encoding.

Am I right about that? The names seem sort of confusable, and even
reading ISO 9075 drafts I haven't really found any additional detail
on what they wanted 22021 to mean, so I guess as long as I know what
it means in PG, that's as good as it gets. :)

-Chap


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] WIP: Upper planner pathification

[Tom Lane]

Amit Kapila  writes:
> On Sat, Mar 5, 2016 at 10:11 PM, Tom Lane  wrote:
>> Is there some reason why hash and nestloop are safe but merge isn't?

> I think it is because we consider to pushdown hash and nestloop to workers,
> but not merge join and the reason for not pushing mergejoin is that
> currently we don't have executor support for same, more work is needed
> there.

If that's true, then mergejoin paths ought to be marked parallel-unsafe
explicitly (with a comment as to why), not just silently reduced to degree
zero in a manner that looks more like an oversight than anything
intentional.

I also note that the regression tests pass with this patch and parallel
mode forced, which seems unlikely if allowing a parallel worker to execute
a join works for only two out of the three join types.  And checking the
git history for nodeHashjoin.c, nodeHash.c, and nodeNestloop.c shows no
evidence that any of those files have been touched for parallel query,
so it's pretty hard to see a reason why those would work in parallel
queries but nodeMergejoin.c not.

I still say the code as it stands is merely a copy-and-pasteo.

regards, tom lane


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Typo in psql-ref.sgml

[Magnus Hagander]

On Sun, Mar 6, 2016 at 9:38 AM, Guillaume Lelarge 
wrote:

> Hi,
>
> While translating the 9.5 ref/psql-ref.sgml, I found this:
>
> and variables shows help about about
> psql configuration variables
>
> The word "about" is written twice. Sounds like a nice typo to me :)
>
> See attached patch (for 9.5 and HEAD).
>
>
Yeah, not only was it written twice, it was written twice twice :)

Applied! Thanks!

-- 
 Magnus Hagander
 Me: http://www.hagander.net/
 Work: http://www.redpill-linpro.com/

Re: [HACKERS] Parallel Aggregate

[Haribabu Kommi]

On Fri, Mar 4, 2016 at 3:00 PM, David Rowley
 wrote:
> On 17 February 2016 at 17:50, Haribabu Kommi  wrote:
>> Here I attached a draft patch based on previous discussions. It still needs
>> better comments and optimization.
>
> Over in [1] Tom posted a large change to the grouping planner which
> causes large conflict with the parallel aggregation patch. I've been
> looking over Tom's patch and reading the related thread and I've
> observed 3 things:
>
> 1. Parallel Aggregate will be much easier to write and less code to
> base it up top of Tom's upper planner changes. The latest patch does
> add a bit of cruft (e.g create_gather_plan_from_subplan()) which won't
> be required after Tom pushes the changes to the upper planner.
> 2. If we apply parallel aggregate before Tom's upper planner changes
> go in, then Tom needs to reinvent it again when rebasing his patch.
> This seems senseless, so this is why I did this work.
> 3. Based on the thread, most people are leaning towards getting Tom's
> changes in early to allow a bit more settle time before beta, and
> perhaps also to allow other patches to go in after (e.g this)
>
> So, I've done a bit of work and I've rewritten the parallel aggregate
> code to base it on top of Tom's patch posted in [1]. There's a few
> things that are left unsolved at this stage.
>
> 1. exprType() for Aggref still returns the aggtype, where it needs to
> return the trans type for partial agg nodes, this need to return the
> trans type rather than the aggtype. I had thought I might fix this by
> adding a proxy node type that sits in the targetlist until setrefs.c
> where it can be plucked out and replaced by the Aggref. I need to
> investigate this further.
> 2. There's an outstanding bug relating to HAVING clause not seeing the
> right state of aggregation and returning wrong results. I've not had
> much time to look into this yet, but I suspect its an existing bug
> that's already in master from my combine aggregate patch. I will
> investigate this on Sunday.
>

Thanks for updating the patch. Here I attached updated patch
with the additional changes,

1. Now parallel aggregation works with expressions along with aggregate
functions also.
2. Aggref return the trans type instead of agg type, this change adds
the support of parallel aggregate to float aggregates, still it needs a
fix in _equalAggref function.

Pending:
1. Explain plan needs to be corrected for parallel grouping similar like
parallel aggregate.

To apply this patch, first apply the patch in [1]

[1] - http://www.postgresql.org/message-id/14172.1457228...@sss.pgh.pa.us

Regards,
Hari Babu
Fujitsu Australia


parallelagg_v1.patch
Description: Binary data

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

[HACKERS] Typo in psql-ref.sgml

[Guillaume Lelarge]

Hi,

While translating the 9.5 ref/psql-ref.sgml, I found this:

and variables shows help about about
psql configuration variables

The word "about" is written twice. Sounds like a nice typo to me :)

See attached patch (for 9.5 and HEAD).

Thanks.


-- 
Guillaume.
  http://blog.guillaume.lelarge.info
  http://www.dalibo.com
diff --git a/doc/src/sgml/ref/psql-ref.sgml b/doc/src/sgml/ref/psql-ref.sgml
index 6d0cb3d..f2ea63b 100644
--- a/doc/src/sgml/ref/psql-ref.sgml
+++ b/doc/src/sgml/ref/psql-ref.sgml
@@ -597,7 +597,7 @@ EOF
   explained: commands describes psql's
   backslash commands; options describes the command-line
   options that can be passed to psql;
-  and variables shows help about about psql configuration
+  and variables shows help about psql configuration
   variables.
   
   
@@ -2734,7 +2734,7 @@ testdb= \setenv LESS -imx4F
 explained: commands describes psql's
 backslash commands; options describes the command-line
 options that can be passed to psql;
-and variables shows help about about psql configuration
+and variables shows help about psql configuration
 variables.
 
 

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers