Re: [HACKERS] Problem with displaying "wide" tables in psql

[Emre Hasegeli]

2014-02-16 18:37, Sergey Muraviov :

> New code doesn't work with empty strings but I've done minor optimization
> for this case.

It seems better now. I added some new lines and spaces, removed unnecessary
parentheses and marked it as "Ready for Committer".


fix_psql_print_aligned_vertical_v5.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] [BUG] Archive recovery failure on 9.3+.

[Kyotaro HORIGUCHI]

Thank you for committing.

> On 02/14/2014 10:38 AM, Kyotaro HORIGUCHI wrote:
> > Finally, the patch you will find attached is fixed only in
> > styling mentioned above from your last patch. This patch applies
> > current HEAD and I confirmed that it fixes this issue but I have
> > not checked the lastSourceFailed section. Simple file removal
> > could not lead to there.
> 
> Ok, applied. Thanks!

-- 
Kyotaro Horiguchi
NTT Open Source Software Center


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

Re: [HACKERS] gaussian distribution pgbench

[KONDO Mitsumasa]

(2014/02/16 7:38), Fabien COELHO wrote:

   I have updated the patch (v7) based on Mitsumasa latest v6:
   - some code simplifications & formula changes.
   - I've added explicit looping probability computations in comments
 to show the (low) looping probability of the iterative search.
   - I've tried to clarify the sgml documentation.
   - I've removed the 5.0 default value as it was not used anymore.
   - I've renamed some variables to match the naming style around.
Thank you for yor detail review and fix some code! I checked your modification 
version,

it seems better than previos version and very helpful for documents.


* Mathematical soundness

   I've checked again the mathematical soundness for the methods involved.

   After further thoughts, I'm not that sure that there is not a bias induced
   by taking the second value based on "cos" when the first based on "sin"
   as failed the test. So I removed the cos computation for the gaussian 
version,
   and simplified the code accordingly. This mean that it may be a little
   less efficient, but I'm more confident that there is no bias.
I tried to confirm which method is better. However, at the end of the day, it is 
not a problem because other part of implementations have bigger overhead in 
pgbench client. We like simple implementaion so I agree with your modification 
version. And I tested this version, there is no overhead in creating gaussian and 
exponential random number with minimum threshold that is most overhead situation.



* Conclusion

   If Mitsumasa-san is okay with the changes I have made, I would suggest
   to accept this patch.
Attached patch based on v7 is added output that is possibility of access record 
when we use exponential option
in the end of pgbench result. It is caluculated by a definite integral method for 
e^-x.

If you check it and think no problem, please mark it ready for commiter.
Ishii-san will review this patch:)

Regards,
--
Mitsumasa KONDO
NTT Open Source Software Center
*** a/contrib/pgbench/pgbench.c
--- b/contrib/pgbench/pgbench.c
***
*** 98,103  static int	pthread_join(pthread_t th, void **thread_return);
--- 98,106 
  #define LOG_STEP_SECONDS	5	/* seconds between log messages */
  #define DEFAULT_NXACTS	10		/* default nxacts */
  
+ #define MIN_GAUSSIAN_THRESHOLD		2.0	/* minimum threshold for gauss */
+ #define MIN_EXPONENTIAL_THRESHOLD	2.0	/* minimum threshold for exp */
+ 
  int			nxacts = 0;			/* number of transactions per client */
  int			duration = 0;		/* duration in seconds */
  
***
*** 169,174  bool		is_connect;			/* establish connection for each transaction */
--- 172,185 
  bool		is_latencies;		/* report per-command latencies */
  int			main_pid;			/* main process id used in log filename */
  
+ /* gaussian distribution tests: */
+ double		stdev_threshold;   /* standard deviation threshold */
+ booluse_gaussian = false;
+ 
+ /* exponential distribution tests: */
+ double		exp_threshold;   /* threshold for exponential */
+ bool		use_exponential = false;
+ 
  char	   *pghost = "";
  char	   *pgport = "";
  char	   *login = NULL;
***
*** 330,335  static char *select_only = {
--- 341,428 
  	"SELECT abalance FROM pgbench_accounts WHERE aid = :aid;\n"
  };
  
+ /* --exponential case */
+ static char *exponential_tpc_b = {
+ 	"\\set nbranches " CppAsString2(nbranches) " * :scale\n"
+ 	"\\set ntellers " CppAsString2(ntellers) " * :scale\n"
+ 	"\\set naccounts " CppAsString2(naccounts) " * :scale\n"
+ 	"\\setexponential aid 1 :naccounts :exp_threshold\n"
+ 	"\\setrandom bid 1 :nbranches\n"
+ 	"\\setrandom tid 1 :ntellers\n"
+ 	"\\setrandom delta -5000 5000\n"
+ 	"BEGIN;\n"
+ 	"UPDATE pgbench_accounts SET abalance = abalance + :delta WHERE aid = :aid;\n"
+ 	"SELECT abalance FROM pgbench_accounts WHERE aid = :aid;\n"
+ 	"UPDATE pgbench_tellers SET tbalance = tbalance + :delta WHERE tid = :tid;\n"
+ 	"UPDATE pgbench_branches SET bbalance = bbalance + :delta WHERE bid = :bid;\n"
+ 	"INSERT INTO pgbench_history (tid, bid, aid, delta, mtime) VALUES (:tid, :bid, :aid, :delta, CURRENT_TIMESTAMP);\n"
+ 	"END;\n"
+ };
+ 
+ /* --exponential with -N case */
+ static char *exponential_simple_update = {
+ 	"\\set nbranches " CppAsString2(nbranches) " * :scale\n"
+ 	"\\set ntellers " CppAsString2(ntellers) " * :scale\n"
+ 	"\\set naccounts " CppAsString2(naccounts) " * :scale\n"
+ 	"\\setexponential aid 1 :naccounts :exp_threshold\n"
+ 	"\\setrandom bid 1 :nbranches\n"
+ 	"\\setrandom tid 1 :ntellers\n"
+ 	"\\setrandom delta -5000 5000\n"
+ 	"BEGIN;\n"
+ 	"UPDATE pgbench_accounts SET abalance = abalance + :delta WHERE aid = :aid;\n"
+ 	"SELECT abalance FROM pgbench_accounts WHERE aid = :aid;\n"
+ 	"INSERT INTO pgbench_history (tid, bid, aid, delta, mtime) VALUES (:tid, :bid, :aid, :delta, CURRENT_TIMESTAMP);\n"
+ 	"END;\n"
+ };
+ 
+ /* --exponential with -S case */
+ static char *exponential_select_only = {
+ 	"\\set naccounts " CppAsString2(na

Re: [HACKERS] CREATE FOREIGN TABLE ( ... LIKE ... )

[Andres Freund]

On 2014-02-16 20:27:09 -0800, David Fetter wrote:
> On Sat, Feb 15, 2014 at 03:14:03PM +0100, Andres Freund wrote:
> > On 2014-01-31 18:16:18 +0100, Vik Fearing wrote:
> > > On 01/25/2014 06:25 AM, David Fetter wrote:
> > > > Please find attached the next rev :)
> > > 
> > > This version looks committable to me, so I am marking it as such.
> > 
> > This doesn't contain a single regression test, I don't see how that's
> > ok. Marking as waiting on author.
> 
> It now contains regression tests.  Re-marking.

I don't think this really has gone above Needs Review yet.

Greetings,

Andres Freund

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


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

Re: [HACKERS] Ctrl+C from sh can shut down daemonized PostgreSQL cluster

[Bjorn Munch]

On 14/02 14.57, Kevin Grittner wrote:
> We have had a case where a production cluster was accidentally shut
> down by a customer who used Ctrl+C in the same sh session in which
> they had (long before) run pg_ctl start.  We have only seen this in
> sh on Solaris.  Other shells on Solaris don't behave this way, nor
> does sh on tested versions of Linux.  Nevertheless, the problem is
> seen on the default shell for a supported OS.

What Solaris version, and what version of sh?  sh on Solaris isn't
necessarily the "real" bourne shell. In Solaris 11 it's actually
ksh93.

I've seen a sort-of opposite problem which does not appear in stock
Solaris 10 or 11 but in OpenSolaris, at least the version I used to
have on my desktop.

And this was not PostgreSQL but MySQL There's a script mysqld_safe
which will automatically restart the mysqld server if it dies. But in
OpenSolaris with ksh version '93t', if I killed mysqld, the shell that
started it also died. I never could figure out why. Solaris 11 with
ksh '93u' does not have this problem. Nor does Solaris 10 with "real" sh.

Is this customer by any chance running OpenSolaris?

- Bjorn


-- 
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: show xid and xmin in pg_stat_activity and pg_stat_replication

[Christian Kruse]

Hi Robert,

Am 15.02.14 05:03, schrieb Robert Haas:
> Well, this version of the patch reveals a mighty interesting point: a
> lot of the people who are calling pgstat_fetch_stat_beentry() don't
> need this additional information and might prefer not to pay the cost
> of fetching it.

Well, the cost is already paid due to the fact that this patch uses
LocalPgBackendStatus instead of PgBackendStatus in
pgstat_read_current_status(). And pgstat_fetch_stat_beentry() returns
a pointer instead of a copy, so the cost is rather small, too.

> None of pg_stat_get_backend_pid,
> pg_stat_get_backend_dbid, pg_stat_get_backend_userid,
> pg_stat_get_backend_activity, pg_stat_get_backend_activity,
> pg_stat_get_backend_waiting, pg_stat_get_backend_activity_start,
> pg_stat_get_backend_xact_start, pg_stat_get_backend_start,
> pg_stat_get_backend_client_addr, pg_stat_get_backend_client_port,
> pg_stat_get_backend_client_port, and pg_stat_get_db_numbackends
> actually need this new information; it's only ever used in one place.
> So it seems like it might be wise to have pgstat_fetch_stat_beentry
> continue to return the PgBackendStatus * and add a new function
> pgstat_fetch_stat_local_beentry to fetch the LocalPgBackendStatus *;
> then most of these call sites wouldn't need to change.

This is true for now. But one of the purposes of using
LocalPgBackendStatus instead of PgBackendStatus was to be able to add
more fields like this in future. And thus we might need to change this
in future, so why not do it now?

And I also agree to Andres.

> It would still be the case that pgstat_read_current_status() pays the
> price of fetching this information even if pg_stat_get_activity is
> never called.  But since that's probably by far the most commonly-used
> API for this information, that's probably OK.

I agree.

I will change it if this is really wanted, but I think it would be a
good idea to do it this way.

Best regards,

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




signature.asc
Description: OpenPGP digital signature

Re: [HACKERS] Problem with displaying "wide" tables in psql

[Sergey Muraviov]

Thanks.


2014-02-17 12:22 GMT+04:00 Emre Hasegeli :

> 2014-02-16 18:37, Sergey Muraviov :
>
> > New code doesn't work with empty strings but I've done minor optimization
> > for this case.
>
> It seems better now. I added some new lines and spaces, removed unnecessary
> parentheses and marked it as "Ready for Committer".
>



-- 
Best regards,
Sergey Muraviov

Re: [HACKERS] Exposing currentTransactionWALVolume

[KONDO Mitsumasa]

(2014/02/15 23:04), Andres Freund wrote:

Hi Simon,

On 2014-01-14 17:12:35 +, Simon Riggs wrote:

  /*
- * MarkCurrentTransactionIdLoggedIfAny
+ * ReportTransactionInsertedWAL
   *
- * Remember that the current xid - if it is assigned - now has been wal logged.
+ * Remember that the current xid - if it is assigned - has now inserted WAL
   */
  void
-MarkCurrentTransactionIdLoggedIfAny(void)
+ReportTransactionInsertedWAL(uint32 insertedWALVolume)
  {
+   currentTransactionWALVolume += insertedWALVolume;
if (TransactionIdIsValid(CurrentTransactionState->transactionId))
CurrentTransactionState->didLogXid = true;
  }


Not a big fan of combining those two. One works on the toplevel
transaction, the other on the current subtransaction... The new name
also ignores that it's only taking effect if there's actually a
transaction in progress.

Oh, yes. I don't have good idea, but we need to change function name or add new
function for WAL adding volume. If it will be fixed, I set ready for commiter,
because I cannot see any bad point in this patch.

Regards,
--
Mitsumasa KONDO
NTT Open Source Software Center


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

Re: [HACKERS] narwhal and PGDLLIMPORT

[Dave Page]

On Fri, Feb 14, 2014 at 5:32 PM, Tom Lane  wrote:
> I wrote:
>> Hiroshi Inoue  writes:
>>> One thing I'm wondering about is that plperl is linking perlxx.lib
>>> not libperlxx.a. I made a patch following plpython and it also
>>> works here.
>>> Is it worth trying?
>
>> I hadn't noticed that part of plpython's Makefile before.  Man,
>> that's an ugly technique :-(.  Still, there's little about this
>> platform that isn't ugly.  Let's try it and see what happens.
>
> And what happens is this:
> http://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=narwhal&dt=2014-02-14%2017%3A00%3A02
> namely, it gets through plperl now and then chokes with the same
> symptoms on pltcl.  So I guess we need the same hack in pltcl.
> The fun never stops ...
>
> (BTW, narwhal is evidently not trying to build plpython.  I wonder
> why not?)

Not sure - it's certainly installed on the box. I've enabled it for
now, and will see what happens.

-- 
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: 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] [bug fix] psql \copy doesn't end if backend is killed

[MauMau]

From: "Tom Lane" 

I just noticed this CF entry pertaining to the same problem that Stephen
Frost reported a couple days ago:
http://www.postgresql.org/message-id/20140211205336.gu2...@tamriel.snowman.net

I believe it's been adequately fixed as of commits fa4440f516 and
b8f00a46bc, but if you'd test that those handle your problem cases,
I'd appreciate it.


I confirmed that the problem disappeared.  I'll delete my CommitFest entry 
in several days.


Regards
MauMau



--
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] narwhal and PGDLLIMPORT

[Craig Ringer]

On 02/16/2014 07:03 AM, Andres Freund wrote:
> On 2014-02-15 17:48:00 -0500, Tom Lane wrote:
>> Marco Atzeri  writes:
>>>   32 $ grep -rH in6addr_any *
>>> cygwin/in6.h:extern const struct in6_addr in6addr_any;
>>> cygwin/version.h:  in6addr_any, in6addr_loopback.
>>
>> So how come there's a declspec on the getopt.h variables, but not this
>> one?
> 
> Well, those are real constants, so they probably are fully contained in
> the static link library, no need to dynamically resolve them.

If they're externs that're then defined in a single module, that's not
the case.

Only if they're *not* declared extern, and thus added to each
compilation unit then de-duplicated during linking, may they be safely
referred to without a __declspec(dllimport) because each module gets its
own copy.

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


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

Re: [HACKERS] Long paths for tablespace leads to uninterruptible hang in Windows

[Craig Ringer]

On 02/14/2014 10:57 AM, Bruce Momjian wrote:
> On Tue, Jan  7, 2014 at 12:33:33PM +0530, Amit Kapila wrote:

>> Further update on this issue:
>>
>> Microsoft has suggested a workaround for stat API. Their suggestion
>> is to use 'GetFileAttributesEx' instead of stat, when I tried their
>> suggestion, it also gives me same problem as stat.
>>
>> Still they have not told anything about other API's
>> (rmdir, RemoveDirectory) which has same problem.
> 
> Where are we on this?  Is there a check we should add in our code?

This is fascinating - I spent some time chasing the same symptoms in my
Jenkins build slave, and eventually tracked it down to path lengths. gcc
was just hanging uninterruptibly in a win32 syscall, and nothing short
of a reboot would deal with it.

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


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

Re: [HACKERS] Add min and max execute statement time in pg_stat_statement

[Rajeev rastogi]

On 12 February 2014 12:16, KONDO Mitsumasa Wrote:
 
> Hi Rajeev,
> 
> > (2014/01/29 17:31), Rajeev rastogi wrote:
> >> No Issue, you can share me the test cases, I will take the
> performance report.
> Attached patch is supported to latest pg_stat_statements. It includes
> min, max, and stdev statistics. Could you run compiling test on your
> windows enviroments?
> I think compiling error was fixed.

It got compiled successfully on Windows.

Thanks and Regards,
Kumar Rajeev Rastogi


-- 
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] GiST support for inet datatypes

[Andres Freund]

On 2014-02-17 14:40:07 +0200, Emre Hasegeli wrote:
> 2014-02-07 22:41, Robert Haas :
> 
> > Generally, modifying already-release .sql files for extensions is a no-no...
> 
> I prepared separate patches for btree_gist extension with more options.
> First one (btree-gist-drop-default-inet-v1.patch) removes DEFAULT keyword
> only from the inet and the cidr operator classes. Second one
> (btree-gist-drop-default-all-v1.patch) removes DEFAULT keyword for all
> operator classes. I think it is more consistent to remove it from all.
> Third one (btree-gist-drop-inet-v1.patch) removes the inet and the cidr
> operator classes altogether. It is suggested by Tom Lane [1] on bug #5705.
> The new GiST operator class includes basic comparison operators except !=
> so it may be the right time to remove support from btree_gist. Fourth one
> (btree-gist-drop-inet-and-default-v1.patch) is the second one and the third
> one together.
> 
> [1] http://www.postgresql.org/message-id/10183.1287526...@sss.pgh.pa.us

> diff --git a/contrib/btree_gist/Makefile b/contrib/btree_gist/Makefile
> index ba4af14..d5b1fd7 100644
> --- a/contrib/btree_gist/Makefile
> +++ b/contrib/btree_gist/Makefile
> @@ -9,7 +9,7 @@ OBJS =  btree_gist.o btree_utils_num.o btree_utils_var.o 
> btree_int2.o \
>  btree_numeric.o
>  
>  EXTENSION = btree_gist
> -DATA = btree_gist--1.0.sql btree_gist--unpackaged--1.0.sql
> +DATA = btree_gist--1.1.sql btree_gist--unpackaged--1.0.sql

You need to add a file for going from 1.0 to 1.1.

Greetings,

Andres Freund

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


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

Re: [HACKERS] narwhal and PGDLLIMPORT

[Andres Freund]

Hi,

I just wanted to mention that it should probably not be too hard to
emulate the current windows behaviour in gcc/clang elf targets. Somebody
(won't be me) could add a --emulate-windows-linkage configure flag or
such.
By mapping PGDLLIMPORT to__attribute__((section(...))) it should be
relatively straightforward to put all exported variables into a special
section and use a linker script to change the visibility of the default
data, bss, rodata sections...



Greetings,

Andres Freund

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


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

Re: [HACKERS] Create function prototype as part of PG_FUNCTION_INFO_V1

[Alvaro Herrera]

Tom Lane wrote:
> Peter Eisentraut  writes:
> > On 2/15/14, 10:22 AM, Tom Lane wrote:
> >> Yes it does; people who fail to remove their manual externs will get
> >> Windows-only build failures (or at least warnings; it's not very clear
> >> which declaration will win).
> 
> > The manual externs and the automatically provided ones are exactly the
> > same.  Why would that fail?
> 
> Maybe I'm remembering the wrong patch.  I thought what this patch was
> intending was to put PGDLLEXPORT into the automatically-provided externs.

This hunk is the essence of this patch:

 #define PG_FUNCTION_INFO_V1(funcname) \

 
+Datum funcname(PG_FUNCTION_ARGS); \

 
 extern PGDLLEXPORT const Pg_finfo_record * 
CppConcat(pg_finfo_,funcname)(void); \  

 


Note that PGDLLEXPORT is already there.  This patch is just about
additionally providing the prototype.

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


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

Re: [HACKERS] Draft release notes up for review

[Alvaro Herrera]

Josh Berkus wrote:
> On 02/16/2014 03:41 PM, Tom Lane wrote:
> > Draft release notes for 9.3.3 are committed and can be read at
> > http://www.postgresql.org/docs/devel/static/release-9-3-3.html
> > Any comments before I start transposing them into the back branches?
> 
> Major:
> 
> Do we have an explantion of what a multixact is, anywhere, so that we
> can link it?

Is this enough?
http://www.postgresql.org/docs/devel/static/routine-vacuuming.html#VACUUM-FOR-MULTIXACT-WRAPAROUND


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


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

Re: [HACKERS] CREATE FOREIGN TABLE ( ... LIKE ... )

[Michael Paquier]

On Mon, Feb 17, 2014 at 6:28 PM, Andres Freund  wrote:
> I don't think this really has gone above Needs Review yet.
I am not sure that this remark makes the review of this patch much
progressing :(

By the way, I spent some time looking at it and here are some comments:
- Regression tests added are too sensitive with the other tests. For
example by not dropping tables or creating new tables on another tests
run before foreign_data you would need to update the output of this
test as well, something rather unfriendly.
- Regression coverage is limited (there is nothing done for comments
and default expressions)
- regression tests are added in postgres_fdw. This should be perhaps
the target of another patch so I removed them for now as this is only
a core feature (if I am wrong here don't hesitate). Same remark about
information_schema though, those tests are too fragile as they are.
- Documentation had some issues IMO:
-- A bracket was missing before "column_name"...
-- like_option should be clear about what it supports or not, more
precisely that it supports only default expressions and comments
-- some typos and formatting inconsistencies found
- In the case of CREATE TABLE, like_option is bypassed based on the
nature of the object linked, and not based on the nature of the object
created, so for CREATE FOREIGN TABLE, using this argument, I do not
think that we should simply ignore the options not directly supported
but return an error or a warning at least to user (attached patch
returns an ERROR). Documentation needs to reflect that precisely to
let the user know what can be and cannot be done.

After testing the patch, well it does what it is aimed for and it
works. It is somewhat unfortunate that we cannot enforce the name of
columns hidden behind LIKE directly with CREATE, but this would result
in some kludging in the code. It can as well be done simply with ALTER
FOREIGN TABLE.

All those comments result in the patch attached, which I think is in a
state close to committable, so I am marking it as "ready for
committer" (feel free to scream at me if you do not think so). Note
that the patch attached is not using context diffs but git diffs
(really I tried!) because of filterdiff that skipped a block of code
in parse_utilcmd.c.
Regards,
-- 
Michael
diff --git a/doc/src/sgml/ref/create_foreign_table.sgml b/doc/src/sgml/ref/create_foreign_table.sgml
index 1ef4b5e..ecef3c0 100644
--- a/doc/src/sgml/ref/create_foreign_table.sgml
+++ b/doc/src/sgml/ref/create_foreign_table.sgml
@@ -19,7 +19,8 @@
  
 
 CREATE FOREIGN TABLE [ IF NOT EXISTS ] table_name ( [
-column_name data_type [ OPTIONS ( option 'value' [, ... ] ) ] [ COLLATE collation ] [ column_constraint [ ... ] ]
+  { column_name data_type [ OPTIONS ( option 'value' [, ... ] ) ] [ COLLATE collation ] [ column_constraint [ ... ] ]
+| LIKE source_table [ like_option ... ] }
 [, ... ]
 ] )
   SERVER server_name
@@ -31,6 +32,10 @@ CREATE FOREIGN TABLE [ IF NOT EXISTS ] table_name
 { NOT NULL |
   NULL |
   DEFAULT default_expr }
+
+and like_option is:
+
+{ INCLUDING | EXCLUDING } { DEFAULTS | COMMENTS | ALL }
 
  
 
@@ -114,6 +119,29 @@ CREATE FOREIGN TABLE [ IF NOT EXISTS ] table_name

 

+LIKE source_table [ like_option ... ]
+
+ 
+  The LIKE clause specifies a table from which
+  the new foreign table automatically copies all column names and
+  their data types.
+ 
+ 
+  Default expressions for the copied column definitions will only be
+  copied if INCLUDING DEFAULTS is specified.
+  Defaults that call database-modification functions, like
+  nextval, create a linkage between the original and
+  new tables.  The default behavior is to exclude default expressions,
+  resulting in the copied columns in the new table having null defaults.
+ 
+ 
+  The LIKE clause can also be used to copy columns from
+  views, tables, or composite types.
+ 
+
+   
+
+   
 NOT NULL
 
  
diff --git a/src/backend/parser/parse_utilcmd.c b/src/backend/parser/parse_utilcmd.c
index eb07ca3..aec39b8 100644
--- a/src/backend/parser/parse_utilcmd.c
+++ b/src/backend/parser/parse_utilcmd.c
@@ -649,8 +649,8 @@ transformTableConstraint(CreateStmtContext *cxt, Constraint *constraint)
 /*
  * transformTableLikeClause
  *
- * Change the LIKE  portion of a CREATE TABLE statement into
- * column definitions which recreate the user defined column portions of
+ * Change the LIKE  portion of a CREATE [FOREIGN] TABLE statement
+ * into column definitions which recreate the user defined column portions of
  * .
  */
 static void
@@ -668,12 +668,6 @@ transformTableLikeClause(CreateStmtContext *cxt, TableLikeClause *table_like_cla
 	setup_parser_errposition_callback(&pcbstate, cxt->pstate,
 	  table_like_clause->relation->location);
 
-	/* we could support LIKE in many cases, but worry about it another day */
-	if (cxt->isforeign)
-		ereport(ERROR,
-(errcode(ERRCODE_FEATURE_N

Re: [HACKERS] patch: option --if-exists for pg_dump

[Alvaro Herrera]

Jeevan Chalke escribió:

> If yes, then in my latest attached patch, these lines are NOT AT ALL there.
> I have informed on my comment that I have fixed these in my version of
> patch,
> but still you got unstable build. NOT sure how. Seems like you are applying
> wrong patch.
> 
> Will you please let us know what's going wrong ?

The commitfest app is not a chat area.  When you add new versions of a
patch, please mark them as "patch" (not "comment") and make sure to
provide the message-id of the latest version.

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


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

Re: [HACKERS] Review: tests for client programs

[Alvaro Herrera]

Pavel Stehule escribió:
> 2014-02-09 4:16 GMT+01:00 Peter Eisentraut :

> > > a) Configure doesn't check a required IRC::Run module
> >
> > Clearly, we will need to figure out something about how to require this
> > module, and possibly others in the future, as we expand the tests.
> > Having configure check for it is not necessarily the best solution --
> > What is configure supposed to do if it can't find it?
> 
> there can be option "--with-client-tests" and this option should to require
> IRC::Run

A configure option seems a workable idea.

In the future we might want to use the Perl test framework for other
things, so perhaps --enable-perl-testing or something like that.  See
for instance
http://www.postgresql.org/message-id/64aaac31739cef275839932f16fda...@biglumber.com

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


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

Re: [HACKERS] CREATE FOREIGN TABLE ( ... LIKE ... )

[Andres Freund]

On 2014-02-17 23:07:45 +0900, Michael Paquier wrote:
> On Mon, Feb 17, 2014 at 6:28 PM, Andres Freund  wrote:
> > I don't think this really has gone above Needs Review yet.
> I am not sure that this remark makes the review of this patch much
> progressing :(

Uh. What should I then say if a patch is marked as ready for committer
by the author, after it previously had been marked such when it clearly
wasn't? Your review just seems to confirm that it wasn't ready?
If the patch is isn't marked "needs review" in the CF it's less likely
to get timely review. And when a committer looks at the patch it'll just
be determined at not being ready again, making it less likely to get
committed.

Greetings,

Andres Freund

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


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

Re: [HACKERS] Auto-tuning work_mem and maintenance_work_mem

[Tom Lane]

Gavin Flower  writes:
> On 17/02/14 15:26, Robert Haas wrote:
>> I don't really know about cpu_tuple_cost.  Kevin's often advocated
>> raising it, but I haven't heard anyone else advocate for that.  I
>> think we need data points from more people to know whether or not
>> that's a good idea in general.

> Processors have been getting faster, relative to spinning rust, over the 
> years.  So it puzzles me why anybody would want to raise the 
> cpu_tuple_cost!

The case where this is sensible is where your database mostly fits in
RAM, so that the cost of touching the underlying spinning rust isn't
so relevant.  The default cost settings are certainly not very good
for such scenarios.

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] narwhal and PGDLLIMPORT

[Tom Lane]

Dave Page  writes:
> On Fri, Feb 14, 2014 at 5:32 PM, Tom Lane  wrote:
>> (BTW, narwhal is evidently not trying to build plpython.  I wonder
>> why not?)

> Not sure - it's certainly installed on the box. I've enabled it for
> now, and will see what happens.

Sigh ... stop the presses.

In 9.3, narwhal is *still* showing a PGDLLIMPORT-type failure that no
other Windows critter is unhappy about:

dlltool --export-all --output-def worker_spi.def worker_spi.o
dllwrap -o worker_spi.dll --def worker_spi.def worker_spi.o -L../../src/port 
-L../../src/common -Wl,--allow-multiple-definition -L/mingw/lib  
-Wl,--as-needed   -L../../src/backend -lpostgres
Info: resolving _MyBgworkerEntry by linking to __imp__MyBgworkerEntry 
(auto-import)
fu01.o(.idata$3+0xc): undefined reference to `libpostgres_a_iname'
fu02.o(.idata$3+0xc): undefined reference to `libpostgres_a_iname'
nmth00.o(.idata$4+0x0): undefined reference to `_nm__MyBgworkerEntry'
collect2: ld returned 1 exit status

So we are back to square one AFAICS: we still have no idea why narwhal
is pickier than everything else.  (BTW, to save people the trouble of
looking: MyBgworkerEntry is marked PGDLLIMPORT in HEAD but not 9.3.)

Also, in HEAD narwhal is building things OK, but then seems to be
dumping core in the dblink regression test, leaving one with not a very
warm feeling about whether the contrib executables it's building are
any good.

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] narwhal and PGDLLIMPORT

[Dave Page]

On Mon, Feb 17, 2014 at 2:58 PM, Tom Lane  wrote:
> Dave Page  writes:
>> On Fri, Feb 14, 2014 at 5:32 PM, Tom Lane  wrote:
>>> (BTW, narwhal is evidently not trying to build plpython.  I wonder
>>> why not?)
>
>> Not sure - it's certainly installed on the box. I've enabled it for
>> now, and will see what happens.
>
> Sigh ... stop the presses.
>
> In 9.3, narwhal is *still* showing a PGDLLIMPORT-type failure that no
> other Windows critter is unhappy about:
>
> dlltool --export-all --output-def worker_spi.def worker_spi.o
> dllwrap -o worker_spi.dll --def worker_spi.def worker_spi.o -L../../src/port 
> -L../../src/common -Wl,--allow-multiple-definition -L/mingw/lib  
> -Wl,--as-needed   -L../../src/backend -lpostgres
> Info: resolving _MyBgworkerEntry by linking to __imp__MyBgworkerEntry 
> (auto-import)
> fu01.o(.idata$3+0xc): undefined reference to `libpostgres_a_iname'
> fu02.o(.idata$3+0xc): undefined reference to `libpostgres_a_iname'
> nmth00.o(.idata$4+0x0): undefined reference to `_nm__MyBgworkerEntry'
> collect2: ld returned 1 exit status
>
> So we are back to square one AFAICS: we still have no idea why narwhal
> is pickier than everything else.  (BTW, to save people the trouble of
> looking: MyBgworkerEntry is marked PGDLLIMPORT in HEAD but not 9.3.)
>
> Also, in HEAD narwhal is building things OK, but then seems to be
> dumping core in the dblink regression test, leaving one with not a very
> warm feeling about whether the contrib executables it's building are
> any good.

Well, as we know, Narwhal is really quite old now. I think I built it
seven+ years ago. Is it really worth banging heads against walls to
support something that noone in their right mind should be using for a
build these days?


-- 
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: 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] narwhal and PGDLLIMPORT

[Andres Freund]

On 2014-02-17 15:02:15 +, Dave Page wrote:
> On Mon, Feb 17, 2014 at 2:58 PM, Tom Lane  wrote:
> >> Not sure - it's certainly installed on the box. I've enabled it for
> >> now, and will see what happens.
> >
> > Sigh ... stop the presses.
> >
> > In 9.3, narwhal is *still* showing a PGDLLIMPORT-type failure that no
> > other Windows critter is unhappy about:
> >
> > dlltool --export-all --output-def worker_spi.def worker_spi.o
> > dllwrap -o worker_spi.dll --def worker_spi.def worker_spi.o 
> > -L../../src/port -L../../src/common -Wl,--allow-multiple-definition 
> > -L/mingw/lib  -Wl,--as-needed   -L../../src/backend -lpostgres
> > Info: resolving _MyBgworkerEntry by linking to __imp__MyBgworkerEntry 
> > (auto-import)
> > fu01.o(.idata$3+0xc): undefined reference to `libpostgres_a_iname'
> > fu02.o(.idata$3+0xc): undefined reference to `libpostgres_a_iname'
> > nmth00.o(.idata$4+0x0): undefined reference to `_nm__MyBgworkerEntry'
> > collect2: ld returned 1 exit status
> >
> > So we are back to square one AFAICS: we still have no idea why narwhal
> > is pickier than everything else.  (BTW, to save people the trouble of
> > looking: MyBgworkerEntry is marked PGDLLIMPORT in HEAD but not 9.3.)
> >
> > Also, in HEAD narwhal is building things OK, but then seems to be
> > dumping core in the dblink regression test, leaving one with not a very
> > warm feeling about whether the contrib executables it's building are
> > any good.
> 
> Well, as we know, Narwhal is really quite old now. I think I built it
> seven+ years ago. Is it really worth banging heads against walls to
> support something that noone in their right mind should be using for a
> build these days?

The problem is that lots of those issues are bugs that actually cause
problems for msvc builds. If there were tests in worker_spi it'd quite
possibly crash when run in 9.3. The problem is rather that the other
animals are *not* erroring.

Unless I am missing something.

Greetings,

Andres Freund

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


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

Re: [HACKERS] narwhal and PGDLLIMPORT

[Tom Lane]

Andres Freund  writes:
> On 2014-02-17 15:02:15 +, Dave Page wrote:
>> On Mon, Feb 17, 2014 at 2:58 PM, Tom Lane  wrote:
>>> In 9.3, narwhal is *still* showing a PGDLLIMPORT-type failure that no
>>> other Windows critter is unhappy about:

>> Well, as we know, Narwhal is really quite old now. I think I built it
>> seven+ years ago. Is it really worth banging heads against walls to
>> support something that noone in their right mind should be using for a
>> build these days?

> The problem is that lots of those issues are bugs that actually cause
> problems for msvc builds. If there were tests in worker_spi it'd quite
> possibly crash when run in 9.3. The problem is rather that the other
> animals are *not* erroring.

Exactly.

Although on second thought, the lack of complaints from other Windows
animals can probably be blamed on the fact that we didn't back-port
any of the recent hacking on the Windows build processes.  Maybe we
should think about doing so, now that the dust seems to have settled.

We still need to know why narwhal is crashing on dblink though.
I have a bad feeling that that may indicate still-unresolved
linkage problems.

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] Decimal values in

[Masterprojekt Naumann1]

Dear Dev-List,

inside execProcnode.c's ExecProcNode method we want to extract the value of
a tuple for a specific attribute. This works great for integers and
strings, but we are not able to figure out how to do this for floating
point numbers. Below is some example code snippet to show our problem:

TupleTableSlot *
ExecProcNode(PlanState *node) {
TupleTableSlot *result;
...
bool isNull;
Datum datum = slot_getattr(result,0, &isNull);

Form_pg_attribute *attrList = result->tts_tupleDescriptor->attrs;

if(attrList[0]->atttypid==INT4OID){
int value = (int) (datum);
...
} else if(attrList[0]->atttypid==VARCHAROID){
char* value = TextDatumGetCString(datum);
...
//this does not work :(
} else if(attrList[0]->atttypid== /*what is the right
OID*/){
//the value does not seem to be stored in the datum
float value = (float) (datum);
...
}
...
}

How can we get those values?

Yours sincerely, Fabian Tschirschnitz.

Re: [HACKERS] narwhal and PGDLLIMPORT

[Andres Freund]

On 2014-02-17 10:21:12 -0500, Tom Lane wrote:
> Andres Freund  writes:
> > On 2014-02-17 15:02:15 +, Dave Page wrote:
> >> On Mon, Feb 17, 2014 at 2:58 PM, Tom Lane  wrote:
> >>> In 9.3, narwhal is *still* showing a PGDLLIMPORT-type failure that no
> >>> other Windows critter is unhappy about:
> 
> >> Well, as we know, Narwhal is really quite old now. I think I built it
> >> seven+ years ago. Is it really worth banging heads against walls to
> >> support something that noone in their right mind should be using for a
> >> build these days?
> 
> > The problem is that lots of those issues are bugs that actually cause
> > problems for msvc builds. If there were tests in worker_spi it'd quite
> > possibly crash when run in 9.3. The problem is rather that the other
> > animals are *not* erroring.
> 
> Exactly.
> 
> Although on second thought, the lack of complaints from other Windows
> animals can probably be blamed on the fact that we didn't back-port
> any of the recent hacking on the Windows build processes.  Maybe we
> should think about doing so, now that the dust seems to have settled.

Yea, at the very least the gendef.pl thing should be backported,
possibly the mingw --disable-auto-import thing as well. But it's
probably a gooid idea to wait till the branches are stamped?

> We still need to know why narwhal is crashing on dblink though.
> I have a bad feeling that that may indicate still-unresolved
> linkage problems.

It's odd:

[53019d05.f58:2] LOG:  server process (PID 2428) exited with exit code 128
[53019d05.f58:3] DETAIL:  Failed process was running: SELECT *
FROM dblink('dbname=contrib_regression','SELECT * FROM foo') AS t(a 
int, b text, c text[])
WHERE t.a > 7;
[53019d05.f58:4] LOG:  server process (PID 2428) exited with exit code 0
[53019d05.f58:5] LOG:  terminating any other active server processes
[53019d06.e9c:2] WARNING:  terminating connection because of crash of another 
server process
[53019d06.e9c:3] DETAIL:  The postmaster has commanded this server process to 
roll back the curreGreetings,

Not sure if that's actually a segfault and not something else. Why is
the same death reported twice? With different exit codes?

Andres Freund

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


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

[HACKERS] Do you know the reason for increased max latency due to xlog scaling?

[MauMau]

Hello Heikki san,

I'm excited about your great work, xlog scaling.  I'm looking forward to the 
release of 9.4.


Please let me ask you about your performance data on the page:

http://hlinnaka.iki.fi/xloginsert-scaling/padding/

I'm worried about the big increase in max latency.  Do you know the cause? 
More frequent checkpoints caused by increased WAL volume thanks to enhanced 
performance?


Although I'm not sure this is related to what I'm asking, the following code 
fragment in WALInsertSlotAcquireOne() catched my eyes.  Shouldn't the if 
condition be "slotno == -1" instead of "!="?  I thought this part wants to 
make inserters to use another slot on the next insertion, when they fail to 
acquire the slot immediately.  Inserters pass slotno == -1.  I'm sorry if I 
misunderstood the code.


/*
 * If we couldn't get the slot immediately, try another slot next time.
 * On a system with more insertion slots than concurrent inserters, this
 * causes all the inserters to eventually migrate to a slot that no-one
 * else is using. On a system with more inserters than slots, it still
 * causes the inserters to be distributed quite evenly across the slots.
 */
if (slotno != -1 && retry)
 slotToTry = (slotToTry + 1) % num_xloginsert_slots;

Regards
MauMau



--
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] Decimal values in

[Alvaro Herrera]

Masterprojekt Naumann1 escribió:
> Dear Dev-List,
> 
> inside execProcnode.c's ExecProcNode method we want to extract the value of
> a tuple for a specific attribute. This works great for integers and
> strings, but we are not able to figure out how to do this for floating
> point numbers. Below is some example code snippet to show our problem:

"DECIMAL_OID" (you probably mean NUMERICOID) points to datatype numeric,
which is not floating point but a variable length datatype with its own
special encoding for storage.  If you want floating point you need
FLOAT4OID and FLOAT8OID, and columns created with types float and
"double precision", respectively.

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


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

Re: [HACKERS] Do you know the reason for increased max latency due to xlog scaling?

[Andres Freund]

Hi,

On 2014-02-18 00:43:54 +0900, MauMau wrote:
> Please let me ask you about your performance data on the page:
> 
> http://hlinnaka.iki.fi/xloginsert-scaling/padding/
> 
> I'm worried about the big increase in max latency.  Do you know the cause?
> More frequent checkpoints caused by increased WAL volume thanks to enhanced
> performance?

I don't see much evidence of increased latency there? You can't really
compare the latency when the throughput is significantly different.

> Although I'm not sure this is related to what I'm asking, the following code
> fragment in WALInsertSlotAcquireOne() catched my eyes.  Shouldn't the if
> condition be "slotno == -1" instead of "!="?  I thought this part wants to
> make inserters to use another slot on the next insertion, when they fail to
> acquire the slot immediately.  Inserters pass slotno == -1.  I'm sorry if I
> misunderstood the code.

I think you're right.

Greetings,

Andres Freund

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


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

Re: [HACKERS] narwhal and PGDLLIMPORT

[Tom Lane]

Andres Freund  writes:
> On 2014-02-17 10:21:12 -0500, Tom Lane wrote:
>> Although on second thought, the lack of complaints from other Windows
>> animals can probably be blamed on the fact that we didn't back-port
>> any of the recent hacking on the Windows build processes.  Maybe we
>> should think about doing so, now that the dust seems to have settled.

> Yea, at the very least the gendef.pl thing should be backported,
> possibly the mingw --disable-auto-import thing as well. But it's
> probably a gooid idea to wait till the branches are stamped?

Certainly --- I'm not touching that till the releases are out ;-).
Anything that's broken now has been broken in past releases too,
so it's not worth the risk.

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] Auto-tuning work_mem and maintenance_work_mem

[Bruce Momjian]

On Sun, Feb 16, 2014 at 09:26:47PM -0500, Robert Haas wrote:
> > So, would anyone like me to create patches for any of these items before
> > we hit 9.4 beta?  We have added autovacuum_work_mem, and increasing
> > work_mem and maintenance_work_mem by 4x is a simple operation.  Not sure
> > about the others.  Or do we just keep this all for 9.5?
> 
> I don't think anyone objected to increasing the defaults for work_mem
> and maintenance_work_mem by 4x, and a number of people were in favor,
> so I think we should go ahead and do that.  If you'd like to do the
> honors, by all means!

OK, patch attached.

> The current bgwriter_lru_maxpages value limits the background writer
> to a maximum of 4MB/s.  If one imagines shared_buffers = 8GB, that
> starts to seem rather low, but I don't have a good feeling for what a
> better value would be.
> 
> The current vacuum cost delay settings limit autovacuum to about
> 2.6MB/s.  I am inclined to think we need a rather large bump there,
> like 10x, but maybe it would be more prudent to do a smaller bump,
> like say 4x, to avoid changing the default behavior too dramatically
> between releases.  IOW, I guess I'm proposing raising
> vacuum_cost_limit from 200 to 800.
> 
> I don't really know about cpu_tuple_cost.  Kevin's often advocated
> raising it, but I haven't heard anyone else advocate for that.  I
> think we need data points from more people to know whether or not
> that's a good idea in general.

Robert, can you take the lead on these remaining possible changes?  We
don't have time for any controversial changes but things everyone can
agree on, like work_mem, should be implemented for 9.4.

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

  + Everyone has their own god. +
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
new file mode 100644
index e12778b..47bdebf
*** a/doc/src/sgml/config.sgml
--- b/doc/src/sgml/config.sgml
*** include 'filename'
*** 1213,1219 
 
  Specifies the amount of memory to be used by internal sort operations
  and hash tables before writing to temporary disk files. The value
! defaults to one megabyte (1MB).
  Note that for a complex query, several sort or hash operations might be
  running in parallel; each operation will be allowed to use as much memory
  as this value specifies before it starts to write data into temporary
--- 1213,1219 
 
  Specifies the amount of memory to be used by internal sort operations
  and hash tables before writing to temporary disk files. The value
! defaults to four megabytes (4MB).
  Note that for a complex query, several sort or hash operations might be
  running in parallel; each operation will be allowed to use as much memory
  as this value specifies before it starts to write data into temporary
*** include 'filename'
*** 1239,1245 
  Specifies the maximum amount of memory to be used by maintenance
  operations, such as VACUUM, CREATE
  INDEX, and ALTER TABLE ADD FOREIGN KEY.  It defaults
! to 16 megabytes (16MB).  Since only one of these
  operations can be executed at a time by a database session, and
  an installation normally doesn't have many of them running
  concurrently, it's safe to set this value significantly larger
--- 1239,1245 
  Specifies the maximum amount of memory to be used by maintenance
  operations, such as VACUUM, CREATE
  INDEX, and ALTER TABLE ADD FOREIGN KEY.  It defaults
! to 64 megabytes (64MB).  Since only one of these
  operations can be executed at a time by a database session, and
  an installation normally doesn't have many of them running
  concurrently, it's safe to set this value significantly larger
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
new file mode 100644
index 86afde1..aa5a875
*** a/src/backend/utils/misc/guc.c
--- b/src/backend/utils/misc/guc.c
*** static struct config_int ConfigureNamesI
*** 1773,1779 
  			GUC_UNIT_KB
  		},
  		&work_mem,
! 		1024, 64, MAX_KILOBYTES,
  		NULL, NULL, NULL
  	},
  
--- 1773,1779 
  			GUC_UNIT_KB
  		},
  		&work_mem,
! 		4096, 64, MAX_KILOBYTES,
  		NULL, NULL, NULL
  	},
  
*** static struct config_int ConfigureNamesI
*** 1784,1790 
  			GUC_UNIT_KB
  		},
  		&maintenance_work_mem,
! 		16384, 1024, MAX_KILOBYTES,
  		NULL, NULL, NULL
  	},
  
--- 1784,1790 
  			GUC_UNIT_KB
  		},
  		&maintenance_work_mem,
! 		65536, 1024, MAX_KILOBYTES,
  		NULL, NULL, NULL
  	},
  
diff --git a/src/backend/utils/misc/postgresql.conf.sample b/src/backend/utils/misc/postgresql.conf.sample
new file mode 100644
index 480c9e9..07341e7
*** a/src/backend/utils/misc/postgresql.conf.sample
--- b/src/backend/utils/misc/postgresql.conf.

Re: [HACKERS] Auto-tuning work_mem and maintenance_work_mem

[Andres Freund]

On 2014-02-16 21:26:47 -0500, Robert Haas wrote:
> I don't think anyone objected to increasing the defaults for work_mem
> and maintenance_work_mem by 4x, and a number of people were in favor,
> so I think we should go ahead and do that.  If you'd like to do the
> honors, by all means!

Actually, I object to increasing work_mem by default. In my experience
most of the untuned servers are backing some kind of web application and
often run with far too many connections. Increasing work_mem for those
is dangerous.

> I don't really know about cpu_tuple_cost.  Kevin's often advocated
> raising it, but I haven't heard anyone else advocate for that.  I
> think we need data points from more people to know whether or not
> that's a good idea in general.

FWIW It's a good idea in my experience.

Greetings,

Andres Freund

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


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

Re: [HACKERS] Auto-tuning work_mem and maintenance_work_mem

[Stephen Frost]

* Andres Freund (and...@2ndquadrant.com) wrote:
> On 2014-02-16 21:26:47 -0500, Robert Haas wrote:
> > I don't think anyone objected to increasing the defaults for work_mem
> > and maintenance_work_mem by 4x, and a number of people were in favor,
> > so I think we should go ahead and do that.  If you'd like to do the
> > honors, by all means!
> 
> Actually, I object to increasing work_mem by default. In my experience
> most of the untuned servers are backing some kind of web application and
> often run with far too many connections. Increasing work_mem for those
> is dangerous.

And I still disagree with this- even in those cases.  Those same untuned
servers are running dirt-simple queries 90% of the time and they won't
use any more memory from this, while the 10% of the queries which are
more complicated will greatly improve.

> > I don't really know about cpu_tuple_cost.  Kevin's often advocated
> > raising it, but I haven't heard anyone else advocate for that.  I
> > think we need data points from more people to know whether or not
> > that's a good idea in general.
> 
> FWIW It's a good idea in my experience.

I'm in favor of this also but I'm also in the camp of "gee, more data
would be nice".

Thanks,

Stephen


signature.asc
Description: Digital signature

Re: [HACKERS] Auto-tuning work_mem and maintenance_work_mem

[Andres Freund]

On 2014-02-17 11:31:56 -0500, Stephen Frost wrote:
> * Andres Freund (and...@2ndquadrant.com) wrote:
> > On 2014-02-16 21:26:47 -0500, Robert Haas wrote:
> > > I don't think anyone objected to increasing the defaults for work_mem
> > > and maintenance_work_mem by 4x, and a number of people were in favor,
> > > so I think we should go ahead and do that.  If you'd like to do the
> > > honors, by all means!
> > 
> > Actually, I object to increasing work_mem by default. In my experience
> > most of the untuned servers are backing some kind of web application and
> > often run with far too many connections. Increasing work_mem for those
> > is dangerous.
> 
> And I still disagree with this- even in those cases.  Those same untuned
> servers are running dirt-simple queries 90% of the time and they won't
> use any more memory from this, while the 10% of the queries which are
> more complicated will greatly improve.

Uh. Paging.

Greetings,

Andres Freund

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


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

Re: [HACKERS] Do you know the reason for increased max latency due to xlog scaling?

[MauMau]

From: "Andres Freund" 

On 2014-02-18 00:43:54 +0900, MauMau wrote:
I'm worried about the big increase in max latency.  Do you know the 
cause?
More frequent checkpoints caused by increased WAL volume thanks to 
enhanced

performance?


I don't see much evidence of increased latency there? You can't really
compare the latency when the throughput is significantly different.


For example, please see the max latencies of test set 2 (PG 9.3) and test 
set 4 (xlog scaling with padding).  They are 207.359 and 1219.422 
respectively.  The throughput is of course greatly improved, but I think the 
response time should not be sacrificed as much as possible.  There are some 
users who are sensitive to max latency, such as stock exchange and online 
games.



Although I'm not sure this is related to what I'm asking, the following 
code

fragment in WALInsertSlotAcquireOne() catched my eyes.  Shouldn't the if
condition be "slotno == -1" instead of "!="?  I thought this part wants 
to
make inserters to use another slot on the next insertion, when they fail 
to
acquire the slot immediately.  Inserters pass slotno == -1.  I'm sorry if 
I

misunderstood the code.


I think you're right.


Thanks for your confirmation.  I'd be glad if the fix could bring any 
positive impact on max latency.


Regards
MauMau



--
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] Decimal values in

[Tom Lane]

Alvaro Herrera  writes:
> Masterprojekt Naumann1 escribió:
>> inside execProcnode.c's ExecProcNode method we want to extract the value of
>> a tuple for a specific attribute. This works great for integers and
>> strings, but we are not able to figure out how to do this for floating
>> point numbers. Below is some example code snippet to show our problem:

> "DECIMAL_OID" (you probably mean NUMERICOID) points to datatype numeric,
> which is not floating point but a variable length datatype with its own
> special encoding for storage.  If you want floating point you need
> FLOAT4OID and FLOAT8OID, and columns created with types float and
> "double precision", respectively.

Also, you should not be using casts, but the appropriate DatumGetXXX
macro.  In some cases those reduce to a cast, but your code ought not
assume that.

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] Do you know the reason for increased max latency due to xlog scaling?

[Andres Freund]

On 2014-02-18 01:35:52 +0900, MauMau wrote:
> From: "Andres Freund" 
> >On 2014-02-18 00:43:54 +0900, MauMau wrote:
> >>I'm worried about the big increase in max latency.  Do you know the
> >>cause?
> >>More frequent checkpoints caused by increased WAL volume thanks to
> >>enhanced
> >>performance?
> >
> >I don't see much evidence of increased latency there? You can't really
> >compare the latency when the throughput is significantly different.
> 
> For example, please see the max latencies of test set 2 (PG 9.3) and test
> set 4 (xlog scaling with padding).  They are 207.359 and 1219.422
> respectively.  The throughput is of course greatly improved, but I think the
> response time should not be sacrificed as much as possible.  There are some
> users who are sensitive to max latency, such as stock exchange and online
> games.

You need to compare both at the same throughput to have any meaningful
comparison.

Greetings,

Andres Freund

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


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

Re: [HACKERS] Ctrl+C from sh can shut down daemonized PostgreSQL cluster

[Bruce Momjian]

On Mon, Feb 17, 2014 at 10:38:29AM +0100, Bjorn Munch wrote:
> On 14/02 14.57, Kevin Grittner wrote:
> > We have had a case where a production cluster was accidentally shut
> > down by a customer who used Ctrl+C in the same sh session in which
> > they had (long before) run pg_ctl start.  We have only seen this in
> > sh on Solaris.  Other shells on Solaris don't behave this way, nor
> > does sh on tested versions of Linux.  Nevertheless, the problem is
> > seen on the default shell for a supported OS.
> 
> What Solaris version, and what version of sh?  sh on Solaris isn't
> necessarily the "real" bourne shell. In Solaris 11 it's actually
> ksh93.

This was Solaris 9.

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

  + Everyone has their own god. +


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

Re: [HACKERS] pg_basebackup skips pg_replslot directory

[Andres Freund]

Hi,

On 2014-02-18 02:01:58 +0900, Sawada Masahiko wrote:
> I found strange behavior of PostgreSQL of HEAD while using pg_basebackup.
> pg_basebackup skips pg_replslot directory since
> 858ec11858a914d4c380971985709b6d6b7dd6fc commit.
> 
> But pg_repslot direcotry is needed to start replication. So the
> standby server which is created by
> pg_baseback can not start.
> I got following FATAL error when the standby server starts.

Yes. Fujuii has submitted a similar patch, I was hoping he'd commit
it...

Greetings,

Andres Freund

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


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

[HACKERS] pg_basebackup skips pg_replslot directory

[Sawada Masahiko]

Hi all,

I found strange behavior of PostgreSQL of HEAD while using pg_basebackup.
pg_basebackup skips pg_replslot directory since
858ec11858a914d4c380971985709b6d6b7dd6fc commit.

But pg_repslot direcotry is needed to start replication. So the
standby server which is created by
pg_baseback can not start.
I got following FATAL error when the standby server starts.

FATAL:  could not open directory "pg_replslot": No such file or directory

Is this a bug?
Attached file solves it by including pg_replslot directory as empty directory.

Please give feedback.


Regards,

---
Sawada Masahiko


basebackup_pg_replslot.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] Ctrl+C from sh can shut down daemonized PostgreSQL cluster

[Bruce Momjian]

On Mon, Feb 17, 2014 at 10:38:29AM +0100, Bjorn Munch wrote:
> On 14/02 14.57, Kevin Grittner wrote:
> > We have had a case where a production cluster was accidentally shut
> > down by a customer who used Ctrl+C in the same sh session in which
> > they had (long before) run pg_ctl start.  We have only seen this in
> > sh on Solaris.  Other shells on Solaris don't behave this way, nor
> > does sh on tested versions of Linux.  Nevertheless, the problem is
> > seen on the default shell for a supported OS.
> 
> What Solaris version, and what version of sh?  sh on Solaris isn't
> necessarily the "real" bourne shell. In Solaris 11 it's actually
> ksh93.
> 
> I've seen a sort-of opposite problem which does not appear in stock
> Solaris 10 or 11 but in OpenSolaris, at least the version I used to
> have on my desktop.
> 
> And this was not PostgreSQL but MySQL There's a script mysqld_safe
> which will automatically restart the mysqld server if it dies. But in
> OpenSolaris with ksh version '93t', if I killed mysqld, the shell that
> started it also died. I never could figure out why. Solaris 11 with
> ksh '93u' does not have this problem. Nor does Solaris 10 with "real" sh.
> 
> Is this customer by any chance running OpenSolaris?

FYI, this email post has a header line that causes all replies to go
_only_ to the group email address:

Mail-Followup-To: pgsql-hackers@postgresql.org

I assume it is something related to the Oracle mail server or something
configured by the email author.

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

  + Everyone has their own god. +


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

Re: [HACKERS] pg_basebackup skips pg_replslot directory

[Sawada Masahiko]

On Tue, Feb 18, 2014 at 2:07 AM, Andres Freund  wrote:
> Hi,
>
> On 2014-02-18 02:01:58 +0900, Sawada Masahiko wrote:
>> I found strange behavior of PostgreSQL of HEAD while using pg_basebackup.
>> pg_basebackup skips pg_replslot directory since
>> 858ec11858a914d4c380971985709b6d6b7dd6fc commit.
>>
>> But pg_repslot direcotry is needed to start replication. So the
>> standby server which is created by
>> pg_baseback can not start.
>> I got following FATAL error when the standby server starts.
>
> Yes. Fujuii has submitted a similar patch, I was hoping he'd commit
> it...
>

I did not notice it.
Thank you for info!

Regards,

---
Sawada Masahiko


-- 
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] Draft release notes up for review

[Tom Lane]

Josh Berkus  writes:
> On 02/16/2014 03:41 PM, Tom Lane wrote:
>> Draft release notes for 9.3.3 are committed and can be read at
>> http://www.postgresql.org/docs/devel/static/release-9-3-3.html
>> Any comments before I start transposing them into the back branches?

> Major:

> Do we have an explantion of what a multixact is, anywhere, so that we
> can link it?

Fixed.  I did a bit of wordsmithing on the text Alvaro pointed to, too.

> Minor:

> ECPG or ecpg?  Pick one or the other.

AFAICS, "ecpg" is the vast majority case in the release notes, so
that's what I've used.

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] pg_basebackup skips pg_replslot directory

[Andres Freund]

On 2014-02-18 02:16:19 +0900, Sawada Masahiko wrote:
> On Tue, Feb 18, 2014 at 2:07 AM, Andres Freund  wrote:
> > Hi,
> >
> > On 2014-02-18 02:01:58 +0900, Sawada Masahiko wrote:
> >> I found strange behavior of PostgreSQL of HEAD while using pg_basebackup.
> >> pg_basebackup skips pg_replslot directory since
> >> 858ec11858a914d4c380971985709b6d6b7dd6fc commit.
> >>
> >> But pg_repslot direcotry is needed to start replication. So the
> >> standby server which is created by
> >> pg_baseback can not start.
> >> I got following FATAL error when the standby server starts.
> >
> > Yes. Fujuii has submitted a similar patch, I was hoping he'd commit
> > it...
> >
> 
> I did not notice it.
> Thank you for info!

His patch and some discussion is at/around
http://archives.postgresql.org/message-id/CAHGQGwGvb0qXP7Q76xLUkGO%2BwE9SyJzvzF%3DQBOS-mxgiz0vfKw%40mail.gmail.com

Thanks for the patch,

Andres Freund

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


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

Re: [HACKERS] Auto-tuning work_mem and maintenance_work_mem

[Robert Haas]

On Mon, Feb 17, 2014 at 11:19 AM, Andres Freund  wrote:
> On 2014-02-16 21:26:47 -0500, Robert Haas wrote:
>> I don't think anyone objected to increasing the defaults for work_mem
>> and maintenance_work_mem by 4x, and a number of people were in favor,
>> so I think we should go ahead and do that.  If you'd like to do the
>> honors, by all means!
>
> Actually, I object to increasing work_mem by default. In my experience
> most of the untuned servers are backing some kind of web application and
> often run with far too many connections. Increasing work_mem for those
> is dangerous.

I think you may be out-voted.  An awful lot of people have voiced
support for the idea of raising this value, and there is no rule that
our default should be the smallest value that anyone will ever find
useful.  We do tend to err on the side of conservatism and aim for a
relatively low-end machine, and I agree with that policy, but there is
such a thing as going overboard.  With the proposed defaults, a user
with one sort or hash in every session, each of which uses the
entirety of work_mem, is on the hook for 400MB.  If you're trying to
handle 100 connections on a machine that does not have 400MB of
working memory available, you are probably in for a bad time of it.

Now, if you're saying that people raise max_connections to say 1000
*and do nothing else* perhaps that makes the argument more plausible.
But I don't think it makes it very much more plausible.  Even a
high-end system is likely to deliver terrible performance if the user
has 1000 simultaneously-active connections; one with only a few GB of
memory is going to be crushed like a bug.

I'll note that in 9.3, we quadrupled the default size of
shared_buffers when we got out from under the POSIX shared memory
limits and AFAIK we've had zero complaints about that.  It is entirely
possible, even likely, that there is a machine out there somewhere for
which the old value of 32MB is preferable, and those people can
configure a smaller value.  But that's not typical.  And neither do I
believe that the typical PostgreSQL user wants a 2MB sort to spill to
disk.

-- 
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] Ctrl+C from sh can shut down daemonized PostgreSQL cluster

[Tom Lane]

Bruce Momjian  writes:
> On Mon, Feb 17, 2014 at 10:38:29AM +0100, Bjorn Munch wrote:
>> What Solaris version, and what version of sh?  sh on Solaris isn't
>> necessarily the "real" bourne shell. In Solaris 11 it's actually
>> ksh93.

> This was Solaris 9.

Isn't that out of support by Oracle?

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] Auto-tuning work_mem and maintenance_work_mem

[Andres Freund]

On 2014-02-17 12:23:58 -0500, Robert Haas wrote:
> On Mon, Feb 17, 2014 at 11:19 AM, Andres Freund  
> wrote:
> > On 2014-02-16 21:26:47 -0500, Robert Haas wrote:
> >> I don't think anyone objected to increasing the defaults for work_mem
> >> and maintenance_work_mem by 4x, and a number of people were in favor,
> >> so I think we should go ahead and do that.  If you'd like to do the
> >> honors, by all means!
> >
> > Actually, I object to increasing work_mem by default. In my experience
> > most of the untuned servers are backing some kind of web application and
> > often run with far too many connections. Increasing work_mem for those
> > is dangerous.
> 
> I think you may be out-voted.

I realize that, but I didn't want to let the "I don't think anyone
objected" stand :)

> With the proposed defaults, a user with one sort or hash in every
> session, each of which uses the entirety of work_mem, is on the hook
> for 400MB.  If you're trying to handle 100 connections on a machine
> that does not have 400MB of working memory available, you are probably
> in for a bad time of it.

Sure, if that's all they do it's fine. But often enough queries aren't
that simple. Lots of the ORMs commonly used for web applications tend to
create lots of JOINs to gather all the data and also use sorting for paging.

Greetings,

Andres Freund

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


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

Re: [HACKERS] Auto-tuning work_mem and maintenance_work_mem

[Tom Lane]

Andres Freund  writes:
> On 2014-02-17 12:23:58 -0500, Robert Haas wrote:
>> I think you may be out-voted.

> I realize that, but I didn't want to let the "I don't think anyone
> objected" stand :)

FWIW, I think we need to be pretty gradual about this sort of thing,
because push-back from the field is the only way to know if we've gone
too far for average users.  I'm OK with raising work_mem 4X in one go,
but I'd complain if it were 10X, or if we were also raising other
resource consumption limits in the same release.

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] Ctrl+C from sh can shut down daemonized PostgreSQL cluster

[Bruce Momjian]

On Mon, Feb 17, 2014 at 12:25:33PM -0500, Tom Lane wrote:
> Bruce Momjian  writes:
> > On Mon, Feb 17, 2014 at 10:38:29AM +0100, Bjorn Munch wrote:
> >> What Solaris version, and what version of sh?  sh on Solaris isn't
> >> necessarily the "real" bourne shell. In Solaris 11 it's actually
> >> ksh93.
> 
> > This was Solaris 9.
> 
> Isn't that out of support by Oracle?

It certainly might be --- I have no idea.  What surprised me is that we
are relying solely on system() to block signals to pg_ctl-spawned
servers.  The question is whether that is sufficient and whether we
should be doing more.  I don't think we have to make adjustments just
for Solaris 9.

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

  + Everyone has their own god. +


-- 
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: option --if-exists for pg_dump

[Alvaro Herrera]

Jeevan Chalke escribió:

I don't understand this code.  (Well, it's pg_dump.)  Or maybe I do
understand it, and it's not doing what you think it's doing.  I mean, in
this part:

> diff --git a/src/bin/pg_dump/pg_backup_archiver.c 
> b/src/bin/pg_dump/pg_backup_archiver.c
> index 7fc0288..c08a0d3 100644
> --- a/src/bin/pg_dump/pg_backup_archiver.c
> +++ b/src/bin/pg_dump/pg_backup_archiver.c
> @@ -413,8 +413,84 @@ RestoreArchive(Archive *AHX)
>   /* Select owner and schema as necessary */
>   _becomeOwner(AH, te);
>   _selectOutputSchema(AH, te->namespace);
> - /* Drop it */
> - ahprintf(AH, "%s", te->dropStmt);
> +
> + if (*te->dropStmt != '\0')
> + {
> + /* Inject IF EXISTS clause to DROP part 
> when required. */
> + if (ropt->if_exists)

It does *not* modify te->dropStmt, it only sends ahprint() a different
version of what was stored (injected the wanted IF EXISTS clause).  If
that is correct, then why are we, in this other part, trying to remove
the IF EXISTS clause?

> @@ -2942,9 +3018,39 @@ _getObjectDescription(PQExpBuffer buf, TocEntry *te, 
> ArchiveHandle *AH)
>   strcmp(type, "OPERATOR CLASS") == 0 ||
>   strcmp(type, "OPERATOR FAMILY") == 0)
>   {
> - /* Chop "DROP " off the front and make a modifiable copy */
> - char   *first = pg_strdup(te->dropStmt + 5);
> - char   *last;
> + char*first;
> + char*last;
> +
> + /*
> +  * Object description is based on dropStmt statement which may 
> have
> +  * IF EXISTS clause.  Thus we need to update an offset such 
> that it
> +  * won't be included in the object description.
> +  */

Maybe I am mistaken and the te->dropStmt already contains the IF EXISTS
bit for some reason; but if so I don't know why that is.  Care to
explain?

I also think that _getObjectDescription() becomes overworked after this
patch.  I wonder if we should be storing te->objIdentity so that we can
construct the ALTER OWNER command without going to as much trouble as
parsing the DROP command.  Is there a way to do that? Maybe we can ask
the server for the object identity, for example.  There is a new
function to do that in 9.3 which perhaps we can now use.

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


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

Re: [HACKERS] [bug fix] "pg_ctl stop" times out when it should respond quickly

[Alvaro Herrera]

MauMau escribió:

> pg_ctl timed out waiting for the zombie postgres.
> 
> maumau 19621 18849  0 15:21 pts/900:00:00 [postgres] 
> maumau 20253 18849  0 15:22 pts/900:00:00 
> /maumau/postgresql-9.4/src/test/regress/./tmp_check/install//maumau/pgsql/bin/pg_ctl
> stop -D /maumau/postgresql-9.4/src/test/regress/./tmp_check/data -s
> -m fast
> 
> pg_regress must wait for postgres to terminate by calling waitpid(),
> because it invoked postgres directly.  The attached
> pg_regress_pg_stop.patch does this.  If you like the combination of
> this and the original fix for pg_ctl in one patch, please use
> pg_stop_fail_v3.patch.

The pg_regress part is ugly.  However, pg_regress is doing something
unusual when starting postmaster itself, so the ugly coding to stop it
seems to match.  If we wanted to avoid the ugliness here, the right fix
would be to use pg_ctl to start postmaster as well as to stop it.  But
that'd come at a price, because we would need more ugly code to figure
out postmaster's PID.  All in all, the compromise proposed by this patch
seems acceptable.  If we really wanted to make all this real pretty, we
could provide a "libpg_ctl" library to start and stop postmaster, as
well as query the PID.  Probably not worth the trouble.

I would apply this patch to all supported branches after this week's
release.

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


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

Re: [HACKERS] Ctrl+C from sh can shut down daemonized PostgreSQL cluster

[Alvaro Herrera]

Bruce Momjian wrote:

> FYI, this email post has a header line that causes all replies to go
> _only_ to the group email address:
> 
>   Mail-Followup-To: pgsql-hackers@postgresql.org
> 
> I assume it is something related to the Oracle mail server or something
> configured by the email author.

Most likely, Bjorn has followup_to set to true:
http://www.mutt.org/doc/manual/manual-6.html#followup_to

I very much doubt that the mail server is injecting such a header.

Amusingly, Mutt also has an option to control whether to honor this
header:
http://www.mutt.org/doc/manual/manual-6.html#honor_followup_to

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


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

Re: [HACKERS] Auto-tuning work_mem and maintenance_work_mem

[Robert Haas]

On Mon, Feb 17, 2014 at 11:33 AM, Andres Freund  wrote:
> On 2014-02-17 11:31:56 -0500, Stephen Frost wrote:
>> * Andres Freund (and...@2ndquadrant.com) wrote:
>> > On 2014-02-16 21:26:47 -0500, Robert Haas wrote:
>> > > I don't think anyone objected to increasing the defaults for work_mem
>> > > and maintenance_work_mem by 4x, and a number of people were in favor,
>> > > so I think we should go ahead and do that.  If you'd like to do the
>> > > honors, by all means!
>> >
>> > Actually, I object to increasing work_mem by default. In my experience
>> > most of the untuned servers are backing some kind of web application and
>> > often run with far too many connections. Increasing work_mem for those
>> > is dangerous.
>>
>> And I still disagree with this- even in those cases.  Those same untuned
>> servers are running dirt-simple queries 90% of the time and they won't
>> use any more memory from this, while the 10% of the queries which are
>> more complicated will greatly improve.
>
> Uh. Paging.

What about 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] Auto-tuning work_mem and maintenance_work_mem

[Andres Freund]

On 2014-02-17 13:33:17 -0500, Robert Haas wrote:
> On Mon, Feb 17, 2014 at 11:33 AM, Andres Freund  
> wrote:
> >> And I still disagree with this- even in those cases.  Those same untuned
> >> servers are running dirt-simple queries 90% of the time and they won't
> >> use any more memory from this, while the 10% of the queries which are
> >> more complicated will greatly improve.
> >
> > Uh. Paging.
> 
> What about it?

It's often the source of a good portion of the queries and load in web
applications. Multiple joins and more than one row... I have several
time seen stats changes or bad to-be-sorted columns cause large amounts
of memory to be used.

Anyway, I've stated my opinion that I do not think it's a good idea to
raise that particular default (while agreeing with all the others) and I
know I am in the minority, so I don't think we need to argue this out...

Greetings,

Andres Freund

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


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

[HACKERS] 8.2 -> 8.4 Upgrade: No More "ldaps://"?

[Jim Seymour]

Hi There,

Tried to upgrade from 8.2.21 to 8.4.19 this morning and ran into a
wall: It would appear the 

hostssl all all  0.0.0.0/0  ldap "ldaps://..."

syntax is no longer supported?

Searched.  Asked on the IRC channel.  It would seem that in 8.4.x
there's no way to perform a "straight SSL" (not TLS) connect to an LDAP
server anymore?

Thanks,
Jim
-- 
Note: My mail server employs *very* aggressive anti-spam
filtering.  If you reply to this email and your email is
rejected, please accept my apologies and let me know via my
web form at .


-- 
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] Memory ordering issue in LWLockRelease, WakeupWaiters, WALInsertSlotRelease

[Robert Haas]

On Sat, Feb 15, 2014 at 11:17 AM, Andres Freund  wrote:
> On 2014-02-15 16:18:00 +0100, Andres Freund wrote:
>> On 2014-02-15 10:06:41 -0500, Tom Lane wrote:
>> > Andres Freund  writes:
>> > > My current conclusion is that backporting barriers.h is by far the most
>> > > reasonable way to go. The compiler problems have been ironed out by
>> > > now...
>> >
>> > -1.  IMO that code is still quite unproven, and what's more, the
>> > problem we're discussing here is completely hypothetical.  If it
>> > were real, we'd have field evidence of it.  We've not had that
>> > much trouble seeing instances of even very narrow race-condition
>> > windows in the past.
>>
>> Well, the problem is that few of us have access to interesting !x86
>> machines to run tests, and that's where we'd see problems (since x86
>> gives enough guarantees to avoid this unless the compiler reorders
>> stuff). I am personally fine with just using volatiles to avoid
>> reordering in the older branches, but Florian argued against it.
>
> Here's patches doing that. The 9.3 version also applies to 9.2; the 9.1
> version applies back to 8.4.

I have no confidence that this isn't going to be real bad for performance.

-- 
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] Auto-tuning work_mem and maintenance_work_mem

[Bruce Momjian]

On Mon, Feb 17, 2014 at 07:39:47PM +0100, Andres Freund wrote:
> On 2014-02-17 13:33:17 -0500, Robert Haas wrote:
> > On Mon, Feb 17, 2014 at 11:33 AM, Andres Freund  
> > wrote:
> > >> And I still disagree with this- even in those cases.  Those same untuned
> > >> servers are running dirt-simple queries 90% of the time and they won't
> > >> use any more memory from this, while the 10% of the queries which are
> > >> more complicated will greatly improve.
> > >
> > > Uh. Paging.
> > 
> > What about it?
> 
> It's often the source of a good portion of the queries and load in web
> applications. Multiple joins and more than one row... I have several
> time seen stats changes or bad to-be-sorted columns cause large amounts
> of memory to be used.

Perhaps we should have said there was "general agreement" to increase
work_mem and maintenence_work_mem by 4x, not that there was 100%
agreement.  It would be nice to have 100% agreement, but if we _require_
that then defaults would probably never be changed.

> Anyway, I've stated my opinion that I do not think it's a good idea to
> raise that particular default (while agreeing with all the others) and I
> know I am in the minority, so I don't think we need to argue this out...

OK, good.  If you did feel there was need for more discussion, we would
need to push this change to PG 9.5.

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

  + Everyone has their own god. +


-- 
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] Memory ordering issue in LWLockRelease, WakeupWaiters, WALInsertSlotRelease

[Andres Freund]

On 2014-02-17 13:49:01 -0500, Robert Haas wrote:
> On Sat, Feb 15, 2014 at 11:17 AM, Andres Freund  
> wrote:
> > On 2014-02-15 16:18:00 +0100, Andres Freund wrote:
> >> On 2014-02-15 10:06:41 -0500, Tom Lane wrote:
> >> > Andres Freund  writes:
> >> > > My current conclusion is that backporting barriers.h is by far the most
> >> > > reasonable way to go. The compiler problems have been ironed out by
> >> > > now...
> >> >
> >> > -1.  IMO that code is still quite unproven, and what's more, the
> >> > problem we're discussing here is completely hypothetical.  If it
> >> > were real, we'd have field evidence of it.  We've not had that
> >> > much trouble seeing instances of even very narrow race-condition
> >> > windows in the past.
> >>
> >> Well, the problem is that few of us have access to interesting !x86
> >> machines to run tests, and that's where we'd see problems (since x86
> >> gives enough guarantees to avoid this unless the compiler reorders
> >> stuff). I am personally fine with just using volatiles to avoid
> >> reordering in the older branches, but Florian argued against it.
> >
> > Here's patches doing that. The 9.3 version also applies to 9.2; the 9.1
> > version applies back to 8.4.
> 
> I have no confidence that this isn't going to be real bad for performance.

It's just a write barrier which evaluates to a pure compiler barrier on
x86 anyway?
And it's in a loop that's only entered when the kernel is entered anyway
to wake up the other backend.

What should that affect significantly?

Greetings,

Andres Freund

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


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

Re: [HACKERS] nextVictimBuffer in README

[Robert Haas]

On Thu, Feb 13, 2014 at 12:10 PM, Vik Fearing  wrote:
> While reading through src/backend/storage/buffer/README and looking at
> the code that it describes, I noticed that the case is wrong for
> nextVictimBuffer.
>
> It's no big deal really, but the attached trivial patch makes the README
> match the code.

Thanks, committed.

-- 
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] Memory ordering issue in LWLockRelease, WakeupWaiters, WALInsertSlotRelease

[Robert Haas]

On Mon, Feb 17, 2014 at 1:55 PM, Andres Freund  wrote:
> On 2014-02-17 13:49:01 -0500, Robert Haas wrote:
>> On Sat, Feb 15, 2014 at 11:17 AM, Andres Freund  
>> wrote:
>> > On 2014-02-15 16:18:00 +0100, Andres Freund wrote:
>> >> On 2014-02-15 10:06:41 -0500, Tom Lane wrote:
>> >> > Andres Freund  writes:
>> >> > > My current conclusion is that backporting barriers.h is by far the 
>> >> > > most
>> >> > > reasonable way to go. The compiler problems have been ironed out by
>> >> > > now...
>> >> >
>> >> > -1.  IMO that code is still quite unproven, and what's more, the
>> >> > problem we're discussing here is completely hypothetical.  If it
>> >> > were real, we'd have field evidence of it.  We've not had that
>> >> > much trouble seeing instances of even very narrow race-condition
>> >> > windows in the past.
>> >>
>> >> Well, the problem is that few of us have access to interesting !x86
>> >> machines to run tests, and that's where we'd see problems (since x86
>> >> gives enough guarantees to avoid this unless the compiler reorders
>> >> stuff). I am personally fine with just using volatiles to avoid
>> >> reordering in the older branches, but Florian argued against it.
>> >
>> > Here's patches doing that. The 9.3 version also applies to 9.2; the 9.1
>> > version applies back to 8.4.
>>
>> I have no confidence that this isn't going to be real bad for performance.
>
> It's just a write barrier which evaluates to a pure compiler barrier on
> x86 anyway?
> And it's in a loop that's only entered when the kernel is entered anyway
> to wake up the other backend.
>
> What should that affect significantly?

On x86, presumably nothing.  On other architectures, I don't know what
the impact is, but I don't accept a hand-wavy assertion that there
shouldn't be any as evidence that there won't be.

-- 
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] Memory ordering issue in LWLockRelease, WakeupWaiters, WALInsertSlotRelease

[Andres Freund]

On 2014-02-17 14:06:43 -0500, Robert Haas wrote:
> On Mon, Feb 17, 2014 at 1:55 PM, Andres Freund  wrote:
> > On 2014-02-17 13:49:01 -0500, Robert Haas wrote:
> > It's just a write barrier which evaluates to a pure compiler barrier on
> > x86 anyway?
> > And it's in a loop that's only entered when the kernel is entered anyway
> > to wake up the other backend.
> >
> > What should that affect significantly?
> 
> On x86, presumably nothing.  On other architectures, I don't know what
> the impact is, but I don't accept a hand-wavy assertion that there
> shouldn't be any as evidence that there won't be.

Directly afterwards there's a syscall that needs to do internal locking
(because it's essentially doing IPC). Which combined certainly is much
more expensive then a write barrier.
And any !x86 architecture that has more heavyweight write barriers
really *needs* a barrier there since you only need more heavywheight
write barriers if the architecture doesn't guarantee total store
order. This isn't a performance optimization, it's correctness.

What's the way to resolve this then? I don't have access to any big !x86
machines.

Greetings,

Andres Freund

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


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

Re: [HACKERS] 8.2 -> 8.4 Upgrade: No More "ldaps://"?

[Tom Lane]

Jim Seymour  writes:
> Tried to upgrade from 8.2.21 to 8.4.19 this morning and ran into a
> wall: It would appear the 
> hostssl all all  0.0.0.0/0  ldap "ldaps://..."
> syntax is no longer supported?

The 8.4 release notes say that there were incompatible changes in the
format of pg_hba.conf entries for LDAP authentication, and this is one:
you're supposed to use the ldaptls option now.

AFAICS from the relevant commit (7356381ef), there is no change in
functionality between what we did for "ldaps:" and what we do now
for "ldaptls".

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] Auto-tuning work_mem and maintenance_work_mem

[Gavin Flower]

On 18/02/14 03:48, Tom Lane wrote:

Gavin Flower  writes:

On 17/02/14 15:26, Robert Haas wrote:

I don't really know about cpu_tuple_cost.  Kevin's often advocated
raising it, but I haven't heard anyone else advocate for that.  I
think we need data points from more people to know whether or not
that's a good idea in general.

Processors have been getting faster, relative to spinning rust, over the
years.  So it puzzles me why anybody would want to raise the
cpu_tuple_cost!

The case where this is sensible is where your database mostly fits in
RAM, so that the cost of touching the underlying spinning rust isn't
so relevant.  The default cost settings are certainly not very good
for such scenarios.

regards, tom lane

Thanks.

That is obvious... once you pointed it out!


Cheers,
Gavin


--
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] 8.2 -> 8.4 Upgrade: No More "ldaps://"?

[Jim Seymour]

On Mon, 17 Feb 2014 14:18:40 -0500
Tom Lane  wrote:

> Jim Seymour  writes:
> > Tried to upgrade from 8.2.21 to 8.4.19 this morning and ran into a
> > wall: It would appear the 
> > hostssl all all  0.0.0.0/0  ldap "ldaps://..."
> > syntax is no longer supported?
> 
> The 8.4 release notes say that there were incompatible changes in the
> format of pg_hba.conf entries for LDAP authentication, and this is
> one: you're supposed to use the ldaptls option now.

Yes, I saw that, but when I tried

ldap ldapserver=... ldapport=636 ldaptls=1

it failed.

> 
> AFAICS from the relevant commit (7356381ef), there is no change in
> functionality between what we did for "ldaps:" and what we do now
> for "ldaptls".

That very well could be.  I always *assumed* that "ldaps://" meant it
was doing SSL on port 636.  After all: That's what SMTPS means, for
example.  But I got to thinking, and looking at my OpenLDAP config and
thought "Hmmm... I wonder...?" and removed "ldapport=636" from my
pg_hba.conf and, lo and behold, it worked!

Thanks for the follow-up, Tom.

Regards,
Jim
-- 
Note: My mail server employs *very* aggressive anti-spam
filtering.  If you reply to this email and your email is
rejected, please accept my apologies and let me know via my
web form at .


-- 
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-tuning work_mem and maintenance_work_mem

[Peter Geoghegan]

On Mon, Feb 17, 2014 at 8:31 AM, Stephen Frost  wrote:
>> Actually, I object to increasing work_mem by default. In my experience
>> most of the untuned servers are backing some kind of web application and
>> often run with far too many connections. Increasing work_mem for those
>> is dangerous.
>
> And I still disagree with this- even in those cases.  Those same untuned
> servers are running dirt-simple queries 90% of the time and they won't
> use any more memory from this, while the 10% of the queries which are
> more complicated will greatly improve.

+1


-- 
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] GiST support for inet datatypes

[Tom Lane]

Emre Hasegeli  writes:
> 2014-02-17 14:54, Andres Freund :
>> You need to add a file for going from 1.0 to 1.1.

> Thank you for the notice. I added them to the patches which touch only two
> of the operator classes. It drops and re-creates operator classes as there
> is not ALTER OPERATOR CLASS DROP DEFAULT command.

Dropping an operator class is quite unacceptable, as it will cause indexes
based on that class to go away (or more likely, cause the upgrade to fail,
if you didn't use CASCADE).  What we've done in the past for changes that
are nominally unsupported is to make the upgrade scripts tweak the system
catalogs directly.

More generally, it doesn't look to me like these upgrade scripts are
complete; shouldn't they be creating some new objects, not just replacing
old ones?

We need to have a discussion as to whether it's actually sane for an
upgrade to remove the DEFAULT marking on a pre-existing opclass.  It
strikes me that this would for instance break pg_dump dumps, in the sense
that the reloaded index would probably now have a different opclass
than before (since pg_dump would see no need to have put an explicit
opclass name into CREATE INDEX if it was the default in the old database).
Even if the new improved opclass is in all ways better, that would be
catastrophic for pg_upgrade I suspect.  Unless the new opclass is
on-disk-compatible with the old; in which case we shouldn't be creating
a new opclass at all, but just modifying the definition of the old one.

In short we probably need to think a bit harder about what this patch is
proposing to do.  It seems fairly likely to me that some other approach
would be a better idea.

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] Memory ordering issue in LWLockRelease, WakeupWaiters, WALInsertSlotRelease

[Heikki Linnakangas]

On 02/10/2014 08:33 PM, Heikki Linnakangas wrote:

On 02/10/2014 08:03 PM, Tom Lane wrote:

Heikki Linnakangas  writes:

On 02/10/2014 06:41 PM, Andres Freund wrote:

Well, it's not actually using any lwlock.c code, it's a special case
locking logic, just reusing the datastructures. That said, I am not
particularly happy about the amount of code it's duplicating from
lwlock.c. Pretty much all of WALInsertSlotReleaseOne and most of
WALInsertSlotAcquireOne() is a copied.



I'm not too happy with the amount of copy-paste myself, but there was
enough difference to regular lwlocks that I didn't want to bother all
lwlocks with the xlog-specific stuff either. The WAL insert slots do
share the LWLock-related PGPROC fields though, and semaphore. I'm all
ears if you have ideas on that..


I agree that if the behavior is considerably different, we don't really
want to try to make LWLockAcquire/Release cater to both this and their
standard behavior.  But why not add some additional functions in lwlock.c
that do what xlog wants?  If we're going to have mostly-copy-pasted logic,
it'd at least be better if it was in the same file, and not somewhere
that's not even in the same major subtree.


Ok, I'll try to refactor it that way, so that we can see if it looks better.


This is what I came up with. I like it, I didn't have to contort lwlocks 
as much as I feared. I added one field to LWLock structure, which is 
used to store the position of how far a WAL inserter has progressed. The 
LWLock code calls it just "value", without caring what's stored in it, 
and it's used by new functions LWLockWait and LWLockWakeup to implement 
the behavior the WAL insertion slots have, to wake up other processes 
waiting for the slot without releasing it.


This passes regression tests, but I'll have to re-run the performance 
tests with this. One worry is that if the padded size of the LWLock 
struct is smaller than cache line, neighboring WAL insertion locks will 
compete for the cache line. Another worry is that since I added a field 
to LWLock struct, it might now take 64 bytes on platforms where it used 
to be 32 bytes before. That wastes some memory.


- Heikki
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index 508970a..b148f70 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -86,7 +86,7 @@ int			sync_method = DEFAULT_SYNC_METHOD;
 int			wal_level = WAL_LEVEL_MINIMAL;
 int			CommitDelay = 0;	/* precommit delay in microseconds */
 int			CommitSiblings = 5; /* # concurrent xacts needed to sleep */
-int			num_xloginsert_slots = 8;
+int			num_xloginsert_locks = 8;
 
 #ifdef WAL_DEBUG
 bool		XLOG_DEBUG = false;
@@ -289,7 +289,7 @@ XLogRecPtr	XactLastRecEnd = InvalidXLogRecPtr;
  * (which is almost but not quite the same as a pointer to the most recent
  * CHECKPOINT record).	We update this from the shared-memory copy,
  * XLogCtl->Insert.RedoRecPtr, whenever we can safely do so (ie, when we
- * hold an insertion slot).  See XLogInsert for details.  We are also allowed
+ * hold an insertion lock).  See XLogInsert for details.  We are also allowed
  * to update from XLogCtl->RedoRecPtr if we hold the info_lck;
  * see GetRedoRecPtr.  A freshly spawned backend obtains the value during
  * InitXLOGAccess.
@@ -363,63 +363,6 @@ typedef struct XLogwrtResult
 
 
 /*
- * A slot for inserting to the WAL. This is similar to an LWLock, the main
- * difference is that there is an extra xlogInsertingAt field that is protected
- * by the same mutex. Unlike an LWLock, a slot can only be acquired in
- * exclusive mode.
- *
- * The xlogInsertingAt field is used to advertise to other processes how far
- * the slot owner has progressed in inserting the record. When a backend
- * acquires a slot, it initializes xlogInsertingAt to 1, because it doesn't
- * yet know where it's going to insert the record. That's conservative
- * but correct; the new insertion is certainly going to go to a byte position
- * greater than 1. If another backend needs to flush the WAL, it will have to
- * wait for the new insertion. xlogInsertingAt is updated after finishing the
- * insert or when crossing a page boundary, which will wake up anyone waiting
- * for it, whether the wait was necessary in the first place or not.
- *
- * A process can wait on a slot in two modes: LW_EXCLUSIVE or
- * LW_WAIT_UNTIL_FREE. LW_EXCLUSIVE works like in an lwlock; when the slot is
- * released, the first LW_EXCLUSIVE waiter in the queue is woken up. Processes
- * waiting in LW_WAIT_UNTIL_FREE mode are woken up whenever the slot is
- * released, or xlogInsertingAt is updated. In other words, a process in
- * LW_WAIT_UNTIL_FREE mode is woken up whenever the inserter makes any progress
- * copying the record in place. LW_WAIT_UNTIL_FREE waiters are always added to
- * the front of the queue, while LW_EXCLUSIVE waiters are appended to the end.
- *
- * To join the wait queue, a process must set MyProc->lwWaitMode to the 

Re: [HACKERS] Memory ordering issue in LWLockRelease, WakeupWaiters, WALInsertSlotRelease

[Andres Freund]

On 2014-02-17 22:30:54 +0200, Heikki Linnakangas wrote:
> This is what I came up with. I like it, I didn't have to contort lwlocks as
> much as I feared. I added one field to LWLock structure, which is used to
> store the position of how far a WAL inserter has progressed. The LWLock code
> calls it just "value", without caring what's stored in it, and it's used by
> new functions LWLockWait and LWLockWakeup to implement the behavior the WAL
> insertion slots have, to wake up other processes waiting for the slot
> without releasing it.
> 
> This passes regression tests, but I'll have to re-run the performance tests
> with this. One worry is that if the padded size of the LWLock struct is
> smaller than cache line, neighboring WAL insertion locks will compete for
> the cache line. Another worry is that since I added a field to LWLock
> struct, it might now take 64 bytes on platforms where it used to be 32 bytes
> before. That wastes some memory.

Why don't you allocate them in a separate tranche, from xlog.c? Then you
can store them inside whatever bigger object you want, guaranteeing
exactly the alignment you need. possibly you even can have the extra
value in the enclosing object?

I'd very much like to keep the core lwlocks size from increasing much, I
plan to work on inlineing them in the BufferDescriptors and keeping it
smaller does increase cache hit ratio..

Greetings,

Andres Freund

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


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

Re: [HACKERS] GiST support for inet datatypes

[Emre Hasegeli]

2014-02-17 22:16, Tom Lane :

> More generally, it doesn't look to me like these upgrade scripts are
> complete; shouldn't they be creating some new objects, not just replacing
> old ones?

The actual patches are on the previous mail [1]. I was just trying
to solve the problem that btree_gist cannot be loaded because of
the new operator class.

> In short we probably need to think a bit harder about what this patch is
> proposing to do.  It seems fairly likely to me that some other approach
> would be a better idea.

How about only removing the inet and the cidr operator classes
from btree_gist. btree-gist-drop-inet-v2.patch does that.

[1] 
http://www.postgresql.org/message-id/cae2gyzxc0fxewe59sfduznq24c+frbdmgxwwvbyvmeanate...@mail.gmail.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] GiST support for inet datatypes

[Tom Lane]

Emre Hasegeli  writes:
> How about only removing the inet and the cidr operator classes
> from btree_gist. btree-gist-drop-inet-v2.patch does that.

I'm not sure which part of "no" you didn't understand, but to be
clear: you don't get to break existing installations.

Assuming that this opclass is sufficiently better than the existing one,
it would sure be nice if it could become the default; but I've not seen
any proposal in this thread that would allow that without serious upgrade
problems.  I think the realistic alternatives so far are (1) new opclass
is not the default, or (2) this patch gets rejected.

We should probably expend some thought on a general approach to
replacing the default opclass for a datatype, because I'm sure this
will come up again.  Right now I don't see a feasible way.

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] patch: option --if-exists for pg_dump

[Pavel Stehule]

Hello


2014-02-17 18:10 GMT+01:00 Alvaro Herrera :

> Jeevan Chalke escribió:
>
> I don't understand this code.  (Well, it's pg_dump.)  Or maybe I do
> understand it, and it's not doing what you think it's doing.  I mean, in
> this part:
>
> > diff --git a/src/bin/pg_dump/pg_backup_archiver.c
> b/src/bin/pg_dump/pg_backup_archiver.c
> > index 7fc0288..c08a0d3 100644
> > --- a/src/bin/pg_dump/pg_backup_archiver.c
> > +++ b/src/bin/pg_dump/pg_backup_archiver.c
> > @@ -413,8 +413,84 @@ RestoreArchive(Archive *AHX)
> >   /* Select owner and schema as necessary */
> >   _becomeOwner(AH, te);
> >   _selectOutputSchema(AH, te->namespace);
> > - /* Drop it */
> > - ahprintf(AH, "%s", te->dropStmt);
> > +
> > + if (*te->dropStmt != '\0')
> > + {
> > + /* Inject IF EXISTS clause to DROP
> part when required. */
> > + if (ropt->if_exists)
>
> It does *not* modify te->dropStmt, it only sends ahprint() a different
> version of what was stored (injected the wanted IF EXISTS clause).  If
> that is correct, then why are we, in this other part, trying to remove
> the IF EXISTS clause?
>

we should not to modify te->dropStmt, because only in this fragment a DROP
STATEMENT is produced. This additional logic ensures correct syntax for all
variation of DROP.

When I wrote this patch I had a initial problem with understanding relation
between pg_dump and pg_restore. And I pushed IF EXISTS to all related DROP
statements producers. But I was wrong. All the drop statements are reparsed
and transformed and serialized in this fragment. So only this fragment
should be modified. IF EXISTS clause can be injected before, when you read
plain text dump (produced by pg_dump --if-exists) in pg_restore.


>
> > @@ -2942,9 +3018,39 @@ _getObjectDescription(PQExpBuffer buf, TocEntry
> *te, ArchiveHandle *AH)
> >   strcmp(type, "OPERATOR CLASS") == 0 ||
> >   strcmp(type, "OPERATOR FAMILY") == 0)
> >   {
> > - /* Chop "DROP " off the front and make a modifiable copy */
> > - char   *first = pg_strdup(te->dropStmt + 5);
> > - char   *last;
> > + char*first;
> > + char*last;
> > +
> > + /*
> > +  * Object description is based on dropStmt statement which
> may have
> > +  * IF EXISTS clause.  Thus we need to update an offset
> such that it
> > +  * won't be included in the object description.
> > +  */
>
> Maybe I am mistaken and the te->dropStmt already contains the IF EXISTS
> bit for some reason; but if so I don't know why that is.  Care to
> explain?
>

pg_restore is available to read plain dump produced by pg_dump --if-exists.
It is way how IF EXISTS can infect te->dropStmt


>
> I also think that _getObjectDescription() becomes overworked after this
> patch.  I wonder if we should be storing te->objIdentity so that we can
> construct the ALTER OWNER command without going to as much trouble as
> parsing the DROP command.  Is there a way to do that? Maybe we can ask
> the server for the object identity, for example.  There is a new
> function to do that in 9.3 which perhaps we can now use.
>
>
do you think a pg_describe_object function?

Probably it is possible, but its significantly much more invasive change,
you should to get objidentity, that is not trivial

Regards

Pavel


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

Re: [HACKERS] patch: option --if-exists for pg_dump

[Alvaro Herrera]

Pavel Stehule escribió:

> 2014-02-17 18:10 GMT+01:00 Alvaro Herrera :

> > Maybe I am mistaken and the te->dropStmt already contains the IF EXISTS
> > bit for some reason; but if so I don't know why that is.  Care to
> > explain?
> 
> pg_restore is available to read plain dump produced by pg_dump --if-exists.
> It is way how IF EXISTS can infect te->dropStmt

Makes sense, I guess.

> > I also think that _getObjectDescription() becomes overworked after this
> > patch.  I wonder if we should be storing te->objIdentity so that we can
> > construct the ALTER OWNER command without going to as much trouble as
> > parsing the DROP command.  Is there a way to do that? Maybe we can ask
> > the server for the object identity, for example.  There is a new
> > function to do that in 9.3 which perhaps we can now use.
>
> do you think a pg_describe_object function?
> 
> Probably it is possible, but its significantly much more invasive change,
> you should to get objidentity, that is not trivial

I was thinking in pg_identify_object().  It can be given the values used
to construct the CatalogId of each tocEntry.

But yes, it is more invasive.

I'd guess that would be a project related to cleaning up the ALTER
OWNER.  What we have now looks like an kludge.

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


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

Re: [HACKERS] Ctrl+C from sh can shut down daemonized PostgreSQL cluster

[Bjorn Munch]

On 17/02 12.25, Tom Lane wrote:
> Bruce Momjian  writes:
> > On Mon, Feb 17, 2014 at 10:38:29AM +0100, Bjorn Munch wrote:
> >> What Solaris version, and what version of sh?  sh on Solaris isn't
> >> necessarily the "real" bourne shell. In Solaris 11 it's actually
> >> ksh93.
> 
> > This was Solaris 9.
> 
> Isn't that out of support by Oracle?

Not completely, final EOL is October 31 this year.

- Bjorn


-- 
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] Ctrl+C from sh can shut down daemonized PostgreSQL cluster

[Bjorn Munch]

On 17/02 14.54, Alvaro Herrera wrote:
> Bruce Momjian wrote:
> 
> > FYI, this email post has a header line that causes all replies to go
> > _only_ to the group email address:
> > 
> > Mail-Followup-To: pgsql-hackers@postgresql.org
> > 
> > I assume it is something related to the Oracle mail server or something
> > configured by the email author.
> 
> Most likely, Bjorn has followup_to set to true:
>   http://www.mutt.org/doc/manual/manual-6.html#followup_to
> 
> I very much doubt that the mail server is injecting such a header.

That would be it yes. :-) I hit 'L' to reply to the mailing list only
and that would by default also set this, I suppose. Nobody's
complained before. :-)

- Bjorn (also a mutt user)



-- 
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] CREATE FOREIGN TABLE ( ... LIKE ... )

[Andres Freund]

On 2014-02-17 23:07:45 +0900, Michael Paquier wrote:
> On Mon, Feb 17, 2014 at 6:28 PM, Andres Freund  wrote:
> > I don't think this really has gone above Needs Review yet.
> I am not sure that this remark makes the review of this patch much
> progressing :(
> 
> By the way, I spent some time looking at it and here are some
> comments:

David just pinged me and tricked me into having a quick look :)

Unless I miss something this possibly allows column definition to slip
by that shouldn't because normally all fdw column definitions are passed
through transformColumnDefinition() which does some checks, but the
copied ones aren't.
I haven't looked long enough to see whether that's currently
problematic, but even if not, it's sure a trap waiting to spring.

Greetings,

Andres Freund

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


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

Re: [HACKERS] CREATE FOREIGN TABLE ( ... LIKE ... )

[Michael Paquier]

On Tue, Feb 18, 2014 at 7:22 AM, Andres Freund  wrote:
> On 2014-02-17 23:07:45 +0900, Michael Paquier wrote:
>> On Mon, Feb 17, 2014 at 6:28 PM, Andres Freund  
>> wrote:
>> > I don't think this really has gone above Needs Review yet.
>> I am not sure that this remark makes the review of this patch much
>> progressing :(
>>
>> By the way, I spent some time looking at it and here are some
>> comments:
>
> David just pinged me and tricked me into having a quick look :)
>
> Unless I miss something this possibly allows column definition to slip
> by that shouldn't because normally all fdw column definitions are passed
> through transformColumnDefinition() which does some checks, but the
> copied ones aren't.
> I haven't looked long enough to see whether that's currently
> problematic, but even if not, it's sure a trap waiting to spring.
transformColumnDefinition contains checks about serial and constraints
mainly. The only thing that could be problematic IMO is the process
done exclusively for foreign tables which is the creation of some
ALTER FOREIGN TABLE ALTER COLUMN commands when per-column options are
detected, something that is not passed to a like'd table with this
patch. This may meritate a comment in the code.
Actually after more thinking I think that it would make sense to have
another INCLUDING/EXCLUDING option for foreign tables: OPTIONS to pass
the column options when link is done from another foreign table. This
should be another patch though.
Regards,
-- 
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] Memory ordering issue in LWLockRelease, WakeupWaiters, WALInsertSlotRelease

[Peter Geoghegan]

On Wed, Feb 12, 2014 at 3:55 AM, MauMau  wrote:
> FYI, the following stack traces are the ones obtained during two instances
> of hang.

You mentioned a hang during a B-Tree insert operation - do you happen
to have a backtrace that relates to that?


-- 
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] Auto-tuning work_mem and maintenance_work_mem

[Jeff Janes]

On Sun, Feb 16, 2014 at 6:26 PM, Robert Haas  wrote:



> The current bgwriter_lru_maxpages value limits the background writer
>  to a maximum of 4MB/s.  If one imagines shared_buffers = 8GB, that
> starts to seem rather low, but I don't have a good feeling for what a
> better value would be.
>

I don't quite understand the point of bgwriter_lru_maxpages in the first
place.  What is it supposed to protect us from?

I wonder if that isn't an artefact from when the checkpointer was the same
process as the background writer, to prevent the background writer
functionality from starving the checkpointer functionality.


Cheers,

Jeff

Re: [HACKERS] Ctrl+C from sh can shut down daemonized PostgreSQL cluster

[Tom Lane]

Bruce Momjian  writes:
> It certainly might be --- I have no idea.  What surprised me is that we
> are relying solely on system() to block signals to pg_ctl-spawned
> servers.  The question is whether that is sufficient and whether we
> should be doing more.  I don't think we have to make adjustments just
> for Solaris 9.

We aren't relying on system(); it does no such thing, according to the
POSIX spec.  If it did, pg_ctl would be unable to print any errors to the
terminal, because dissociating from the foreground process group generally
also disables your ability to print on the terminal.

I poked around in the POSIX spec a bit, and if I'm reading it correctly,
the only thing that typically results in the postmaster becoming
dissociated from the terminal is use of "&" to launch it.  In a shell
with job control, that should result in the process being put into a
"background" process group that won't receive terminal signals nor be
permitted to do I/O to it.  This is distinct from dissociating altogether
because you can use "fg" to return the process to foreground; if we did a
setsid() we'd lose that option, if I'm reading the standards correctly.
So I'm loath to see the postmaster doing setsid() for itself.

POSIX also mandates that interactive shells have job control enabled by
default.

However ... the "&" isn't issued in the user's interactive shell.  It's
seen by the shell launched by pg_ctl's system() call.  So it appears to
be standards-conforming if that shell does nothing to move the launched
postmaster into the background.

The POSIX spec describes a shell switch -m that forces subprocesses
to be launched in their own process groups.  So maybe what we ought
to do is teach pg_ctl to do something like

   system("set -m; postgres ...");

Dunno if this is really portable, though it ought to be.

Alternatively, we could do what the comments in pg_ctl have long thought
desirable, namely get rid of use of system() in favor of fork()/exec().
With that, pg_ctl could do a setsid() inside the child process.

Or we could wait to see if anybody reports this sort of behavior in
a shell that won't be out of support before 9.4 gets out the door.

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] Changeset Extraction v7.6.1

[Robert Haas]

On Fri, Feb 14, 2014 at 4:55 AM, Andres Freund  wrote:
> [ patches ]

Having now had a little bit of opportunity to reflect on the State Of
This Patch, I'd like to step back from the minutia upon which I've
been commenting in my previous emails and articulate three high-level
concerns about this patch.  In so doing, I would like to specifically
request that other folks on this mailing list comment on the extent to
which they do or do not believe these concerns to be valid.  I believe
I've mentioned all of these concerns at least to some degree
previously, but they've been mixed in with other things, so I want to
take this opportunity to call them out more clearly.

1. How safe is it to try to do decoding inside of a regular backend?
What we're doing here is entering a special mode where we forbid the
use of regular snapshots in favor of requiring the use of "decoding
snapshots", and forbid access to non-catalog relations.  We then run
through the decoding process; and then exit back into regular mode.
On entering and on exiting this special mode, we
InvalidateSystemCaches().  I don't see a big problem with having
special backends (e.g. walsender) use this special mode, but I'm less
convinced that it's wise to try to set things up so that we can switch
back and forth between decoding mode and regular mode in a single
backend.  I worry that won't end up working out very cleanly, and I
think the prohibition against using this special mode in an
XID-bearing transaction is merely a small downpayment on future pain
in this area.  That having been said, I can't pretend at this point
either to understand the genesis of this particular restriction or
what other problems are likely to crop up in trying to allow this
mode-switching.  So it's possible that I'm overblowing it, but it's
makin' me nervous.

2. I think the snapshot-export code is fundamentally misdesigned.  As
I said before, the idea that we're going to export one single snapshot
at one particular point in time strikes me as extremely short-sighted.
 For example, consider one-to-many replication where clients may join
or depart the replication group at any time.  Whenever somebody joins,
we just want a  pair such that they can apply all
changes after the LSN except for XIDs that would have been visible to
the snapshot.  And in fact, we don't even need any special machinery
for that; the client can just make a connection and *take a snapshot*
once decoding is initialized enough.  This code is going to great
pains to be able to export a snapshot at the precise point when all
transactions that were running in the first xl_running_xacts record
seen after the start of decoding have ended, but there's nothing
magical about that point, except that it's the first point at which a
freshly-taken snapshot is guaranteed to be good enough to establish an
initial state for any table in the database.

But do you really want to keep that snapshot around long enough to
copy the entire database?  I bet you don't: if the database is big,
holding back xmin for long enough to copy the whole thing isn't likely
to be fun.  You might well want to copy one table at a time, with
progressively newer snapshots, and apply to each table only those
transactions that weren't part of the initial snapshot for that table.
 Many other patterns are possible.  What you've got baked in here
right now is suitable only for the simplest imaginable case, and yet
we're paying a substantial price in implementation complexity for it.
Frankly, this code is *ugly*; the fact that SnapBuildExportSnapshot()
needs to start a transaction so that it can push out a snapshot.  I
think that's a pretty awful abuse of the transaction machinery, and
the whole point of it, AFAICS, is to eliminate flexibility that we'd
have with simpler approaches.

3. As this feature is proposed, the only plugin we'll ship with 9.4 is
a test_decoding plugin which, as its own documentation says, "doesn't
do anything especially useful."  What exactly do we gain by forcing
users who want to make use of these new capabilities to write C code?
You previously stated that it wasn't possible (or there wasn't time)
to write something generic, but how hard is it, really?  Sure, people
who are hard-core should have the option to write C code, and I'm
happy that they do.  But that shouldn't, IMHO anyway, be a requirement
to use that feature, and I'm having trouble understanding why we're
making it one.  The test_decoding plugin doesn't seem tremendously
much simpler than something that someone could actually use, so why
not make that the goal?

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] Changeset Extraction v7.6.1

[Tom Lane]

Robert Haas  writes:
> Having now had a little bit of opportunity to reflect on the State Of
> This Patch, I'd like to step back from the minutia upon which I've
> been commenting in my previous emails and articulate three high-level
> concerns about this patch.  In so doing, I would like to specifically
> request that other folks on this mailing list comment on the extent to
> which they do or do not believe these concerns to be valid.
> ...

> 1. How safe is it to try to do decoding inside of a regular backend?
> What we're doing here is entering a special mode where we forbid the
> use of regular snapshots in favor of requiring the use of "decoding
> snapshots", and forbid access to non-catalog relations.  We then run
> through the decoding process; and then exit back into regular mode.
> On entering and on exiting this special mode, we
> InvalidateSystemCaches().

How often is such a mode switch expected to happen?  I would expect
frequent use of InvalidateSystemCaches() to be pretty much disastrous
for performance, even absent any of the possible bugs you're worried
about.  It would likely be better to design things so that a decoder
backend does only that.

> 2. I think the snapshot-export code is fundamentally misdesigned.

Your concerns here sound reasonable, but I can't say I've got any
special insight into it.

> 3. As this feature is proposed, the only plugin we'll ship with 9.4 is
> a test_decoding plugin which, as its own documentation says, "doesn't
> do anything especially useful."  What exactly do we gain by forcing
> users who want to make use of these new capabilities to write C code?

TBH, if that's all we're going to ship, I'm going to vote against
committing this patch to 9.4 at all.  Let it wait till 9.5 when we
might be able to build something useful on it.  To point out just
one obvious problem, how much confidence can we have in the APIs
being right if there are no usable clients?  Even if they're right,
what benefit do we get from freezing them one release before anything
useful is going to happen?

The most recent precedent I can think of is the FDW APIs, which I'd
be the first to admit are still in flux.  But we didn't ship anything
there without non-toy contrib modules to exercise it.  If we had,
we'd certainly have regretted it, because in the creation of those
contrib modules we found flaws in the initial design.

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] Need to update this comment in xlog.c?

[Amit Langote]

Hi,

Should "background writer" in the following comment be "checkpointer" post-9.2?

src/backend/access/transam/xlog.c

/*
 * Statistics for current checkpoint are collected in this global struct.
 * Because only the background writer or a stand-alone backend can perform
 * checkpoints, this will be unused in normal backends.
 */
CheckpointStatsData CheckpointStats;


--
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] Changeset Extraction v7.6.1

[Robert Haas]

On Mon, Feb 17, 2014 at 9:10 PM, Tom Lane  wrote:
>> 3. As this feature is proposed, the only plugin we'll ship with 9.4 is
>> a test_decoding plugin which, as its own documentation says, "doesn't
>> do anything especially useful."  What exactly do we gain by forcing
>> users who want to make use of these new capabilities to write C code?
>
> TBH, if that's all we're going to ship, I'm going to vote against
> committing this patch to 9.4 at all.  Let it wait till 9.5 when we
> might be able to build something useful on it.  To point out just
> one obvious problem, how much confidence can we have in the APIs
> being right if there are no usable clients?  Even if they're right,
> what benefit do we get from freezing them one release before anything
> useful is going to happen?

I actually have a lot of confidence that the APIs are almost entirely
right, except maybe for the snapshot-related stuff and possibly one or
two other minor details.  And I have every confidence that 2ndQuadrant
is going to put out decoding modules that do useful stuff.  I also
assume Slony is going to ship one at some point.  EnterpriseDB's xDB
replication server will need one, so someone at EDB will have to go
write that.  And if Bucardo or Londiste want to use this
infrastructure, they'll need their own, too.  What I don't understand
is why it's cool to make each of those replication solutions bring its
own to the table.  I mean if they want to, so that they can generate
exactly the format they want with no extra overhead, sure, cool.  What
I don't understand is why we're not taking the test_decoding module,
polishing it up a little to produce some nice, easily
machine-parseable output, calling it basic_decoding, and shipping
that.  Then people who want something else can build it, but people
who are happy with something basic will already have it.

What I actually suspect is going to happen if we ship this as-is is
that people are going to start building logical replication solutions
on top of the test_decoding module even though it explicitly says that
it's just test code.  This is *really* cool technology and people are
*hungry* for it.  But writing C is hard, so if there's not a polished
plugin available, I bet people are going to try to use the
not-polished one.  I think we try to get out ahead of that.

-- 
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] Changeset Extraction v7.6.1

[Peter Geoghegan]

On Mon, Feb 17, 2014 at 6:35 PM, Robert Haas  wrote:
> What I actually suspect is going to happen if we ship this as-is is
> that people are going to start building logical replication solutions
> on top of the test_decoding module even though it explicitly says that
> it's just test code.  This is *really* cool technology and people are
> *hungry* for it.  But writing C is hard, so if there's not a polished
> plugin available, I bet people are going to try to use the
> not-polished one.  I think we try to get out ahead of that.

Tom made a comparison with FDWs, so I'll make another. The Multicorn
module made FDW authorship much more accessible by wrapping it in a
Python interface, I believe with some success. I don't want to stand
in the way of building a fully-featured test_decoding module, but I
think that those that would misuse test_decoding as it currently
stands can be redirected to a third-party wrapper. As you say, it's
pretty cool stuff, so it seems likely that someone will build one for
us.


-- 
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

[HACKERS] Rowcounts marked by create_foreignscan_path()

[Etsuro Fujita]

Why does create_foreignscan_path() not set the rowcounts based on
ParamPathInfo when the path is a parameterized path?  Please find
attached a patch.

Thanks,

Best regards,
Etsuro Fujita
*** a/src/backend/optimizer/util/pathnode.c
--- b/src/backend/optimizer/util/pathnode.c
***
*** 1722,1733  create_foreignscan_path(PlannerInfo *root, RelOptInfo *rel,
List *fdw_private)
  {
ForeignPath *pathnode = makeNode(ForeignPath);
  
pathnode->path.pathtype = T_ForeignScan;
pathnode->path.parent = rel;
!   pathnode->path.param_info = get_baserel_parampathinfo(root, rel,
!   
  required_outer);
!   pathnode->path.rows = rows;
pathnode->path.startup_cost = startup_cost;
pathnode->path.total_cost = total_cost;
pathnode->path.pathkeys = pathkeys;
--- 1722,1740 
List *fdw_private)
  {
ForeignPath *pathnode = makeNode(ForeignPath);
+   ParamPathInfo *param_info = get_baserel_parampathinfo(root, rel,
+   
  required_outer);
  
pathnode->path.pathtype = T_ForeignScan;
pathnode->path.parent = rel;
!   pathnode->path.param_info = param_info;
! 
!   /* Mark the path with the correct row estimate */
!   if (param_info)
!   pathnode->path.rows = param_info->ppi_rows;
!   else
!   pathnode->path.rows = rows;
! 
pathnode->path.startup_cost = startup_cost;
pathnode->path.total_cost = total_cost;
pathnode->path.pathkeys = pathkeys;

-- 
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] Rowcounts marked by create_foreignscan_path()

[Tom Lane]

Etsuro Fujita  writes:
> Why does create_foreignscan_path() not set the rowcounts based on
> ParamPathInfo when the path is a parameterized path?

The calling FDW is supposed to do that; note the header comment.
I'm not sure that it'd be an improvement to change the API spec
to be "create_foreignscan_path has no intelligence, except that
sometimes it will decide to override your rows estimate anyway;
nonetheless, it takes your cost estimate as gospel".

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] Rowcounts marked by create_foreignscan_path()

[Etsuro Fujita]

(2014/02/18 12:03), Tom Lane wrote:
> Etsuro Fujita  writes:
>> Why does create_foreignscan_path() not set the rowcounts based on
>> ParamPathInfo when the path is a parameterized path?

> The calling FDW is supposed to do that; note the header comment.

Understood.  However, ISTM postgresGetForeignPaths() doesn't work like
that.  It uses the same rowcount for all paths of a same parameterization?

Thanks,

Best regards,
Etsuro Fujita


-- 
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] Rowcounts marked by create_foreignscan_path()

[Tom Lane]

Etsuro Fujita  writes:
> (2014/02/18 12:03), Tom Lane wrote:
>> The calling FDW is supposed to do that; note the header comment.

> Understood.  However, ISTM postgresGetForeignPaths() doesn't work like
> that.  It uses the same rowcount for all paths of a same parameterization?

That's what we want no?

Anyway, the point of using ppi_rows would be to enforce that all the
rowcount estimates for a given parameterized relation are the same.
In the FDW case, all those estimates are the FDW's responsibility,
and so making them consistent is also its responsibility IMO.

Another way of looking at this is that none of the pathnode creation
routines in pathnode.c are responsible for setting rowcount estimates.
That's done by whatever is setting the cost estimate; this must be so,
else the cost estimate is surely bogus.  So any way you slice it, the
FDW has to get it right.

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] Description for pg_replslot in docs

[Amit Kapila]

Description for contents of PGDATA is mentioned at
following page in docs:
http://www.postgresql.org/docs/devel/static/storage-file-layout.html

Isn't it better to have description of pg_replslot in the same
place?

With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.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] Description for pg_replslot in docs

[Michael Paquier]

On Tue, Feb 18, 2014 at 12:43 PM, Amit Kapila  wrote:
> Description for contents of PGDATA is mentioned at
> following page in docs:
> http://www.postgresql.org/docs/devel/static/storage-file-layout.html
>
> Isn't it better to have description of pg_replslot in the same
> place?
Definitely. +1.
-- 
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 fix] "pg_ctl stop" times out when it should respond quickly

[Michael Paquier]

On Tue, Feb 18, 2014 at 1:29 AM, Alvaro Herrera
 wrote:
> MauMau escribió:
> The pg_regress part is ugly.  However, pg_regress is doing something
> unusual when starting postmaster itself, so the ugly coding to stop it
> seems to match.  If we wanted to avoid the ugliness here, the right fix
> would be to use pg_ctl to start postmaster as well as to stop it.  But
> that'd come at a price, because we would need more ugly code to figure
> out postmaster's PID.  All in all, the compromise proposed by this patch
> seems acceptable.  If we really wanted to make all this real pretty, we
> could provide a "libpg_ctl" library to start and stop postmaster, as
> well as query the PID.  Probably not worth the trouble.
This might not be worth the trouble for this bug, but actually it
could be useful to many third-part tools and extensions to have a
common and generic way to do things. I have seen many utilities using
a copy/paste of pg_ctl functions and still maintain some of them...
Regards,
-- 
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] inherit support for foreign tables

[Shigeru Hanada]

Hi Fujita-san,

Thanks for the reviewing.

2014-02-10 21:00 GMT+09:00 Etsuro Fujita :
> (2014/02/07 21:31), Etsuro Fujita wrote:
>> So, I've modified the patch so
>> that we continue to disallow SET STORAGE on a foreign table *in the same
>> manner as before*, but, as your patch does, allow it on an inheritance
>> hierarchy that contains foreign tables, with the semantics that we
>> quietly ignore the foreign tables and apply the operation to the plain
>> tables, by modifying the ALTER TABLE simple recursion mechanism.
>> Attached is the updated version of the patch.

I'm not sure that allowing ALTER TABLE against parent table affects
descendants even some of them are foreign table.  I think the rule
should be simple enough to understand for users, of course it should
be also consistent and have backward compatibility.

If foreign table can be modified through inheritance tree, this kind
of change can be done.

1) create foreign table as a child of a ordinary table
2) run ALTER TABLE parent, the foreign table is also changed
3) remove foreign table from the inheritance tree by ALTER TABLE child
NO INHERIT parent
4) here we can't do same thing as 2), because it is not a child anymore.

So IMO we should determine which ALTER TABLE features are allowed to
foreign tables, and allow them regardless of the recursivity.

Comments?
-- 
Shigeru HANADA


-- 
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: option --if-exists for pg_dump

[Jeevan Chalke]

On Mon, Feb 17, 2014 at 7:43 PM, Alvaro Herrera wrote:

> Jeevan Chalke escribió:
>
> > If yes, then in my latest attached patch, these lines are NOT AT ALL
> there.
> > I have informed on my comment that I have fixed these in my version of
> > patch,
> > but still you got unstable build. NOT sure how. Seems like you are
> applying
> > wrong patch.
> >
> > Will you please let us know what's going wrong ?
>
> The commitfest app is not a chat area.


Hmm. Extremely sorry about that.


> When you add new versions of a
> patch, please mark them as "patch" (not "comment") and make sure to
> provide the message-id of the latest version.
>
>
Ohh, I was needed to mark it as patch and NOT comment (with message id).
And since I had marked it as comment, commitfest app was taking previous
patch
and not the latest one.
My bad. Will keep this in mind.

Thanks


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



-- 
Jeevan B Chalke
Principal Software Engineer, Product Development
EnterpriseDB Corporation
The Enterprise PostgreSQL Company

Re: [HACKERS] Need to update this comment in xlog.c?

[Heikki Linnakangas]

On 02/18/2014 04:30 AM, Amit Langote wrote:

Hi,

Should "background writer" in the following comment be "checkpointer" post-9.2?

src/backend/access/transam/xlog.c

/*
  * Statistics for current checkpoint are collected in this global struct.
  * Because only the background writer or a stand-alone backend can perform
  * checkpoints, this will be unused in normal backends.
  */
CheckpointStatsData CheckpointStats;


Thanks, fixed.

- Heikki


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