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

[Amit Langote]

Hi,

On 2016/02/16 18:25, Kyotaro HORIGUCHI wrote:
> At Tue, 16 Feb 2016 10:39:27 +0900, Amit Langote wrote:
>> On 2016/02/15 20:21, Kyotaro HORIGUCHI wrote:
>>> CREATE FUNCTION
>>>  pg_stat_get_command_progress(IN cmdtype integer)
>>>  RETURNS SETOF integer[] as $$
>>>
>>> SELECT * from pg_stat_get_command_progress(PROGRESS_COMMAND_VACUUM) as x
>>>  x
>>> -_
>>> {1233, 16233, 1, }
>>> {3244, 16236, 2, }
>>> 
>>
>> I am not sure what we would pass as argument to the (SQL) function
>> pg_stat_get_command_progress() in the system view definition for
>> individual commands - what is PROGRESS_COMMAND_VACUUM exactly? Would
>> string literals like "vacuum", "cluster", etc. to represent command names
>> work?
> 
> Sorry, it is a symbol to tell pg_stat_get_command_progress() to
> return stats numbers of backends running VACUUM. It should have
> been COMMAND_LAZY_VACUUM for this patch. If we want progress of
> CREATE INDEX, it would be COMMAND_CREATE_INDEX.

Oh I see:

CREATE VIEW pg_stat_vacuum_prgress AS
  SELECT * from pg_stat_get_command_progress(PROGRESS_COMMAND_VACUUM) as x

is actually:

CREATE VIEW pg_stat_vacuum_prgress AS
  SELECT * from pg_stat_get_command_progress(1) as x

where PROGRESS_COMMAND_VACUUM is 1 in backend code (macro, enum,
whatever).  I was confused because we never say relkind = RELKIND_INDEX in
SQL queries, :)

>> That said, there is discussion upthread about more precise reporting on
>> index vacuuming by utilizing the lazy_tid_reaped() (the index bulk delete
>> callback) as a place where we can report what index block number we are
>> at.  I think that would mean the current IndexBulkDeleteCallback signature
>> is insufficient, which is the following:
>>
>> /* Typedef for callback function to determine if a tuple is bulk-deletable */
>> typedef bool (*IndexBulkDeleteCallback) (ItemPointer itemptr, void *state);
>>
>> One more parameter would be necessary:
>>
>> typedef bool (*IndexBulkDeleteCallback) (ItemPointer itemptr, BlockNumber
>> current_index_blkno, void *state);
> 
> It could work for btree but doesn't for, for example,
> gin. ginbulkdelete finds the next page in the following way.
> 
>>  blkno = GinPageGetOpaque(page)->rightlink;
> 
> We should use another value to fagure the progress. If the
> callback is called centainly the same or around the same number
> of times with the total page numbers, the callback should just
> increment a static counter for processed pages.
> 
>> That would also require changing all the am specific vacuumpage routines
>> (like btvacuumpage) to also pass the new argument. Needless to say, some
>> bookkeeping information would also need to be kept in LVRelStats (the
>> "state" in above signature).
>>
>> Am I missing something?
> 
> So, maybe missing the case of other than btree..

More or less, the callback is called maxoffset number of times for all
index pages containing pointers to heap tuples. Robert said upthread that
counting in granularity lower than pages may not be useful:

"Let's report blocks, not tuples. The reason is that
pg_class.reltuples is only an estimate and might be wildly wrong on
occasion, but the length of the relation in blocks can be known with
certainty."

With the existing interface of the callback, it's difficult to keep the
count of pages, hence a proposal to enhance the interface. Also, now I
wonder whether scanned_index_pages will always converge to whatever
total_index_pages we get from RelationGetNumberOfBlocks(index), because
callback is not called for *every* index page and tends to differ per
index method (am). Thanks for pointing me to confirm so.

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] Freeze avoidance of very large table.

[Masahiko Sawada]

On Wed, Feb 17, 2016 at 4:29 PM, Masahiko Sawada  wrote:
> On Wed, Feb 17, 2016 at 4:08 AM, Bruce Momjian  wrote:
>> On Tue, Feb 16, 2016 at 03:57:01PM -0300, Alvaro Herrera wrote:
>>> Masahiko Sawada wrote:
>>> > On Wed, Feb 17, 2016 at 12:02 AM, Bruce Momjian  wrote:
>>> > > On Tue, Feb 16, 2016 at 11:56:25PM +0900, Masahiko Sawada wrote:
>>> > >> > I agreed on ripping out the converter plugin ability of pg_upgrade.
>>> > >> > Remember pg_upgrade was originally written by EnterpriseDB staff, 
>>> > >> > and I
>>> > >> > think they expected their closed-source fork of Postgres might need a
>>> > >> > custom page converter someday, but it never needed one, and at this
>>> > >> > point I think having the code in there is just making things more
>>> > >> > complex.  I see _no_ reason for community Postgres to use a plugin
>>> > >> > converter because we are going to need that code for every upgrade 
>>> > >> > from
>>> > >> > pre-9.6 to 9.6+, so why not just hard-code in the functions we need. 
>>> > >> >  We
>>> > >> > can remove it once 9.5 is end-of-life.
>>> > >> >
>>> > >>
>>> > >> Hm, we should rather remove the source code around PAGE_CONVERSION and
>>> > >> page.c at 9.6?
>>> > >
>>> > > Yes.  I can do it if you wish.
>>> >
>>> > I see. I understand that page-converter code would be useful for some
>>> > future cases, but makes thing more complex.
>>>
>>> If we're not going to use it, let's get rid of it right away.  There's
>>> no point in having a feature that adds complexity just because we might
>>> find some hypothetical use of it in a not-yet-imagined future.
>>
>> Agreed.  We can always add it later if we need it.
>>
>
> Attached patch gets rid of page conversion code.
>

Sorry, previous patch is incorrect..
Fixed version patch attached.

Regards,

--
Masahiko Sawada


Remove_page_conversion_from_pg_upgrade_v2.patch
Description: binary/octet-stream

-- 
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] about google summer of code 2016

[Amit Langote]

Hi Shubham,

On 2016/02/17 16:27, Shubham Barai wrote:
> Hello everyone,
> 
> I am currently pursuing my bachelor of engineering in computer science
> at Maharashtra
> Institute of Technology, Pune ,India. I am very excited about contributing
> to postgres through google summer of code program.
> 
> Is postgres   applying for gsoc 2016 as mentoring organization ?

I think it does.  Track this page for updates:
http://www.postgresql.org/developer/summerofcode/

You can contact one of the people listed on that page for the latest.

I didn't find for 2016 but here is the PostgreSQL wiki page for the last
year's GSoC page: https://wiki.postgresql.org/wiki/GSoC_2015#Project_Ideas

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] Freeze avoidance of very large table.

[Masahiko Sawada]

On Wed, Feb 17, 2016 at 4:08 AM, Bruce Momjian  wrote:
> On Tue, Feb 16, 2016 at 03:57:01PM -0300, Alvaro Herrera wrote:
>> Masahiko Sawada wrote:
>> > On Wed, Feb 17, 2016 at 12:02 AM, Bruce Momjian  wrote:
>> > > On Tue, Feb 16, 2016 at 11:56:25PM +0900, Masahiko Sawada wrote:
>> > >> > I agreed on ripping out the converter plugin ability of pg_upgrade.
>> > >> > Remember pg_upgrade was originally written by EnterpriseDB staff, and 
>> > >> > I
>> > >> > think they expected their closed-source fork of Postgres might need a
>> > >> > custom page converter someday, but it never needed one, and at this
>> > >> > point I think having the code in there is just making things more
>> > >> > complex.  I see _no_ reason for community Postgres to use a plugin
>> > >> > converter because we are going to need that code for every upgrade 
>> > >> > from
>> > >> > pre-9.6 to 9.6+, so why not just hard-code in the functions we need.  
>> > >> > We
>> > >> > can remove it once 9.5 is end-of-life.
>> > >> >
>> > >>
>> > >> Hm, we should rather remove the source code around PAGE_CONVERSION and
>> > >> page.c at 9.6?
>> > >
>> > > Yes.  I can do it if you wish.
>> >
>> > I see. I understand that page-converter code would be useful for some
>> > future cases, but makes thing more complex.
>>
>> If we're not going to use it, let's get rid of it right away.  There's
>> no point in having a feature that adds complexity just because we might
>> find some hypothetical use of it in a not-yet-imagined future.
>
> Agreed.  We can always add it later if we need it.
>

Attached patch gets rid of page conversion code.

Regards,

--
Masahiko Sawada


Remove_page_conversion_from_pg_upgrade.patch
Description: binary/octet-stream

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

[HACKERS] about google summer of code 2016

[Shubham Barai]

Hello everyone,

I am currently pursuing my bachelor of engineering in computer science
at Maharashtra
Institute of Technology, Pune ,India. I am very excited about contributing
to postgres through google summer of code program.

Is postgres   applying for gsoc 2016 as mentoring organization ?


Thanks,
Shubham Barai

Re: [HACKERS] proposal: PL/Pythonu - function ereport

[Catalin Iacob]

After I unzip the patch it doesn't apply.
patch says:
patch:  Only garbage was found in the patch input.

It's a combined diff, the git-diff manual says this about it:
Chunk header format is modified to prevent people from accidentally
feeding it to patch -p1. Combined diff format was created for review
of merge commit changes, and was not meant for apply.

Thanks for doing the test changes. It definitely shows the change is
big. The tests at least were heavily relying on both the str
conversion and on passing more than one argument. Sad face.

You're going to hate me but seeing this I changed my mind about 5,
requiring all those extra str calls is too much change in behaviour. I
was going to propose passing everything through str (so message,
detail, hint but also schema, table) but thinking about it some more,
I think I have something better.

Python 3 has keyword only arguments. It occurs to me they're exactly
for "optional extra stuff" like detail, hint etc.
Python 2 doesn't have that notion but you can kind of fake it since
you get an args tuple and a kwargs dictionary.

What about keeping the same behaviour for multiple positional
arguments (single one -> make it str, multiple -> use str of the args
tuple) and requiring users to pass detail, hint only as keyword
arguments? Code wise it would only mean adding PyObject* kw to
PLy_output and adding some code to extract detail, hint etc. from kw.
This would also allow getting rid of the GUC since backward
compatibility is fully preserved.

Again, sorry for all the back and forth but it still feels like we
haven't nailed the design to something satisfactory. All the tests you
needed to change are a hint towards that.


-- 
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] On Conflict Do nothing errors IF conflict and there is a data type length or check failure

[Amit Langote]

Hi,

On 2016/02/17 14:34, Regina Obe wrote:
> I'm guessing this is by design but just wanted to confirm that since it
> makes this feature not as useful for us.  
> 
> It also wasn't absolutely clear to me from the documentation.
> 
> We are running PostgreSQL 9.5.1 and if we do something like:
> 
> CREATE TABLE test(field1 varchar(5) primary key, field2 varchar(3));
> 
> INSERT INTO test(field1, field2) VALUES ('test','tes');
> 
> INSERT INTO test(field1,field2) VALUES('test', 'test')
> ON CONFLICT(field1) DO NOTHING;
> 
> It triggers an error:
> 
> ERROR:  value too long for type character varying(3)
> 
> I think it does this for check constraints too.
> 
> 
> Even though the record under consideration would be thrown out anyway.

I think the error occurs long before it would/could be determined that it
won't be inserted anyway (the latter being execution time). It would be
quite out-of-place to document such behaviors under ON CONFLICT or check
constraints description, IMHO.

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] Figures in docs

[Tatsuo Ishii]

> Hi. 
> 
> In DocBook 4.2 sgml dtd, figure tag is supported already.
> that was implemented for multi output format.

Ok, there's no technical problems with figures then.  MySQL docs has
some nice figures. I am jealous.

> I remember that very old postgresql document has some picture (eg.
> system architecture, ERD ...).

So it seems to be a matter of policy? At some point we prefer not to
include figures, maybe.

> when release new version, these might be changed, nevertheless these can
> not been.
>
> this proposer is not about xml or sgml.
> how can new figures be modified for new documents.

Keep the source files (LibreOffice Draw for example).

> regards ioseph.
>  
> 2016-02-17 (수), 08:26 +0300, Oleg Bartunov:
>> 
>> 
>> On Wed, Feb 17, 2016 at 4:17 AM, Tatsuo Ishii 
>> wrote:
>> It seems there's no figures/diagrams in our docs. I vaguely
>> recall that
>> we used to have a few diagrams in our docs. If so, was there
>> any
>> technical reason to remove them?
>> 
>> I don't know the reason, but it's shame, we are still in sgml ! 
>> 
>> We already do our translations (as others) in xml using custom
>> scripting.  xml provides us better integration with available tools
>> and ability to insert graphics. Last time we asked in -docs about
>> moving to xml and Alexander demonstrated acceptable speed of xml
>> build, but there were no reply from Peter, who is (?) responsible for
>> our documentation infrastructure. Probably, we should just created a
>> patch and submit to commitfest.  You can check this thread
>> http://www.postgresql.org/message-id/1428009501118.85...@postgrespro.ru
>> 
>> 
>> 
>> Best regards,
>> --
>> Tatsuo Ishii
>> SRA OSS, Inc. Japan
>> English: http://www.sraoss.co.jp/index_en.php
>> Japanese:http://www.sraoss.co.jp
>> 
>> 
>> --
>> Sent via pgsql-hackers mailing list
>> (pgsql-hackers@postgresql.org)
>> To make changes to your subscription:
>> http://www.postgresql.org/mailpref/pgsql-hackers
>> 
>> 
> 
> 


-- 
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] Bug in StartupSUBTRANS

[Michael Paquier]

On Sun, Feb 14, 2016 at 9:03 AM, Jeff Janes  wrote:
> I've repeated the crash/recovery/wrap-around testing with the proposed
> fix in place, and this pre-existing problem seems to be fixed now, and
> I have found no other problems.
>
> I've attached a new version, incorporating comments from Tom and Michael.

Thanks, this patch looks good to me this way.

I have added an entry in the CF app to not lose track of this bug:
https://commitfest.postgresql.org/9/526/
That's the most I can do.
-- 
Michael


-- 
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] Figures in docs

[Ioseph Kim]

Hi. 

In DocBook 4.2 sgml dtd, figure tag is supported already.
that was implemented for multi output format.

I remember that very old postgresql document has some picture (eg.
system architecture, ERD ...).
when release new version, these might be changed, nevertheless these can
not been.

this proposer is not about xml or sgml.
how can new figures be modified for new documents.

regards ioseph.
 
2016-02-17 (수), 08:26 +0300, Oleg Bartunov:
> 
> 
> On Wed, Feb 17, 2016 at 4:17 AM, Tatsuo Ishii 
> wrote:
> It seems there's no figures/diagrams in our docs. I vaguely
> recall that
> we used to have a few diagrams in our docs. If so, was there
> any
> technical reason to remove them?
> 
> I don't know the reason, but it's shame, we are still in sgml ! 
> 
> We already do our translations (as others) in xml using custom
> scripting.  xml provides us better integration with available tools
> and ability to insert graphics. Last time we asked in -docs about
> moving to xml and Alexander demonstrated acceptable speed of xml
> build, but there were no reply from Peter, who is (?) responsible for
> our documentation infrastructure. Probably, we should just created a
> patch and submit to commitfest.  You can check this thread
> http://www.postgresql.org/message-id/1428009501118.85...@postgrespro.ru
> 
> 
> 
> Best regards,
> --
> Tatsuo Ishii
> SRA OSS, Inc. Japan
> English: http://www.sraoss.co.jp/index_en.php
> Japanese:http://www.sraoss.co.jp
> 
> 
> --
> Sent via pgsql-hackers mailing list
> (pgsql-hackers@postgresql.org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-hackers
> 
> 




-- 
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] planstats.sgml

[Mark Kirkwood]

On 16/02/16 20:01, Tatsuo Ishii wrote:

Tatsuo Ishii  writes:

While reading planstats.sgml, I encounted a sentence which I don't
understand.



 These numbers are current as of the last VACUUM or
 ANALYZE on the table.  The planner then fetches the
 actual current number of pages in the table (this is a cheap operation,
 not requiring a table scan).  If that is different from
 relpages then
 reltuples is scaled accordingly to
 arrive at a current number-of-rows estimate.  In this case the value of
 relpages is up-to-date so the rows estimate is
 the same as reltuples.



I don't understand the last sentence (In this case...). For me it
seems it is talking about the case when replages is not different from
what the planner fetches from the table. If so, why "In this case"?


I think what it meant is "In the example above, ...".  Feel free to
change it if you think that is clearer.


Oh, I see. I'm going to change "In this case" to "In the example above".


Done.



Yeah, that is clearer.



--
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] extend pgbench expressions with functions

[Michael Paquier]

On Tue, Feb 16, 2016 at 11:12 PM, Fabien COELHO  wrote:
>
> Hello Robert,
>
>>> However, for obvious reasons the committer opinion prevails:-)
>>
>>
>> You're welcome to solicit other opinions.  I'm not unwilling to give
>> way if there's a chorus of support for the way you've done it.
>
>
> Hmmm. I do not expect much chorus on such a minor subject:-)
>
>> But to me it sounds like you're saying that it doesn't really matter
>> whether the system is scalable and maintainable because we only have 5
>> functions right now, which is a design philosophy with which I just don't
>> agree.
>
>
> The design does not aim at scalability but at simplicity, otherwise I would
> have done things quite differently: with many functions the "switch" based
> selection does not scale anyway.
>
> Anyway, attached are two patches, one for each approach.
>
> The array (second patch) is not too bad if one agrees with a maximum number
> of arguments, and also as I did not change the list structure coming from
> the parser, so it does not need to manage the number of arguments in the
> function structure. The code for min/max is also simpler when dealing with
> an array instead of a linked list. I do not like much array references in
> the code, so I tried to avoid them by using lval/rval scalars for operator
> operands.
>
> Please choose the one you prefer, and I'll adapt the remaining stuff to your
> choice.

For those two patches and HEAD, it is possible to do a direct
performance comparison for simple operator functions, as those are
being switched to behave as functions. So doing the following for 50M
"transactions" on my laptop:
\set aid 1 + 1
pgbench -f addition.sql -t 5000

I have the following:
HEAD: 3.5~3.7M TPS
list method: 3.6~3.7M TPS
array method: 3.4~3.5M TPS
So all approaches have a comparable performance.

Btw, patch 2 is returning a warning for me:
pgbench.c:1060:16: warning: comparison of constant
-9223372036854775808 with expression of type 'int' is always false
[-Wtautological-constant-out-of-range-compare]
if (lval == PG_INT64_MIN)
It is trying to compare a 32b integer with an int64 value, evalFunc
needed an int64.
-- 
Michael


-- 
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] Figures in docs

[Tatsuo Ishii]

> On Wed, Feb 17, 2016 at 4:17 AM, Tatsuo Ishii  wrote:
> 
>> It seems there's no figures/diagrams in our docs. I vaguely recall that
>> we used to have a few diagrams in our docs. If so, was there any
>> technical reason to remove them?
>>
> 
> I don't know the reason, but it's shame, we are still in sgml !
> 
> We already do our translations (as others) in xml using custom scripting.
> xml provides us better integration with available tools and ability to
> insert graphics. Last time we asked in -docs about moving to xml and
> Alexander demonstrated acceptable speed of xml build, but there were no
> reply from Peter, who is (?) responsible for our documentation
> infrastructure. Probably, we should just created a patch and submit to
> commitfest.  You can check this thread
> http://www.postgresql.org/message-id/1428009501118.85...@postgrespro.ru

Well, there are some PostgreSQL doc translation projects are running
including translation for Japanese, which I am working on.

If we are going to change the manual format and/or the build system, I
need to confirm it does work for Japanese as well. In theory because
the Japanese translation project uses UTF-8, there should be no
problem as far as the whole build system works for UTF-8. But I want
to confirm first...

BTW, are you going to propose a mega patch which changes all the sgml
files to xml files?

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


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

[HACKERS] On Conflict Do nothing errors IF conflict and there is a data type length or check failure

[Regina Obe]

I'm guessing this is by design but just wanted to confirm that since it
makes this feature not as useful for us.  

It also wasn't absolutely clear to me from the documentation.

We are running PostgreSQL 9.5.1 and if we do something like:

CREATE TABLE test(field1 varchar(5) primary key, field2 varchar(3));

INSERT INTO test(field1, field2) VALUES ('test','tes');

INSERT INTO test(field1,field2) VALUES('test', 'test')
ON CONFLICT(field1) DO NOTHING;

It triggers an error:

ERROR:  value too long for type character varying(3)

I think it does this for check constraints too.


Even though the record under consideration would be thrown out anyway.


Thanks,
Regina







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

[HACKERS] Fix handling of invalid sockets returned by PQsocket()

[Michael Paquier]

Hi all,

After looking at Alvaro's message mentioning the handling of
PQsocket() for invalid sockets, I just had a look by curiosity at
other calls of this routine, and found a couple of issues:
1) In vacuumdb.c, init_slot() does not check for the return value of PQsocket():
slot->sock = PQsocket(conn);
2) In isolationtester.c, try_complete_step() should do the same.
3) In pg_recvlogical.c for StreamLogicalLog() I am spotting the same problem.
I guess those ones should be fixed as well, no?

The patch attached addresses those issues.

This has been raised in this message, but beginning a new thread makes
more sense:
http://www.postgresql.org/message-id/cab7npqttzoiuvygnonlvnzysstusofhkeo9ftrqbkwj36uc...@mail.gmail.com
Regards,
-- 
Michael
diff --git a/src/bin/pg_basebackup/pg_recvlogical.c b/src/bin/pg_basebackup/pg_recvlogical.c
index 832f9f9..b4325b0 100644
--- a/src/bin/pg_basebackup/pg_recvlogical.c
+++ b/src/bin/pg_basebackup/pg_recvlogical.c
@@ -360,6 +360,14 @@ StreamLogicalLog(void)
 			struct timeval timeout;
 			struct timeval *timeoutptr = NULL;
 
+			if (PQsocket(conn) < 0)
+			{
+fprintf(stderr,
+		_("%s: bad socket: \"%s\"\n"),
+		progname, strerror(errno));
+goto error;
+			}
+
 			FD_ZERO(_mask);
 			FD_SET(PQsocket(conn), _mask);
 
diff --git a/src/bin/scripts/vacuumdb.c b/src/bin/scripts/vacuumdb.c
index c6afcd5..e81123f 100644
--- a/src/bin/scripts/vacuumdb.c
+++ b/src/bin/scripts/vacuumdb.c
@@ -70,7 +70,7 @@ static void DisconnectDatabase(ParallelSlot *slot);
 
 static int	select_loop(int maxFd, fd_set *workerset, bool *aborting);
 
-static void init_slot(ParallelSlot *slot, PGconn *conn);
+static void init_slot(ParallelSlot *slot, PGconn *conn, const char *progname);
 
 static void help(const char *progname);
 
@@ -421,14 +421,14 @@ vacuum_one_database(const char *dbname, vacuumingOptions *vacopts,
 	 * array contains the connection.
 	 */
 	slots = (ParallelSlot *) pg_malloc(sizeof(ParallelSlot) * concurrentCons);
-	init_slot(slots, conn);
+	init_slot(slots, conn, progname);
 	if (parallel)
 	{
 		for (i = 1; i < concurrentCons; i++)
 		{
 			conn = connectDatabase(dbname, host, port, username, prompt_password,
    progname, false, true);
-			init_slot(slots + i, conn);
+			init_slot(slots + i, conn, progname);
 		}
 	}
 
@@ -917,11 +917,18 @@ select_loop(int maxFd, fd_set *workerset, bool *aborting)
 }
 
 static void
-init_slot(ParallelSlot *slot, PGconn *conn)
+init_slot(ParallelSlot *slot, PGconn *conn, const char *progname)
 {
 	slot->connection = conn;
 	slot->isFree = true;
 	slot->sock = PQsocket(conn);
+
+	if (slot->sock < 0)
+	{
+		fprintf(stderr, _("%s: bad socket: %s\n"), progname,
+strerror(errno));
+		exit(1);
+	}
 }
 
 static void
diff --git a/src/test/isolation/isolationtester.c b/src/test/isolation/isolationtester.c
index 0a9d25c..3e13a39 100644
--- a/src/test/isolation/isolationtester.c
+++ b/src/test/isolation/isolationtester.c
@@ -720,6 +720,12 @@ try_complete_step(Step *step, int flags)
 	PGresult   *res;
 	bool		canceled = false;
 
+	if (sock < 0)
+	{
+		fprintf(stderr, "bad socket: %s\n", strerror(errno));
+		exit_nicely();
+	}
+
 	gettimeofday(_time, NULL);
 	FD_ZERO(_set);
 

-- 
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] Figures in docs

[Oleg Bartunov]

On Wed, Feb 17, 2016 at 4:17 AM, Tatsuo Ishii  wrote:

> It seems there's no figures/diagrams in our docs. I vaguely recall that
> we used to have a few diagrams in our docs. If so, was there any
> technical reason to remove them?
>

I don't know the reason, but it's shame, we are still in sgml !

We already do our translations (as others) in xml using custom scripting.
xml provides us better integration with available tools and ability to
insert graphics. Last time we asked in -docs about moving to xml and
Alexander demonstrated acceptable speed of xml build, but there were no
reply from Peter, who is (?) responsible for our documentation
infrastructure. Probably, we should just created a patch and submit to
commitfest.  You can check this thread
http://www.postgresql.org/message-id/1428009501118.85...@postgrespro.ru


> Best regards,
> --
> Tatsuo Ishii
> SRA OSS, Inc. Japan
> English: http://www.sraoss.co.jp/index_en.php
> Japanese:http://www.sraoss.co.jp
>
>
> --
> 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] tsearch_extras extension

[Oleg Bartunov]

On Wed, Feb 17, 2016 at 6:57 AM, Tim Abbott  wrote:

> Just following up here since I haven't gotten a reply -- I'd love to work
> with someone from the Postgres community on a plan to make the
> tsearch_extras functionality available as part of mainline postgres.
>
>
> -Tim Abbott
>
> On Wed, Feb 3, 2016 at 9:41 PM, Tim Abbott  wrote:
>
>> Hello,
>>
>> I'm a maintainer of the Zulip open source group chat application.  Zulip
>> depends on a small (~200 lines) postgres extension called tsearch_extras (
>> https://github.com/zbenjamin/tsearch_extras) that returns the actual
>> (offset, length) pairs of all the matches for a postgres full text search
>> query.  This extension allows Zulip to do its own highlighting of the full
>> text search matches, using a more complicated method than what Postgres
>> supports natively.
>>
>> I think tsearch_extras is probably of value to others using postgres
>> full-text search (and I'd certainly love to get out of the business of
>> maintaining an out-of-tree postgres extension), so I'm wondering if this
>> feature (or a variant of it) would be of interest to upstream?
>>
>> Thanks!
>>
>> -Tim Abbott
>>
>> (See
>> http://www.postgresql.org/message-id/flat/52c7186d.8010...@strangersgate.com#52c7186d.8010...@strangersgate.com
>> for the discussion on postgres mailing lists that caused us to write this
>> module in the first place.)
>>
>
Tim,

take a look on this patch (https://commitfest.postgresql.org/7/385/) and
contact author.  It contains everything you need to your purposes.

btw, Stas, check on status "Returned with feedback" !


Regards,
Oleg

Re: [HACKERS] Bug with int2

[Feng Tian]

Ah, thanks!

On Tue, Feb 16, 2016 at 7:54 PM, Peter Geoghegan  wrote:

> On Tue, Feb 16, 2016 at 7:27 PM, Feng Tian  wrote:
> > I run into the following.   Seems this is a bug for -32768, which should
> be
> > a valid smallint value.
>
> This isn't a bug. You see the error only due to operator precedence:
>
> postgres=# select (-32768)::int2;
>   int2
> ─
>  -32,768
> (1 row)
>
> --
> Peter Geoghegan
>

Re: [HACKERS] proposal: PL/Pythonu - function ereport

[Pavel Stehule]

I noticed the patch isn't registered in the March CF, please do that.
> It would be a pity to miss 9.6 because of not registering it in time.
>

https://commitfest.postgresql.org/9/525/

Regards

Pavel

[HACKERS] commitfest application doesn't see new patch

[Pavel Stehule]

Hi

In a entry https://commitfest.postgresql.org/9/525/ the commitfest
application show last patch

Latest attachment (incompat.py) at 2016-02-04 09:13:55 from Catalin Iacob


although last patch is from yesterday

Attachment: plpython-enhanced-error-02.patch.gz

Regards

Pavel

Re: [HACKERS] proposal: function parse_ident

[Pavel Stehule]

2016-02-17 1:38 GMT+01:00 Jim Nasby :

> On 2/11/16 1:27 AM, Pavel Stehule wrote:
>
>> I editorialized the docs and some of the comments. In particular, I
>> documented behavior of not truncating, and recommended casting to
>> name[] if user cares about that. Added a unit test to verify that
>> works. BTW, I saw mention in the thread about not truncated spaces,
>> but the function *does* truncate them, unless they're inside quotes,
>> where they're legitimate.
>>
>>
>> ok
>>
>
> I missed some of my edits. Updated patch with those in place attached.
>
> Also added test for invalid characters.
>>
>> I think "strict" would be more in line with other uses in code.
>> There are currently no other occurrences of 'strictmode' in the
>> code. There are loads of references to 'strict', but I didn't go
>> through all of them to see if any were used as externally visible
>> function parameter names.
>>
>>
>> I am sorry, I don't understand to this point. You unlike the name of
>> parameter "strictmode" ? Have you any proposal? Maybe "restrictive" ?
>>
>
> I would just call it strict. There's precedent for that in the code.
>

+1

fixed in attached patch


>
> The almost all code +/- is related to human readable error messages. We
>> can move some code to separate static functions - read_quoted_ident,
>> read_unquoted_ident, but there will be some magic about parameters, and
>> the code will not be much better, than it is now.
>>
>
> What I'm saying is that most places that need to do de-quoting or similar
> just run a simple while loop and use an in_quote variable to track whether
> they're inside a quote or not. See backend/utils/adt/rowtypes.c line 199
> for an example.
>
> As I said, I don't have a strong opinion on it, so if you prefer it this
> way that's fine with me.


yes, I don't see string differences between for(;;) and break and
while(var). I prefer current state.

Regards

Pavel


>
> --
> Jim Nasby, Data Architect, Blue Treble Consulting, Austin TX
> Experts in Analytics, Data Architecture and PostgreSQL
> Data in Trouble? Get it in Treble! http://BlueTreble.com
>
diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
new file mode 100644
index f9eea76..9eed19a
*** a/doc/src/sgml/func.sgml
--- b/doc/src/sgml/func.sgml
***
*** 1821,1826 
--- 1821,1847 

 
  
+  parse_ident
+ 
+ parse_ident(str text,
+[ strictmode boolean DEFAULT true ] )
+
+text[]
+Split qualified identifier into array parts.
+When strictmode is false, extra characters after the identifier are ignored.
+This is useful for parsing identifiers for objects like functions and arrays that may have trailing
+characters. By default, extra characters after the last identifier are considered an error.
+second parameter is false, then chars after last identifier are ignored. Note that this function
+does not truncate quoted identifiers. If you care about that you should cast the result of this
+ 	   function to name[].
+
+parse_ident('"SomeSchema".someTable')
+"SomeSchema,sometable"
+   
+ 
+   
+
+ 
   pg_client_encoding
  
  pg_client_encoding()
diff --git a/src/backend/catalog/system_views.sql b/src/backend/catalog/system_views.sql
new file mode 100644
index 923fe58..9a65bc9
*** a/src/backend/catalog/system_views.sql
--- b/src/backend/catalog/system_views.sql
*** RETURNS jsonb
*** 965,967 
--- 965,974 
  LANGUAGE INTERNAL
  STRICT IMMUTABLE
  AS 'jsonb_set';
+ 
+ CREATE OR REPLACE FUNCTION
+   parse_ident(str text, strict boolean DEFAULT true)
+ RETURNS text[]
+ LANGUAGE INTERNAL
+ STRICT IMMUTABLE
+ AS 'parse_ident';
diff --git a/src/backend/parser/scansup.c b/src/backend/parser/scansup.c
new file mode 100644
index 2b4ab20..7aa5b76
*** a/src/backend/parser/scansup.c
--- b/src/backend/parser/scansup.c
*** scanstr(const char *s)
*** 130,135 
--- 130,144 
  char *
  downcase_truncate_identifier(const char *ident, int len, bool warn)
  {
+ 	return downcase_identifier(ident, len, warn, true);
+ }
+ 
+ /*
+  * a workhorse for downcase_truncate_identifier
+  */
+ char *
+ downcase_identifier(const char *ident, int len, bool warn, bool truncate)
+ {
  	char	   *result;
  	int			i;
  	bool		enc_is_single_byte;
*** downcase_truncate_identifier(const char
*** 158,169 
  	}
  	result[i] = '\0';
  
! 	if (i >= NAMEDATALEN)
  		truncate_identifier(result, i, warn);
  
  	return result;
  }
  
  /*
   * truncate_identifier() --- truncate an identifier to NAMEDATALEN-1 bytes.
   *
--- 167,179 
  	}
  	result[i] = '\0';
  
! 	if (i >= NAMEDATALEN && truncate)
  		truncate_identifier(result, i, warn);
  
  	return result;
  }
  
+ 
  /*
   * truncate_identifier() --- truncate an identifier to NAMEDATALEN-1 

Re: [HACKERS] Parallel Aggregate

[Haribabu Kommi]

On Sat, Feb 13, 2016 at 3:51 PM, Robert Haas  wrote:
> On Sun, Feb 7, 2016 at 8:21 PM, Haribabu Kommi
>  wrote:future.
>> Here I attached updated patch with the corrections.
>
> So, what about the main patch, for parallel aggregation itself?  I'm
> reluctant to spend too much time massaging combine functions if we
> don't have the infrastructure to use them.

Here I attached a draft patch based on previous discussions. It still needs
better comments and optimization.

Overview:
1. Before creating the plan for the best path, verify whether parallel aggregate
plan is possible or not? If possible check whether it is the cheapest plan
compared to normal aggregate? If parallel is cheaper then replace the best
path with the cheapest_partial_path.

2. while generating parallel aggregate plan, first generate targetlist of
partial aggregate by generating bare aggregate references and group by
expressions.

3. Change the aggref->aggtype with aggtranstype in the partial aggregate
targetlist to return a proper tuple data from worker.

4. Generate partial aggregate node using the generated targetlist.

5. Add gather and finalize aggregate nodes on top of partial aggregate plan.

To do:
1. Optimize the aggregate cost calculation mechanism, currently it is used
many times.
2. Better comments and etc.

Please verify whether the patch is in the right direction as per your
expectation?

Regards,
Hari Babu
Fujitsu Australia


parallelagg_poc_v7.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] Re: [COMMITTERS] pgsql: Introduce group locking to prevent parallel processes from deadl

[Tom Lane]

Robert Haas  writes:
> Hmm, that's true.  I don't think it actually matters all that much,
> because proclock->tag.myProc->lockGroupLeader == NULL has pretty much
> the same effect as if proclock->tag.myProc->lockGroupLeader ==
> proclock->tag.myProc.  But not completely.  One problem is that we
> don't currently assume that 8-byte writes are atomic, so somebody
> might see the group leader field half-set, which would be bad.

Yes, exactly.  I'm not certain if there are any real platforms where
a pointer-sized write wouldn't be atomic (it sure sounds inefficient
for that to be true), but we have not assumed that to date and I'd
just as soon not start here.

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] Re: [COMMITTERS] pgsql: Introduce group locking to prevent parallel processes from deadl

[Robert Haas]

On Tue, Feb 16, 2016 at 3:54 PM, Tom Lane  wrote:
> Robert Haas  writes:
>> 1. It removes the groupLeader flag from PGPROC.  You're right: we don't need 
>> it.
>
> It occurs to me that this claim is bogus:
>
>> We institute a further coding rule that a process cannot join or leave a lock
>> group while owning any PROCLOCK.  Therefore, given a lock manager lock
>> sufficient to examine PROCLOCK *proclock, it also safe to examine
>> proclock->tag.myProc->lockGroupLeader (but not the other fields mentioned in
>> the previous paragraph).
>
> Yes, we can legislate that workers have to join before taking any lock,
> and if processes only leave the group at death then that end of it is not
> a problem either; but it is patently not the case that a process will hold
> no locks when it calls BecomeLockGroupLeader().
>
> We may be able to salvage this simplification anyway, but it will require
> a tighter specification of when it's safe to look at
> proclock->tag.myProc->lockGroupLeader.
>
> Another possibility is to forget about executor-time
> BecomeLockGroupLeader(), and just say that any process that isn't a worker
> always becomes its own group leader at startup.  Then the above para is
> true as written.  I don't know what downsides this might have; do your
> changes in the lock manager try to optimize the null-lockGroupLeader case?

Hmm, that's true.  I don't think it actually matters all that much,
because proclock->tag.myProc->lockGroupLeader == NULL has pretty much
the same effect as if proclock->tag.myProc->lockGroupLeader ==
proclock->tag.myProc.  But not completely.  One problem is that we
don't currently assume that 8-byte writes are atomic, so somebody
might see the group leader field half-set, which would be bad.

But, yes, there are some optimizations for that case.  For example, in
LockCheckConflicts:

/* If no group locking, it's definitely a conflict. */
if (leader == NULL)
{
PROCLOCK_PRINT("LockCheckConflicts: conflicting (simple)",
   proclock);
return STATUS_FOUND;
}

ProcSleep also has a check of this sort.  In theory those
optimizations are quite important, because they can avoid extra passes
over the lock queue which theoretically turn O(N) algorithms into
O(N^2) or O(N^2) into O(N^3) or whatever.  But in practice I'm not
sure that the lock queues are ever long enough for that to become an
issue.  And if they are, the fact that your system is lock-bound is
probably a much bigger problem that a few extra CPU cycles inside the
lock manager to figure that out.  But I don't want to be too flip
about that analysis - there might be some scenario where the extra
cycles do hurt.

-- 
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: Commitfest Bug (was: [HACKERS] Re: Reusing abbreviated keys during second pass of ordered [set] aggregates)

[Pavel Stehule]

2016-02-17 3:19 GMT+01:00 Jim Nasby :

> On 2/16/16 12:38 AM, Michael Paquier wrote:
>
>> When a patch with status "Ready for committer" on CF N is moved to CF
>> (N+1), its status is automatically changed to "Needs Review". That's
>> something to be aware of when cleaning up the CF app entries.
>>
>
> I agree with Alvarro; this seems like a bug to me. If a patch is ready for
> committer in CF N, why would it suddenly not be ready in N+1?
>

+1,

This behave is pretty unpleasant and frustrating.

Regards

Pavel


> --
> Jim Nasby, Data Architect, Blue Treble Consulting, Austin TX
> Experts in Analytics, Data Architecture and PostgreSQL
> Data in Trouble? Get it in Treble! http://BlueTreble.com
>
>
> --
> 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] Re: [COMMITTERS] pgsql: Introduce group locking to prevent parallel processes from deadl

[Robert Haas]

On Tue, Feb 16, 2016 at 8:37 PM, Craig Ringer  wrote:
> On 14 February 2016 at 08:05, Robert Haas  wrote:
>> The concept of a
>> lock group is formally separate from the concept of a parallel group
>> created by a ParallelContext, but it is not clear that there will ever
>> be any other context in which a lock group will be a good idea.
>
> Just coming back to this in terms of what Stephen and I raised: Robert, do
> you think this design as it stands can handle cases where a normal
> standalone backend gets promoted to a lock-group leader that others can then
> join?

That's the exact intended purpose of it.

-- 
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] proposal: enhancing slow query log, and autoexplain log about waiting on lock before query exec time

[Pavel Stehule]

2016-02-17 3:43 GMT+01:00 Jim Nasby :

> On 2/14/16 11:24 AM, Pavel Stehule wrote:
>
>> > We have a patch, that inject logs about the time waiting on locks
>> before
>> > query execution. This feature helps us lot of, and I hope, it can be
>> > generally useful.
>>
>> Doesn't log_lock_waits cover that territory already?
>>
>> It does. But It creates different log entry - and can be hard to join
>> slow query with log entry sometimes lot of lines before. This proposal
>> is about taking important information comfortably - and log parsing and
>> processing is simpler.
>>
>
> I'm all for anything that improves visibility into locking, but it seems
> like this is more a band-aid than a fix. Certainly any real analysis of
> logfiles means you're stuck with something like pgBadger. If this would
> significantly simply pgBadger's job then great, but I don't think that's
> the case.
>
> What would be useful logging-wise is if the log line for the query itself
> could contain lock wait time, but that doesn't sound like what you're
> proposing?
>

I hope, so I propose this idea. First time I wanted talk about the idea.
Next step is the talk about format.


>
> What I think would be far more useful is adding lock wait time info to
> pg_stat_statements and maybe pg_stat_*_tables.


If we can enhance primary log, auto_explain, then we can do same with
pg_stat_statements.

lock statistics in table or database level would be great - it is good
simple indicator about application health, but it is for another proposal
(and patch). I can propose it, or I can collaborate on it with pleasure.

Regards

Pavel



>
> --
> Jim Nasby, Data Architect, Blue Treble Consulting, Austin TX
> Experts in Analytics, Data Architecture and PostgreSQL
> Data in Trouble? Get it in Treble! http://BlueTreble.com
>

Re: [HACKERS] tsearch_extras extension

[Tim Abbott]

Just following up here since I haven't gotten a reply -- I'd love to work
with someone from the Postgres community on a plan to make the
tsearch_extras functionality available as part of mainline postgres.


-Tim Abbott

On Wed, Feb 3, 2016 at 9:41 PM, Tim Abbott  wrote:

> Hello,
>
> I'm a maintainer of the Zulip open source group chat application.  Zulip
> depends on a small (~200 lines) postgres extension called tsearch_extras (
> https://github.com/zbenjamin/tsearch_extras) that returns the actual
> (offset, length) pairs of all the matches for a postgres full text search
> query.  This extension allows Zulip to do its own highlighting of the full
> text search matches, using a more complicated method than what Postgres
> supports natively.
>
> I think tsearch_extras is probably of value to others using postgres
> full-text search (and I'd certainly love to get out of the business of
> maintaining an out-of-tree postgres extension), so I'm wondering if this
> feature (or a variant of it) would be of interest to upstream?
>
> Thanks!
>
> -Tim Abbott
>
> (See
> http://www.postgresql.org/message-id/flat/52c7186d.8010...@strangersgate.com#52c7186d.8010...@strangersgate.com
> for the discussion on postgres mailing lists that caused us to write this
> module in the first place.)
>
>

Re: [HACKERS] Bug with int2

[Peter Geoghegan]

On Tue, Feb 16, 2016 at 7:27 PM, Feng Tian  wrote:
> I run into the following.   Seems this is a bug for -32768, which should be
> a valid smallint value.

This isn't a bug. You see the error only due to operator precedence:

postgres=# select (-32768)::int2;
  int2
─
 -32,768
(1 row)

-- 
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] Bug with int2

[Michael Paquier]

On Wed, Feb 17, 2016 at 12:27 PM, Feng Tian  wrote:
> ftian=# select -32768::int2;
> ERROR:  smallint out of range

But 32768 is not. You should just use parenthesis, a cast does not
take into account the minus sign here:
=# select (-32768)::int2;
  int2

 -32768
(1 row)
-- 
Michael


-- 
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] Bug with int2

[Tom Lane]

Feng Tian  writes:
> I run into the following.   Seems this is a bug for -32768, which should be
> a valid smallint value.

> ftian=# select -32768::int2;
> ERROR:  smallint out of range

You have the wrong idea about the precedence of those operators.
"select (-32768)::int2" works.

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] How are CREATE EXTENSION ... VERSION or ALTER EXTENSION ... UPDATE TO ... intended to work?

[Tom Lane]

Chapman Flack  writes:
> I've been looking a little more deeply at the extension mechanism,
> trying to answer my own question about what happens once there have
> been several releases of an extension, and the extensions directory
> is now populated with a bunch of files quux--1.0.sql, quux--1.1.sql,
> quux--1.0--1.1.sql, quux--1.1--1.0.sql, ..., quux.control.
> And somewhere in $libdir there are quux-1.0.so, quux-1.1.so.

Well, at least so far as the existing extensions in contrib are concerned,
there are *not* version numbers in the .so filenames.  This means you
can't have more than one version of the .so installed at once, but we've
not really found a need for that.  It's usually feasible --- and desirable
--- to keep ABI compatibility to the extent that the new .so can be
swapped in for the old without needing to change the SQL function
definitions immediately.

I agree that MODULE_PATHNAME isn't an adequate mechanism if you want
to have version numbers in the .so filenames.  We could think about
providing some solution for that, perhaps along the lines of a %v
escape such as you suggest, but it would take awhile for that to get
into the field.  Avoiding MODULE_PATHNAME in favor of writing out
the versioned .so name in the .sql files is probably the path of
least resistance.

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

[HACKERS] Bug with int2

[Feng Tian]

I run into the following.   Seems this is a bug for -32768, which should be
a valid smallint value.

Test was run on 9.4.5.

Thanks,
Feng

ftian=# select 32767::int2;


 int2
---
 32767
(1 row)

ftian=# select -32767::int2;
 ?column?
--
   -32767
(1 row)

ftian=# select 32768::int2;


ERROR:  smallint out of range
ftian=# select -32768::int2;
ERROR:  smallint out of range
ftian=#

[HACKERS] How are CREATE EXTENSION ... VERSION or ALTER EXTENSION ... UPDATE TO ... intended to work?

[Chapman Flack]

I've been looking a little more deeply at the extension mechanism,
trying to answer my own question about what happens once there have
been several releases of an extension, and the extensions directory
is now populated with a bunch of files quux--1.0.sql, quux--1.1.sql,
quux--1.0--1.1.sql, quux--1.1--1.0.sql, ..., quux.control.
And somewhere in $libdir there are quux-1.0.so, quux-1.1.so.

The .sql scripts pretty much all CREATE OR REPLACE the function quux()
AS 'MODULE_PATHNAME', 'quux', and module_pathname is set in quux.control.
That file was most recently written when quux-1.1 was installed, so it
defines module_pathname as $libdir/quux-1.1 and the default_version is 1.1.

So it's clear how a plain CREATE EXTENSION quux; works. No question there.
But what is intended to happen if I want to CREATE EXTENSION quux
VERSION 1.0; ?

If there is still a file quux--1.0.sql in extensions/ (which there
may very well be), it will be executed. But it may still contain
CREATE OR REPLACE FUNCTION quux() ... AS 'MODULE_PATHNAME', 'quux'
(which was, after all, correct back when that was the current release)
but now the definition in the current quux.control will cause
MODULE_PATHNAME to expand to quux-1.1.so, not 1.0 as expected.

At least, that's what would happen if each new quux release just ships
new extension files (a new quux.control, new main quux--1.x.sql, and a
few quux--1.y--1.x.sql update files), but does not overwrite the older
quux--1.y.sql files the site may already have in the extensions/ directory.

A workaround could be that each new quux release installer either removes
all pre-existing older quux--*.sql files (so then you would never have
the option to downgrade or create from an older version) ... or overwrites
them all with new versions that hardcode the older shared-object names
instead of using the magic MODULE_PATHNAME. A sort of 'freeze' operation.

That seems to lead to a situation where the *simple* part of the extension
build script is the part that actually builds the extension, along with the
.control file and the new main quux--1.x.sql file (which can just be a
boilerplate with the version injected in two places, and otherwise never
change between releases), while a steadily-growing part of the build script
will only be there to generate overwriting versions of older .sql files
that have their corresponding older module pathnames hardcoded in.

OR ... avoid using module_pathname and just generate every .sql file
with the correct pathname injected ... then older files don't have to
be overwritten when newer versions of the extension are installed, the
previously-installed ones can just be left in place, and they will always
refer to the correct version of the module. That would work, but leave
module_pathname rather useless.

It seems to me that maybe this scheme is missing something like a
%v substitution that can be specified in the .control file as part
of the module_pathname value:

module_pathname = '$libdir/quux-%v'

which the system would expand to the effective version string whenever
substituting MODULE_PATHNAME into any of the .sql scripts.

"Effective version string" would need careful attention; I think if the
upgrade planner picks a sequence of .sql scripts to execute, within each
script in turn, %v needs to expand to the 'destination' version of that
script, that is, the version string that appears last in that script's
filename.

I think with a change like that, there would be less danger that
extension build scripts grow to an unnecessary and awkward complexity
just to deal with curating the collection of .sql scripts associated
with past versions.

Maybe it should be a new keyword, like module_pathname_versioned, just
to avoid changing the meaning of anybody's current module_pathname that
might have a literal %v.

OR ... am I completely overlooking a better way of using the facility
as it now exists?

-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] Declarative partitioning

[Amit Langote]

On 2016/02/15 10:55, Amit Langote wrote:
> required. There is also basic planner support but no support yet to enable
> constraint exclusion on partitions (which will be fulfilled shortly by
> installing equivalent check constraints on partitions).

Just to follow up on this - attached now adds equivalent check constraint
with partition creation. Constraint exclusion should work. No regression
tests yet though.

>From now on, instead of attaching multiple files like in the previous
message, I will send a single tar.gz which will contain patches created by
git-format-patch.

Thanks,
Amit


decl-part-20160217-1.tar.gz
Description: application/gzip

-- 
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: enhancing slow query log, and autoexplain log about waiting on lock before query exec time

[Jim Nasby]

On 2/14/16 11:24 AM, Pavel Stehule wrote:

> We have a patch, that inject logs about the time waiting on locks before
> query execution. This feature helps us lot of, and I hope, it can be
> generally useful.

Doesn't log_lock_waits cover that territory already?

It does. But It creates different log entry - and can be hard to join
slow query with log entry sometimes lot of lines before. This proposal
is about taking important information comfortably - and log parsing and
processing is simpler.


I'm all for anything that improves visibility into locking, but it seems 
like this is more a band-aid than a fix. Certainly any real analysis of 
logfiles means you're stuck with something like pgBadger. If this would 
significantly simply pgBadger's job then great, but I don't think that's 
the case.


What would be useful logging-wise is if the log line for the query 
itself could contain lock wait time, but that doesn't sound like what 
you're proposing?


What I think would be far more useful is adding lock wait time info to 
pg_stat_statements and maybe pg_stat_*_tables.

--
Jim Nasby, Data Architect, Blue Treble Consulting, Austin TX
Experts in Analytics, Data Architecture and PostgreSQL
Data in Trouble? Get it in Treble! http://BlueTreble.com


--
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] [patch] Proposal for \crosstabview in psql

[Jim Nasby]

On 2/11/16 4:21 AM, Dean Rasheed wrote:

Thinking about this some more though, perhaps*sorting*  the columns is
the wrong way to be thinking about it. Perhaps a better approach would
be to allow the columns to be*listed*  (optionally, using a separate
query). Something like the following (don't get too hung up on the
syntax):

SELECT name,
to_char(date, 'Mon') AS month,
sum(amount) AS amount
  FROM invoices
  GROUP BY 1,2
  ORDER BY name
\crosstabview cols = (select to_char(d, 'Mon') from
generate_series('2000-01-01'::date, '2000-12-01', '1 month') d)


My concern with that is that often you don't know what the columns will 
be, because you don't know what exact data the query will produce. So to 
use this syntax you'd have to re-create a huge chunk of the original 
query. :(

--
Jim Nasby, Data Architect, Blue Treble Consulting, Austin TX
Experts in Analytics, Data Architecture and PostgreSQL
Data in Trouble? Get it in Treble! http://BlueTreble.com


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

Commitfest Bug (was: [HACKERS] Re: Reusing abbreviated keys during second pass of ordered [set] aggregates)

[Jim Nasby]

On 2/16/16 12:38 AM, Michael Paquier wrote:

When a patch with status "Ready for committer" on CF N is moved to CF
(N+1), its status is automatically changed to "Needs Review". That's
something to be aware of when cleaning up the CF app entries.


I agree with Alvarro; this seems like a bug to me. If a patch is ready 
for committer in CF N, why would it suddenly not be ready in N+1?

--
Jim Nasby, Data Architect, Blue Treble Consulting, Austin TX
Experts in Analytics, Data Architecture and PostgreSQL
Data in Trouble? Get it in Treble! http://BlueTreble.com


--
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] Re: [COMMITTERS] pgsql: Introduce group locking to prevent parallel processes from deadl

[Craig Ringer]

On 14 February 2016 at 08:05, Robert Haas  wrote:


>
> The concept of a
> lock group is formally separate from the concept of a parallel group
> created by a ParallelContext, but it is not clear that there will ever
> be any other context in which a lock group will be a good idea.


Just coming back to this in terms of what Stephen and I raised: Robert, do
you think this design as it stands can handle cases where a normal
standalone backend gets promoted to a lock-group leader that others can
then join?

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

Re: [HACKERS] pglogical - logical replication contrib module

[Craig Ringer]

On 17 February 2016 at 00:54, Oleg Bartunov  wrote:


>
> DDL support is what it's missed for now.
>
>
TBH, based on experience with DDL replication and deparse in BDR, it's
going to be missing for a while yet too, or at least not comprehensively
present without caveats or exceptions.


Some DDL operations don't translate well to a series of replicatable
actions. The case I hit the most is

   ALTER TABLE mytable ADD COLUMN somecolumn sometype NOT NULL DEFAULT
some_function();

This is executed (simplified) by taking an ACCESS EXCLUSIVE lock, changing
the catalogs but not making the changes visible yet, rewriting the table,
and committing to make the rewritten table and the catalog changes visible.

That won't work well with logical replication. We currently capture DDL
with event triggers and log them to a table for later logical decoding and
replay - that's the "recognised" way. The trouble being that replaying that
statement will result in an unnecessary full table rewrite on the
downstream. Then we have to decode and send stream of changes to a table
called pg_temp_, truncate the copy of mytable on the
downstream that we just rewrote and apply those rows instead.

Of course all that only works sensibly if you have exactly one upstream and
the downstream copy of the table is treated as (or enforced as) read-only.

Improving this probably needs DDL deparse to be smarter. Rather than just
emitting something that can be reconstructed into the SQL text of the DDL
it needs to emit one or more steps that are semantically the same but allow
us to skip the rewrite. Along the lines of:

* ALTER TABLE mytable ADD COLUMN somecolumn sometype;
* ALTER TABLE mytable ALTER COLUMN somecolumn DEFAULT some_function();
* 
* ALTER TABLE mytable ALTER COLUMN somecolumn NOT NULL;

Alternately the downstream would need a hook that lets it intercept and
prevent table rewrites caused by ALTER TABLE and similar. So it can instead
just do a truncate and wait for the new rows to come from the master.

Note that all this means the standby has to hold an ACCESS EXCLUSIVE lock
on the table during all of replay. That shouldn't be necessary, all we
really need is an EXCLUSIVE lock since concurrent SELECTs are fine. No idea
how to do that.



Deparse is also just horribly complicated to get right. There are so many
clauses and subclauses and variants of statements. Each of which must be
perfect.



Not everything has a simple and obvious mapping on the downstream side
either. TRUNCATE ... CASCADE is the obvious one. You do a cascade truncate
on the master - do you want that to replicate as a cascaded truncate on the
replica, or a truncate of only those tables that actually got truncated on
the master? If the replica has additional tables with FKs pointing at
tables replica the TRUNCATE would truncate those too if you replicate it as
CASCADE; if you don't the truncate will fail instead. Really, both are
probably wrong as far as the user is concerned, but we can't truncate just
the tables truncated on the master, ignore the FK relationships, and leave
dangling FK references either.


All this means that DDL replication is probably only going to make sense in
scenarios where there's exactly one master and the replica obeys some rules
like "don't create FKs pointing from non-replicated tables to tables
replicated from somewhere else". A concept we currently have no way to
express or enforce like we do persistent-to-UNLOGGED FKs.



Then there's global objects. Something as simple as:

CREATE ROLE fred;

CREATE TABLE blah(...) OWNER fred;

will break replication because we only see the CREATE TABLE, not the CREATE
ROLE. If we instead replayed the CREATE ROLE and there were multiple
connections between different DBs on an upstream and downstream apply would
fail on all but one. But we can't anyway since there's no way to capture
that CREATE ROLE from any DB except the one it was executed in, which might
not even be one of the ones doing replication.

I strongly suspect we'll need logical decoding to be made aware of such
global DDL and decode it from the WAL writes to the system catalogs. Which
will be fun - but at least modifications to the shared catalogs are a lot
simpler than the sort of gymnastics done by ALTER TABLE, etc.



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

[HACKERS] Figures in docs

[Tatsuo Ishii]

It seems there's no figures/diagrams in our docs. I vaguely recall that
we used to have a few diagrams in our docs. If so, was there any
technical reason to remove them?

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


-- 
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] exposing pg_controldata and pg_config as functions

[Michael Paquier]

On Tue, Feb 16, 2016 at 11:36 PM, Joe Conway  wrote:
> Thanks!

OK. I think I'm good now. Thanks for the quick update.
-- 
Michael


-- 
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] custom function for converting human readable sizes to bytes

[Vitaly Burovoy]

On 2/16/16, Vitaly Burovoy  wrote:
> On 2/16/16, Dean Rasheed  wrote:
>> Fixing that in parse_memory_unit() would be messy because it assumes a
>> base unit of kB, so it would require a negative multiplier, and
>> pg_size_bytes() would have to be taught to divide by the magnitude of
>> negative multipliers in the same way that guc.c does.

Now parse_memory_unit returns -1024 for bytes as divider, constant
"bytes" has moved there.
Add new memory_units_bytes_hint which differs from an original
memory_units_int by "bytes" size unit.
Allow hintmsg be NULL and if so, skip setting dereferenced variable to
memory_units_bytes_hint.

>> ISTM that it would be far less code, and much simpler and more
>> readable to just parse the supported units directly in
>> pg_size_bytes(), rather than trying to share code with guc.c, when the
>> supported units are actually different and may well diverge further in
>> the future.
>
> Initially it was not shared with guc.c and now it is by the letter[1]
> of Oleksandr Shulgin and Robert Haas.
>
>> I'll try to hack something up, and see what it looks like.
>>
>> Regards,
>> Dean

Current version contains correct hint message, it passes tests.
Besides mentioned above there are some comment rewordings, and
variable renaming.

[1]http://www.postgresql.org/message-id/ca+tgmoanot7ugjsbibfuqgachk2lzrypmkjvvugp5r197yu...@mail.gmail.com

-- 
Best regards,
Vitaly Burovoy


pg-size-bytes-11chgd2.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] proposal: function parse_ident

[Jim Nasby]

On 2/11/16 1:27 AM, Pavel Stehule wrote:

I editorialized the docs and some of the comments. In particular, I
documented behavior of not truncating, and recommended casting to
name[] if user cares about that. Added a unit test to verify that
works. BTW, I saw mention in the thread about not truncated spaces,
but the function *does* truncate them, unless they're inside quotes,
where they're legitimate.


ok


I missed some of my edits. Updated patch with those in place attached.


Also added test for invalid characters.

I think "strict" would be more in line with other uses in code.
There are currently no other occurrences of 'strictmode' in the
code. There are loads of references to 'strict', but I didn't go
through all of them to see if any were used as externally visible
function parameter names.


I am sorry, I don't understand to this point. You unlike the name of
parameter "strictmode" ? Have you any proposal? Maybe "restrictive" ?


I would just call it strict. There's precedent for that in the code.


The almost all code +/- is related to human readable error messages. We
can move some code to separate static functions - read_quoted_ident,
read_unquoted_ident, but there will be some magic about parameters, and
the code will not be much better, than it is now.


What I'm saying is that most places that need to do de-quoting or 
similar just run a simple while loop and use an in_quote variable to 
track whether they're inside a quote or not. See 
backend/utils/adt/rowtypes.c line 199 for an example.


As I said, I don't have a strong opinion on it, so if you prefer it this 
way that's fine with me.

--
Jim Nasby, Data Architect, Blue Treble Consulting, Austin TX
Experts in Analytics, Data Architecture and PostgreSQL
Data in Trouble? Get it in Treble! http://BlueTreble.com
diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
index 139aa2b..b4a2898 100644
--- a/doc/src/sgml/func.sgml
+++ b/doc/src/sgml/func.sgml
@@ -1778,6 +1778,27 @@
   

 
+ parse_ident
+
+parse_ident(str 
text,
+   [ strictmode boolean DEFAULT 
true ] )
+   
+   text[]
+   Split qualified identifier into array 
parts.
+   When strictmode is false, extra characters after 
the identifier are ignored.
+   This is useful for parsing identifiers for objects like functions and 
arrays that may have trailing
+   characters. By default, extra characters after the last identifier are 
considered an error.
+   second parameter is false, then chars after last identifier are 
ignored. Note that this function
+   does not truncate quoted identifiers. If you care about that you should 
cast the result of this
+  function to name[].
+   
+   parse_ident('"SomeSchema".someTable')
+   "SomeSchema,sometable"
+  
+
+  
+   
+
  pg_client_encoding
 
 pg_client_encoding()
diff --git a/src/backend/catalog/system_views.sql 
b/src/backend/catalog/system_views.sql
index 923fe58..61d5b80 100644
--- a/src/backend/catalog/system_views.sql
+++ b/src/backend/catalog/system_views.sql
@@ -965,3 +965,10 @@ RETURNS jsonb
 LANGUAGE INTERNAL
 STRICT IMMUTABLE
 AS 'jsonb_set';
+
+CREATE OR REPLACE FUNCTION
+  parse_ident(str text, strictmode boolean DEFAULT true)
+RETURNS text[]
+LANGUAGE INTERNAL
+STRICT IMMUTABLE
+AS 'parse_ident';
diff --git a/src/backend/parser/scansup.c b/src/backend/parser/scansup.c
index 2b4ab20..7aa5b76 100644
--- a/src/backend/parser/scansup.c
+++ b/src/backend/parser/scansup.c
@@ -130,6 +130,15 @@ scanstr(const char *s)
 char *
 downcase_truncate_identifier(const char *ident, int len, bool warn)
 {
+   return downcase_identifier(ident, len, warn, true);
+}
+
+/*
+ * a workhorse for downcase_truncate_identifier
+ */
+char *
+downcase_identifier(const char *ident, int len, bool warn, bool truncate)
+{
char   *result;
int i;
boolenc_is_single_byte;
@@ -158,12 +167,13 @@ downcase_truncate_identifier(const char *ident, int len, 
bool warn)
}
result[i] = '\0';
 
-   if (i >= NAMEDATALEN)
+   if (i >= NAMEDATALEN && truncate)
truncate_identifier(result, i, warn);
 
return result;
 }
 
+
 /*
  * truncate_identifier() --- truncate an identifier to NAMEDATALEN-1 bytes.
  *
diff --git a/src/backend/utils/adt/misc.c b/src/backend/utils/adt/misc.c
index 6a306f3..f7d60b1 100644
--- a/src/backend/utils/adt/misc.c
+++ b/src/backend/utils/adt/misc.c
@@ -21,12 +21,15 @@
 #include 
 
 #include "access/sysattr.h"
+#include "access/htup_details.h"
 #include "catalog/catalog.h"
+#include "catalog/namespace.h"
 #include "catalog/pg_tablespace.h"
 #include "catalog/pg_type.h"
 #include "commands/dbcommands.h"
 #include "funcapi.h"
 #include "miscadmin.h"
+#include "parser/scansup.h"
 #include "parser/keywords.h"
 #include "postmaster/syslogger.h"
 

Re: [HACKERS] auto_explain sample rate

[Julien Rouhaud]

On 16/02/2016 22:51, Alvaro Herrera wrote:
> Julien Rouhaud wrote:
> 
> Hijacking this macro is just too obscure:
> 
>>  #define auto_explain_enabled() \
>>  (auto_explain_log_min_duration >= 0 && \
>> - (nesting_level == 0 || auto_explain_log_nested_statements))
>> + (nesting_level == 0 || auto_explain_log_nested_statements) && \
>> + current_query_sampled)
> 
> because it then becomes hard to figure out that assigning to _sampled is
> what makes the enabled() check pass or not depending on sampling:
> 
>> @@ -191,6 +211,14 @@ _PG_fini(void)
>>  static void
>>  explain_ExecutorStart(QueryDesc *queryDesc, int eflags)
>>  {
>> +/*
>> + * For ratio sampling, randomly choose top-level statement. Either
>> + * all nested statements will be explained or none will.
>> + */
>> +if (auto_explain_log_min_duration >= 0 && nesting_level == 0)
>> +current_query_sampled = (random() < auto_explain_sample_ratio *
>> +MAX_RANDOM_VALUE);
>> +
>>  if (auto_explain_enabled())
>>  {
> 
> I think it's better to keep the "enabled" macro unmodified, and just add
> another conditional to the "if" test there.
> 

Thanks for looking at this!

Agreed, it's too obscure. Attached v4 fixes as you said.

-- 
Julien Rouhaud
http://dalibo.com - http://dalibo.org
diff --git a/contrib/auto_explain/auto_explain.c b/contrib/auto_explain/auto_explain.c
index 0950e18..fa564fd 100644
--- a/contrib/auto_explain/auto_explain.c
+++ b/contrib/auto_explain/auto_explain.c
@@ -29,6 +29,7 @@ static bool auto_explain_log_triggers = false;
 static bool auto_explain_log_timing = true;
 static int	auto_explain_log_format = EXPLAIN_FORMAT_TEXT;
 static bool auto_explain_log_nested_statements = false;
+static double auto_explain_sample_ratio = 1;
 
 static const struct config_enum_entry format_options[] = {
 	{"text", EXPLAIN_FORMAT_TEXT, false},
@@ -47,10 +48,14 @@ static ExecutorRun_hook_type prev_ExecutorRun = NULL;
 static ExecutorFinish_hook_type prev_ExecutorFinish = NULL;
 static ExecutorEnd_hook_type prev_ExecutorEnd = NULL;
 
+/* Is the current query sampled, per backend */
+static bool current_query_sampled = true;
+
 #define auto_explain_enabled() \
 	(auto_explain_log_min_duration >= 0 && \
 	 (nesting_level == 0 || auto_explain_log_nested_statements))
 
+
 void		_PG_init(void);
 void		_PG_fini(void);
 
@@ -62,6 +67,7 @@ static void explain_ExecutorFinish(QueryDesc *queryDesc);
 static void explain_ExecutorEnd(QueryDesc *queryDesc);
 
 
+
 /*
  * Module load callback
  */
@@ -159,6 +165,19 @@ _PG_init(void)
 			 NULL,
 			 NULL);
 
+	DefineCustomRealVariable("auto_explain.sample_ratio",
+			 "Percentage of queries to process.",
+			NULL,
+			_explain_sample_ratio,
+			1.0,
+			0.0,
+			1.0,
+			PGC_SUSET,
+			0,
+			NULL,
+			NULL,
+			NULL);
+
 	EmitWarningsOnPlaceholders("auto_explain");
 
 	/* Install hooks. */
@@ -191,7 +210,15 @@ _PG_fini(void)
 static void
 explain_ExecutorStart(QueryDesc *queryDesc, int eflags)
 {
-	if (auto_explain_enabled())
+	/*
+	 * For ratio sampling, randomly choose top-level statement. Either
+	 * all nested statements will be explained or none will.
+	 */
+	if (auto_explain_log_min_duration >= 0 && nesting_level == 0)
+		current_query_sampled = (random() < auto_explain_sample_ratio *
+MAX_RANDOM_VALUE);
+
+	if (auto_explain_enabled() && current_query_sampled)
 	{
 		/* Enable per-node instrumentation iff log_analyze is required. */
 		if (auto_explain_log_analyze && (eflags & EXEC_FLAG_EXPLAIN_ONLY) == 0)
@@ -210,7 +237,7 @@ explain_ExecutorStart(QueryDesc *queryDesc, int eflags)
 	else
 		standard_ExecutorStart(queryDesc, eflags);
 
-	if (auto_explain_enabled())
+	if (auto_explain_enabled() && current_query_sampled)
 	{
 		/*
 		 * Set up to track total elapsed time in ExecutorRun.  Make sure the
@@ -280,7 +307,7 @@ explain_ExecutorFinish(QueryDesc *queryDesc)
 static void
 explain_ExecutorEnd(QueryDesc *queryDesc)
 {
-	if (queryDesc->totaltime && auto_explain_enabled())
+	if (queryDesc->totaltime && auto_explain_enabled() && current_query_sampled)
 	{
 		double		msec;
 
diff --git a/doc/src/sgml/auto-explain.sgml b/doc/src/sgml/auto-explain.sgml
index d527208..9d40e65 100644
--- a/doc/src/sgml/auto-explain.sgml
+++ b/doc/src/sgml/auto-explain.sgml
@@ -203,6 +203,23 @@ LOAD 'auto_explain';
  
 

+
+   
+
+ auto_explain.sample_ratio (real)
+ 
+  auto_explain.sample_ratio configuration parameter
+ 
+
+
+ 
+  auto_explain.sample_ratio causes auto_explain to only
+  explain X percent statements in each session.  In case of nested
+  statements, either all will be explained or none. Only superusers can
+  change this setting.
+ 
+
+   
   
 
   

-- 
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] PostgreSQL Audit Extension

[Bruce Momjian]

On Fri, Feb  5, 2016 at 01:16:25PM -0500, Stephen Frost wrote:
> Looking at pgaudit and the other approaches to auditing which have been
> developed (eg: applications which sit in front of PG and essentially
> have to reimplement large bits of PG to then audit the commands sent
> before passing them to PG, or hacks which try to make sense out of log
> files full of SQL statements) make it quite clear, in my view, that
> attempts to bolt-on auditing to PG result in a poorer solution, from a
> technical perspective, than what this project is known for and capable
> of.  To make true progress towards that, however, we need to get past
> the thinking that auditing doesn't need to be in-core or that it should
> be a second-class citizen feature or that we don't need it in PG.

Coming in late here, but the discussion around how to maintain the
auditing code seems very similar to how to handle the logical
replication of DDL commands.  First, have we looked into hooking
auditing into scanning logical replication contents, and second, how are
we handling the logical replication of DDL and could we use the same
approach for auditing?

-- 
  Bruce Momjian  http://momjian.us
  EnterpriseDB http://enterprisedb.com

+ As you are, so once was I. As I am, so you will be. +
+ Roman grave inscription +


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

[HACKERS] Tab Completion for CREATE DATABASE ... TEMPLATE ...

[Sehrope Sarkuni]

We use "CREATE DATABASE foo TEMPLATE foo_bk" to restore development
databases to a known snapshot (ex: prior to testing DB migrations).
Currently psql only autocompletes "foo_bk" if it's marked as a template
database. It's mildly inconvenient to have to type out the entire database
name (as they're not marked as templates).

The CREATE DATABASE command allows a super user to use any database as the
template, and a non-super user (with CREATEDB privilege) to use any
database of which it's the owner.

The attached patch updates the psql "CREATE DATABASE ... TEMPLATE "
completion to match what the command actually allows.

Regards,
-- Sehrope Sarkuni
Founder & CEO | JackDB, Inc. | https://www.jackdb.com/
diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
index 5f27120..fc1edb0 100644
--- a/src/bin/psql/tab-complete.c
+++ b/src/bin/psql/tab-complete.c
@@ -599,8 +599,13 @@ static const SchemaQuery Query_for_list_of_matviews = {
 "OR '\"' || nspname || '\"' ='%s') "
 
 #define Query_for_list_of_template_databases \
-"SELECT pg_catalog.quote_ident(datname) FROM pg_catalog.pg_database "\
-" WHERE substring(pg_catalog.quote_ident(datname),1,%d)='%s' AND datistemplate"
+"SELECT pg_catalog.quote_ident(d.datname) "\
+"FROM pg_catalog.pg_database d "\
+" JOIN pg_catalog.pg_user u ON u.usesysid = d.datdba "\
+"WHERE substring(pg_catalog.quote_ident(datname),1,%d)='%s' "\
+"   AND (d.datistemplate "\
+" OR u.usename = USER "\
+" OR (SELECT z.usesuper FROM pg_catalog.pg_user z WHERE z.usename = USER))"
 
 #define Query_for_list_of_databases \
 "SELECT pg_catalog.quote_ident(datname) FROM pg_catalog.pg_database "\

-- 
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: PL/Pythonu - function ereport

[Pavel Stehule]

Hi

2016-02-16 17:18 GMT+01:00 Catalin Iacob :

> On Sat, Feb 13, 2016 at 4:26 PM, Pavel Stehule 
> wrote:
> > I am sending new version. Current version does:
>
> Hello,
>
> I had a look and I like the big picture.
>
> Tests fail when built against Python 3.5 with stuff like this in
> regression.diffs:
> -ERROR:  plpy.Error: stop on error
> -DETAIL:  some detail
> -HINT:  some hint
> -CONTEXT:  Traceback (most recent call last):
> -  PL/Python function "elog_test", line 18, in 
> -plpy.error('stop on error', 'some detail','some hint')
> -PL/Python function "elog_test"
> +ERROR:  could not convert Python Unicode object to bytes
> +DETAIL:  TypeError: bad argument type for built-in operation
> +CONTEXT:  PL/Python function "elog_test"
>

fixed - the object serialization fails on Py_None

>
> This is related to the use of PyString_AsString and the changed
> semantics of that in Python 3 (due to the fact that strings are now
> Unicode objects and so on). Didn't have time to dig more deeply into
> the exact cause.
>
> Similarly, there are alternative expected test outputs that you didn't
> update, for example src/pl/plpython/expected/plpython_types_3.out so
> tests fail on some Python versions due to those as well.
>
> > 1. the plpy utility functions can use all ErrorData fields,
> > 2. there are no new functions,
> > 3. via GUC plpythonu.legacy_custom_exception we can return previous
> behave,
> > 4. only exception Error is raised with natural structure - no composite
> > value spidata.
> > 5. fields: message, detail and hint are implicitly translated to string
> - it
> > decrease a necessity of legacy mode
>
> I disagree that 5. is a good idea. I think we should just treat
> message, detail and hint like the other ones (schema, table etc.). Use
> s in PyArg_ParseTupleAndKeywords and let the user explicitly cast to a
> string. Explicit is better than implicit. The way you did it you keep
> part of the weird old interface which used to cast to string for you.
> We shouldn't keep warts of the old behaviour, especially with the GUC
> to ask for the old behaviour.
>

removed @5


>
> By the way, getting rid of object_to_string and its usage in
> PLy_output_kw removed some "ERROR: could not convert Python Unicode
> object to bytes" failures in the tests so I'm quite sure that the
> usage of PyString_AsString is responsible for those.
>
> I don't like that you set legacy_custom_exception=true in some
> existing tests, probably to avoid changing them to the new behaviour.
> We should trust that the new behaviour is what we want and the tests
> should reflect that. If it's too much work, remember we're asking
> users to do the same work to convert their code. We have
> elog_test_legacy to test elog_test_legacy=true, the rest of the tests
> should use legacy_custom_exception=false.
>

all regress tests now works in new mode

Regards

Pavel


plpython-enhanced-error-02.patch.gz
Description: GNU Zip compressed 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] auto_explain sample rate

[Alvaro Herrera]

Julien Rouhaud wrote:

Hijacking this macro is just too obscure:

>  #define auto_explain_enabled() \
>   (auto_explain_log_min_duration >= 0 && \
> -  (nesting_level == 0 || auto_explain_log_nested_statements))
> +  (nesting_level == 0 || auto_explain_log_nested_statements) && \
> +  current_query_sampled)

because it then becomes hard to figure out that assigning to _sampled is
what makes the enabled() check pass or not depending on sampling:

> @@ -191,6 +211,14 @@ _PG_fini(void)
>  static void
>  explain_ExecutorStart(QueryDesc *queryDesc, int eflags)
>  {
> + /*
> +  * For ratio sampling, randomly choose top-level statement. Either
> +  * all nested statements will be explained or none will.
> +  */
> + if (auto_explain_log_min_duration >= 0 && nesting_level == 0)
> + current_query_sampled = (random() < auto_explain_sample_ratio *
> + MAX_RANDOM_VALUE);
> +
>   if (auto_explain_enabled())
>   {

I think it's better to keep the "enabled" macro unmodified, and just add
another conditional to the "if" test there.

-- 
Álvaro Herrerahttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, 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] auto_explain sample rate

[Julien Rouhaud]

On 25/08/2015 14:45, Michael Paquier wrote:
> On Fri, Jul 17, 2015 at 2:53 PM, Craig Ringer  wrote:
>> On 7 July 2015 at 21:37, Julien Rouhaud  wrote:
>>
>>> Well, I obviously missed that pg_srand48() is only used if the system
>>> lacks random/srandom, sorry for the noise.  So yes, random() must be
>>> used instead of pg_lrand48().
>>>
>>> I'm attaching a new version of the patch fixing this issue just in case.
>>
>> Thanks for picking this up. I've been trying to find time to come back
>> to it but been swamped in priority work.
> 
> For now I am marking that as returned with feedback.
> 

PFA v3 of the patch, rebased on current head. It fixes the last issue
(sample a percentage of queries).

I'm adding it to the next commitfest.

-- 
Julien Rouhaud
http://dalibo.com - http://dalibo.org
diff --git a/contrib/auto_explain/auto_explain.c b/contrib/auto_explain/auto_explain.c
index 0950e18..4edb08e 100644
--- a/contrib/auto_explain/auto_explain.c
+++ b/contrib/auto_explain/auto_explain.c
@@ -29,6 +29,7 @@ static bool auto_explain_log_triggers = false;
 static bool auto_explain_log_timing = true;
 static int	auto_explain_log_format = EXPLAIN_FORMAT_TEXT;
 static bool auto_explain_log_nested_statements = false;
+static double auto_explain_sample_ratio = 1;
 
 static const struct config_enum_entry format_options[] = {
 	{"text", EXPLAIN_FORMAT_TEXT, false},
@@ -47,9 +48,14 @@ static ExecutorRun_hook_type prev_ExecutorRun = NULL;
 static ExecutorFinish_hook_type prev_ExecutorFinish = NULL;
 static ExecutorEnd_hook_type prev_ExecutorEnd = NULL;
 
+/* Is the current query sampled, per backend */
+static bool current_query_sampled = true;
+
 #define auto_explain_enabled() \
 	(auto_explain_log_min_duration >= 0 && \
-	 (nesting_level == 0 || auto_explain_log_nested_statements))
+	 (nesting_level == 0 || auto_explain_log_nested_statements) && \
+	 current_query_sampled)
+
 
 void		_PG_init(void);
 void		_PG_fini(void);
@@ -62,6 +68,7 @@ static void explain_ExecutorFinish(QueryDesc *queryDesc);
 static void explain_ExecutorEnd(QueryDesc *queryDesc);
 
 
+
 /*
  * Module load callback
  */
@@ -159,6 +166,19 @@ _PG_init(void)
 			 NULL,
 			 NULL);
 
+	DefineCustomRealVariable("auto_explain.sample_ratio",
+			 "Percentage of queries to process.",
+			NULL,
+			_explain_sample_ratio,
+			1.0,
+			0.0,
+			1.0,
+			PGC_SUSET,
+			0,
+			NULL,
+			NULL,
+			NULL);
+
 	EmitWarningsOnPlaceholders("auto_explain");
 
 	/* Install hooks. */
@@ -191,6 +211,14 @@ _PG_fini(void)
 static void
 explain_ExecutorStart(QueryDesc *queryDesc, int eflags)
 {
+	/*
+	 * For ratio sampling, randomly choose top-level statement. Either
+	 * all nested statements will be explained or none will.
+	 */
+	if (auto_explain_log_min_duration >= 0 && nesting_level == 0)
+		current_query_sampled = (random() < auto_explain_sample_ratio *
+MAX_RANDOM_VALUE);
+
 	if (auto_explain_enabled())
 	{
 		/* Enable per-node instrumentation iff log_analyze is required. */
diff --git a/doc/src/sgml/auto-explain.sgml b/doc/src/sgml/auto-explain.sgml
index d527208..9d40e65 100644
--- a/doc/src/sgml/auto-explain.sgml
+++ b/doc/src/sgml/auto-explain.sgml
@@ -203,6 +203,23 @@ LOAD 'auto_explain';
  
 

+
+   
+
+ auto_explain.sample_ratio (real)
+ 
+  auto_explain.sample_ratio configuration parameter
+ 
+
+
+ 
+  auto_explain.sample_ratio causes auto_explain to only
+  explain X percent statements in each session.  In case of nested
+  statements, either all will be explained or none. Only superusers can
+  change this setting.
+ 
+
+   
   
 
   

-- 
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] Re: [COMMITTERS] pgsql: Introduce group locking to prevent parallel processes from deadl

[Tom Lane]

Robert Haas  writes:
> 1. It removes the groupLeader flag from PGPROC.  You're right: we don't need 
> it.

It occurs to me that this claim is bogus:

> We institute a further coding rule that a process cannot join or leave a lock
> group while owning any PROCLOCK.  Therefore, given a lock manager lock
> sufficient to examine PROCLOCK *proclock, it also safe to examine
> proclock->tag.myProc->lockGroupLeader (but not the other fields mentioned in
> the previous paragraph).

Yes, we can legislate that workers have to join before taking any lock,
and if processes only leave the group at death then that end of it is not
a problem either; but it is patently not the case that a process will hold
no locks when it calls BecomeLockGroupLeader().

We may be able to salvage this simplification anyway, but it will require
a tighter specification of when it's safe to look at
proclock->tag.myProc->lockGroupLeader.

Another possibility is to forget about executor-time
BecomeLockGroupLeader(), and just say that any process that isn't a worker
always becomes its own group leader at startup.  Then the above para is
true as written.  I don't know what downsides this might have; do your
changes in the lock manager try to optimize the null-lockGroupLeader 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] Remove or weaken hints about "effective resolution of sleep delays is 10 ms"?

[Jeff Janes]

On Tue, Feb 16, 2016 at 12:06 AM, Robert Haas  wrote:
> On Wed, Feb 10, 2016 at 5:15 PM, Andres Freund  wrote:
>> Hi,
>>
>> Several places in our docs have blurbs like
>>> Note that on many systems, the effective resolution of sleep delays is
>>> 10 milliseconds; setting wal_writer_delay to a value that
>>> is not a multiple of 10 might have the same results as setting it to
>>> the next higher multiple of 10.
>> Afaik that's not the case on any recent operating system/hardware. So
>> perhaps we should just remove all of those blurbs, or just replace them
>> with something like "on some older systems the effective resolution of
>> sleep delays is limited to multiples of 10 milliseconds"?
>
> Hmm, is that true?  What we do we think the resolution is on modern
> systems?  I would not have guessed that to be inaccurate.

time perl -le 'my $wait=0.1; select undef,undef,undef,$wait
foreach (1..1/$wait*1); warn "should be 1 second"'


On CentOS 6.7 (not exactly modern) and Ubuntu Trusty, if I ask for 10
microsecond delays, I get between 70 and 100 microseconds delays
(depending on the VM software, mostly, it seems).  So at least 100
fold better than the 10ms.


Of course if processes are fighting over CPU, you might do worse.


-- 
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: PL/Pythonu - function ereport

[Pavel Stehule]

2016-02-16 21:06 GMT+01:00 Catalin Iacob :

> On Tue, Feb 16, 2016 at 5:18 PM, Catalin Iacob 
> wrote:
> > We have
> > elog_test_legacy to test elog_test_legacy=true, the rest of the tests
> > should use legacy_custom_exception=false.
>
> This should have been:
> We have elog_test_legacy to test legacy_custom_exception=true, the
> rest of the tests should use legacy_custom_exception=false.
>

I understand - I fixed regress tests for new behave


>
> I noticed the patch isn't registered in the March CF, please do that.
> It would be a pity to miss 9.6 because of not registering it in time.
>

I'll do it. Now, I fixing 3.4 Python code. There are more issues with
"could not convert Python Unicode object to bytes"

Regards

Pavel

Re: [HACKERS] Re: [COMMITTERS] pgsql: Introduce group locking to prevent parallel processes from deadl

[Tom Lane]

Robert Haas  writes:
> Yeah, you're right.  Attached is a draft patch that tries to clean up
> that and a bunch of other things that you raised.

I reviewed this patch.  I don't have any particular comment on the changes
in deadlock.c; I haven't studied the code closely enough to know if those
are right.  I did think that the commentary elsewhere could be improved
some more, so attached is a delta patch on top of yours that shows what
I suggest.

I've not done anything here about removing lockGroupLeaderIdentifier,
although I will be happy to send a patch for that unless you know of
some reason I missed why it's necessary.

regards, tom lane

diff --git a/src/backend/storage/lmgr/README b/src/backend/storage/lmgr/README
index 8eaa91c..5a62c8f 100644
*** a/src/backend/storage/lmgr/README
--- b/src/backend/storage/lmgr/README
*** acquire an AccessShareLock while the oth
*** 603,609 
  This might seem dangerous and could be in some cases (more on that below), but
  if we didn't do this then parallel query would be extremely prone to
  self-deadlock.  For example, a parallel query against a relation on which the
! leader had already AccessExclusiveLock would hang, because the workers would
  try to lock the same relation and be blocked by the leader; yet the leader
  can't finish until it receives completion indications from all workers.  An
  undetected deadlock results.  This is far from the only scenario where such a
--- 603,609 
  This might seem dangerous and could be in some cases (more on that below), but
  if we didn't do this then parallel query would be extremely prone to
  self-deadlock.  For example, a parallel query against a relation on which the
! leader already had AccessExclusiveLock would hang, because the workers would
  try to lock the same relation and be blocked by the leader; yet the leader
  can't finish until it receives completion indications from all workers.  An
  undetected deadlock results.  This is far from the only scenario where such a
*** quickly enough for this interlock to fai
*** 664,690 
  
  A PGPROC's lockGroupLeader is NULL for processes not involved in parallel
  query. When a process wants to cooperate with parallel workers, it becomes a
! lock group leader, which means setting this field back to point to its own
  PGPROC. When a parallel worker starts up, it points this field at the leader,
  with the above-mentioned interlock. The lockGroupMembers field is only used in
! the leader; it is a list of the workers. The lockGroupLink field is used to
! link the leader and all workers into the leader's list. All of these fields
! are protected by the lock manager locks; the lock manager lock that protects
! the lockGroupLeaderIdentifier, lockGroupLeader, and lockGroupMembers fields in
! a given PGPROC is chosen by taking pgprocno modulo the number of lock manager
! partitions. This unusual arrangement has a major advantage: the deadlock
! detector can count on the fact that no lockGroupLeader field can change while
! the deadlock detector is running, because it knows that it holds all the lock
! manager locks.  A PGPROC's lockGroupLink is protected by the lock manager
! partition lock for the group of which it is a part.  If it's not part of any
! group, this field is unused and can only be examined or modified by the
! process that owns the PGPROC.
  
  We institute a further coding rule that a process cannot join or leave a lock
  group while owning any PROCLOCK.  Therefore, given a lock manager lock
! sufficient to examine PROCLOCK *proclock, it also safe to examine
! proclock->tag.myProc->lockGroupLeader (but not the other fields mentioned in
! the previous paragraph).
  
  User Locks (Advisory Locks)
  ---
--- 664,689 
  
  A PGPROC's lockGroupLeader is NULL for processes not involved in parallel
  query. When a process wants to cooperate with parallel workers, it becomes a
! lock group leader, which means setting this field to point to its own
  PGPROC. When a parallel worker starts up, it points this field at the leader,
  with the above-mentioned interlock. The lockGroupMembers field is only used in
! the leader; it is a list of the member PGPROCs of the lock group (the leader
! and all workers). The lockGroupLink field is the list link for this list.
! 
! All four of these fields are considered to be protected by a lock manager
! partition lock.  The partition lock that protects these fields within a given
! lock group is chosen by taking the leader's pgprocno modulo the number of lock
! manager partitions.  This unusual arrangement has a major advantage: the
! deadlock detector can count on the fact that no lockGroupLeader field can
! change while the deadlock detector is running, because it knows that it holds
! all the lock manager locks.  Also, holding this single lock allows safe
! manipulation of the lockGroupMembers list for the lock group.
  
  

Re: [HACKERS] proposal: PL/Pythonu - function ereport

[Catalin Iacob]

On Tue, Feb 16, 2016 at 5:18 PM, Catalin Iacob  wrote:
> We have
> elog_test_legacy to test elog_test_legacy=true, the rest of the tests
> should use legacy_custom_exception=false.

This should have been:
We have elog_test_legacy to test legacy_custom_exception=true, the
rest of the tests should use legacy_custom_exception=false.

I noticed the patch isn't registered in the March CF, please do that.
It would be a pity to miss 9.6 because of not registering it in time.


-- 
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] Re: [COMMITTERS] pgsql: Introduce group locking to prevent parallel processes from deadl

[Tom Lane]

Robert Haas  writes:
> Yeah, you're right.  Attached is a draft patch that tries to clean up
> that and a bunch of other things that you raised.

I've been reviewing this patch, and I had a question: why do we need to
bother with a lockGroupLeaderIdentifier field?  It seems totally redundant
with the leader's pid field, ie, why doesn't BecomeLockGroupMember simply
compare leader->pid with the PID it's passed?  For some more safety, it
could also verify that leader->lockGroupLeader == leader; but I don't
see what the extra field is buying us.

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] Freeze avoidance of very large table.

[Bruce Momjian]

On Tue, Feb 16, 2016 at 03:57:01PM -0300, Alvaro Herrera wrote:
> Masahiko Sawada wrote:
> > On Wed, Feb 17, 2016 at 12:02 AM, Bruce Momjian  wrote:
> > > On Tue, Feb 16, 2016 at 11:56:25PM +0900, Masahiko Sawada wrote:
> > >> > I agreed on ripping out the converter plugin ability of pg_upgrade.
> > >> > Remember pg_upgrade was originally written by EnterpriseDB staff, and I
> > >> > think they expected their closed-source fork of Postgres might need a
> > >> > custom page converter someday, but it never needed one, and at this
> > >> > point I think having the code in there is just making things more
> > >> > complex.  I see _no_ reason for community Postgres to use a plugin
> > >> > converter because we are going to need that code for every upgrade from
> > >> > pre-9.6 to 9.6+, so why not just hard-code in the functions we need.  
> > >> > We
> > >> > can remove it once 9.5 is end-of-life.
> > >> >
> > >>
> > >> Hm, we should rather remove the source code around PAGE_CONVERSION and
> > >> page.c at 9.6?
> > >
> > > Yes.  I can do it if you wish.
> > 
> > I see. I understand that page-converter code would be useful for some
> > future cases, but makes thing more complex.
> 
> If we're not going to use it, let's get rid of it right away.  There's
> no point in having a feature that adds complexity just because we might
> find some hypothetical use of it in a not-yet-imagined future.

Agreed.  We can always add it later if we need it.

-- 
  Bruce Momjian  http://momjian.us
  EnterpriseDB http://enterprisedb.com

+ As you are, so once was I. As I am, so you will be. +
+ Roman grave inscription +


-- 
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] Freeze avoidance of very large table.

[Alvaro Herrera]

Masahiko Sawada wrote:
> On Wed, Feb 17, 2016 at 12:02 AM, Bruce Momjian  wrote:
> > On Tue, Feb 16, 2016 at 11:56:25PM +0900, Masahiko Sawada wrote:
> >> > I agreed on ripping out the converter plugin ability of pg_upgrade.
> >> > Remember pg_upgrade was originally written by EnterpriseDB staff, and I
> >> > think they expected their closed-source fork of Postgres might need a
> >> > custom page converter someday, but it never needed one, and at this
> >> > point I think having the code in there is just making things more
> >> > complex.  I see _no_ reason for community Postgres to use a plugin
> >> > converter because we are going to need that code for every upgrade from
> >> > pre-9.6 to 9.6+, so why not just hard-code in the functions we need.  We
> >> > can remove it once 9.5 is end-of-life.
> >> >
> >>
> >> Hm, we should rather remove the source code around PAGE_CONVERSION and
> >> page.c at 9.6?
> >
> > Yes.  I can do it if you wish.
> 
> I see. I understand that page-converter code would be useful for some
> future cases, but makes thing more complex.

If we're not going to use it, let's get rid of it right away.  There's
no point in having a feature that adds complexity just because we might
find some hypothetical use of it in a not-yet-imagined future.

-- 
Álvaro Herrerahttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, 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] Doubt in 9.5 release note

[Bruce Momjian]

On Wed, Jan 27, 2016 at 01:31:45PM +0900, Tatsuo Ishii wrote:
> > Tatsuo Ishii  writes:
> >> I saw following item in release-9.5.sgml:
> >> 
> >>
> >> Support comments on domain
> >> constraints (lvaro Herrera)
> >>
> >>   
> > 
> >> It seems the release note has nothing to do with the commit. Also,
> >> commenting on DOMAIN constraints is not new in 9.5, I think.
> > 
> > Yeah, it is.  See 7eca575d1c28f6eee2bf4564f3d458d10c4a8f47, which is
> > the commit that should have been listed here, likely.
> 
> Are you willing to fix it?

Fixed and patched back to 9.5.

-- 
  Bruce Momjian  http://momjian.us
  EnterpriseDB http://enterprisedb.com

+ As you are, so once was I. As I am, so you will be. +
+ Roman grave inscription +


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

[HACKERS] Question about Restart point and checkpoint_segments

[Benoit Lobréau]

Hello,

I am using a hot_standby setup on PostgreSQL 9.1
While I was testing, I found out that only checkpoint_timeout (+ a
checkpoint since the last restart point) could trigger a restart point.

The code (bgwriter.c) seems to confirm this:

/*
* Check progress against WAL segments written and checkpoint_segments.
*
* We compare the current WAL insert location against the location
* computed before calling CreateCheckPoint. The code in XLogInsert that
* actually triggers a checkpoint when checkpoint_segments is exceeded
* compares against RedoRecptr, so this is not completely accurate.
* However, it's good enough for our purposes, we're only calculating an
* estimate anyway.
*/
if (!RecoveryInProgress())  ===> Only in case of primary
{
   recptr = GetInsertRecPtr();
   elapsed_xlogs =
   (((double) (int32) (recptr.xlogid -
ckpt_start_recptr.xlogid)) * XLogSegsPerFile +
   ((double) recptr.xrecoff - (double)
ckpt_start_recptr.xrecoff) / XLogSegSize) /
   CheckPointSegments;

   if (progress < elapsed_xlogs)  ===> progress in volume
   {
   ckpt_cached_elapsed = elapsed_xlogs;
   return false;
   }
}

/*
* Check progress against time elapsed and checkpoint_timeout.
*/
gettimeofday(, NULL);
elapsed_time = ((double) ((pg_time_t) now.tv_sec - ckpt_start_time) +

now.tv_usec / 100.0) / CheckPointTimeout;


if (progress < elapsed_time)  ===> progress in time
{
   ckpt_cached_elapsed = elapsed_time;
   return false;
}

/* It looks like we're on schedule. */
return true;

I also found a post from Simon Riggs [1]: "checkpoint_segments is ignored
on standby."

The documentation is stating the opposite [2]: "In standby mode, a
restartpoint is also triggered if checkpoint_segments log segments have
been replayed since last restartpoint and at least one checkpoint record
has been replayed."

Since I am not a native english speaker, maybe I misunderstood the
documentation. But to me, it looks wrong.
If it's indeed wrong. Could you explain why checkpoint_segments doesn't
trigger a restart_point in standby mode ?

Thank you
Benoit

[1]
http://www.postgresql.org/message-id/CA+U5nMKdf7odZzYNnoRkkCZmJpGEy=oqbu9nan_zva_rtzi...@mail.gmail.com
[2] http://www.postgresql.org/docs/9.1/static/wal-configuration.html

Re: [HACKERS] pglogical - logical replication contrib module

[Oleg Bartunov]

On Tue, Feb 16, 2016 at 5:38 PM, Bruce Momjian  wrote:

> On Tue, Jan 26, 2016 at 08:14:26PM -0800, Joshua Drake wrote:
> > On 12/31/2015 03:34 PM, Petr Jelinek wrote:
> > >Hi,
> > >
> > >I'd like to submit the replication solution which is based on the
> > >pglogical_output [1] module (which is obviously needed for this to
> > >compile).
> >
> > This is fantastic! However, history presents itself here and
> > PostgreSQL in the past has not "blessed" a single solution for
> > Replication. Obviously that changed a bit with streaming replication
> > but this is a bit different than that. As I understand it, PgLogical
> > is Logical Replication (similar to Slony and Londiste). I wouldn't
> > be surprised (although I don't know) if Slony were to start using
> > some of the pglogical_output module features in the future.
> >
> > If we were to accept PgLogical into core, it will become the default
> > blessed solution for PostgreSQL. While that is great in some ways
> > it is a different direction than the project has taken in the past.
> > Is this what we want to do?
>
> Replying late here, but I think with binary replication, we decided
> that, assuming you were happy with the features provided, our streaming
> binary replication solution was going to be the best and recommended way
> of doing it.
>
> I don't think we ever had that feeling with Slony or Londiste in that
> there were so many limitations and so many different ways of
> implementing logical replication that we never recommended a best way.
>
> So, the question is, do we feel that PgLogical is best and recommended
> way to do logical replication.  If it is, then having it in core makes
> sense.
>

DDL support is what it's missed for now.


>
> --
>   Bruce Momjian  http://momjian.us
>   EnterpriseDB http://enterprisedb.com
>
> + As you are, so once was I. As I am, so you will be. +
> + Roman grave inscription +
>
>
> --
> 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] Small PATCH: check of 2 Perl modules

[Victor Wagner]

On Tue, 16 Feb 2016 12:23:56 -0300
Alvaro Herrera  wrote:

> ... but I agree with the point upthread that this should wait to see
> what happens with the CMake stuff, since this is not a newly
> introduced problem.

I doubt, that CMake stuff would be ready for 9.6. There is just one
commitfest left, and it would be quite a radical change.

And even if would appear in the next release, it cannot be easily
backpatched to 9.5. So we'll probably live with autoconf until the
end-of-life of 9.5 series at least.  



-- 
   Victor Wagner 


-- 
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] ALTER ... OWNER TO ... CASCADE

[Vladimir Borodin]

> 16 февр. 2016 г., в 18:20, Alvaro Herrera  
> написал(а):
> 
> Vladimir Borodin wrote:
> 
>>> Moreover, the use case you've sketched (ie, change ownership of all
>>> objects inside a database) doesn't actually have anything to do with
>>> following dependencies.  It's a lot closer to REASSIGN OWNED ... in
>>> fact, it's not clear to me why REASSIGN OWNED doesn't solve that
>>> use-case already.
>> 
>> Sometimes I hit the following. You have created a database and schema
>> inside it from the superuser (i.e. postgres). Than you want to change
>> ownership of whole database to another user (i.e. alice), but only
>> this database, not all other objects in all other databases. It seems
>> that REASSIGN OWNED doesn’t solve this already.
> 
> So essentially you want to change all the objects in the database except
> those that were created together with the database itself (i.e. those
> that were copied from the template database).  

Yes. Without such syntax it is now done in a really awful way now, i.e. [0].

[0] 
https://github.com/saltstack/salt/blob/405d0aef1cf11bb56b5d2320b176f6992e6cdf3b/salt/modules/postgres.py#L1806-L1847

> That seems a reasonable
> use-case, but I'm not sure that this ALTER .. OWNER CASCADE is the right
> thing for that -- What object would you start with?  Each schema other
> than pg_catalog, pg_toast, information_schema?  As I recall, the problem
> is that REASSIGN OWNED refuses to work on pinned objects.  Maybe what
> you want is something like
>  REASSIGN OWNED BY xyz IN SCHEMA public TO xyzxxz
> i.e., an extension of the current REASSIGN OWNED BY command?

Well, I don’t know what syntax and implementation would be correct. I just want 
to give a specific user all rights to manage all objects in a specific database 
(which was created from postgres user earlier). It would be really useful.

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


--
Да пребудет с вами сила…
https://simply.name/ru

Re: [HACKERS] proposal: PL/Pythonu - function ereport

[Catalin Iacob]

On Sat, Feb 13, 2016 at 4:26 PM, Pavel Stehule  wrote:
> I am sending new version. Current version does:

Hello,

I had a look and I like the big picture.

Tests fail when built against Python 3.5 with stuff like this in
regression.diffs:
-ERROR:  plpy.Error: stop on error
-DETAIL:  some detail
-HINT:  some hint
-CONTEXT:  Traceback (most recent call last):
-  PL/Python function "elog_test", line 18, in 
-plpy.error('stop on error', 'some detail','some hint')
-PL/Python function "elog_test"
+ERROR:  could not convert Python Unicode object to bytes
+DETAIL:  TypeError: bad argument type for built-in operation
+CONTEXT:  PL/Python function "elog_test"

This is related to the use of PyString_AsString and the changed
semantics of that in Python 3 (due to the fact that strings are now
Unicode objects and so on). Didn't have time to dig more deeply into
the exact cause.

Similarly, there are alternative expected test outputs that you didn't
update, for example src/pl/plpython/expected/plpython_types_3.out so
tests fail on some Python versions due to those as well.

> 1. the plpy utility functions can use all ErrorData fields,
> 2. there are no new functions,
> 3. via GUC plpythonu.legacy_custom_exception we can return previous behave,
> 4. only exception Error is raised with natural structure - no composite
> value spidata.
> 5. fields: message, detail and hint are implicitly translated to string - it
> decrease a necessity of legacy mode

I disagree that 5. is a good idea. I think we should just treat
message, detail and hint like the other ones (schema, table etc.). Use
s in PyArg_ParseTupleAndKeywords and let the user explicitly cast to a
string. Explicit is better than implicit. The way you did it you keep
part of the weird old interface which used to cast to string for you.
We shouldn't keep warts of the old behaviour, especially with the GUC
to ask for the old behaviour.

By the way, getting rid of object_to_string and its usage in
PLy_output_kw removed some "ERROR: could not convert Python Unicode
object to bytes" failures in the tests so I'm quite sure that the
usage of PyString_AsString is responsible for those.

I don't like that you set legacy_custom_exception=true in some
existing tests, probably to avoid changing them to the new behaviour.
We should trust that the new behaviour is what we want and the tests
should reflect that. If it's too much work, remember we're asking
users to do the same work to convert their code. We have
elog_test_legacy to test elog_test_legacy=true, the rest of the tests
should use legacy_custom_exception=false.


-- 
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] Freeze avoidance of very large table.

[Masahiko Sawada]

On Wed, Feb 17, 2016 at 12:02 AM, Bruce Momjian  wrote:
> On Tue, Feb 16, 2016 at 11:56:25PM +0900, Masahiko Sawada wrote:
>> > I agreed on ripping out the converter plugin ability of pg_upgrade.
>> > Remember pg_upgrade was originally written by EnterpriseDB staff, and I
>> > think they expected their closed-source fork of Postgres might need a
>> > custom page converter someday, but it never needed one, and at this
>> > point I think having the code in there is just making things more
>> > complex.  I see _no_ reason for community Postgres to use a plugin
>> > converter because we are going to need that code for every upgrade from
>> > pre-9.6 to 9.6+, so why not just hard-code in the functions we need.  We
>> > can remove it once 9.5 is end-of-life.
>> >
>>
>> Hm, we should rather remove the source code around PAGE_CONVERSION and
>> page.c at 9.6?
>
> Yes.  I can do it if you wish.

I see. I understand that page-converter code would be useful for some
future cases, but makes thing more complex.
So I will post the patch without page-converter If no objection from
other hackers.

Regards,

--
Masahiko Sawada


-- 
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] Small PATCH: check of 2 Perl modules

[Alvaro Herrera]

Victor Wagner wrote:

> Not everyone have "standard perl installation" nowadays. Most Linux
> users, for example, use Perl package from the distrubution, and
> distributions love to strip down standard perl installation putting its
> parts into separate packages, some of which might be optional.
> 
> For example, in Centos 6 it is part of perl-Test-Simple package. which
> might be not included into docker images or simular minimal systems for
> container-based deployment.

That's a good point ...

> So, it worth few lines of the configure.in to remind user that "Your
> perl installation is not standard enough".

... but I agree with the point upthread that this should wait to see
what happens with the CMake stuff, since this is not a newly introduced
problem.

-- 
Álvaro Herrerahttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, 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] [WIP] ALTER ... OWNER TO ... CASCADE

[Alvaro Herrera]

Vladimir Borodin wrote:

> > Moreover, the use case you've sketched (ie, change ownership of all
> > objects inside a database) doesn't actually have anything to do with
> > following dependencies.  It's a lot closer to REASSIGN OWNED ... in
> > fact, it's not clear to me why REASSIGN OWNED doesn't solve that
> > use-case already.
> 
> Sometimes I hit the following. You have created a database and schema
> inside it from the superuser (i.e. postgres). Than you want to change
> ownership of whole database to another user (i.e. alice), but only
> this database, not all other objects in all other databases. It seems
> that REASSIGN OWNED doesn’t solve this already.

So essentially you want to change all the objects in the database except
those that were created together with the database itself (i.e. those
that were copied from the template database).  That seems a reasonable
use-case, but I'm not sure that this ALTER .. OWNER CASCADE is the right
thing for that -- What object would you start with?  Each schema other
than pg_catalog, pg_toast, information_schema?  As I recall, the problem
is that REASSIGN OWNED refuses to work on pinned objects.  Maybe what
you want is something like
  REASSIGN OWNED BY xyz IN SCHEMA public TO xyzxxz
i.e., an extension of the current REASSIGN OWNED BY command?

-- 
Álvaro Herrerahttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, 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] [WIP] ALTER ... OWNER TO ... CASCADE

[Dmitry Ivanov]

> Sometimes I hit the following. You have created a database and schema inside
> it from the superuser (i.e. postgres). Than you want to change ownership of
> whole database to another user (i.e. alice), but only this database, not
> all other objects in all other databases. 

Actually, it skips all files that belong to irrelevant databases:

/*
 * We only operate on shared objects and objects in the current
 * database
 */
if (sdepForm->dbid != MyDatabaseId &&
sdepForm->dbid != InvalidOid)
continue;

> It seems that REASSIGN OWNED doesn’t solve this already.

Yes, it doesn't solve this case. This is due to the fact that if the superuser 
that created the database is 'pinned' (e.g. postgres), it is impossible to 
track any object which depends on him, since such a dependency is not present 
in the pg_shdepend (pay attention to the comment below):

if (isSharedObjectPinned(AuthIdRelationId, roleid, sdepRel))
{
.
ereport(ERROR,
(errcode(ERRCODE_DEPENDENT_OBJECTS_STILL_EXIST),
 errmsg("cannot reassign ownership

/*
 * There's no need to tell the whole truth, which is that we
 * didn't track these dependencies at all ...
 */
}

This prevents you from doing something like:

test=# reassign owned by postgres to test;
ERROR:  cannot reassign ownership of objects owned by role postgres because 
they are required by the database system

I think that my solution might fit better.

-- 
Dmitry Ivanov
Postgres Professional: http://www.postgrespro.com
Russian Postgres 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] [PROPOSAL] Improvements of Hunspell dictionaries support

[Artur Zakirov]

I attached a new version of the patch.

On 10.02.2016 19:46, Teodor Sigaev wrote:



I duplicate the patch here.


it's very good thing to update disctionaries to support modern versions.
And thank you for improving documentation. Also I've impressed by long
description in spell.c header.

Som notices about code:

1
  struct SPELL. Why do you remove union p? You leave comment
  about using d struct instead of flag field and as can see
  it's right comment. It increases size of SPELL structure.


Fixed.



2 struct AFFIX. I'm agree with Alvaro taht sum of sizes of bit fields
should be less or equal to size of integer. In opposite case, suppose,
we can get undefined behavior. Please, split  bitfields  to two integers.


Fixed.



3 unsigned char flagval[65000];
   Is it forbidden to use 6 number? In any case, decodeFlag() doesn't
   restrict return value. I suggest to enlarge array to 1<<16 and add limit
   to return value of decodeFlag().


flagval array was enlarged. Added verification of return value of 
DecodeFlag() for for various FLAG parameter (FM_LONG, FM_NUM and FM_CHAR).




4
  I'd like to see a short comment describing at least new functions


Added some comments which describe new functions and old functions for 
loading dictionaries into PostgreSQL. This patch adds new functions and 
modifies functions which is used for loading dictionaries.


At the moment, comments does not describe functions which used for word 
normalization. But I can add more comments.




5
  Pls, add tests for new code.




Added tests. Old sample dictionaries files was moved to the folder 
"dicts". New sample dictionaries files was added:

- hunspell_sample_long.affix
- hunspell_sample_long.dict
- hunspell_sample_num.affix
- hunspell_sample_num.dict

--
Artur Zakirov
Postgres Professional: http://www.postgrespro.com
Russian Postgres Company
*** a/doc/src/sgml/textsearch.sgml
--- b/doc/src/sgml/textsearch.sgml
***
*** 2615,2632  SELECT plainto_tsquery('supernova star');
 
  
 
! To create an Ispell dictionary, use the built-in
! ispell template and specify several parameters:
 
! 
  
! CREATE TEXT SEARCH DICTIONARY english_ispell (
  TEMPLATE = ispell,
! DictFile = english,
! AffFile = english,
! StopWords = english
! );
  
  
 
  Here, DictFile, AffFile, and StopWords
--- 2615,2655 
 
  
 
! To create an Ispell dictionary perform these steps:
 
!
! 
!  
!   download dictionary configuration files. OpenOffice
!   extension files have the .oxt extension. It is necessary
!   to extract .aff and .dic files, change extensions
!   to .affix and .dict. For some dictionary
!   files it is also needed to convert characters to the UTF-8 encoding
!   with commands (for example, for norwegian language dictionary):
  
! iconv -f ISO_8859-1 -t UTF-8 -o nn_no.affix nn_NO.aff
! iconv -f ISO_8859-1 -t UTF-8 -o nn_no.dict nn_NO.dic
! 
!  
! 
! 
!  
!   copy files to the $SHAREDIR/tsearch_data directory
!  
! 
! 
!  
!   load files into PostgreSQL with the following command:
! 
! CREATE TEXT SEARCH DICTIONARY english_hunspell (
  TEMPLATE = ispell,
! DictFile = en_us,
! AffFile = en_us,
! Stopwords = english);
  
+  
+ 
+
  
 
  Here, DictFile, AffFile, and StopWords
***
*** 2643,2648  CREATE TEXT SEARCH DICTIONARY english_ispell (
--- 2666,2720 
 
  
 
+ The .affix file of Ispell has the following structure:
+ 
+ prefixes
+ flag *A:
+ .   >   RE  # As in enter > reenter
+ suffixes
+ flag T:
+ E   >   ST  # As in late > latest
+ [^AEIOU]Y   >   -Y,IEST # As in dirty > dirtiest
+ [AEIOU]Y>   EST # As in gray > grayest
+ [^EY]   >   EST # As in small > smallest
+ 
+
+
+ And the .dict file has the following structure:
+ 
+ lapse/ADGRS
+ lard/DGRS
+ large/PRTY
+ lark/MRS
+ 
+
+ 
+
+ Format of the .dict file is:
+ 
+ basic_form/affix_class_name
+ 
+
+ 
+
+ In the .affix file every affix flag is described in the
+ following format:
+ 
+ condition > [-stripping_letters,] adding_affix
+ 
+
+ 
+
+ Here, condition has a format similar to the format of regular expressions.
+ It can use groupings [...] and [^...].
+ For example, [AEIOU]Y means that the last letter of the word
+ is "y" and the penultimate letter is "a",
+ "e", "i", "o" or "u".
+ [^EY] means that the last letter is neither "e"
+ nor "y".
+
+ 
+
  Ispell dictionaries support splitting compound words;
  a useful feature.
  Notice that the affix file should specify a special flag using the
***
*** 2663,2668  SELECT ts_lexize('norwegian_ispell', 'sjokoladefabrikk');
--- 2735,2796 
  
 
  
+
+ MySpell is very similar to Hunspell.
+ The .affix file of Hunspell has the 

Re: [HACKERS] Remove or weaken hints about "effective resolution of sleep delays is 10 ms"?

[Tom Lane]

Robert Haas  writes:
> On Tue, Feb 16, 2016 at 3:50 AM, Andres Freund  wrote:
>> To back up my claim on this, read man 7 time
>> (e.g. http://linux.die.net/man/7/time), especially "The software clock,
>> HZ, and jiffies" and "High-resolution timers". To quote the most salient
>> point:

> Interesting, thanks.

Yeah.  "grep resolution /proc/timer_list" on my RHEL6 box says that the
timers all have 1ns resolution!

Given this, I'm on board with just removing the weasel-wording about
timer resolution, except maybe for commit_delay where useful values
are small enough that it's a hazard on old systems.

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] Freeze avoidance of very large table.

[Bruce Momjian]

On Tue, Feb 16, 2016 at 11:56:25PM +0900, Masahiko Sawada wrote:
> > I agreed on ripping out the converter plugin ability of pg_upgrade.
> > Remember pg_upgrade was originally written by EnterpriseDB staff, and I
> > think they expected their closed-source fork of Postgres might need a
> > custom page converter someday, but it never needed one, and at this
> > point I think having the code in there is just making things more
> > complex.  I see _no_ reason for community Postgres to use a plugin
> > converter because we are going to need that code for every upgrade from
> > pre-9.6 to 9.6+, so why not just hard-code in the functions we need.  We
> > can remove it once 9.5 is end-of-life.
> >
> 
> Hm, we should rather remove the source code around PAGE_CONVERSION and
> page.c at 9.6?

Yes.  I can do it if you wish.

-- 
  Bruce Momjian  http://momjian.us
  EnterpriseDB http://enterprisedb.com

+ As you are, so once was I. As I am, so you will be. +
+ Roman grave inscription +


-- 
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] commitfest problem ?

[Artur Zakirov]

On 16.02.2016 17:50, Oleg Bartunov wrote:

This entry https://commitfest.postgresql.org/8/419/ contains very
unrelated patches from another commitfest. I think


Oleg


No, this is not commitfest problem. The part of the thread "[PROPOSAL] 
Improvements of Hunspell dictionaries support" 
(http://www.postgresql.org/message-id/56aa02ee.6090...@postgrespro.ru) 
was moved to the thread "[PATCH] we have added support for box type in 
SP-GiST index" by mistake.


I had noticed it too late. I was writing into the wrong thread. And now 
commitfest shows wrong last attachment.


--
Artur Zakirov
Postgres Professional: http://www.postgrespro.com
Russian Postgres 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: [JDBC] [HACKERS] Packaging of postgresql-jdbc

[Tom Lane]

Pavel Raiskup  writes:
> Oh, Pavel probably forgot to CC pgjdbc, fixing.

In fact, this is completely off-topic for pgsql-hackers, please confine
the discussion to pgsql-jdbc.

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] Freeze avoidance of very large table.

[Masahiko Sawada]

On Tue, Feb 16, 2016 at 6:13 AM, Bruce Momjian  wrote:
> On Wed, Feb 10, 2016 at 04:39:15PM +0900, Kyotaro HORIGUCHI wrote:
>> > I still agree with this plugin approach, but I felt it's still
>> > complicated a bit, and I'm concerned that patch size has been
>> > increased.
>> > Please give me feedbacks.
>>
>> Yeah, I feel the same. What make it worse, the plugin mechanism
>> will get further complex if we make it more flexible for possible
>> usage as I proposed above. It is apparently too complicated for
>> deciding whether to load *just one*, for now, converter
>> function. And no additional converter is in sight.
>>
>> I incline to pull out all the plugin stuff of pg_upgrade. We are
>> so prudent to make changes of file formats so this kind of events
>> will happen with several-years intervals. The plugin mechanism
>> would be valuable if we are encouraged to change file formats
>> more frequently and freely by providing it, but such situation
>> absolutely introduces more untoward things..
>
> I agreed on ripping out the converter plugin ability of pg_upgrade.
> Remember pg_upgrade was originally written by EnterpriseDB staff, and I
> think they expected their closed-source fork of Postgres might need a
> custom page converter someday, but it never needed one, and at this
> point I think having the code in there is just making things more
> complex.  I see _no_ reason for community Postgres to use a plugin
> converter because we are going to need that code for every upgrade from
> pre-9.6 to 9.6+, so why not just hard-code in the functions we need.  We
> can remove it once 9.5 is end-of-life.
>

Hm, we should rather remove the source code around PAGE_CONVERSION and
page.c at 9.6?

Regards,

--
Masahiko Sawada


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

[HACKERS] commitfest problem ?

[Oleg Bartunov]

This entry https://commitfest.postgresql.org/8/419/ contains very unrelated
patches from another commitfest. I think


Oleg

Re: [HACKERS] pglogical - logical replication contrib module

[Bruce Momjian]

On Tue, Jan 26, 2016 at 08:14:26PM -0800, Joshua Drake wrote:
> On 12/31/2015 03:34 PM, Petr Jelinek wrote:
> >Hi,
> >
> >I'd like to submit the replication solution which is based on the
> >pglogical_output [1] module (which is obviously needed for this to
> >compile).
> 
> This is fantastic! However, history presents itself here and
> PostgreSQL in the past has not "blessed" a single solution for
> Replication. Obviously that changed a bit with streaming replication
> but this is a bit different than that. As I understand it, PgLogical
> is Logical Replication (similar to Slony and Londiste). I wouldn't
> be surprised (although I don't know) if Slony were to start using
> some of the pglogical_output module features in the future.
> 
> If we were to accept PgLogical into core, it will become the default
> blessed solution for PostgreSQL. While that is great in some ways
> it is a different direction than the project has taken in the past.
> Is this what we want to do?

Replying late here, but I think with binary replication, we decided
that, assuming you were happy with the features provided, our streaming
binary replication solution was going to be the best and recommended way
of doing it.

I don't think we ever had that feeling with Slony or Londiste in that
there were so many limitations and so many different ways of
implementing logical replication that we never recommended a best way.

So, the question is, do we feel that PgLogical is best and recommended
way to do logical replication.  If it is, then having it in core makes
sense.

-- 
  Bruce Momjian  http://momjian.us
  EnterpriseDB http://enterprisedb.com

+ As you are, so once was I. As I am, so you will be. +
+ Roman grave inscription +


-- 
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] exposing pg_controldata and pg_config as functions

[Joe Conway]

On 02/15/2016 11:20 PM, Michael Paquier wrote:
> Here are just a couple of cosmetic comments.

> Missing markup  around PostgreSQL.

Added, but I'll note that there are tons of locations in the docs where
we do not do that. Maybe they should all be made consistent.

> +   Application. There is a System Information Function
> Why is using upper-case characters necessary here? This could just say
> system function.

It is capitalized because it refers to a section in the docs. I followed
it with  as well, so it ends up reading:
"There is a System Information Function (Section 9.25) called pg_config
which underlies this view."

I guess I could be convinced to lower case it, but I thought this looked
better.

> The paragraph in func.sgml is a copy-paste of the one in
> catalogs.sgml. We may want to remove the duplication.

The paragraphs are mostly copy-paste, but slightly different (toward the
end). There is both a function and a system view -- why would we not
want a description in both places?


> +   /* let the caller know we're sending back a tuplestore */
> +   rsinfo->returnMode = SFRM_Materialize;
> I guess one can recognize your style here for SRF functions :)

Old habits die hard ;-)

> @@ -61,7 +74,7 @@ libpgcommon_srv.a: $(OBJS_SRV)
>  # a hack that might fail someday if there is a *_srv.o without a
>  # corresponding *.o, but it works for now.
>  %_srv.o: %.c %.o
> -   $(CC) $(CFLAGS) $(subst -DFRONTEND,, $(CPPFLAGS)) -c $< -o $@
> +   $(CC) $(CFLAGS) $(subst -DFRONTEND ,, $(CPPFLAGS)) -c $< -o $@
> Diff noise?

No, intentional. The original version leaves a white space at the
beginning of CPPFLAGS after removing -DFRONTEND.

> --- /dev/null
> +++ b/src/common/config_info.c
> [...]
> + * IDENTIFICATION
> + *   src/common/controldata_utils.c
> This is incorrect.

Right -- found and fixed several of these with Alvaro's help after
posting. Also fixed Copyright dates (s/2015/2016/) on the new files.


> + * IDENTIFICATION
> + *   src/backend/utils/misc/pg_config.c
> + *
> + */
> I am nitpicking here but this header block should have a long
> "" at its bottom.

Fair enough -- fixed.

Thanks!

Joe

-- 
Crunchy Data - http://crunchydata.com
PostgreSQL Support for Secure Enterprises
Consulting, Training, & Open Source Development
diff --git a/doc/src/sgml/catalogs.sgml b/doc/src/sgml/catalogs.sgml
index 412c845..7b71768 100644
*** a/doc/src/sgml/catalogs.sgml
--- b/doc/src/sgml/catalogs.sgml
***
*** 7350,7355 
--- 7350,7360 
   
  
   
+   pg_config
+   compile-time configuration parameters
+  
+ 
+  
pg_cursors
open cursors
   
***
*** 7609,7614 
--- 7614,7668 

   
  
+  
+   pg_config
+ 
+   
+pg_config
+   
+ 
+   
+The view pg_config describes the
+compile-time configuration parameters of the currently installed
+version of PostgreSQL. It is intended, for example, to
+be used by software packages that want to interface to
+PostgreSQL to facilitate finding the required header
+files and libraries. It provides the same basic information as the
+ PostgreSQL Client
+Application. There is a System Information Function
+() called pg_config
+which underlies this view.
+   
+ 
+   
+pg_config Columns
+
+ 
+  
+   Name
+   Type
+   Description
+  
+ 
+ 
+ 
+  
+   name
+   text
+   The parameter name
+  
+ 
+  
+   setting
+   text
+   The parameter value
+  
+ 
+
+   
+ 
+  
+ 
   
pg_cursors
  
diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
index f9eea76..0e17ca3 100644
*** a/doc/src/sgml/func.sgml
--- b/doc/src/sgml/func.sgml
*** SELECT * FROM pg_ls_dir('.') WITH ORDINA
*** 15003,15008 
--- 15003,15014 

  

+pg_config()
+setof record
+get list of compile-time configure variable names and their values
+   
+ 
+   
 pg_is_other_temp_schema(oid)
 boolean
 is schema another session's temporary schema?
*** SET search_path TO schema
  
 
+ pg_config
+
+ 
+
+ pg_config returns a set of records describing the
+ compile-time configuration parameters of the currently installed
+ version of PostgreSQL. It is intended, for example, to
+ be used by software packages that want to interface to
+ PostgreSQL to facilitate finding the required header
+ files and libraries. The name column contains the
+ parameter name. The setting column contains the
+ parameter value. It provides the same basic information as the
+  PostgreSQL Client
+ Application. There is also a  system
+ view.
+
+ 
+
  version
 
  
diff --git a/src/backend/catalog/system_views.sql b/src/backend/catalog/system_views.sql
index 923fe58..abf9a70 100644
*** a/src/backend/catalog/system_views.sql
--- 

Re: [HACKERS] extend pgbench expressions with functions

[Fabien COELHO]

Hello Robert,


However, for obvious reasons the committer opinion prevails:-)


You're welcome to solicit other opinions.  I'm not unwilling to give
way if there's a chorus of support for the way you've done it.


Hmmm. I do not expect much chorus on such a minor subject:-)

But to me it sounds like you're saying that it doesn't really matter 
whether the system is scalable and maintainable because we only have 5 
functions right now, which is a design philosophy with which I just 
don't agree.


The design does not aim at scalability but at simplicity, otherwise I 
would have done things quite differently: with many functions the "switch" 
based selection does not scale anyway.


Anyway, attached are two patches, one for each approach.

The array (second patch) is not too bad if one agrees with a maximum 
number of arguments, and also as I did not change the list structure 
coming from the parser, so it does not need to manage the number of 
arguments in the function structure. The code for min/max is also simpler 
when dealing with an array instead of a linked list. I do not like much 
array references in the code, so I tried to avoid them by using lval/rval 
scalars for operator operands.


Please choose the one you prefer, and I'll adapt the remaining stuff to 
your choice.


--
Fabien.diff --git a/doc/src/sgml/ref/pgbench.sgml b/doc/src/sgml/ref/pgbench.sgml
index ade1b53..f39f341 100644
--- a/doc/src/sgml/ref/pgbench.sgml
+++ b/doc/src/sgml/ref/pgbench.sgml
@@ -786,7 +786,7 @@ pgbench  options  dbname
   
 
   
-   
+   
 
  \set varname expression
 
@@ -798,8 +798,10 @@ pgbench  options  dbname
   The expression may contain integer constants such as 5432,
   references to variables :variablename,
   and expressions composed of unary (-) or binary operators
-  (+, -, *, /, %)
-  with their usual associativity, and parentheses.
+  (+, -, *, /,
+  %) with their usual associativity,
+  function calls, and
+  parentheses.
  
 
  
@@ -994,6 +996,62 @@ END;
 
  
 
+ 
+  Built-In Functions
+
+   
+ The following functions are built into pgbench and
+ may be used in conjunction with
+ \set.
+   
+
+   
+   
+pgbench Functions
+
+ 
+  
+   Function
+   Return Type
+   Description
+   Example
+   Result
+  
+ 
+ 
+  
+   abs(a)
+   same as a
+   integer value
+   abs(-17)
+   17
+  
+  
+   debug(a)
+   same as a 
+   print to stderr the given argument
+   debug(5432)
+   5432
+  
+  
+   max(i [, ... ] )
+   integer
+   maximum value
+   max(5, 4, 3, 2)
+   5
+  
+  
+   min(i [, ... ] )
+   integer
+   minimum value
+   min(5, 4, 3, 2)
+   2
+  
+ 
+ 
+   
+ 
+
  
   Per-Transaction Logging
 
diff --git a/src/bin/pgbench/exprparse.y b/src/bin/pgbench/exprparse.y
index 06ee04b..cac4d5e 100644
--- a/src/bin/pgbench/exprparse.y
+++ b/src/bin/pgbench/exprparse.y
@@ -16,10 +16,13 @@
 
 PgBenchExpr *expr_parse_result;
 
+static PgBenchExprList *make_elist(PgBenchExpr *exp, PgBenchExprList *list);
 static PgBenchExpr *make_integer_constant(int64 ival);
 static PgBenchExpr *make_variable(char *varname);
-static PgBenchExpr *make_op(char operator, PgBenchExpr *lexpr,
+static PgBenchExpr *make_op(const char *operator, PgBenchExpr *lexpr,
 		PgBenchExpr *rexpr);
+static int find_func(const char *fname);
+static PgBenchExpr *make_func(const int fnumber, PgBenchExprList *args);
 
 %}
 
@@ -31,13 +34,15 @@ static PgBenchExpr *make_op(char operator, PgBenchExpr *lexpr,
 	int64		ival;
 	char	   *str;
 	PgBenchExpr *expr;
+	PgBenchExprList *elist;
 }
 
+%type  elist
 %type  expr
-%type  INTEGER
-%type  VARIABLE
+%type  INTEGER function
+%type  VARIABLE FUNCTION
 
-%token INTEGER VARIABLE
+%token INTEGER VARIABLE FUNCTION
 %token CHAR_ERROR /* never used, will raise a syntax error */
 
 /* Precedence: lowest to highest */
@@ -49,16 +54,25 @@ static PgBenchExpr *make_op(char operator, PgBenchExpr *lexpr,
 
 result: expr{ expr_parse_result = $1; }
 
+elist:  	{ $$ = NULL; }
+	| expr 	{ $$ = make_elist($1, NULL); }
+	| elist ',' expr		{ $$ = make_elist($3, $1); }
+	;
+
 expr: '(' expr ')'			{ $$ = $2; }
 	| '+' expr %prec UMINUS	{ $$ = $2; }
-	| '-' expr %prec UMINUS	{ $$ = make_op('-', make_integer_constant(0), $2); }
-	| expr '+' expr			{ $$ = make_op('+', $1, $3); }
-	| expr '-' expr			{ $$ = make_op('-', $1, $3); }
-	| expr '*' expr			{ $$ = make_op('*', $1, $3); }
-	| expr '/' expr			{ $$ = make_op('/', $1, $3); }
-	| expr '%' expr			{ $$ = make_op('%', $1, $3); }
+	| '-' expr %prec UMINUS	{ $$ = make_op("-", make_integer_constant(0), $2); }
+	| expr '+' expr			{ $$ = make_op("+", $1, $3); }
+	| expr '-' expr			{ $$ = make_op("-", $1, $3); }
+	| expr '*' expr			{ $$ = make_op("*", $1, $3); }
+	| expr '/' expr			{ $$ = make_op("/", $1, $3); }
+	| expr '%' expr			{ 

Re: [HACKERS] Small PATCH: check of 2 Perl modules

[Victor Wagner]

On Mon, 15 Feb 2016 08:34:11 -0500
Peter Eisentraut  wrote:

> On 2/12/16 8:20 AM, Eugene Kazakov wrote:
> > TAP-tests need two Perl modules: Test::More and IPC::Run.
> > 
> > The patch adds checking of modules in configure.in and configure.  
> 
> I think those modules are part of a standard Perl installation.

Not everyone have "standard perl installation" nowadays. Most Linux
users, for example, use Perl package from the distrubution, and
distributions love to strip down standard perl installation putting its
parts into separate packages, some of which might be optional.

For example, in Centos 6 it is part of perl-Test-Simple package. which
might be not included into docker images or simular minimal systems for
container-based deployment.

So, it worth few lines of the configure.in to remind user that "Your
perl installation is not standard enough".
-- 
 



-- 
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] pgbench - allow backslash-continuations in custom scripts

[Robert Haas]

On Mon, Feb 15, 2016 at 1:04 AM, Kyotaro HORIGUCHI
 wrote:
> I'm sorry, but I didn't understood the 'submission notes' exactly
> means. Is it precise descriptions in source comments? or commit
> message of git-commit?

Write a detailed email explaining each change that is part of the
patch and why it is there.  Attach the patch to that same email.

-- 
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] Packaging of postgresql-jdbc

[Dave Cramer]

On 16 February 2016 at 07:50, Craig Ringer  wrote:

> On 16 February 2016 at 20:15, Pavel Kajaba  wrote:
>
>> Hello pg-hackers,
>>
>> I need advice about postgresql-jdbc driver.
>>
>> Current version in Fedora is behind latest version of postgresql-jdbc
>> (1200 vs 1207).
>>
>> We are trying to package latest version into Fedora, but there are
>> dependencies, which are not useless in Fedora (waffle-jna)
>
>
> Which *are* useless in Fedora. I know that was just an editing mistake.
> It's a library used in PgJDBC for windows SSPI support.
>
> I don't really see the problem here. If your packaging policy prevents you
> from incorporating it, patch it out. It's use is simple, self-contained and
> already optional.
>
>
>> and ones which we are not 100% open source (osgi-enterprise). We talked
>> with upstream quite intensively but not been able to find any solution
>> which would meet our requirements.
>>
>
> ... which you should probably outline here, because otherwise nobody  will
> understand the problem.
>
>
>> We think that it's not a good, when open-source project depending on
>> packages, which licence is not 100% clear.
>>
>
> Well, frankly, that's Java. So long as they're soft-dependencies I really
> don't care.
>
>
I've already explained the JDBC position here.

There is an impedance mismatch between the java ecosystem and distros.

We have moved to maven as have most other java projects.


As Craig said, if you want to build it, patch it out, and create a
ant/Makefile to make the jar.

Dave Cramer

da...@postgresintl.com
www.postgresintl.com

Re: [HACKERS] Declarative partitioning

[Robert Haas]

On Fri, Jan 22, 2016 at 8:54 AM, Tomas Vondra
 wrote:
> I'd like to comment on the one thing and that's the syntax. It seems to me
> we're really trying to reinvent the wheel and come up with our own version
> of the syntax. Is there a particular reason why not to look at the syntax of
> the other databases and adapt as much of the existing syntax as possible?

I'm not sure this is at all standardized.  How do we decide whose
syntax to adopt?  Oracle's just because it's Oracle?  I don't
particularly like inheriting the limitations of Oracle's syntax - such
as at most 2 levels of partitioning, weird VALUES LESS THAN syntax
that doesn't look like anything else we do in PG and has no way to
specify an opclass, etc.

> I think that's for a few reasons - firstly it makes the life much easier for
> the DBAs and users who are either migrating to PostgreSQL or have to manage
> a mix of databases. Secondly, it serves as a valuable source of engineering
> info, preventing the "I haven't thought of this use case" problem.
>
> An example of this is the proposed syntax for adding a partition
>
>   CREATE TABLE measurement_fail
>   PARTITION OF measurement
>   FOR VALUES START ('2006-02-15') END ('2006-03-01');
>
> which seems a bit awkward as both the databases I'm familiar with (Oracle
> and Sybase) use ALTER TABLE to do this
>
>   ALTER TABLE measurement
> ADD PARTITION measurement_fail VALUES LESS THAN ( ... )
>
> And so on for the other commands.

One thing to think about is that pg_dump --binary-upgrade needs to
decorate each command that creates a relation.  I guess you could
decorate ALTER TABLE with the same stuff we currently apply to CREATE
TABLE but...

> That being said, I entirely agree with Simon (and others) that getting the
> planner part work is the crucial part of the patch. But I also think that a
> proper abstraction (thanks to good syntax) may be a valuable hint how to
> define the catalogs and such.

No argument on that from me.

-- 
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] Packaging of postgresql-jdbc

[Pavel Raiskup]

Oh, Pavel probably forgot to CC pgjdbc, fixing.

Forwarded message from Craig Ringer:

On 16 February 2016 at 20:15, Pavel Kajaba  wrote:
> Hello pg-hackers,
>
> I need advice about postgresql-jdbc driver.
>
> Current version in Fedora is behind latest version of postgresql-jdbc
> (1200 vs 1207).
>
> We are trying to package latest version into Fedora, but there are
> dependencies, which are not useless in Fedora (waffle-jna)


Which *are* useless in Fedora. I know that was just an editing mistake.
It's a library used in PgJDBC for windows SSPI support.

I don't really see the problem here. If your packaging policy prevents you
from incorporating it, patch it out. It's use is simple, self-contained and
already optional.


> and ones which we are not 100% open source (osgi-enterprise). We talked
> with upstream quite intensively but not been able to find any solution
> which would meet our requirements.
>

... which you should probably outline here, because otherwise nobody  will
understand the problem.


> We think that it's not a good, when open-source project depending on
> packages, which licence is not 100% clear.
>

Well, frankly, that's Java. So long as they're soft-dependencies I really
don't care.



-- 
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] Sequence Access Method WIP

[Alexander Korotkov]

On Sat, Jan 30, 2016 at 3:37 PM, Petr Jelinek  wrote:

> On 29 January 2016 at 23:59, Robert Haas  wrote:
> > On Fri, Jan 29, 2016 at 5:24 PM, Tom Lane  wrote:
> >> Alexander Korotkov  writes:
> >>> On Fri, Jan 29, 2016 at 6:36 PM, Alvaro Herrera <
> alvhe...@2ndquadrant.com>
> >>> wrote:
>  I'm thinking we'd do CREATE ACCESS METHOD foobar TYPE INDEX or
> something
>  like that.
> >>
> >>> I would prefer "CREATE {INDEX | SEQUENCE | ... } ACCESS METHOD name
> HANDLER
> >>> handler;", but I don't insist.
> >>
> >> I think that Alvaro's idea is less likely to risk future grammar
> >> conflicts.  We've had enough headaches from CREATE [ UNIQUE ] INDEX
> >> [ CONCURRENTLY ] to make me really wary of variables in the
> statement-name
> >> part of the syntax.
> >
> > Strong +1.  If we put the type of access method immediately after
> > CREATE, I'm almost positive we'll regret it for exactly that reason.
> >
>
> Just as a note, CREATE SEQUENCE ACCESS METHOD already causes grammar
> conflict now, that's why my proposal was different, I didn't want to
> add more keywords. I think Alvaro's proposal is fine as well.
>
> The other point is that we are creating ACCESS METHOD object so that's
> what should be after CREATE.
>
> In any case this is slightly premature IMHO as DDL is somewhat unless
> until we have sequence access methods implementation we can agree on,
> or the generic WAL logging so that custom indexes can be crash safe.


I've changed syntax of CREATE ACCESS METHOD syntax in the thread about
index access method extendability.
Now it is "CREATE ACCESS METHOD name TYPE INDEX HANDLER handler;". New
column amtype of pg_am stores access method type.
http://www.postgresql.org/message-id/capphfdu9gln7kuicwegsp50caamwx8q-jwzbpehc92rvfhz...@mail.gmail.com
It could be easily extended to "CREATE ACCESS METHOD name TYPE SEQUENCE
HANDLER handler;".

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

Re: [HACKERS] Declarative partitioning

[Robert Haas]

On Fri, Jan 15, 2016 at 5:48 AM, Amit Langote
 wrote:
> If we have a CREATE statement for each partition, how do we generalize
> that to partitions at different levels? For example, if we use something
> like the following to create a partition of parent_name:
>
> CREATE PARTITION partition_name OF parent_name FOR VALUES ...
> WITH ... TABLESPACE ...
>
> Do we then say:
>
> CREATE PARTITION subpartition_name OF partition_name ...
>
> to create a level 2 partition (sub-partition) of parent_name?

Yes, exactly.

Personally, I would be more inclined to make this a CREATE TABLE statement, like

CREATE TABLE partition_name PARTITION OF parent_name FOR VALUES ...
CREATE TABLE subpartition_name PARTITION OF partition_name FOR VALUES ...

> I ask that also because it's related to the choice of syntax to use to
> declare the partition key for the multi-level case. I'm considering the
> SUBPARTITION BY notation and perhaps we could generalize it to more than
> just 2 levels. So, for the above case, parent_name would have been created as:
>
> CREATE TABLE parent_name PARTITION BY ... SUBPARTITION BY ...
>
> Needless to say, when subpartition_name is created with the command we saw
> a moment ago, the root partitioned table would be locked. In fact, adding
> a partition anywhere in the hierarchy needs an exclusive lock on the root
> table. Also, partition rule (the FOR VALUES clause) would be validated
> against PARTITION BY or SUBPARTITION BY clause at the respective level.
>
> Although, I must admit I feel a little uneasy about the inherent asymmetry
> in using SUBPARTITION BY for key declaration whereas piggybacking CREATE
> PARTITION for creating sub-partitions. Is there a better way?

I think if you've got SUBPARTITION as a keyword in the syntax
anywhere, you're doing it wrong.  The toplevel object shouldn't really
care whether its children are themselves partitioned or not.

> Do we want this at all? It seems difficult to generalize this to
> multi-level hierarchy of more than 2 levels.

It doesn't do anything for me.  There may be somebody who wants it,
but I don't see much value myself.

> After thinking some more on this - I think that identical tuple
> descriptors may not just be a nice-to-have but critical in some cases. For
> example, consider built-in/trigger-less tuple routing. I'd imagine that
> the partition to insert a tuple into would be determined just before
> calling heap_insert() in ExecInsert() and CopyFrom(). That means the
> HeapTuple that is passed to heap_insert() to insert into the partition
> would be based on the root table's tuple descriptor. Note also that the
> tuple would have passed through BR, IR triggers, constraints of the root
> table. When the data is eventually queried from partitions directly, or
> well even via the root table (considering existing executor capabilities),
> partition's tuple descriptor at that point had better match the data that
> went onto the disk. That means we had better keep at least the following
> things in sync: number of attributes, name, position (attnum), type,
> notnull-ness of individual attributes. So in order to do that, recursively
> apply ADD/DROP COLUMN, SET WITH/WITHOUT OIDS, RENAME COLUMN, ALTER COLUMN
> TYPE, SET/DROP NOT NULL on the root table to all the partitions and
> prevent those sub-commands to be directly applied to any table
> (partitions) in the partitioning hierarchy but the root. I further don't
> see the point of allowing to set (or drop) column defaults in partitions
> because now INSERT or COPY FROM cannot be directly applied to partitions.
> Similar argument could be made for BR, IR triggers and CHECK constraints.
> Am I missing something in all of this?

Well, in the end there are basically two choices.  Either tuple
descriptors have to match exactly, and then you can reuse a tuple
intended for one partition for some other partition without
projection; or else they don't, and you need to project.  I'm not sure
that projection is expensive enough to make disallowing mismatched
tuple descriptors a necessary design choice - and certainly that
design choice is awkward from a UI standpoint, because we will
sometimes not be able to attach a partition for a reason that the user
can neither see in the \d output nor correct.  But on the flip side,
not having to worry about projection is nice.

> An alternative to doing any of that very well may be to design
> trigger-less tuple routing to be smarter about possible mismatch of the
> tuple descriptors but I haven't given that a lot of thought. Is that
> really an alternative worth looking into?

Yes.

> On one hand, I think to keep treating "partition hierarchies" as
> "inheritance hierachies" might have some issues. I am afraid that
> documented inheritance semantics may not be what we want to keep using for
> the new partitioned tables. By that, I mean all the user-facing behaviors
> where inheritance has some bearing. Should it also affect new 

Re: [HACKERS] GetExistingLocalJoinPath() vs. the docs

[Ashutosh Bapat]

On Mon, Feb 15, 2016 at 9:11 PM, Stephen Frost  wrote:

> Greetings,
>
> While getting back into the thread Re: Optimization for updating foreign
> tables in Postgres FDW, I noticed some issues with the docs and
> GetExistingLocalJoinPath():
>
> GetExistingLocalJoinPath() exists in src/backend/foreign/foreign.c, but
> the docs include discussion of it under 54.2 - Foreign Data Wrapper
> Callback Routines.  Shouldn't this be under 54.3. Foreign Data Wrapper
> Helper Functions?


Yes


> Also, the prototype is incorrect in the docs (it
> doesn't return a void)


Thanks for pointing that out.


> and the paragraph about what it's for could do
> with some wordsmithing.
>

Any specific suggestions?


>
> A link from 54.2 to 54.3 which mentions it would be fine, of course.
>

Ok

PFA patch fixing those things.


-- 
Best Wishes,
Ashutosh Bapat
EnterpriseDB Corporation
The Postgres Database Company


geljp_doc.patch
Description: application/download

-- 
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] Packaging of postgresql-jdbc

[Craig Ringer]

On 16 February 2016 at 20:15, Pavel Kajaba  wrote:

> Hello pg-hackers,
>
> I need advice about postgresql-jdbc driver.
>
> Current version in Fedora is behind latest version of postgresql-jdbc
> (1200 vs 1207).
>
> We are trying to package latest version into Fedora, but there are
> dependencies, which are not useless in Fedora (waffle-jna)


Which *are* useless in Fedora. I know that was just an editing mistake.
It's a library used in PgJDBC for windows SSPI support.

I don't really see the problem here. If your packaging policy prevents you
from incorporating it, patch it out. It's use is simple, self-contained and
already optional.


> and ones which we are not 100% open source (osgi-enterprise). We talked
> with upstream quite intensively but not been able to find any solution
> which would meet our requirements.
>

... which you should probably outline here, because otherwise nobody  will
understand the problem.


> We think that it's not a good, when open-source project depending on
> packages, which licence is not 100% clear.
>

Well, frankly, that's Java. So long as they're soft-dependencies I really
don't care.

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

Re: [HACKERS] extend pgbench expressions with functions

[Robert Haas]

On Tue, Feb 16, 2016 at 5:18 AM, Fabien COELHO  wrote:
>>> Good point. One simple idea here would be to use a custom pgbench
>>> script that has no SQL commands and just calculates the values of some
>>> parameters to measure the impact without depending on the backend,
>>> with a fixed number of transactions.
>>
>> Sure, we could do that.  But whether it materially changes pgbench -S
>> results, say, is a lot more important.
>
>
> Indeed. Several runs on my laptop:
>
>   ~ 40-54 tps with master using:
> \set naccounts 10 * :scale
> \setrandom aid 1 :naccounts
>
>   ~ 43-53 tps with full function patch using:
> \set naccounts 10 * :scale
> \setrandom aid 1 :naccounts
>
>   ~ 73-89 tps with full function patch using:
> \set aid random(1, 10 * :scale)
>
> The performance is pretty similar on the same script. The real pain is
> variable management, avoiding some is a win.

Wow, that's pretty nice.

-- 
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] extend pgbench expressions with functions

[Robert Haas]

On Tue, Feb 16, 2016 at 3:53 AM, Fabien COELHO  wrote:
> My opinion is that the submitted version is simple and fine for the purpose,
> and the plan you suggest replaces 5*2 repetitions by a lot of code and
> complexity, which is not desirable and should be avoided.
>
> However, for obvious reasons the committer opinion prevails:-)

You're welcome to solicit other opinions.  I'm not unwilling to give
way if there's a chorus of support for the way you've done it.  But to
me it sounds like you're saying that it doesn't really matter whether
the system is scalable and maintainable because we only have 5
functions right now, which is a design philosophy with which I just
don't agree.

-- 
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] Remove or weaken hints about "effective resolution of sleep delays is 10 ms"?

[Robert Haas]

On Tue, Feb 16, 2016 at 3:50 AM, Andres Freund  wrote:
> On 2016-02-16 09:13:09 +0100, Andres Freund wrote:
>> What we do we think the resolution is on modern
>> >systems?  I would not have guessed that to be inaccurate.
>>
>> Depends in a lot of factors. The biggest being how busy you're system
>> is. On an mostly idle system (i.e. workout so CPUs being
>> overcommitted) you can get resolutions considerably below one
>> millisecond. HPET can get you very low latencies, making OS scheduling
>> latencies the dominant factor, but one that can be tuned.
>
> To back up my claim on this, read man 7 time
> (e.g. http://linux.die.net/man/7/time), especially "The software clock,
> HZ, and jiffies" and "High-resolution timers". To quote the most salient
> point:

Interesting, thanks.

-- 
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] postgres_fdw vs. force_parallel_mode on ppc

[Andrew Dunstan]

On 02/15/2016 07:57 PM, Tom Lane wrote:

Noah Misch  writes:

On Mon, Feb 15, 2016 at 07:31:40PM -0500, Robert Haas wrote:

Oh, crap.  I didn't realize that TEMP_CONFIG didn't affect the contrib
test suites.  Is there any reason for that, or is it just kinda where
we ended up?

To my knowledge, it's just the undesirable place we ended up.

Yeah.  +1 for fixing that, if it's not unreasonably painful.





+1 for fixing it everywhere.

Historical note: back when TEMP_CONFIG was implemented, the main 
regression set was just about the only set of tests the buildfarm ran 
using a temp install. That wasn't even available for contrib and the 
PLs, IIRC.


cheers

andrew





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

[HACKERS] Packaging of postgresql-jdbc

[Pavel Kajaba]

Hello pg-hackers,

I need advice about postgresql-jdbc driver.

Current version in Fedora is behind latest version of postgresql-jdbc (1200 vs 
1207).

We are trying to package latest version into Fedora, but there are 
dependencies, which are not useless in Fedora (waffle-jna) and ones which we 
are not 100% open source (osgi-enterprise). We talked with upstream quite 
intensively but not been able to find any solution which would meet our 
requirements.

We think that it's not a good, when open-source project depending on packages, 
which licence is not 100% clear. 

Do you have any ideas how we could solve this problem?

It's quite easy to remove it but it's not best solutions because there is a 
chance that with each release we would have to work on removing. (Gentoo is 
doing this way : 
https://gitweb.gentoo.org/repo/gentoo.git/tree/dev-java/jdbc-postgresql )

Other distros seem to have similar problem 
(https://launchpad.net/ubuntu/+source/libpgjava).
 
We will be thankful for any idea.

Pavel Kajaba


-- 
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] ALTER ... OWNER TO ... CASCADE

[Oleg Bartunov]

On Mon, Feb 15, 2016 at 7:25 PM, Tom Lane  wrote:

> Teodor Sigaev  writes:
> >> So basically, a generic CASCADE facility sounds like a lot of work to
> >> produce something that would seldom be anything but a foot-gun.
>
> > DELETE FROM  or TRUNCATE could be a foot-gun too, but it's not a reason
> to
> > remove tham. I faced with problem when I tried to change owner of
> datadase with
> > all objects inside. Think, this feature could be useful although it
> should
> > restricted to superuser obly.
>
> That's a pretty weak argument, and I do not think you have thought through
> all the consequences.  It is not hard at all to imagine cases where using
> this sort of thing could be a security vulnerability.  Are you familiar
> with the reasons why Unix systems don't typically allow users to "give
> away" ownership of files?  The same problem exists here.
>

yes, I remember AT and BSD :)



>
> To be concrete about it:
>
> 1. Alice does, say, "CREATE EXTENSION cube".
>
> 2. Bob creates a security-definer function owned by himself, using a
>"cube"-type parameter so that it's dependent on the extension.
>(It needn't actually do anything with that parameter.)
>
> 3. Alice does ALTER EXTENSION cube OWNER TO charlie CASCADE.
>
> 4. Bob now has a security-definer function owned by (and therefore
>executing as) Charlie, whose contents were determined by Bob.
>Game over for Charlie ... and for everyone else too, if Charlie is
>a superuser, which is not unlikely for an extension owner.
>
> The only way Alice can be sure that the ALTER EXTENSION is safe is if
> she manually inspects every dependent object, in which case she might
> as well not use CASCADE.
>
> Moreover, the use case you've sketched (ie, change ownership of all
> objects inside a database) doesn't actually have anything to do with
> following dependencies.  It's a lot closer to REASSIGN OWNED ... in
> fact, it's not clear to me why REASSIGN OWNED doesn't solve that
> use-case already.
>
> I remain of the opinion that this is a terrible idea.
>

+1, I also suggest to check REASSIGN OWNED.


>
>
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] custom function for converting human readable sizes to bytes

[Vitaly Burovoy]

On 2/16/16, Vitaly Burovoy  wrote:
> On 2/16/16, Dean Rasheed  wrote:
>> On 16 February 2016 at 05:01, Pavel Stehule 
>> wrote:
>>> 2016-02-15 10:16 GMT+01:00 Dean Rasheed :
>> Fixing that in parse_memory_unit() would be messy because it assumes a
>> base unit of kB, so it would require a negative multiplier, and
>> pg_size_bytes() would have to be taught to divide by the magnitude of
>> negative multipliers in the same way that guc.c does.

Well... pg_size_bytes already knows what negative multiplier is (see
the constant skip_unit), but it seems it is hard to teach other
functions expecting values in KB to understand negative multipliers as
dividers.

> I guess the best way is to simply make "bytes" a valid size unit even
> in GUC by adding it to the memory_unit_conversion_table

Oops. I forgot they are not in bytes.

> with reflecting it in memory_units_hint and removing
> an extra checking from pg_size_bytes.
>
>> ISTM that it would be far less code, and much simpler and more
>> readable to just parse the supported units directly in
>> pg_size_bytes(), rather than trying to share code with guc.c, when the
>> supported units are actually different and may well diverge further in
>> the future.

Initially it was not shared with guc.c and now it is by the letter[1]
of Oleksandr Shulgin and Robert Haas.

I guess since parse_memory_unit uses memory_unit_conversion_table
where values (not for GUC_UNIT_KB but nevertheless) can be negatives
and for KB it can be a single value (-1024), move "bytes" check to
parse_memory_unit, add a constant memory_units_bytes_hint as
copy-paste with included "bytes" size unit (and with a comment it is
special for parse_memory_unit function).

Also change "skip_unit" in dbsize.c to -1024.

>> I'll try to hack something up, and see what it looks like.

[1]http://www.postgresql.org/message-id/ca+tgmoanot7ugjsbibfuqgachk2lzrypmkjvvugp5r197yu...@mail.gmail.com
-- 
Best regards,
Vitaly Burovoy


-- 
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] custom function for converting human readable sizes to bytes

[Vitaly Burovoy]

On 2/16/16, Dean Rasheed  wrote:
> On 16 February 2016 at 05:01, Pavel Stehule 
> wrote:
>> 2016-02-15 10:16 GMT+01:00 Dean Rasheed :
>>> Is there any reason not to make
>>> pg_size_bytes() return numeric?
>>>
>> This is a question. I have not a strong opinion about it. There are no
>> any
>> technical objection - the result will be +/- same. But you will enforce
>> Numeric into outer expression evaluation.
>>
>> The result will not be used together with function pg_size_pretty, but
>> much
>> more with functions pg_relation_size, pg_relation_size, .. and these
>> functions doesn't return Numeric. These functions returns bigint. Bigint
>> is
>> much more natural type for this purpose.
>>
>> Is there any use case for Numeric?
>>
>
> [Shrug] I don't really have a strong opinion about it either, but it
> seemed that maybe the function might have some wider uses beyond just
> comparing object sizes, and since it's already computing the numeric
> size, it might as well just return it.

I agree with Pavel, it leads to a comparison in numeric, which is
overhead. int8 can always be casted to numeric on-demand, but not vice
versa. The main reasons to return int8 instead of numeric are
performance and inability to imagine use case where so big numbers
could take place.

> Looking at the rest of the patch, I think there are other things that
> need tidying up -- the unit parsing code for one. This is going to
> some effort to reuse the memory_unit_conversion_table from guc.c, but
> the result is that it has added quite a bit of new code and now the
> responsibility for parsing different units is handled by different
> functions in different files, which IMO is quite messy. Worse, the
> error message is wrong and misleading:
>
> select pg_size_bytes('10 bytes'); -- OK
> select pg_size_bytes('10 gallons');
> ERROR:  invalid size: "10 gallons"
> DETAIL:  Invalid size unit "gallons"
> HINT:  Valid units for this parameter are "kB", "MB", "GB", and "TB".
>
> which fails to mention that "bytes" is also a valid unit.

:-D

Yes, it is fail. I missed it...

> Fixing that in parse_memory_unit() would be messy because it assumes a
> base unit of kB, so it would require a negative multiplier, and
> pg_size_bytes() would have to be taught to divide by the magnitude of
> negative multipliers in the same way that guc.c does.

I guess the best way is to simply make "bytes" a valid size unit even
in GUC by adding it to the memory_unit_conversion_table with
reflecting it in memory_units_hint and removing an extra checking from
pg_size_bytes.

> ISTM that it would be far less code, and much simpler and more
> readable to just parse the supported units directly in
> pg_size_bytes(), rather than trying to share code with guc.c, when the
> supported units are actually different and may well diverge further in
> the future.
>
> I'll try to hack something up, and see what it looks like.
>
> Regards,
> Dean

-- 
Best regards,
Vitaly Burovoy


-- 
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] custom function for converting human readable sizes to bytes

[Pavel Stehule]

2016-02-16 11:25 GMT+01:00 Dean Rasheed :

> On 16 February 2016 at 05:01, Pavel Stehule 
> wrote:
> > 2016-02-15 10:16 GMT+01:00 Dean Rasheed :
> >> Is there any reason not to make
> >> pg_size_bytes() return numeric?
> >>
> > This is a question. I have not a strong opinion about it. There are no
> any
> > technical objection - the result will be +/- same. But you will enforce
> > Numeric into outer expression evaluation.
> >
> > The result will not be used together with function pg_size_pretty, but
> much
> > more with functions pg_relation_size, pg_relation_size, .. and these
> > functions doesn't return Numeric. These functions returns bigint. Bigint
> is
> > much more natural type for this purpose.
> >
> > Is there any use case for Numeric?
> >
>
> [Shrug] I don't really have a strong opinion about it either, but it
> seemed that maybe the function might have some wider uses beyond just
> comparing object sizes, and since it's already computing the numeric
> size, it might as well just return it.
>

I see only one disadvantage - when we return numeric, then all following
expression will be in numeric, and for some functions, or some expression
the users will have to cast explicitly to bigint.


>
>
> Looking at the rest of the patch, I think there are other things that
> need tidying up -- the unit parsing code for one. This is going to
> some effort to reuse the memory_unit_conversion_table from guc.c, but
> the result is that it has added quite a bit of new code and now the
> responsibility for parsing different units is handled by different
> functions in different files, which IMO is quite messy. Worse, the
> error message is wrong and misleading:
>
> select pg_size_bytes('10 bytes'); -- OK
> select pg_size_bytes('10 gallons');
> ERROR:  invalid size: "10 gallons"
> DETAIL:  Invalid size unit "gallons"
> HINT:  Valid units for this parameter are "kB", "MB", "GB", and "TB".
>
> which fails to mention that "bytes" is also a valid unit.
>

true


>
> Fixing that in parse_memory_unit() would be messy because it assumes a
> base unit of kB, so it would require a negative multiplier, and
> pg_size_bytes() would have to be taught to divide by the magnitude of
> negative multipliers in the same way that guc.c does.
>
> ISTM that it would be far less code, and much simpler and more
> readable to just parse the supported units directly in
> pg_size_bytes(), rather than trying to share code with guc.c, when the
> supported units are actually different and may well diverge further in
> the future.
>

yes, if we have different units, then independent control tables has more
sense.



>
> I'll try to hack something up, and see what it looks like.
>
> Regards,
> Dean
>

Re: [HACKERS] custom function for converting human readable sizes to bytes

[Dean Rasheed]

On 16 February 2016 at 05:01, Pavel Stehule  wrote:
> 2016-02-15 10:16 GMT+01:00 Dean Rasheed :
>> Is there any reason not to make
>> pg_size_bytes() return numeric?
>>
> This is a question. I have not a strong opinion about it. There are no any
> technical objection - the result will be +/- same. But you will enforce
> Numeric into outer expression evaluation.
>
> The result will not be used together with function pg_size_pretty, but much
> more with functions pg_relation_size, pg_relation_size, .. and these
> functions doesn't return Numeric. These functions returns bigint. Bigint is
> much more natural type for this purpose.
>
> Is there any use case for Numeric?
>

[Shrug] I don't really have a strong opinion about it either, but it
seemed that maybe the function might have some wider uses beyond just
comparing object sizes, and since it's already computing the numeric
size, it might as well just return it.


Looking at the rest of the patch, I think there are other things that
need tidying up -- the unit parsing code for one. This is going to
some effort to reuse the memory_unit_conversion_table from guc.c, but
the result is that it has added quite a bit of new code and now the
responsibility for parsing different units is handled by different
functions in different files, which IMO is quite messy. Worse, the
error message is wrong and misleading:

select pg_size_bytes('10 bytes'); -- OK
select pg_size_bytes('10 gallons');
ERROR:  invalid size: "10 gallons"
DETAIL:  Invalid size unit "gallons"
HINT:  Valid units for this parameter are "kB", "MB", "GB", and "TB".

which fails to mention that "bytes" is also a valid unit.

Fixing that in parse_memory_unit() would be messy because it assumes a
base unit of kB, so it would require a negative multiplier, and
pg_size_bytes() would have to be taught to divide by the magnitude of
negative multipliers in the same way that guc.c does.

ISTM that it would be far less code, and much simpler and more
readable to just parse the supported units directly in
pg_size_bytes(), rather than trying to share code with guc.c, when the
supported units are actually different and may well diverge further in
the future.

I'll try to hack something up, and see what it looks like.

Regards,
Dean


-- 
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] Identifying a message in emit_log_hook.

[Simon Riggs]

On 16 February 2016 at 09:47, Kyotaro HORIGUCHI <
horiguchi.kyot...@lab.ntt.co.jp> wrote:


> I'm planning to change the error level of a specific error
> message.  We can use emit_log_hook for this purpose but we have
> no reliable means to identify what a message is.
>
> For messages without valid sqlerrcode, filename:lineno could be
> used to identify but lineno is too unstable.  One possible and
> most straightforward way to solve this is defining identifiers
> covering all error messages but such identifiers are too hard to
> manage.  ErrorData.message could also be used but NLS translation
> and placeholders prevent it from being identified by simple means
> like strcmp(3).
>
> As a solution for this problem, I'd like to porpose to have an
> additional member in the struct ErrorData to hold a message id,
> that is, the format string for errmsg(). This is not best but
> useful enough.
>
> It is somewhat a crude way, but the attached small patch would
> do.
>
> Is there any opinions or suggestions?
>

First, I support the concept of message ids and any work you do in moving
this forwards.

If you were to require message ids, how would this work for extensions?
Many things write to the log, so this solution would rely upon 100%
compliance with your new approach. I think that is unlikely to happen.

I suggest an approach that allows optionally accessing a message id by
hashing the English message text. That would allow people to identify
messages without relying on people labelling everything correctly, as well
as writing filters that do not depend upon language.

I'm guessing this would require making the pre-translated error text
available to plugins as well as translated form.

-- 
Simon Riggshttp://www.2ndQuadrant.com/

PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Re: [HACKERS] extend pgbench expressions with functions

[Fabien COELHO]

Hello Robert,


Good point. One simple idea here would be to use a custom pgbench
script that has no SQL commands and just calculates the values of some
parameters to measure the impact without depending on the backend,
with a fixed number of transactions.


Sure, we could do that.  But whether it materially changes pgbench -S
results, say, is a lot more important.


Indeed. Several runs on my laptop:

  ~ 40-54 tps with master using:
\set naccounts 10 * :scale
\setrandom aid 1 :naccounts

  ~ 43-53 tps with full function patch using:
\set naccounts 10 * :scale
\setrandom aid 1 :naccounts

  ~ 73-89 tps with full function patch using:
\set aid random(1, 10 * :scale)

The performance is pretty similar on the same script. The real pain is 
variable management, avoiding some is a win.


However, as you suggest, the tps impact even with -M prepared -S is 
nought, because the internal scripting time in pgbench is much smaller 
than the time to do actual connecting and querying.


--
Fabien.


--
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] Identifying a message in emit_log_hook.

[Pavel Stehule]

Hi

2016-02-16 10:47 GMT+01:00 Kyotaro HORIGUCHI <
horiguchi.kyot...@lab.ntt.co.jp>:

> Hello.
>
> I'm planning to change the error level of a specific error
> message.  We can use emit_log_hook for this purpose but we have
> no reliable means to identify what a message is.
>
> For messages without valid sqlerrcode, filename:lineno could be
> used to identify but lineno is too unstable.  One possible and
> most straightforward way to solve this is defining identifiers
> covering all error messages but such identifiers are too hard to
> manage.  ErrorData.message could also be used but NLS translation
> and placeholders prevent it from being identified by simple means
> like strcmp(3).
>
> As a solution for this problem, I'd like to porpose to have an
> additional member in the struct ErrorData to hold a message id,
> that is, the format string for errmsg(). This is not best but
> useful enough.
>
> It is somewhat a crude way, but the attached small patch would
> do.
>
> Is there any opinions or suggestions?
>

It looks like workaround. The fixing missing sqlerrcode is much better.

Regards

Pavel



>
> regards,
>
> --
> Kyotaro Horiguchi
> NTT Open Source Software Center
>
> diff --git a/src/backend/utils/error/elog.c
> b/src/backend/utils/error/elog.c
> index 9005b26..2d13101 100644
> --- a/src/backend/utils/error/elog.c
> +++ b/src/backend/utils/error/elog.c
> @@ -801,6 +801,7 @@ errmsg(const char *fmt,...)
> CHECK_STACK_DEPTH();
> oldcontext = MemoryContextSwitchTo(edata->assoc_context);
>
> +   edata->message_id = fmt;
> EVALUATE_MESSAGE(edata->domain, message, false, true);
>
> MemoryContextSwitchTo(oldcontext);
> @@ -830,6 +831,7 @@ errmsg_internal(const char *fmt,...)
> CHECK_STACK_DEPTH();
> oldcontext = MemoryContextSwitchTo(edata->assoc_context);
>
> +   edata->message_id = fmt;
> EVALUATE_MESSAGE(edata->domain, message, false, false);
>
> MemoryContextSwitchTo(oldcontext);
> @@ -853,6 +855,7 @@ errmsg_plural(const char *fmt_singular, const char
> *fmt_plural,
> CHECK_STACK_DEPTH();
> oldcontext = MemoryContextSwitchTo(edata->assoc_context);
>
> +   edata->message_id = fmt_singular;
> EVALUATE_MESSAGE_PLURAL(edata->domain, message, false);
>
> MemoryContextSwitchTo(oldcontext);
> @@ -1361,6 +1364,7 @@ elog_finish(int elevel, const char *fmt,...)
> recursion_depth++;
> oldcontext = MemoryContextSwitchTo(edata->assoc_context);
>
> +   edata->message_id = fmt;
> EVALUATE_MESSAGE(edata->domain, message, false, false);
>
> MemoryContextSwitchTo(oldcontext);
> @@ -1420,6 +1424,7 @@ format_elog_string(const char *fmt,...)
>
> oldcontext = MemoryContextSwitchTo(ErrorContext);
>
> +   edata->message_id = fmt;
> EVALUATE_MESSAGE(edata->domain, message, false, true);
>
> MemoryContextSwitchTo(oldcontext);
> diff --git a/src/include/utils/elog.h b/src/include/utils/elog.h
> index 326896f..4df76da 100644
> --- a/src/include/utils/elog.h
> +++ b/src/include/utils/elog.h
> @@ -354,6 +354,7 @@ typedef struct ErrorData
> char   *detail_log; /* detail error message for server
> log only */
> char   *hint;   /* hint message */
> char   *context;/* context message */
> +   const char *message_id; /* message id of .message */
> char   *schema_name;/* name of schema */
> char   *table_name; /* name of table */
> char   *column_name;/* name of column */
>
>
> --
> Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-hackers
>
>