Re: [HACKERS] Compiling extensions on Windows

2014-01-12 Thread Craig Ringer
On 01/12/2014 12:00 AM, Tom Lane wrote:
 Craig Ringer cr...@2ndquadrant.com writes:
  We don't set __declspec(dllexport) on extension functions automatically
  when building stand-alone on Windows. So it's necessary to explicitly
  specify PGDLLEXPORT for each function.
 I'm not sure I believe this.  I don't see any PGDLLEXPORT symbols in any
 of the standard contrib modules; how is it that they work?
 

We force all symbols to be exported using the project file generation
tools in the Pg build system, so PGDLLEXPORT is not required there.

That doesn't help stand alone builds. We don't have anything like PGXS
for Windows, so the only real option for building and distributing
reliably compatible extensions is your own Visual Studio project.

Just build in contrib/ in a source tree is not an acceptable answer,
because extensions need binary compatibility with public installer
distributions like the one EDB maintains. Unless you're sure your build
is set up just the same way I'm not at all convinced you're going to get
that, though you may get something that works *most* of the time. This
hasn't historically been a project keen on mostly solutions though.

 So if it's really necessary to change anything here, I'd rather see us
 take the approach of hiding it in PG_FUNCTION_INFO_V1.  What happens
 if we do that and there's also a manually-written prototype?

I've run out of weekend and have to go full-speed on upd. sb. views,
BDR, and event triggers tomorrow, so I don't think I'll have a chance to
test it immediately. I'll check after the CF deadline. That's certainly
one option, anyway, and one that'll solve the immediate issue with
extensions, and would be the most practical short term solution if it works.

Otherwise we can just add DLLEXPORT macros on public functions in
contrib/ and the docs. They're harmless on other platforms - and in fact
can be quite handy.



Those windows droppings can actually be useful: They annotate sources
with a useful piece of information we don't currently use on *nix, but
probably should. They specify a shared library or executable's public
interface.

Imagine that the macro was called PGAPI or PG_PUBLIC_API, not
PGDLLEXPORT. That's what it actually *means*, really.

On *nix we treat the executable and shared lib APIs as the union of the
exported symbols of all compilation units in the object. That's rather
crude, as it means that something very internal to Pg must be public if
it's to be visible across more than one compilation unit, and that if we
wish to hide it we have to squash things together into bigger
compilation units than we might otherwise want.

That's a pain for backward compat, as theoretically all of PostgreSQL's
exported symbols are public API. We don't really offer a good way to
tell what's public and what's not anyway.

Instead, we can and (IMO) should be marking our public API explicitly.
If it's supposed to be usable from a contrib / extension / fdw / etc,
mark it PGDLLEXPORT. Or rename that PG_PUBLIC_API and label it that.
Whatever. On Windows, these expand to __declspec(dllexport) and cause
symbol export. On gcc, we switch to building with -fvisiblity=hidden and
have these macros expand to __attribute__ ((dllexport)) . Access to
non-exported symbols becomes a link error. On ancient GCCs and on other
toolchains we continue to export all symbols by default, as now.

This'd reduce symbol table sizes a bit - though it's really only a big
win for C++ code with monster symbol tables. More importantly, it'd make
it easier to tell what's supposed to be reasonably stable across
versions as public API, and what's totally internal.

(It might be that we have enough exts delving deep in to Pg's guts
already that this is impractical. It'd be fun to see.)

Related references:

  http://gcc.gnu.org/wiki/Visibility
  http://people.redhat.com/drepper/dsohowto.pdf

-- 
 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] Compiling extensions on Windows

2014-01-12 Thread Craig Ringer
On 01/12/2014 04:54 PM, Craig Ringer wrote:
 On 01/12/2014 12:00 AM, Tom Lane wrote:
 So if it's really necessary to change anything here, I'd rather see us
 take the approach of hiding it in PG_FUNCTION_INFO_V1.  What happens
 if we do that and there's also a manually-written prototype?
 
 That's certainly
 one option, anyway, and one that'll solve the immediate issue with
 extensions, and would be the most practical short term solution if it works.

... which it kind-of does.

Turned out to be trivial to test. If the prototype with PGDLLEXPORT
appears *first*, then all is well. If the prototype with PGDLLEXPORT
appears AFTER a user-provided prototype it fails with:

1DemoExtension.c(16): error C2375: 'add_one' : redefinition; different
linkage
1  DemoExtension.c(14) : see declaration of 'add_one'

Two copies of the prototype, both with PGDLLEXPORT, work fine.

So really the question is: Do we care? The current usage of extensions
built outside the Pg tree on Windows is likely to be nearly zero, and is
already fiddly. I'm not too fussed if we make people fix up their
prototypes.

I think we can just emit a prototype for the function from
PG_FUNCTION_INFO_V1 . The only things it's going to break is the odd bit
of weirdly-built, not really supported extensions on Windows.

Are there any platforms that object to prototype redefinition? If not,
we can just emit a prototype on all platforms, not just Windows.

-- 
 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] Syntax of INSERT...ON DUPLICATE KEY LOCK FOR UPDATE

2014-01-12 Thread Craig Ringer
On 01/12/2014 06:42 AM, Peter Geoghegan wrote:
 Someone suggested to me that I solicit opinion on the chosen syntax of
 INSERT...ON DUPLICATE KEY LOCK FOR UPDATE on a separate thread. I'll
 do that here, while also giving a formal user-level definition of the
 feature. I'd like to solicit feedback from a wider set of people than
 those participating in the main thread, while avoiding talking about
 arcane details around locking which have dominated discussions up
 until this point.

After reading the following trimmed text I can see that the idea behind
this solves a real user problem.

It's not beautiful - but we've been rejecting things that solve the
upsert problem for users for ages. This looks technically solid enough
to do the job, useful, and reasonably usable with guidance from the
documentation.

My main question is how (if) this interacts with COPY. Would users need
to continue to COPY to an UNLOGGED or TEMP table, then use this for an
upsert-like operation into the target table? Or is it likely that COPY
can be extended to expose similar functionality - or wrap this into a
copy-on-duplicate-key-update ?


-- 
 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] WIP patch (v2) for updatable security barrier views

2014-01-12 Thread Craig Ringer
On 01/09/2014 06:48 PM, Dean Rasheed wrote:
 On 8 January 2014 10:19, Dean Rasheed dean.a.rash...@gmail.com wrote:
 The assertion failure with inheritance and sublinks is a separate
 issue --- adjust_appendrel_attrs() is not expecting to find any
 unplanned sublinks in the query tree when it is invoked, since they
 would normally have all been planned by that point. However, the
 addition of the new security barrier subqueries after inheritance
 expansion can now insert new sublinks which need to be planned. I'll
 look into how best to make that happen.

 The attached patch does that, which fixes the case you reported.

Dean, any objections to adding this to the current CF, or to my doing so?

I want to adjust the RLS patch to build on top of this patch, splitting
the RLS patch up into a series that can be considered separately. To
have any hope of doing that, I'm going to need to be able to base it on
this patch.

Even if -hackers collectively decides the approach you've posted for
updatable s.b. views isn't the best way at some future point, we can
replace it with a better one then without upsetting users. RLS only
needs quite a high level interface over this, so it should adapt well to
anything that lets it wrap a table into a s.b. qualified subquery.

If there's no better approach forthcoming, then I think this proposal
should be committed. I'll do further testing to see if I can find
anything that breaks it, of course.

I've been bashing my head against this for weeks without great
inspiration - everything I try when doing this in the rewriter creates
three problems for every one it fixes. That's not to say it can't be
done, just that I haven't been able to do it while trying to learn the
basics of the rewriter, planner and executor at the same time ;-)

I've been consistently stuck on how to expand the tlist and inject ctid
(and oid, where required) Vars back *up* the chain of expanded
subqueries after views are expanded in rewrite. We only know the
required tlist and can only access the ctid and oid attrs once we expand
the inner-most view, but we've got no way to walk back up the tree of
subqueries (Query*) to inject Var tlis entries pointing to the
next-inner-most subquery. Need some way to walk back up the nested tree
of queries injecting this info. (I've had another idea about this that I
need to explore tonight, but every previous approach I've tried has
circled back to this same problem).


-- 
 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] Disallow arrays with non-standard lower bounds

2014-01-12 Thread Craig Ringer
On 01/10/2014 07:41 AM, David Fetter wrote:
 On Thu, Jan 09, 2014 at 04:30:25PM -0600, Jim Nasby wrote:
 ISTM that allowing users to pick arbitrary lower array bounds was a
 huge mistake. I've never seen anyone make use of it, can't think of
 any legitimate use cases for it, and hate the stupendous amount of
 extra code needed to deal with it.

 Obviously we can't just drop support, but what about an initdb (or
 hell, even configure) option to disallow arrays with a lower bound
  1? Unfortunately we can't do this with a GUC since you can store
 arrays in a table.
 
 We have dropped support, as you put it, for bigger and harder-hitting
 mistakes than this.

Implicit casts to text, anybody?

Everybody used them. The project dropped them anyway, and did so pretty
much unconditionally at that.

This is a vastly less heavily used feature. I'd be really, really glad
to see it disabled by default at initdb time, with a warning saying the
initdb option to enable it will go away in a few versions.

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


Fwd: [HACKERS] patch: make_timestamp function

2014-01-12 Thread Pavel Stehule
Hello


2014/1/11 Tomas Vondra t...@fuzzy.cz

 Hi,

 I've done a quick review of this patch:

 1) patch applies fine to the current HEAD, with a few hunks offset
by a few lines

 2) the compilation fails because of duplicate OIDs in pg_proc, so
I had to change 3969-3975 to 4033-4039, then it compiles fine


fixed



 3) make installcheck works fine

 4) No regression tests for make_time / make_date.

 5) The documentation is incomplete - make_date / make_time are missing.


two previous points are done by
http://git.postgresql.org/gitweb/?p=postgresql.git;a=commitdiff;h=f901bb50e33ad95593bb68f7b3b55eb2e47607dccommits.
This patch try to complete a ToDo entry.



 6) The documentation should mention that when the 'timezone' parameter
is not set explicitly, the current timezone is used.


fixed



 7) Why do the functions accept only the timezone abbreviation, not the
full name? I find it rather confusing, because the 'timezone' option
uses the full name, and we're using this as the default. But doing
'show timestamp' and using the returned value fails. Is it possible
to fix this somehow?


A only abbreviation is allowed for timetz type. Timestamp can work with
full time zone names. A rules (behave) should be same as input functions
for types: timestamptz and timetz.

postgres=# select '10:10:10 CET'::timetz;
   timetz
─
 10:10:10+01
(1 row)

postgres=# select '10:10:10 Europe/Prague'::timetz;
ERROR:  invalid input syntax for type time with time zone: 10:10:10
Europe/Prague
LINE 1: select '10:10:10 Europe/Prague'::timetz;
   ^

This limit is due used routines limits.

postgres=# select make_timestamptz(2014, 12, 10, 10, 10, 10,
'America/Vancouver');
make_timestamptz

 2014-12-10 19:10:10+01
(1 row)

Time: 0.829 ms
postgres=# select '2014-12-10 10:10:10
America/Vancouver'::timestamptz;  timestamptz

 2014-12-10 19:10:10+01
(1 row)

Time: 0.753 ms

I enhanced a regress tests. I found so work with time zones is not strongly
consistent in different use cases. Operator AT TIME ZONE is more tolerant,
but I use a routines used in input functions and my target was consistent
behave (and results) with input functions.

Regards

Pavel





 regards
 Tomas


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

commit 0ede2fcd1034a8d34fdfd82105e32a36c287ca6f
Author: Pavel Stehule pavel.steh...@gmail.com
Date:   Sun Jan 12 13:05:06 2014 +0100

enhanced doc and tests

diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
index 8579bdd..13d3d89 100644
--- a/doc/src/sgml/func.sgml
+++ b/doc/src/sgml/func.sgml
@@ -6714,6 +6714,32 @@ SELECT SUBSTRING('XY1234Z', 'Y*?([0-9]{1,3})');
row
 entry
  indexterm
+  primarymake_interval/primary
+ /indexterm
+ literal
+  function
+   make_interval(parameteryears/parameter typeint/type DEFAULT 0,
+   parametermonths/parameter typeint/type DEFAULT 0,
+   parameterweeks/parameter typeint/type DEFAULT 0,
+   parameterdays/parameter typeint/type DEFAULT 0,
+   parameterhours/parameter typeint/type DEFAULT 0,
+   parametermins/parameter typeint/type DEFAULT 0,
+   parametersecs/parameter typedouble precision/type DEFAULT 0.0)
+  /function
+ /literal
+/entry
+entrytypeinterval/type/entry
+entry
+ Create interval from years, months, weeks, days, hours, minutes and
+ seconds fields
+/entry
+entryliteralmake_interval(days := 10)/literal/entry
+entryliteral10 days/literal/entry
+   /row
+
+   row
+entry
+ indexterm
   primarymake_time/primary
  /indexterm
  literal
@@ -6735,6 +6761,81 @@ SELECT SUBSTRING('XY1234Z', 'Y*?([0-9]{1,3})');
row
 entry
  indexterm
+  primarymake_timetz/primary
+ /indexterm
+ literal
+  function
+   make_timetz(parameterhour/parameter typeint/type,
+   parametermin/parameter typeint/type,
+   parametersec/parameter typedouble precision/type,
+   optional parametertimezone/parameter typetext/type /optional)
+  /function
+ /literal
+/entry
+entrytypetime with time zone/type/entry
+entry
+ Create time with time zone from hour, minute and seconds fields. When
+ parametertimezone/parameter is not specified, then current time zone
+ is used.
+/entry
+entryliteralmake_timetz(8, 15, 23.5)/literal/entry
+entryliteral08:15:23.5+01/literal/entry
+   /row
+
+   row
+entry
+ indexterm
+  primarymake_timestamp/primary
+ /indexterm
+ literal
+  function
+   

Re: [HACKERS] plpgsql.consistent_into

2014-01-12 Thread Marko Tiikkaja

On 1/12/14, 7:47 AM, Pavel Stehule wrote:

2014/1/12 Marko Tiikkaja ma...@joh.to


Greetings fellow elephants,

I would humbly like to submit for your consideration my proposal for
alleviating pain caused by one of the most annoying footguns in PL/PgSQL:
the behaviour of SELECT .. INTO when the query returns more than one row.
  Some of you might know that no exception is raised in this case (as
opposed to INSERT/UPDATE/DELETE .. INTO, all of them yielding
TOO_MANY_ROWS), which can hide subtle bugs in queries if during testing the
query always returns only one row or the correct one happens to be picked
up every time.  Additionally, the row_count() after execution is always
going to be either 0 or 1, so even if you want to explicitly guard against
potentially broken queries, you can't do so!



It is not bad and, sure, - it is very useful and important

but - it is a redundant to INTO STRICT clause. When you use it, then you
change a INTO behaviour. Is not better to ensure STRICT option than hidden
redefining INTO?


That only works if the query should never return 0 rows either.  If you 
want to allow for missing rows, STRICT is out of the question.



Option INTO (without STRICT clause) is not safe and we should to disallow.
I see a three states (not only two)

a) disallow INTO without STRICT (as preferred for new code)
b) implicit check after every INTO without STRICT
c) without check

these modes should be: strict_required, strict_default, strict_legacy


I can't get excited about this.  Mostly because it doesn't solve the 
problem I'm having.


It is important to be able to execute queries with INTO which might not 
return a row.  That's what FOUND is for.



So I added the following compile-time option:


set plpgsql.consistent_into to true;



This name is not best (there is not clean with it a into should be
consistent)


I agree, but I had to pick something.  One of the three hard problems in 
CS..



Is question, if this functionality should be enabled by GUC to be used for
legacy code (as protection against some kind of hidden bugs)

This topic is interesting idea for me - some checks can be pushed to
plpgsql_check (as errors or warnings) too.

Generally I like proposed functionality, just I am not sure, so hidden
redefining INTO clause (to INTO STRICT) is what we want. We can do it (but
explicitly). I don't know any situation where INTO without STRICT is valid.
Introduction of STRICT option was wrong idea - and now is not way to back.


Note that this is different from implicitly STRICTifying every INTO, 
like I said above.



Regards,
Marko Tiikkaja


--
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] Why does numeric_out produce so many trailing zeros?

2014-01-12 Thread Florian Pflug
On Jan12, 2014, at 08:50 , David Rowley dgrowle...@gmail.com wrote:
 I've been reading the documents on numeric and I can't find any
 information on the reason that a query like this:
 
 test=# select n::numeric / 1 from generate_series(1,2) s(n);
 ?column?
 
  1.
  2.
 (2 rows)
 
 produces results that have so many trailing zeros. Also I'm wondering why
 the first row has 20 trailing zeros and the 2nd row has just 16?

According to the comment in select_div_scale()

  The result scale of a division isn't specified in any SQL standard. For
  PostgreSQL we select a result scale that will give at least
  NUMERIC_MIN_SIG_DIGITS significant digits, so that numeric gives a
  result no less accurate than float8; but use a scale not less than
  either input's display scale.

NUMERIC_MIN_SIG_DIGITS is 16, so that explains why you see at least 16
trailing zeros. You see more for 1/1 because the heuristics in
select_div_scale() can't determine that the result is larger than one
(due to the first digits being equal) and hence add zeros for safety.
Since NUMERIC internally uses base 1, not base 10, once it adds
digits, it adds at least 4, because that corresponds to one digit in
base 1.

That kind of makes sense if you interpret the number of trailing zeros
as an indicator of the precision of the result. There's then a
difference between '1.00' and just '1' - the former implies that the
result is correct up to at least 2 decimal digits, whereas in the latter
case the actual value may very well be 1.4. NUMERIC doesn't seem to use
that definition consistently, though - otherwise, the result of
'1.00' + '1' would have to be '2', not '2.00'. That behaviour seems
to be mandated by the SQL standard, though - the draft 2003 standard
says in part II subclause 6.26 numeric value expression paragraph (1b)

  The precision of the result of addition and subtraction is
  implementation-defined, and the scale is the maximum of S1 and S2.

 Is there any reason that we output any trailing zeros at all?

That also seems to be mandated by the standard. The draft says in
part II subclause 6.12 cast specification paragraph 10, which deals
with casts from numeric to character types

  Let YP be the shortest character string that conforms to the definition
  of exact numeric literal in Subclause 5.3, literal, whose scale
  is the same as the scale of SD and whose interpreted value is the
  absolute value of SV.

subclause 5.3 literal paragrpah (18) says

  The declared type of an exact numeric literal ENL is an
  implementation-defined exact numeric type whose scale is the number of
  digits to the right of the period. There shall be an exact numeric
  type capable of representing the value of ENL exactly.

best regards,
Florian Pflug



-- 
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] [GENERAL] pg_upgrade tablespaces

2014-01-12 Thread Magnus Hagander
On Sun, Jan 12, 2014 at 5:26 AM, Bruce Momjian br...@momjian.us wrote:

 On Sat, Jan 11, 2014 at 12:48:51PM -0800, Adrian Klaver wrote:
  pg_upgrade looks in the pg_tablespace in pre-9.2, and uses a function in
  9.2+.  The query is:
  
   snprintf(query, sizeof(query),
SELECT%s 
FROM  pg_catalog.pg_tablespace 
WHERE spcname != 'pg_default' AND 
  spcname != 'pg_global',
   /* 9.2 removed the spclocation column */
(GET_MAJOR_VERSION(old_cluster.major_version) = 901) ?
  -- spclocation : pg_catalog.pg_tablespace_location(oid) AS
 spclocation);
 
 
  I see, though I have another question. If pg_tablespace and the
  symlinks can get out of sync, as you say below, why is pg_tablespace
  considered the authority? Or to put it another way, why not just
  look at the symlinks as in 9.2+?

 Uh, good question.  I think I used the system tables because they were
 easier to access.  I can't remember if we used the symlinks for some
 things and pg_tablespace for other things in pre-9.2.


If you mean PostgreSQL internally then no, we didn't use pg_tablespace for
anything ever. We only used the symlinks. Which is why it was so easy to
remove.

If you were using it for something inside pg_upgrade I don't know, but the
backend didn't.


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


Re: [HACKERS] Deprecations in authentication

2014-01-12 Thread Magnus Hagander
On Sat, Jan 11, 2014 at 9:45 PM, Peter Eisentraut pete...@gmx.net wrote:

 On Thu, 2013-10-24 at 20:37 +0200, Magnus Hagander wrote:
  On Thu, Oct 24, 2013 at 8:35 PM, Peter Eisentraut pete...@gmx.net
  wrote:
   On 10/18/12, 7:20 AM, Magnus Hagander wrote:
   1. krb5 authentication. We've had gssapi since 8.3 (which means in
  all
   supported versions). krb5 has been deprecated, also since 8.3. Time
  to
   remove it?
  
   OS X Mavericks has now marked just about everything in krb5.h as
   deprecated, leading to compiler warnings.  Which reminded me of this
   thread.  Maybe it's time.
 
  Yeah, it's still sitting on my TODO to get done for 9.4. I guess
  that's another reason...

 Are you still planning to do this?


I am. So I really need to pick up the ball on that :S

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


Re: [HACKERS] Standalone synchronous master

2014-01-12 Thread Florian Pflug
On Jan11, 2014, at 18:53 , Andres Freund and...@2ndquadrant.com wrote:
 On 2014-01-11 18:28:31 +0100, Florian Pflug wrote:
 Hm, I was about to suggest that you can set statement_timeout before
 doing COMMIT to limit the amount of time you want to wait for the
 standby to respond. Interestingly, however, that doesn't seem to work,
 which is weird, since AFAICS statement_timeout simply generates a
 query cancel requester after the timeout has elapsed, and cancelling
 the COMMIT with Ctrl-C in psql *does* work.
 
 I think that'd be a pretty bad API since you won't know whether the
 commit failed or succeeded but replication timed out. There very well
 might have been longrunning constraint triggers or such taking a long
 time.

You could still distinguish these cases because the COMMIT would succeed
with a WARNING if the timeout elapses while waiting for the standby, just
as it does for query cancellations already.

I'm not saying that this is a great API, though - I brought it up only
because I accepting cancellation requests but ignoring timeouts seems
a bit inconsistent to me.

best regards,
Florian Pflug



-- 
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] Syntax of INSERT...ON DUPLICATE KEY LOCK FOR UPDATE

2014-01-12 Thread Andreas Karlsson

On 01/11/2014 11:42 PM, Peter Geoghegan wrote:

I recently suggested that rather than RETURNING REJECTS, we could have
a REJECTING clause, which would see a DML statement project strictly
the complement of what RETURNING projects in the same context. So
perhaps you could also see what RETURNING would not have projected
because a before row trigger returned NULL (i.e. when a before trigger
indicates to not proceed with insertion). That is certainly more
general, and so is perhaps preferable. It's also less verbose, and it
seems less likely to matter that we'll need to make REJECTING a fully
reserved keyword, as compared to REJECTS. (RETURNING is already a
fully reserved keyword not described by the standard, so this makes a
certain amount of sense to me). If nothing else, REJECTING is more
terse than RETURNING REJECTS.


I do not entirely understand what you are proposing here.  Any example 
how this would look compared to your RETURNING REJECTS proposal?


--
Andreas Karlsson


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


[HACKERS] ECPG regression tests generating warnings

2014-01-12 Thread Kevin Grittner
Since 192b4aacad45c16a8a9341644479125977366dab my `make
check-world` runs are generating these two warnings:

desc.pgc:55: WARNING: descriptor outdesc does not exist
desc.pgc:86: WARNING: descriptor outdesc does not exist

The referenced file is in the src/interfaces/ecpg/test/sql/
directory and was modified by the above-mentioned commit.

--
Kevin Grittner
EDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


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


Re: [HACKERS] proposal, patch: allow multiple plpgsql plugins

2014-01-12 Thread Marko Tiikkaja

On 1/9/14, 11:41 PM, Pavel Stehule wrote:

There are two basic questions:

b) will we support same API still - a reference on plugin_info in exec
state is a issue - described in patch.


Pardon my ignorance, but why does the plugin_info have to be in the 
executor state?  If we're going to change the API, can't we pass it 
directly to the callback function?  Am I missing something completely 
obvious? :-)



Regards,
Marko Tiikkaja


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


Re: [HACKERS] proposal, patch: allow multiple plpgsql plugins

2014-01-12 Thread Marko Tiikkaja

On 1/12/14, 5:33 PM, I wrote:

On 1/9/14, 11:41 PM, Pavel Stehule wrote:

There are two basic questions:

b) will we support same API still - a reference on plugin_info in exec
state is a issue - described in patch.


Pardon my ignorance, but why does the plugin_info have to be in the
executor state?  If we're going to change the API, can't we pass it
directly to the callback function?


Oh, I think I'm being stupid -- we'd only have to do what *if* we don't 
want to change the API?  Then my vote is for breaking the API.



Regards,
Marko Tiikkaja


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


Re: [HACKERS] proposal, patch: allow multiple plpgsql plugins

2014-01-12 Thread Pavel Stehule
2014/1/12 Marko Tiikkaja ma...@joh.to

 On 1/12/14, 5:33 PM, I wrote:

 On 1/9/14, 11:41 PM, Pavel Stehule wrote:

 There are two basic questions:

 b) will we support same API still - a reference on plugin_info in exec
 state is a issue - described in patch.


 Pardon my ignorance, but why does the plugin_info have to be in the
 executor state?  If we're going to change the API, can't we pass it
 directly to the callback function?


 Oh, I think I'm being stupid -- we'd only have to do what *if* we don't
 want to change the API?  Then my vote is for breaking the API.


yes. It is my vote too.

It is trouble - but support same API is really ugly - on second hand -
there are only few plpgsql plugins - and every plugin needs recompilation
for new mayor version and fixing will be easy.

Regards

Pavel Stehule




 Regards,
 Marko Tiikkaja



Re: [HACKERS] plpgsql.consistent_into

2014-01-12 Thread Pavel Stehule
2014/1/12 Marko Tiikkaja ma...@joh.to

 On 1/12/14, 7:47 AM, Pavel Stehule wrote:

 2014/1/12 Marko Tiikkaja ma...@joh.to

  Greetings fellow elephants,

 I would humbly like to submit for your consideration my proposal for
 alleviating pain caused by one of the most annoying footguns in PL/PgSQL:
 the behaviour of SELECT .. INTO when the query returns more than one row.
   Some of you might know that no exception is raised in this case (as
 opposed to INSERT/UPDATE/DELETE .. INTO, all of them yielding
 TOO_MANY_ROWS), which can hide subtle bugs in queries if during testing
 the
 query always returns only one row or the correct one happens to be
 picked
 up every time.  Additionally, the row_count() after execution is always
 going to be either 0 or 1, so even if you want to explicitly guard
 against
 potentially broken queries, you can't do so!


 It is not bad and, sure, - it is very useful and important

 but - it is a redundant to INTO STRICT clause. When you use it, then you
 change a INTO behaviour. Is not better to ensure STRICT option than hidden
 redefining INTO?


 That only works if the query should never return 0 rows either.  If you
 want to allow for missing rows, STRICT is out of the question.


hmm - you have true.

try to find better name.

Other questions is using a GUC for legacy code. I am for this checked mode
be default (or can be simply activated for new code)

Regards

Pavel




  Option INTO (without STRICT clause) is not safe and we should to disallow.
 I see a three states (not only two)

 a) disallow INTO without STRICT (as preferred for new code)
 b) implicit check after every INTO without STRICT
 c) without check

 these modes should be: strict_required, strict_default,
 strict_legacy


 I can't get excited about this.  Mostly because it doesn't solve the
 problem I'm having.

 It is important to be able to execute queries with INTO which might not
 return a row.  That's what FOUND is for.


  So I added the following compile-time option:


 set plpgsql.consistent_into to true;


 This name is not best (there is not clean with it a into should be
 consistent)


 I agree, but I had to pick something.  One of the three hard problems in
 CS..


  Is question, if this functionality should be enabled by GUC to be used for
 legacy code (as protection against some kind of hidden bugs)

 This topic is interesting idea for me - some checks can be pushed to
 plpgsql_check (as errors or warnings) too.

 Generally I like proposed functionality, just I am not sure, so hidden
 redefining INTO clause (to INTO STRICT) is what we want. We can do it (but
 explicitly). I don't know any situation where INTO without STRICT is
 valid.
 Introduction of STRICT option was wrong idea - and now is not way to back.


 Note that this is different from implicitly STRICTifying every INTO, like
 I said above.


 Regards,
 Marko Tiikkaja



Re: [HACKERS] [GENERAL] pg_upgrade tablespaces

2014-01-12 Thread Tom Lane
Bruce Momjian br...@momjian.us writes:
 On Sat, Jan 11, 2014 at 12:48:51PM -0800, Adrian Klaver wrote:
 I see, though I have another question. If pg_tablespace and the
 symlinks can get out of sync, as you say below, why is pg_tablespace
 considered the authority? Or to put it another way, why not just
 look at the symlinks as in 9.2+?

 Uh, good question.  I think I used the system tables because they were
 easier to access.  I can't remember if we used the symlinks for some
 things and pg_tablespace for other things in pre-9.2.

Well, pre-9.2 pg_dumpall is going to make use of the pg_tablespace
entries, because it has no other choice.  We could conceivably teach
pg_upgrade to look at the symlinks for itself, but we're not going
to do that in pg_dumpall.  Which means that the intermediate dump
script would contain inconsistent location values anyway if the
catalog entries are wrong.  So I don't see any value in changing the
quoted code in pg_upgrade.

It does however seem reasonable for pg_upgrade to note whether any
of the paths are prefixed by old PGDATA and warn about the risks
involved.

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] Partitioning performance: cache stringToNode() of pg_constraint.ccbin

2014-01-12 Thread Noah Misch
On Sat, Jan 11, 2014 at 02:10:01PM -0500, Bruce Momjian wrote:
 On Mon, Jun  3, 2013 at 03:07:27PM -0400, Noah Misch wrote:
  A colleague, Korry Douglas, observed a table partitioning scenario where
  deserializing pg_constraint.ccbin is a hot spot.  The following test case, a
  simplification of a typical partitioning setup, spends 28% of its time in
  stringToNode() and callees thereof:
 
 Noah, what is the status on this?

Further study revealed a defect in the patch's memory management, and I have
not gotten around to correcting that.

-- 
Noah Misch
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] Compiling extensions on Windows

2014-01-12 Thread Tom Lane
Craig Ringer cr...@2ndquadrant.com writes:
 Turned out to be trivial to test. If the prototype with PGDLLEXPORT
 appears *first*, then all is well. If the prototype with PGDLLEXPORT
 appears AFTER a user-provided prototype it fails with:

That's sort of what I thought would happen.  It's problematic because
putting the extern before the PG_FUNCTION_INFO_V1 is standard practice,
especially if you have the extern in a header file.

 I think we can just emit a prototype for the function from
 PG_FUNCTION_INFO_V1.

Doesn't sound like it; we'll still be forced to modify or remove
manually-written externs in basically all contrib and third-party code,
if we want it to build on Windows.  Which, as I said before, does not
seem to me to be a workable solution path at all.  It would take
years to track down all the third-party code and get it fixed, if
the authors don't themselves build/test on Windows.

And I continue to maintain that it's not acceptable for the Windows port
to require this anyway.  If any other platform came to us and said hey
guys, you need to plaster this non-ANSI-C decoration on every exported
function, what response do you think they'd get?

One last point is that automatically export every external symbol is
exactly what happens for these modules on Unix-ish platforms.  If it
doesn't work the same way on Windows, we're just opening ourselves up to
subtle portability issues.

This needs to be fixed in the Windows build system, not the source code.

regards, tom lane


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


Re: [HACKERS] proposal, patch: allow multiple plpgsql plugins

2014-01-12 Thread Pavel Stehule
Hello

Updated version

I still not happy with plugin_info - it is only per plugin now and should
be per plugin and per function.

Regards

Pavel


2014/1/12 Pavel Stehule pavel.steh...@gmail.com




 2014/1/12 Marko Tiikkaja ma...@joh.to

 On 1/12/14, 5:33 PM, I wrote:

 On 1/9/14, 11:41 PM, Pavel Stehule wrote:

 There are two basic questions:

 b) will we support same API still - a reference on plugin_info in exec
 state is a issue - described in patch.


 Pardon my ignorance, but why does the plugin_info have to be in the
 executor state?  If we're going to change the API, can't we pass it
 directly to the callback function?


 Oh, I think I'm being stupid -- we'd only have to do what *if* we don't
 want to change the API?  Then my vote is for breaking the API.


 yes. It is my vote too.

 It is trouble - but support same API is really ugly - on second hand -
 there are only few plpgsql plugins - and every plugin needs recompilation
 for new mayor version and fixing will be easy.

 Regards

 Pavel Stehule




 Regards,
 Marko Tiikkaja



commit 56c89d4c41e4962d46fd83c6045b5827b51d9dfc
Author: Pavel Stehule pavel.steh...@gmail.com
Date:   Sun Jan 12 20:47:56 2014 +0100

cleaned version, break original plpgsql plugin API

diff --git a/src/pl/plpgsql/src/Makefile b/src/pl/plpgsql/src/Makefile
index 852b0c7..37d17a8 100644
--- a/src/pl/plpgsql/src/Makefile
+++ b/src/pl/plpgsql/src/Makefile
@@ -19,7 +19,7 @@ rpath =
 
 OBJS = pl_gram.o pl_handler.o pl_comp.o pl_exec.o pl_funcs.o pl_scanner.o
 
-DATA = plpgsql.control plpgsql--1.0.sql plpgsql--unpackaged--1.0.sql
+DATA = plpgsql.control plpgsql--1.1.sql plpgsql--1.0--1.1.sql plpgsql--unpackaged--1.0.sql
 
 all: all-lib
 
diff --git a/src/pl/plpgsql/src/pl_exec.c b/src/pl/plpgsql/src/pl_exec.c
index 3749fac..2b6b405 100644
--- a/src/pl/plpgsql/src/pl_exec.c
+++ b/src/pl/plpgsql/src/pl_exec.c
@@ -86,6 +86,22 @@ typedef struct SimpleEcontextStackEntry
 static EState *shared_simple_eval_estate = NULL;
 static SimpleEcontextStackEntry *simple_econtext_stack = NULL;
 
+/*
+ * List of pointers and info of registered plugins.
+ */
+typedef struct PluginPtrEntry
+{
+	PLpgSQL_plugin *plugin_ptr;
+	void *plugin_info;		/* reserved for use by optional plugin */
+	struct PluginPtrEntry *next;
+} PluginPtrEntry;
+
+/*
+ * Allocated in TopMemoryContext
+ */
+static PluginPtrEntry *plugins = NULL;
+static int used_plugin_hook_types = 0;
+
 /
  * Local function forward declarations
  /
@@ -236,6 +252,15 @@ static char *format_expr_params(PLpgSQL_execstate *estate,
 static char *format_preparedparamsdata(PLpgSQL_execstate *estate,
 	   const PreparedParamsData *ppd);
 
+/* Bits for used plugin callback types */
+#define PLUGIN_HOOK_TYPE_FUNC_SETUP	(1  0)
+#define PLUGIN_HOOK_TYPE_FUNC_BEG	(1  2)
+#define PLUGIN_HOOK_TYPE_FUNC_END	(1  3)
+#define PLUGIN_HOOK_TYPE_STMT_BEG	(1  4)
+#define PLUGIN_HOOK_TYPE_STMT_END	(1  5)
+
+#define EXEC_PLUGIN_HOOK_TYPE(type) \
+	((used_plugin_hook_types  (PLUGIN_HOOK_TYPE_ ## type)) == (PLUGIN_HOOK_TYPE_ ## type))
 
 /* --
  * plpgsql_exec_function	Called by the call handler for
@@ -331,10 +356,22 @@ plpgsql_exec_function(PLpgSQL_function *func, FunctionCallInfo fcinfo,
 	exec_set_found(estate, false);
 
 	/*
-	 * Let the instrumentation plugin peek at this function
+	 * Let the instrumentation plugins peek at this function
 	 */
-	if (*plugin_ptr  (*plugin_ptr)-func_beg)
-		((*plugin_ptr)-func_beg) (estate, func);
+	if (EXEC_PLUGIN_HOOK_TYPE(FUNC_BEG))
+	{
+		PluginPtrEntry *pe;
+
+		Assert(plugins != NULL);
+
+		for (pe = plugins; pe != NULL; pe = pe-next)
+		{
+			PLpgSQL_plugin *pl_ptr = pe-plugin_ptr;
+
+			if (pl_ptr-func_beg)
+(pl_ptr-func_beg) (estate, func, pe-plugin_info);
+		}
+	}
 
 	/*
 	 * Now call the toplevel block of statements
@@ -479,10 +516,22 @@ plpgsql_exec_function(PLpgSQL_function *func, FunctionCallInfo fcinfo,
 	estate.err_text = gettext_noop(during function exit);
 
 	/*
-	 * Let the instrumentation plugin peek at this function
+	 * Let the instrumentation plugins peek at this function
 	 */
-	if (*plugin_ptr  (*plugin_ptr)-func_end)
-		((*plugin_ptr)-func_end) (estate, func);
+	if (EXEC_PLUGIN_HOOK_TYPE(FUNC_END))
+	{
+		PluginPtrEntry *pe;
+
+		Assert(plugins != NULL);
+
+		for (pe = plugins; pe != NULL; pe = pe-next)
+		{
+			PLpgSQL_plugin *pl_ptr = pe-plugin_ptr;
+
+			if (pl_ptr-func_end)
+(pl_ptr-func_end) (estate, func, pe-plugin_info);
+		}
+	}
 
 	/* Clean up any leftover temporary memory */
 	plpgsql_destroy_econtext(estate);
@@ -699,10 +748,22 @@ plpgsql_exec_trigger(PLpgSQL_function *func,
 	exec_set_found(estate, false);
 
 	/*
-	 * Let the instrumentation plugin peek at this function
+	 * Let the instrumentation plugins peek at this function
 	 */
-	if (*plugin_ptr  (*plugin_ptr)-func_beg)
-		((*plugin_ptr)-func_beg) (estate, func);
+	if 

Re: [HACKERS] Standalone synchronous master

2014-01-12 Thread Josh Berkus
All,

I'm leading this off with a review of the features offered by the actual
patch submitted.  My general discussion of the issues of Sync Degrade,
which justifies my specific suggestions below, follows that.  Rajeev,
please be aware that other hackers may have different opinions than me
on what needs to change about the patch, so you should collect all
opinions before changing code.

===

 Add a new parameter :

 synchronous_standalone_master = on | off

I think this is a TERRIBLE name for any such parameter.  What does
synchronous standalone even mean?  A better name for the parameter
would be auto_degrade_sync_replication or synchronous_timeout_action
= error | degrade, or something similar.  It would be even better for
this to be a mode of synchronous_commit, except that synchronous_commit
is heavily overloaded already.

Some issues raised by this log script:

LOG:  standby tx0113 is now the synchronous standby with priority 1
LOG:  waiting for standby synchronization
  -- standby wal receiver on the standby is killed (SIGKILL)
LOG:  unexpected EOF on standby connection
LOG:  not waiting for standby synchronization
  -- restart standby so that it connects again
LOG:  standby tx0113 is now the synchronous standby with priority 1
LOG:  waiting for standby synchronization
  -- standby wal receiver is first stopped (SIGSTOP) to make sure

The not waiting for standby synchronization message should be marked
something stronger than LOG.  I'd like ERROR.

Second, you have the master resuming sync rep when the standby
reconnects.  How do you determine when it's safe to do that?  You're
making the assumption that you have a failing sync standby instead of
one which simply can't keep up with the master, or a flakey network
connection (see discussion below).

 a.   Master_to_standalone_cmd: To be executed before master
switches to standalone mode.

 b.  Master_to_sync_cmd: To be executed before master switches from
sync mode to standalone mode.

I'm not at all clear what the difference between these two commands is.
 When would one be excuted, and when would the other be executed?  Also,
renaming ...

Missing features:

a) we should at least send committing clients a WARNING if they have
commited a synchronous transaction and we are in degraded mode.

I know others have dismissed this idea as too talky, but from my
perspective, the agreement with the client for each synchronous commit
is being violated, so each and every synchronous commit should report
failure to sync.  Also, having a warning on every commit would make it
easier to troubleshoot degraded mode for users who have ignored the
other warnings we give them.

b) pg_stat_replication needs to show degraded mode in some way, or we
need pg_sync_rep_degraded(), or (ideally) both.

I'm also wondering if we need a more sophisticated approach to
wal_sender_timeout to go with all this.

===

On 01/11/2014 08:33 PM, Bruce Momjian wrote:
 On Sat, Jan 11, 2014 at 07:18:02PM -0800, Josh Berkus wrote:
 In other words, if we're going to have auto-degrade, the most
 intelligent place for it is in
 RepMgr/HandyRep/OmniPITR/pgPoolII/whatever.  It's also the *easiest*
 place.  Anything we do *inside* Postgres is going to have a really,
 really hard time determining when to degrade.
 
 Well, one goal I was considering is that if a commit is hung waiting for
 slave sync confirmation, and the timeout happens, then the mode is
 changed to degraded and the commit returns success.  I am not sure how
 you would do that in an external tool, meaning there is going to be
 period where commits fail, unless you think there is a way that when the
 external tool changes the mode to degrade that all hung commits
 complete.  That would be nice.

Realistically, though, that's pretty unavoidable.  Any technique which
waits a reasonable interval to determine that the replica isn't going to
respond is liable to go beyond the application's timeout threshold
anyway.  There are undoubtedly exceptions to that, but it will be the
case a lot of the time -- how many applications are willing to wait
*minutes* for a COMMIT?

I also don't see any way to allow the hung transactions to commit
without allowing the walsender to make a decision on degrading.  As I've
outlined elsewhere (and below), the walsender just doesn't have enough
information to make a good decision.

On 01/11/2014 08:52 PM, Amit Kapila wrote: It is better than async mode
in a way such that in async mode it never
 waits for commits to be written to standby, but in this new mode it will
 do so unless it is not possible (all sync standby's goes down).
 Can't we use existing wal_sender_timeout, or even if user expects a
 different timeout because for this new mode, he expects master to wait
 more before it start operating like standalone sync master, we can provide
 a new parameter.

One of the reasons that there's so much disagreement about this feature
is that most of the 

Re: [HACKERS] BUG #8782: Segmentation Fault during initialization

2014-01-12 Thread Noah Misch
On Fri, Jan 10, 2014 at 09:10:47PM -0500, Tom Lane wrote:
 3. Move the MemoryContextInit() call to before set_pglocale_pgservice().

 #3 is not too nice either, because it would mean calling MemoryContextInit
 in main.c which doesn't seem like a great place for it.  On the other
 hand, there is a whole lot of rather random junk getting called from
 main.c; who wants to bet that none of the rest of it can call elog(),
 either now or in the future?
 
 After a few moments' thought, I lean a bit towards #3, but it's a
 weakly held position.  Anyone have other ideas?

I, too, would pick #3.  Not much has reason to run before MemoryContextInit();
the only candidate that comes to mind is pgwin32_install_crashdump_handler().

 One other point here is that I'm pretty sure MemoryContextInit itself
 will try to elog() if it fails.  I don't know if it's worth trying
 to unwind that circularity.  As long as we do it early enough, the
 odds of failure should be about negligible --- certainly I don't
 recall ever seeing a report of a crash there.
 
 Possibly it'd be worth having some check in elog.c that ErrorContext has
 been created, with a very simple print some fixed text to stderr and die
 behavior if not.  That would at least be more useful than a bare crash.

Not sure about what more to do here.

Thanks,
nm

-- 
Noah Misch
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] Standalone synchronous master

2014-01-12 Thread Stephen Frost
* Josh Berkus (j...@agliodbs.com) wrote:
 On 01/11/2014 08:52 PM, Amit Kapila wrote: It is better than async mode
 in a way such that in async mode it never
  waits for commits to be written to standby, but in this new mode it will
  do so unless it is not possible (all sync standby's goes down).
  Can't we use existing wal_sender_timeout, or even if user expects a
  different timeout because for this new mode, he expects master to wait
  more before it start operating like standalone sync master, we can provide
  a new parameter.
 
 One of the reasons that there's so much disagreement about this feature
 is that most of the folks strongly in favor of auto-degrade are thinking
 *only* of the case that the standby is completely down.  There are many
 other reasons for a sync transaction to hang, and the walsender has
 absolutely no way of knowing which is the case.  For example:

Uhh, yea, no, I'm pretty sure those in favor of auto-degrade are very
specifically thinking of cases like Standby is restarting, which is
not a reason for the master to fall over.

 * Transient network issues
 * Standby can't keep up with master
 * Postgres bug
 * Storage/IO issues (think EBS)
 * Standby is restarting
 
 You don't want to handle all of those issues the same way as far as sync
 rep is concerned.  For example, if the standby is restaring, you
 probably want to wait instead of degrading.

*What*?!  Certainly not in any kind of OLTP-type system; a system
restart can easily take minutes.  Clearly, you want to resume once the
standby is back up, which I feel like the people against an auto-degrade
mode are missing, but holding up a commit until the standby finishes
rebooting isn't practical.

 There's also the issue that this patch, and necessarily any
 walsender-level auto-degrade, has IMHO no safe way to resume sync
 replication.  This means that any use who has a network or storage blip
 once a day (again, think AWS) would be constantly in degraded mode, even
 though both the master and the replica are up and running -- and it will
 come as a complete surprise to them when the lose the master and
 discover that they've lost data.

I don't follow this logic at all- why is there no safe way to resume?
You wait til the slave is caught up fully and then go back to sync mode.
If that turns out to be an extended problem then an alarm needs to be
raised, of course.

Thanks,

Stephen


signature.asc
Description: Digital signature


Re: [HACKERS] Standalone synchronous master

2014-01-12 Thread Kevin Grittner
Josh Berkus j...@agliodbs.com wrote:

  Add a new parameter :

 
  synchronous_standalone_master = on | off
 
 I think this is a TERRIBLE name for any such parameter.  What does
 synchronous standalone even mean?  A better name for the parameter
 would be auto_degrade_sync_replication or 
 synchronous_timeout_action
 = error | degrade, or something similar.  It would be even better for
 this to be a mode of synchronous_commit, except that synchronous_commit
 is heavily overloaded already.

+1

 a) we should at least send committing clients a WARNING if they have
 commited a synchronous transaction and we are in degraded mode.
 
 I know others have dismissed this idea as too talky, but from my
 perspective, the agreement with the client for each synchronous commit
 is being violated, so each and every synchronous commit should report
 failure to sync.  Also, having a warning on every commit would make it
 easier to troubleshoot degraded mode for users who have ignored the
 other warnings we give them.

I agree that every synchronous commit on a master which is configured for 
synchronous replication which returns without persisting the work of the 
transaction on both the (local) primary and a synchronous replica should issue 
a WARNING.  That said, the API for some connectors (like JDBC) puts the burden 
on the application or its framework to check for warnings each time and do 
something reasonable if found; I fear that a Venn diagram of those shops which 
would use this new feature and those shops that don't rigorously look for and 
reasonably deal with warnings would have significant overlap.

 b) pg_stat_replication needs to show degraded mode in some way, or we
 need pg_sync_rep_degraded(), or (ideally) both.

+1

Since this new feature, where enabled, would cause synchronous replication to 
provide no guarantees beyond what asynchronous replication does[1], but would 
tend to cause people to have an *expectation* that they have some additional 
protection, I think proper documentation will be a big challenge.


--
Kevin Grittner
EDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

[1]  If I understand correctly, this is what the feature is intended to provide:
- A transaction successfully committed on the primary is guaranteed to be 
visible on the replica?  No, in all modes.
- A transaction successfully committed on the primary is guaranteed *not* to be 
visible on the replica?  No, in all modes.
- A the work of a transaction which has not returned from a commit request may 
be visible on the primary and/or the standby?  Yes in all modes.
- A failure of the primary is guaranteed not to lose successfully committed 
transactions when failing over to the replica?  Yes for sync rep without this 
feature, no for async or when this feature is used.  If things are going well 
up to the moment of primary failure, the feature improves the odds (versus 
async) that successfully committed transactions will not be lost, or may reduce 
the number of successfully committed transactions lost.
- A failure of the replica allows transactions on the primary to continue?  
Read only for sync rep without this feature if the last sync standby has 
failed, read only for some interval and then read write with this feature or if 
there is still another working sync rep target, all transactions without 
interruption with async.


-- 
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] Standalone synchronous master

2014-01-12 Thread Josh Berkus
On 01/12/2014 12:35 PM, Stephen Frost wrote:
 * Josh Berkus (j...@agliodbs.com) wrote:
 You don't want to handle all of those issues the same way as far as sync
 rep is concerned.  For example, if the standby is restaring, you
 probably want to wait instead of degrading.
 
 *What*?!  Certainly not in any kind of OLTP-type system; a system
 restart can easily take minutes.  Clearly, you want to resume once the
 standby is back up, which I feel like the people against an auto-degrade
 mode are missing, but holding up a commit until the standby finishes
 rebooting isn't practical.

Well, then that becomes a reason to want better/more configurability.
In the couple of sync rep sites I admin, I *would* want to wait.

 There's also the issue that this patch, and necessarily any
 walsender-level auto-degrade, has IMHO no safe way to resume sync
 replication.  This means that any use who has a network or storage blip
 once a day (again, think AWS) would be constantly in degraded mode, even
 though both the master and the replica are up and running -- and it will
 come as a complete surprise to them when the lose the master and
 discover that they've lost data.
 
 I don't follow this logic at all- why is there no safe way to resume?
 You wait til the slave is caught up fully and then go back to sync mode.
 If that turns out to be an extended problem then an alarm needs to be
 raised, of course.

So, if you have auto-resume, how do you handle the flaky network case?
 And how would an alarm be raised?

On 01/12/2014 12:51 PM, Kevin Grittner wrote:
 Josh Berkus j...@agliodbs.com wrote:
 I know others have dismissed this idea as too talky, but from my
 perspective, the agreement with the client for each synchronous
 commit is being violated, so each and every synchronous commit
 should report failure to sync.  Also, having a warning on every
 commit would make it easier to troubleshoot degraded mode for users
 who have ignored the other warnings we give them.

 I agree that every synchronous commit on a master which is configured
 for synchronous replication which returns without persisting the work
 of the transaction on both the (local) primary and a synchronous
 replica should issue a WARNING.  That said, the API for some
 connectors (like JDBC) puts the burden on the application or its
 framework to check for warnings each time and do something reasonable
 if found; I fear that a Venn diagram of those shops which would use
 this new feature and those shops that don't rigorously look for and
 reasonably deal with warnings would have significant overlap.

Oh, no question.  However, having such a WARNING would help with
interactive troubleshooting once a problem has been identified, and
that's my main reason for wanting it.

Imagine the case where you have auto-degrade and a flaky network.  The
user would experience problems as performance problems; that is, some
commits take minutes on-again, off-again.  They wouldn't necessarily
even LOOK at the sync rep settings.  So next step is to try walking
through a sample transaction on the command line, and then the
DBA/consultant gets WARNING messages, which gives an idea where the real
problem lies.

-- 
Josh Berkus
PostgreSQL Experts Inc.
http://pgexperts.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] Standalone synchronous master

2014-01-12 Thread Stephen Frost
* Josh Berkus (j...@agliodbs.com) wrote:
 Well, then that becomes a reason to want better/more configurability.

I agree with this- the challenge is figuring out what those options
should be and how we should document them.

 In the couple of sync rep sites I admin, I *would* want to wait.

That's certainly an interesting data point.  One of the specific
use-cases that I'm thinking of is to auto-degrade on a graceful shutdown
of the slave for upgrades and/or maintenance.  Perhaps we don't need
*auto* degrade in that case, but then an actual failure of the slave
will also bring down the master.

  I don't follow this logic at all- why is there no safe way to resume?
  You wait til the slave is caught up fully and then go back to sync mode.
  If that turns out to be an extended problem then an alarm needs to be
  raised, of course.
 
 So, if you have auto-resume, how do you handle the flaky network case?
  And how would an alarm be raised?

Ideally, every time there is a auto-degrade, messages are logs to log
files which are monitored and notices are sent to admins about it
happening, who, upon getting repeated such emails, would realize there's
a problem and work to fix it.

 On 01/12/2014 12:51 PM, Kevin Grittner wrote:
  Josh Berkus j...@agliodbs.com wrote:
  I know others have dismissed this idea as too talky, but from my
  perspective, the agreement with the client for each synchronous
  commit is being violated, so each and every synchronous commit
  should report failure to sync.  Also, having a warning on every
  commit would make it easier to troubleshoot degraded mode for users
  who have ignored the other warnings we give them.
 
  I agree that every synchronous commit on a master which is configured
  for synchronous replication which returns without persisting the work
  of the transaction on both the (local) primary and a synchronous
  replica should issue a WARNING.  That said, the API for some
  connectors (like JDBC) puts the burden on the application or its
  framework to check for warnings each time and do something reasonable
  if found; I fear that a Venn diagram of those shops which would use
  this new feature and those shops that don't rigorously look for and
  reasonably deal with warnings would have significant overlap.
 
 Oh, no question.  However, having such a WARNING would help with
 interactive troubleshooting once a problem has been identified, and
 that's my main reason for wanting it.

I'm in the camp of this being too 'talky'.

 Imagine the case where you have auto-degrade and a flaky network.  The
 user would experience problems as performance problems; that is, some
 commits take minutes on-again, off-again.  They wouldn't necessarily
 even LOOK at the sync rep settings.  So next step is to try walking
 through a sample transaction on the command line, and then the
 DBA/consultant gets WARNING messages, which gives an idea where the real
 problem lies.

Or they look in the logs which hopefully say that their slave keeps
getting disconnected...

Thanks,

Stephen


signature.asc
Description: Digital signature


Re: [HACKERS] plpgsql.consistent_into

2014-01-12 Thread Florian Pflug
On Jan12, 2014, at 06:51 , Marko Tiikkaja ma...@joh.to wrote:
 I would humbly like to submit for your consideration my proposal for 
 alleviating pain caused by one of the most annoying footguns in PL/PgSQL: the 
 behaviour of SELECT .. INTO when the query returns more than one row.  Some 
 of you might know that no exception is raised in this case (as opposed to 
 INSERT/UPDATE/DELETE .. INTO, all of them yielding TOO_MANY_ROWS), which can 
 hide subtle bugs in queries if during testing the query always returns only 
 one row or the correct one happens to be picked up every time.  
 Additionally, the row_count() after execution is always going to be either 0 
 or 1, so even if you want to explicitly guard against potentially broken 
 queries, you can't do so!
 
 So I added the following compile-time option:
 
 set plpgsql.consistent_into to true;

I don't think a GUC is the best way to handle this. Handling
this via a per-function setting similar to #variable_conflict would
IMHO be better.So a function containing

#into_surplus_rows error

would complain whereas

#into_surplus_rows ignore_for_select

would leave the behaviour unchanged.

best regards,
Florian Pflug



-- 
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] plpgsql.consistent_into

2014-01-12 Thread Marko Tiikkaja

On 1/12/14, 10:19 PM, Florian Pflug wrote:

On Jan12, 2014, at 06:51 , Marko Tiikkaja ma...@joh.to wrote:


set plpgsql.consistent_into to true;


I don't think a GUC is the best way to handle this. Handling
this via a per-function setting similar to #variable_conflict would
IMHO be better.


This is exactly what the patch does.  A global default in the form of a 
GUC, and then a per-function option for overriding that default.



Regards,
Marko Tiikkaja


--
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] plpgsql.consistent_into

2014-01-12 Thread Pavel Stehule
2014/1/12 Florian Pflug f...@phlo.org

 On Jan12, 2014, at 06:51 , Marko Tiikkaja ma...@joh.to wrote:
  I would humbly like to submit for your consideration my proposal for
 alleviating pain caused by one of the most annoying footguns in PL/PgSQL:
 the behaviour of SELECT .. INTO when the query returns more than one row.
  Some of you might know that no exception is raised in this case (as
 opposed to INSERT/UPDATE/DELETE .. INTO, all of them yielding
 TOO_MANY_ROWS), which can hide subtle bugs in queries if during testing the
 query always returns only one row or the correct one happens to be picked
 up every time.  Additionally, the row_count() after execution is always
 going to be either 0 or 1, so even if you want to explicitly guard against
 potentially broken queries, you can't do so!
 
  So I added the following compile-time option:
 
  set plpgsql.consistent_into to true;

 I don't think a GUC is the best way to handle this. Handling
 this via a per-function setting similar to #variable_conflict would
 IMHO be better.So a function containing

 #into_surplus_rows error

 would complain whereas

 #into_surplus_rows ignore_for_select

 would leave the behaviour unchanged.


There is  GUC for variable_conflict already too. In this case I would to
enable this functionality everywhere (it is tool how to simply eliminate
some kind of strange bugs) so it needs a GUC.

We have GUC for plpgsql.variable_conflict three years and I don't know
about any problem.

Regards

Pavel



 best regards,
 Florian Pflug



 --
 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] Syntax of INSERT...ON DUPLICATE KEY LOCK FOR UPDATE

2014-01-12 Thread Peter Geoghegan
On Sun, Jan 12, 2014 at 8:12 AM, Andreas Karlsson andr...@proxel.se wrote:
 On 01/11/2014 11:42 PM, Peter Geoghegan wrote:
 I recently suggested that rather than RETURNING REJECTS, we could have
 a REJECTING clause, which would see a DML statement project strictly
 the complement of what RETURNING projects in the same context. So
 perhaps you could also see what RETURNING would not have projected
 because a before row trigger returned NULL (i.e. when a before trigger
 indicates to not proceed with insertion). That is certainly more
 general, and so is perhaps preferable. It's also less verbose, and it
 seems less likely to matter that we'll need to make REJECTING a fully
 reserved keyword, as compared to REJECTS. (RETURNING is already a
 fully reserved keyword not described by the standard, so this makes a
 certain amount of sense to me). If nothing else, REJECTING is more
 terse than RETURNING REJECTS.

 I do not entirely understand what you are proposing here.  Any example how
 this would look compared to your RETURNING REJECTS proposal?

It's very similar - REJECTING is a total generalization of what I
already have. The difference is only that REJECTING is accepted in all
contexts that RETURNING is, and not just with INSERT...ON DUPLICATE
KEY LOCK FOR UPDATE. So you could potentially have REJECTING project
the slot proposed for insertion on an UPDATE where RETURNING would
not. If for example a BEFORE ROW trigger fired, and returned NULL,
perhaps it'd then be possible to project the slot as it was before
being passed to the trigger. Perhaps there is no real demand for that,
but, as I said, from a usability perspective it may be easier to
reason about a feature that projects strictly the complement of what
RETURNING would project in the same context.

-- 
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] plpgsql.consistent_into

2014-01-12 Thread Florian Pflug
On Jan12, 2014, at 22:37 , Pavel Stehule pavel.steh...@gmail.com wrote:
 There is  GUC for variable_conflict already too. In this case I would to
 enable this functionality everywhere (it is tool how to simply eliminate
 some kind of strange bugs) so it needs a GUC. 
 
 We have GUC for plpgsql.variable_conflict three years and I don't know
 about any problem.

I must say I hate behaviour-changing GUCs with quite some passion. IMHO
they tend to cause bugs, not avoid them, in the long run. The pattern
usually is

  1) Code gets written, depends on some particular set of settings
 to work correctly

  2) Code gets reused, with little further testing since it's supposed
 to be battle-proven anyway. Settings get dropped.

  3) Code blows up for those corner-cases where the setting actually
 matter. Debugging is hell, because you effectively have to go
 over the code line-by-line and check if it might be affected by
 some GUC or another.

Only a few days ago I spent more than an hour tracking down a bug
which, as it turned out, was caused by a regex which subtly changed its
meaning depending on whether standard_conforming_strings is on or off.

Some GUCs are unavoidable - standard_conforming_strings, for example
probably still was a good idea, since the alternative would have been
to stick with the historical, non-standard behaviour forever. 

But in this case, my feeling is that the trouble such a GUC may cause
out-weights the potential benefits. I'm all for having a directive like
#consistent_into (though I feel that the name could convey the
meaning better). If we *really* think that this ought to be the default
from 9.4 onward, then we should 

  *) Change it to always complain, except if the function explictly
 specifies #consistent_into on or whatever.

  *) Have pg_dump add that to all plpgsql functions if the server
 version is  9.4 or whatever major release this ends up in

That's all just my opinion of course.

best regards,
Florian Pflug



-- 
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] nested hstore patch

2014-01-12 Thread Erik Rijkers
On Sat, January 11, 2014 22:47, Andrew Dunstan wrote:

 On 01/11/2014 03:03 PM, Erik Rijkers wrote:
 On Sat, January 11, 2014 20:30, Peter Eisentraut wrote:
 The documentation doesn't build.
 corrective patch is here:

 http://www.postgresql.org/message-id/37b9f104d5a838eec9b75f3668517aa5.squir...@webmail.xs4all.nl

 It will be in the next version of the patch posted.


Attached is another handful of doc-fixes...

--- doc/src/sgml/hstore.sgml.orig	2014-01-12 15:37:59.292863864 +0100
+++ doc/src/sgml/hstore.sgml	2014-01-13 00:17:51.454592023 +0100
@@ -55,11 +55,11 @@
The text representation of an typehstore/, used for input and output,
may be formatted as scalar values, hash-like values, array-like values, and
nested array and hash values. Scalar values are simply strings, numeric
-   values, booleans, or literalNULL/. Strings continaining whitespace,
+   values, booleans, or literalNULL/. Strings containing whitespace,
commas, literal=/s or literalgt;/s must be double-quoted. To
include a double quote or a backslash in a key or value, escape it with a
-   backslash. Boolean values may be represnted as literaltrue/, literalt/,
-   literalfalse/, or literalf/. Use quotation marks to represent thes
+   backslash. Boolean values may be represented as literaltrue/, literalt/,
+   literalfalse/, or literalf/. Use quotation marks to represent these
values as strings. The literalNULL/ keyword is case-insensitive.
Double-quote the literalNULL/ to treat it as the ordinary string
quoteNULL/quote. Some examples:
@@ -87,9 +87,9 @@
   /para
 
   para
-   Hashes includes zero or more
+   Hashes include zero or more
replaceablekey/ literal=gt;/ replaceablevalue/ pairs separated
-   by commas, optionally brackted by curly braces. Keys must be strings and
+   by commas, optionally bracketed by curly braces. Keys must be strings and
may not be literalNULL/; values may be any typehstore/ type,
including literalNULL/. Examples:
 
@@ -291,7 +291,7 @@
 
  row
   entrytypehstore/ literal?gt;/ typeinteger//entry
-  entrytypebolean//entry
+  entrytypeboolean//entry
   entryget boolean value for array index (literalNULL/ if not boolean or not present)/entry
   entryliteral'[false,null,44]'::hstore ?gt; 0/literal/entry
   entryliteralf/literal/entry
@@ -316,7 +316,7 @@
  row
   entrytypehstore/ literal#?gt;/ typetext[]//entry
   entrytypeboolean//entry
-  entryget boolean value for key path (literalNULL/ if not booelan or not present)/entry
+  entryget boolean value for key path (literalNULL/ if not boolean or not present)/entry
   entryliteral'foo =gt; {bar =gt; true}'::hstore #?gt; '[foo,bar]'/literal/entry
   entryliteralt/literal/entry
  /row
@@ -905,7 +905,7 @@
 
  row
   entryliteralpretty_print//entry
-  entryAdds add newlines between values and indents nested hashes and arrays./entry
+  entryAdds newlines between values and indents nested hashes and arrays./entry
   entryliteralhstore_print('a=gt;t, t=gt;f, arr=gt;[1,2,3]', pretty_print := true)/literal/entry
   entry
 programlisting
@@ -938,7 +938,7 @@
 
  row
   entryliteraljson//entry
-  entryRetuns the value as a JSON string/entry
+  entryReturns the value as a JSON string/entry
   entryliteralhstore_print('arr=gt;[1,2,3]', json := true)/literal/entry
   entryliteralarr: [1, 2, 3]/literal/entry
  /row
@@ -1007,7 +1007,7 @@
 
   para
But literalpopulate_record()/ supports more complicated records and nested
-   typehstore/a values, as well. It makes an effort to convert
+   typehstore/ values, as well. It makes an effort to convert
from typehstore/ data types to PostgreSQL types, including arrays,
typejson/, and typehstore/ values:
 programlisting
-- 
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] plpgsql.consistent_into

2014-01-12 Thread Gavin Flower

On 13/01/14 11:44, Florian Pflug wrote:

On Jan12, 2014, at 22:37 , Pavel Stehule pavel.steh...@gmail.com wrote:

There is  GUC for variable_conflict already too. In this case I would to
enable this functionality everywhere (it is tool how to simply eliminate
some kind of strange bugs) so it needs a GUC.

We have GUC for plpgsql.variable_conflict three years and I don't know
about any problem.

I must say I hate behaviour-changing GUCs with quite some passion. IMHO
they tend to cause bugs, not avoid them, in the long run. The pattern
usually is

   1) Code gets written, depends on some particular set of settings
  to work correctly

   2) Code gets reused, with little further testing since it's supposed
  to be battle-proven anyway. Settings get dropped.

   3) Code blows up for those corner-cases where the setting actually
  matter. Debugging is hell, because you effectively have to go
  over the code line-by-line and check if it might be affected by
  some GUC or another.

Only a few days ago I spent more than an hour tracking down a bug
which, as it turned out, was caused by a regex which subtly changed its
meaning depending on whether standard_conforming_strings is on or off.

Some GUCs are unavoidable - standard_conforming_strings, for example
probably still was a good idea, since the alternative would have been
to stick with the historical, non-standard behaviour forever.

But in this case, my feeling is that the trouble such a GUC may cause
out-weights the potential benefits. I'm all for having a directive like
#consistent_into (though I feel that the name could convey the
meaning better). If we *really* think that this ought to be the default
from 9.4 onward, then we should

   *) Change it to always complain, except if the function explictly
  specifies #consistent_into on or whatever.

   *) Have pg_dump add that to all plpgsql functions if the server
  version is  9.4 or whatever major release this ends up in

That's all just my opinion of course.

best regards,
Florian Pflug



Possibly there should be a warning put out, whenever someone uses some 
behaviour that requires a GUC set to a non-default value?



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] Compiling extensions on Windows

2014-01-12 Thread Craig Ringer
On 01/13/2014 02:04 AM, Tom Lane wrote:
 Craig Ringer cr...@2ndquadrant.com writes:
 
 I think we can just emit a prototype for the function from
 PG_FUNCTION_INFO_V1.
 
 Doesn't sound like it.

On second thought, agreed. The externs tending to appear in headers
kills that.

In that case - after the rush for the CF close, expect a patch from me
against post-9.4 that adds PGDLLEXPORT to the documentation examples,
and another that adds them for contribs (to help people using them as
examples). Nothing else needed, and it doesn't have to affect the *nix
build process or server/ code in any way.

I'll also add a note in the docs explaining what's wrong if you get an
error about an obviously-present function being missing in your
extension on Windows when it works on other platforms.

 And I continue to maintain that it's not acceptable for the Windows port
 to require this anyway.  If any other platform came to us and said hey
 guys, you need to plaster this non-ANSI-C decoration on every exported
 function, what response do you think they'd get?
 
 One last point is that automatically export every external symbol is
 exactly what happens for these modules on Unix-ish platforms.  If it
 doesn't work the same way on Windows, we're just opening ourselves up to
 subtle portability issues.

The portability issues are easily dealt with by using gcc's symbol
visibility features on *nix, which will produce slightly smaller
binaries with faster linkage too. I'll demo that once I've got the
current work off my plate. Platforms w/o gcc visibility don't need to
care, nothing changes, they just don't get the benefits. Everyone
develops on platforms that have gcc and visibility features so they'll
spot any issues introduced.

As described earlier, doing this helps differentiate internal stuff
from public API if we choose to, as well.

If we don't want to attempt to treat anything as internal,
non-public-use, non-stable API, then there's no point using visibility
- everything not static is public API and should be exported to the
world. That's how things are now. In that case I'd stick with just
improving the docs to cover PGDLLEXPORT, and maybe the contribs.

I do think we should think about actually hiding private internal API
post-9.4, though.

-- 
 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] pgcrypto: implement gen_random_uuid

2014-01-12 Thread Wim Lewis
One comment, this:

  /* get 128 random bits */
  int err = px_get_random_bytes(buf, 16);

might be better to use px_get_pseudo_random_bytes(). UUIDs don't
need to be unguessable or have perfect entropy; they just need to
be collision-resistant. RFC4122 mentions this I think, and if you
look at the ossp-uuid function that this is replacing, it also uses
its internal PRNG for v4 UUIDs rather than strong high-entropy
randomness.

(The downside of requesting strong randomness when you don't need
it is that it can potentially cause the server to block while the
system gathers entropy.)



-- 
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] Compiling extensions on Windows

2014-01-12 Thread Craig Ringer
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

On 01/07/2014 12:41 AM, Joe Conway wrote:
 Yes, this pretty much exactly describes how I build PL/R for
 Windows. I had to match my build system SDK with the one EDB uses
 to get a compatible binary. It would be nice if we had something
 equivalent to PGXS on Windows, or maybe even a community build
 system where authors could get Windows binaries built.

I'd be interested in hearing if you have any luck doing this with a
standlone MSVC build: .dll build type, no manifest, compile as C code,
appropriate include and lib paths, define WIN32, link to postgres.lib,
add PGDLLEXPORT to all declarations of functions with a
PG_FUNCTION_INFO_V1, build.

- -- 
 Craig Ringer   http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training  Services
-BEGIN PGP SIGNATURE-
Version: GnuPG v1.4.15 (GNU/Linux)
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/

iQEcBAEBAgAGBQJS01XNAAoJELBXNkqjr+S2S4QH/icetcqZDmsHFHjZnm3hY+LP
M0+rU8+E3on5V/JmWR3bBESmlYXRYM53LmgwapwCfQ45rsrMSiKACyxtl/XkWsEh
38NTsagjlZtsyZIiUoe9d0szSNQerS86ZDBwXAvJnBSNLQy1AnDQ5tzsFbdeZph4
veL6MnoKNIacfbkEBoCjM0KyYdjnYnt4nRlmbGKQNg/h4Y9KqgJsFFpk0r8dfz+v
KNPPyOmdHIcMyCgJS9hIdAdzc+CPPjYBZC3oVVQAuIOsqccmTykPr4Nh3MVcSfCy
JJTRZhzgU6TdRZIi4adY8Ys39O+TJM2T5Wr1xJ6+Eapnd6L1AiY/08jbZIrsFVc=
=5/Y6
-END PGP SIGNATURE-


-- 
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] [GENERAL] pg_upgrade tablespaces

2014-01-12 Thread Bruce Momjian
On Sun, Jan 12, 2014 at 12:48:40PM -0500, Tom Lane wrote:
 Bruce Momjian br...@momjian.us writes:
  On Sat, Jan 11, 2014 at 12:48:51PM -0800, Adrian Klaver wrote:
  I see, though I have another question. If pg_tablespace and the
  symlinks can get out of sync, as you say below, why is pg_tablespace
  considered the authority? Or to put it another way, why not just
  look at the symlinks as in 9.2+?
 
  Uh, good question.  I think I used the system tables because they were
  easier to access.  I can't remember if we used the symlinks for some
  things and pg_tablespace for other things in pre-9.2.
 
 Well, pre-9.2 pg_dumpall is going to make use of the pg_tablespace
 entries, because it has no other choice.  We could conceivably teach
 pg_upgrade to look at the symlinks for itself, but we're not going
 to do that in pg_dumpall.  Which means that the intermediate dump
 script would contain inconsistent location values anyway if the
 catalog entries are wrong.  So I don't see any value in changing the
 quoted code in pg_upgrade.

OK, agreed.

 It does however seem reasonable for pg_upgrade to note whether any
 of the paths are prefixed by old PGDATA and warn about the risks
 involved.

Uh, the problem is that once you rename the old PGDATA, the
pg_tablespace contents no longer point to the current PGDATA.  The
symlinks, if they used absolute paths, wouldn't point to the current
PGDATA either.

-- 
  Bruce Momjian  br...@momjian.ushttp://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] Compiling extensions on Windows

2014-01-12 Thread Joe Conway
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

On 01/12/2014 06:56 PM, Craig Ringer wrote:
 On 01/07/2014 12:41 AM, Joe Conway wrote:
 Yes, this pretty much exactly describes how I build PL/R for 
 Windows. I had to match my build system SDK with the one EDB
 uses to get a compatible binary. It would be nice if we had
 something equivalent to PGXS on Windows, or maybe even a
 community build system where authors could get Windows binaries
 built.
 
 I'd be interested in hearing if you have any luck doing this with
 a standlone MSVC build: .dll build type, no manifest, compile as C
 code, appropriate include and lib paths, define WIN32, link to
 postgres.lib, add PGDLLEXPORT to all declarations of functions with
 a PG_FUNCTION_INFO_V1, build.

Yes, that's more-or-less how I do it. I checked with EDB to be sure I
had the same SDK (looks like last time I did it it was SDK 7.1), then I:

1) download postgres source
2) copy plr source into contrib directory
3) modify src/tools/msvc/* to make plr look like any other contrib
4) build

As long as the SDK is matched, the resulting plr.dll works fine with
the EDB one-click installer version of postgres (or at least so far).

Joe

- -- 
Joe Conway
credativ LLC: http://www.credativ.us
Linux, PostgreSQL, and general Open Source
Training, Service, Consulting,  24x7 Support
-BEGIN PGP SIGNATURE-
Version: GnuPG v1.4.14 (GNU/Linux)
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/

iQIcBAEBAgAGBQJS01m8AAoJEDfy90M199hlkPEP/A1iCDJeRchfBM4OQB73k0VN
+4o9aVxo3n4jjoOiKI3zctpP6bHQI9/wyI9OYAKnRr3PutttrEyDn1MO3wBffTVN
k84rXyKKROgZOnGpQrE33QG82J1TRHmxZRyeQirQplekWEsB2RpiGu+jZJDjUsLn
svmgPZOka5tc+KqC4OJ3qXprv5OW4h6xMfZmUljn6jdzsuEubE6p4X/XqqGUJmVP
t6caIJ3yKvIcedQ7QZmxWEJXYGGoKlr6qv2Vo99SfqK9lGX6A7DFwAG3DaLmWqa/
rULapnfnET5jUtfncu3zkz6Kok848BsoaSfFZpHyyzvU2RlfAAPWCv0ZnNrSYTsl
s7blVBzanBoX63gy4MwMOcGdHmjPyYjeJ78m53GE28cvv62JEGubISkWoCbmOPYu
+UmxaCPR1l03n/QAv3Qo0fCEtU09OXzYci8NDEKqbdkWt9K/M4bNlv207J0CywiX
pEwQGVciaR5ylKYFZpKpCpgf6OElBVFaUk8uE41swgxM3McXWoPFuKocnhTWd0u6
9Fyb7W8W2kAJeOkiNWcpsX2DNHKMkZ8siTfMufyqHDxVfaJLie/0oTZejnGJf4Lh
vOBpqxdowN6xVhBR796vtzxjLg50vDj8NzOKvn6evtJMnWJkUiRBsY8JrR6g3fII
lb+3NTRa5x0ptApbsUKJ
=wFy+
-END PGP SIGNATURE-


-- 
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] Compiling extensions on Windows

2014-01-12 Thread Craig Ringer
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

On 01/13/2014 11:13 AM, Joe Conway wrote:

 Yes, that's more-or-less how I do it. I checked with EDB to be sure
 I had the same SDK (looks like last time I did it it was SDK 7.1),
 then I:
 
 1) download postgres source 2) copy plr source into contrib
 directory

What I mean is that you should not need a full Pg build tree to
compile extensions. Just as we use PGXS on *nix, so it is possible to
just use Visual Studio to build the extension on Windows as a VS
project like any other.

 As long as the SDK is matched, the resulting plr.dll works fine
 with the EDB one-click installer version of postgres (or at least
 so far).

The SDK shouldn't even need to match. Windows software is expected to
cope with mismatched, mix-and-match C runtimes. Icky, but occasionally
useful.

So long as you don't pass a FILE* across DLL boundaries, or free()
memory that was malloc()'d in another DLL, mixing runtimes should work
fine. I'd be interested in knowing what problems you encountered when
the runtime didn't match.

Maybe it's specific to how we do build within a Pg source tree? I was
able to use three different SDK builds against EDB's latest 9.3
packages - SDK 7.1 (x86 and x64), toolchain v90 (x86), and toolchain
v100 (x86) - to compile and test a moderately trivial extension.

- -- 
 Craig Ringer   http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training  Services
-BEGIN PGP SIGNATURE-
Version: GnuPG v1.4.15 (GNU/Linux)
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/

iQEcBAEBAgAGBQJS01vxAAoJELBXNkqjr+S2zcoIAI75MetuO+osAlSOMdMDAgKh
7rqKu8VPyF2bjFVAiYdZOS9Yvlg3yZDzEG2JMbUSJOz3KtGIirZkIub8hgI4q4/W
6PBg5UZAiwK30AXILTqGBXio/Z+olbCPOKvcVv05OF4WExYLMek5Hc2SKy3UIudj
HHvC10LkSeVvJvNj+rK7SUjQIpwa4a2+cc0gx87z7kd8ElVwJ3D/c1Eb3DM8mdsg
KqIlmWkGolwnk1fb/JoabkO9XVvRjPgpj/aR9Ak6mUH7QTXGMqafdpPeCV9BhGRK
d6MDDT3ncoEzHZd7GsKeqVWBFz9eZ7hLXiQR6rZ7bidvNVc4V7NnM2dr50uhoec=
=pgpf
-END PGP SIGNATURE-


-- 
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] Relocation of tablespaces in pg_basebackup

2014-01-12 Thread Andreas Karlsson

On 01/09/2014 10:10 PM, Steeve Lennmark wrote:

That's a much better solution, I attached a patch with the updated code.

# SELECT oid, pg_tablespace_location(oid) FROM pg_tablespace;
[...]
  16388 | /tmp/tblspc1
  16389 | /tmp/tblspc2

$ pg_basebackup -Xs -D backup/data -T /tmp/tblspc1:$(pwd)/backup/t1 -T
/tmp/tblspc2:$(pwd)/backup/t2

This produces the following now:
$ ls backup/; ls -l backup/data/pg_tblspc/
data  t1  t2
lrwxrwxrwx 1 steevel users 23 Jan  9 20:41 16388 - /home/steevel/backup/t1
lrwxrwxrwx 1 steevel users 23 Jan  9 20:41 16389 - /home/steevel/backup/t2


Looked at the patch quickly and noticed that it does not support paths 
containing colons. Is that an acceptable limitation? The $PATH variable 
in most UNIX shells does not support paths with colons either so such 
naming of directories is already discouraged.


Feel free to add the patch to the upcoming commitfest when you feel it 
is ready for a review.


--
Andreas Karlsson


--
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] Syntax of INSERT...ON DUPLICATE KEY LOCK FOR UPDATE

2014-01-12 Thread Andreas Karlsson

On 01/12/2014 11:20 PM, Peter Geoghegan wrote:

On Sun, Jan 12, 2014 at 8:12 AM, Andreas Karlsson andr...@proxel.se wrote:

On 01/11/2014 11:42 PM, Peter Geoghegan wrote:

I recently suggested that rather than RETURNING REJECTS, we could have
a REJECTING clause, which would see a DML statement project strictly
the complement of what RETURNING projects in the same context. So
perhaps you could also see what RETURNING would not have projected
because a before row trigger returned NULL (i.e. when a before trigger
indicates to not proceed with insertion). That is certainly more
general, and so is perhaps preferable. It's also less verbose, and it
seems less likely to matter that we'll need to make REJECTING a fully
reserved keyword, as compared to REJECTS. (RETURNING is already a
fully reserved keyword not described by the standard, so this makes a
certain amount of sense to me). If nothing else, REJECTING is more
terse than RETURNING REJECTS.


I do not entirely understand what you are proposing here.  Any example how
this would look compared to your RETURNING REJECTS proposal?


It's very similar - REJECTING is a total generalization of what I
already have. The difference is only that REJECTING is accepted in all
contexts that RETURNING is, and not just with INSERT...ON DUPLICATE
KEY LOCK FOR UPDATE. So you could potentially have REJECTING project
the slot proposed for insertion on an UPDATE where RETURNING would
not. If for example a BEFORE ROW trigger fired, and returned NULL,
perhaps it'd then be possible to project the slot as it was before
being passed to the trigger. Perhaps there is no real demand for that,
but, as I said, from a usability perspective it may be easier to
reason about a feature that projects strictly the complement of what
RETURNING would project in the same context.


So simply this?

WITH rej AS (
INSERT INTO foo (a, b, c)
VALUES (44, 1078, 'insert'), (55, 1088, 'insert')
REJECTING a)
UPDATE foo SET c = 'update' FROM rej WHERE foo.a = rej.a;

Another question: have you given any thought on the case where you want 
to use both the successfully inserted tuples and the rejected and use in 
the CTE? Is that even something anyone would want? Would perhaps MERGE 
be more suited for that?


--
Andreas Karlsson


--
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] Compiling extensions on Windows

2014-01-12 Thread Joe Conway
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

On 01/12/2014 07:22 PM, Craig Ringer wrote:
 On 01/13/2014 11:13 AM, Joe Conway wrote: What I mean is that you
 should not need a full Pg build tree to compile extensions. Just as
 we use PGXS on *nix, so it is possible to just use Visual Studio to
 build the extension on Windows as a VS project like any other.

I have never gotten that to work.

 As long as the SDK is matched, the resulting plr.dll works fine 
 with the EDB one-click installer version of postgres (or at
 least so far).
 
 The SDK shouldn't even need to match. Windows software is expected
 to cope with mismatched, mix-and-match C runtimes. Icky, but
 occasionally useful.

Unfortunately my experience tells me otherwise. Mismatched SDK =
plr.dll that does not work correctly with EDB builds of postgres.

 So long as you don't pass a FILE* across DLL boundaries, or free() 
 memory that was malloc()'d in another DLL, mixing runtimes should
 work fine. I'd be interested in knowing what problems you
 encountered when the runtime didn't match.

plr would not load. I'm not good enough with MSVC and Windows
debuggers to figure out why, and don't have the time or interest to
become so, I just know matching the SDK to the postgres build resulted
in it owrking for me and no more complaints of this type from the
field. It would work fine with the postgres build I made, but not with
the EDB build which of course is what virtually everyone running pg on
Windows uses.

 Maybe it's specific to how we do build within a Pg source tree? I
 was able to use three different SDK builds against EDB's latest
 9.3 packages - SDK 7.1 (x86 and x64), toolchain v90 (x86), and
 toolchain v100 (x86) - to compile and test a moderately trivial
 extension.

Trivial is probably the key thing. With PL/R it also has to link
against the R.dll which is built with yet another toolchain on Windows.

Joe

- -- 
Joe Conway
credativ LLC: http://www.credativ.us
Linux, PostgreSQL, and general Open Source
Training, Service, Consulting,  24x7 Support
-BEGIN PGP SIGNATURE-
Version: GnuPG v1.4.14 (GNU/Linux)
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/

iQIcBAEBAgAGBQJS018UAAoJEDfy90M199hl/mAP/2OhXbvrlKSzP6fWi8g9Tez0
PnWaOFXXPIeOi5lJ/o5rC7HrQdiCCBljSBAffq0PKl8SVn2cDwmA5E2n4T3JfZQF
PYGZiYuqoiZ6i+svW7x6XocDnIhJgMJvTvys7ToApjmkD3VEgj7RO8MHQyYmVDsh
A9WIPpyb96mTmzlTLnHZDkfL7MgEof4kTHHC2jOa6i3wMq+zATc6lBTXOcrrGzS8
qd/iIap0kNdwKgLEX/jXip0YOB8SMfxOeHVLV+790JUwnWmBJnbn3XDqFEmj39kK
dGEP8vaxjPppyEmMvkGZd5Hxw6WIGFASTjyn6kXH1VfVqsLNZYO+rwTXnSVtyqH0
aFCKLT7awMBjFSh3plFQcqxeeqElZZaCRNVO5xooQ28xoUoUl/wVYqI0yoF9hKKm
NlJ8jJGB6aEImFlQ7QUg2eZRfMpyYc9J06uaX1+/L3g71O4O2Xzgc6gPVWvYCIQP
BvcNBtUlxA38H5wjiMSlyyz4Si95cIIbqDfUliKZ1Ab0W24en0vnvISxk6v/2GKc
vE9X7GRFUjmUJNoIvkRu0hnzp5S955sO0X6Q6pDmgM2esGRADMTntY0Bcxp8R2qg
qiZkVs1vfuewLmzz4LqixItW9BhMHK3zdGcv07xntNt+EAaz8g3cU5tBZP8CP6y8
JTa/fyFth7hL+ttkH5hc
=9+nY
-END PGP SIGNATURE-


-- 
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] Compiling extensions on Windows

2014-01-12 Thread Craig Ringer
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

On 01/13/2014 11:35 AM, Joe Conway wrote:
 On 01/12/2014 07:22 PM, Craig Ringer wrote:
 On 01/13/2014 11:13 AM, Joe Conway wrote: What I mean is that
 you should not need a full Pg build tree to compile extensions.
 Just as we use PGXS on *nix, so it is possible to just use Visual
 Studio to build the extension on Windows as a VS project like any
 other.
 
 I have never gotten that to work.
 
 As long as the SDK is matched, the resulting plr.dll works fine
  with the EDB one-click installer version of postgres (or at 
 least so far).
 
 The SDK shouldn't even need to match. Windows software is
 expected to cope with mismatched, mix-and-match C runtimes. Icky,
 but occasionally useful.
 
 Unfortunately my experience tells me otherwise. Mismatched SDK = 
 plr.dll that does not work correctly with EDB builds of postgres.

Something for me to play with once this CF is over, then.

- -- 
 Craig Ringer   http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training  Services
-BEGIN PGP SIGNATURE-
Version: GnuPG v1.4.15 (GNU/Linux)
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/

iQEcBAEBAgAGBQJS01/mAAoJELBXNkqjr+S2ZQcH/0rHjCW+6pvg8727dJHSmYfM
fY18VBlgYzjfzgPaFLYgp4OqT7VHl2h1d7AapVo8wRblVD4z1hEFtW6/j56Mx1wm
UTw7rWhs7utA9G+gzWcHJz9VDxED0ROFH+IwurSM85P57ztPKaRhvp6YT3fibhYb
kqw51FrEfvlnhCMi3Art3DGmHtzDRLwGTI03YOr/GRfWsccHPwpRLkpQkyuMsyOX
UBhPenz0OhtkVxEfFSmyVWmu6NzlOQyxgFgl8zW7R9pq4fTBgOfo198RDkGKnCno
9KYq3H8VqLxHgpyR2KP3netrqDvDBzk0xVmgoaJbQyT6HyuQDWH6lIHE+5RTyyM=
=nreR
-END PGP SIGNATURE-


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

2014-01-12 Thread David Fetter
On Wed, Jan 08, 2014 at 03:55:38AM +, Dilip kumar wrote:
 Below attached patch is implementing following todo item..
 machine-readable pg_controldata?
 http://www.postgresql.org/message-id/4b901d73.8030...@agliodbs.com

Please put this in the upcoming Commitfest :)

https://commitfest.postgresql.org/action/commitfest_view?id=21

Cheers,
David.
-- 
David Fetter da...@fetter.org http://fetter.org/
Phone: +1 415 235 3778  AIM: dfetter666  Yahoo!: dfetter
Skype: davidfetter  XMPP: david.fet...@gmail.com
iCal: webcal://www.tripit.com/feed/ical/people/david74/tripit.ics

Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate


-- 
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] [GENERAL] pg_upgrade tablespaces

2014-01-12 Thread Adrian Klaver

On 01/12/2014 07:02 PM, Bruce Momjian wrote:

On Sun, Jan 12, 2014 at 12:48:40PM -0500, Tom Lane wrote:

Bruce Momjian br...@momjian.us writes:

On Sat, Jan 11, 2014 at 12:48:51PM -0800, Adrian Klaver wrote:

I see, though I have another question. If pg_tablespace and the
symlinks can get out of sync, as you say below, why is pg_tablespace
considered the authority? Or to put it another way, why not just
look at the symlinks as in 9.2+?



Uh, good question.  I think I used the system tables because they were
easier to access.  I can't remember if we used the symlinks for some
things and pg_tablespace for other things in pre-9.2.


Well, pre-9.2 pg_dumpall is going to make use of the pg_tablespace
entries, because it has no other choice.  We could conceivably teach
pg_upgrade to look at the symlinks for itself, but we're not going
to do that in pg_dumpall.  Which means that the intermediate dump
script would contain inconsistent location values anyway if the
catalog entries are wrong.  So I don't see any value in changing the
quoted code in pg_upgrade.


OK, agreed.


It does however seem reasonable for pg_upgrade to note whether any
of the paths are prefixed by old PGDATA and warn about the risks
involved.


Uh, the problem is that once you rename the old PGDATA, the
pg_tablespace contents no longer point to the current PGDATA.  The
symlinks, if they used absolute paths, wouldn't point to the current
PGDATA either.



Well the problem is that it actually points to a current PGDATA just the 
wrong one. To use the source installation path and the suggested upgrade 
method from pg_upgrade.


Start.

/usr/local/pgsql/data/tblspc_dir

mv above to

/usr/local/pgsql_old/

install new version of Postgres to

/usr/local/pgsql/data/


In the pgsql_old installation you have symlinks pointing back to the 
current default location. As well pg_tablespace points back to 
/usr/local/pgsql/data/ The issue is that there is not actually anything 
there in the way of a tablespace. So when pg_upgrade runs it tries to 
upgrade from /usr/local/pgsql/data/tblspc_dir to 
/usr/local/pgsql/data/tblspc_dir where the first directory either does 
not exist. or if the user went ahead and created the directory in the 
new installation, is empty. What is really wanted is to upgrade from 
/usr/local/pgsql_old/data/tblspc_dir to 
/usr/local/pgsql/data/tblspc_dir. Right now the only way that happens is 
with user intervention.


--
Adrian Klaver
adrian.kla...@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] [GENERAL] pg_upgrade tablespaces

2014-01-12 Thread Bruce Momjian
On Sun, Jan 12, 2014 at 07:58:52PM -0800, Adrian Klaver wrote:
 Well the problem is that it actually points to a current PGDATA just
 the wrong one. To use the source installation path and the suggested
 upgrade method from pg_upgrade.
 
 Start.
 
 /usr/local/pgsql/data/tblspc_dir
 
 mv above to
 
 /usr/local/pgsql_old/
 
 install new version of Postgres to
 
 /usr/local/pgsql/data/
 
 
 In the pgsql_old installation you have symlinks pointing back to the
 current default location. As well pg_tablespace points back to
 /usr/local/pgsql/data/ The issue is that there is not actually
 anything there in the way of a tablespace. So when pg_upgrade runs
 it tries to upgrade from /usr/local/pgsql/data/tblspc_dir to
 /usr/local/pgsql/data/tblspc_dir where the first directory either
 does not exist. or if the user went ahead and created the directory
 in the new installation, is empty. What is really wanted is to
 upgrade from /usr/local/pgsql_old/data/tblspc_dir to
 /usr/local/pgsql/data/tblspc_dir. Right now the only way that
 happens is with user intervention.

Right, it points to _nothing_ in the _new_ cluster.  Perhaps the
simplest approach would be to check all the pg_tablespace locations to
see if they point at real directories.  If not, we would have to have
the user update pg_tablespace and the symlinks.  :-(  Actually, even in
9.2+, those symlinks are going to point at the same nothing.  That
would support checking the symlinks in all versions.

-- 
  Bruce Momjian  br...@momjian.ushttp://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] Relocation of tablespaces in pg_basebackup

2014-01-12 Thread Alvaro Herrera
Andreas Karlsson wrote:
 On 01/09/2014 10:10 PM, Steeve Lennmark wrote:
 That's a much better solution, I attached a patch with the updated code.

 Looked at the patch quickly and noticed that it does not support
 paths containing colons. Is that an acceptable limitation?

Well, clearly it won't work on Windows when tablespaces are on different
drives, so it doesn't sound so acceptable.


-- 
Á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] [GENERAL] pg_upgrade tablespaces

2014-01-12 Thread Adrian Klaver

On 01/12/2014 08:04 PM, Bruce Momjian wrote:

On Sun, Jan 12, 2014 at 07:58:52PM -0800, Adrian Klaver wrote:

Well the problem is that it actually points to a current PGDATA just
the wrong one. To use the source installation path and the suggested
upgrade method from pg_upgrade.




In the pgsql_old installation you have symlinks pointing back to the
current default location. As well pg_tablespace points back to
/usr/local/pgsql/data/ The issue is that there is not actually
anything there in the way of a tablespace. So when pg_upgrade runs
it tries to upgrade from /usr/local/pgsql/data/tblspc_dir to
/usr/local/pgsql/data/tblspc_dir where the first directory either
does not exist. or if the user went ahead and created the directory
in the new installation, is empty. What is really wanted is to
upgrade from /usr/local/pgsql_old/data/tblspc_dir to
/usr/local/pgsql/data/tblspc_dir. Right now the only way that
happens is with user intervention.


Right, it points to _nothing_ in the _new_ cluster.  Perhaps the
simplest approach would be to check all the pg_tablespace locations to
see if they point at real directories.  If not, we would have to have
the user update pg_tablespace and the symlinks.  :-(  Actually, even in
9.2+, those symlinks are going to point at the same nothing.  That
would support checking the symlinks in all versions.



Agreed.


--
Adrian Klaver
adrian.kla...@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] plpgsql.consistent_into

2014-01-12 Thread Pavel Stehule
2014/1/12 Florian Pflug f...@phlo.org

 On Jan12, 2014, at 22:37 , Pavel Stehule pavel.steh...@gmail.com wrote:
  There is  GUC for variable_conflict already too. In this case I would to
  enable this functionality everywhere (it is tool how to simply eliminate
  some kind of strange bugs) so it needs a GUC.
 
  We have GUC for plpgsql.variable_conflict three years and I don't know
  about any problem.

 I must say I hate behaviour-changing GUCs with quite some passion. IMHO
 they tend to cause bugs, not avoid them, in the long run. The pattern
 usually is

   1) Code gets written, depends on some particular set of settings
  to work correctly

   2) Code gets reused, with little further testing since it's supposed
  to be battle-proven anyway. Settings get dropped.

   3) Code blows up for those corner-cases where the setting actually
  matter. Debugging is hell, because you effectively have to go
  over the code line-by-line and check if it might be affected by
  some GUC or another.

 Only a few days ago I spent more than an hour tracking down a bug
 which, as it turned out, was caused by a regex which subtly changed its
 meaning depending on whether standard_conforming_strings is on or off.

 Some GUCs are unavoidable - standard_conforming_strings, for example
 probably still was a good idea, since the alternative would have been
 to stick with the historical, non-standard behaviour forever.

 But in this case, my feeling is that the trouble such a GUC may cause
 out-weights the potential benefits. I'm all for having a directive like
 #consistent_into (though I feel that the name could convey the
 meaning better). If we *really* think that this ought to be the default
 from 9.4 onward, then we should

   *) Change it to always complain, except if the function explictly
  specifies #consistent_into on or whatever.

   *) Have pg_dump add that to all plpgsql functions if the server
  version is  9.4 or whatever major release this ends up in


I disagree - automatic code injection can is strange. Is not problem inject
code from simple DO statement, but I dislike append lines to source code
without any specific user request.

Maybe this problem with GUC can be solved in 9.4. Some specific GUC can be
ported with database.

Pavel



 That's all just my opinion of course.

 best regards,
 Florian Pflug




Re: [HACKERS] Retain dynamic shared memory segments for postmaster lifetime

2014-01-12 Thread Amit Kapila
On Sun, Jan 12, 2014 at 12:41 PM, Amit Kapila amit.kapil...@gmail.com wrote:
 Currently there is no way user can keep the dsm
 segments if he wants for postmaster lifetime, so I
 have exposed a new API dsm_keep_segment()
 to implement the same.

 The specs and need for this API is already discussed
 in thread:
 http://www.postgresql.org/message-id/ca+tgmoakogujqbedgeykysxud9eaidqx77j2_hxzrgfo3hr...@mail.gmail.com

   We have decided to bump reference count for segment
   and call DuplicateHandle for Windows, but I think it should
   also do what dsm_keep_mapping() does that is
   ResourceOwnerForgetDSM(), else it will give
   Warning: dynamic shared memory leak at transaction end.


 I had used dsm_demo (hacked it a bit) module used
 during initial tests for dsm API's to verify the working on
 Windows. So one idea could be that I can extend
 that module to use this new API, so that it can be tested
 by others as well or if you have any other better way, please
 do let me know.

I have extended test (contrib) module dsm_demo such that now user
can specify during dsm_demo_create the lifespan of segment.
The values it can accept are 0 or 1. Default value is 0.
0 -- means segment will be accessible for session life time
1 -- means segment will be accessible for postmaster life time


The behaviour is as below:
Test -1 (Session life time)
Session - 1
-- here it will create segment for session lifetime
select dsm_demo_create('this message is from session-1', 0);
 dsm_demo_create
-
   82712

Session - 2
-
select dsm_demo_read(82712);
   dsm_demo_read

 this message is from session-1
(1 row)


Session-1
\q

Session-2
postgres=# select dsm_demo_read(82712);
 dsm_demo_read
---

(1 row)

Conclusion of Test-1 : As soon as session which has created segment finished,
the segment becomes non-accessible.


Test -2 (Postmaster life time)
Session - 1
-- here it will create segment for postmaster lifetime
select dsm_demo_create('this message is from session-1', 1);
 dsm_demo_create
-
   82712

Session - 2
-
select dsm_demo_read(82712);
   dsm_demo_read

 this message is from session-1
(1 row)


Session-1
\q

Session-2
postgres=# select dsm_demo_read(82712);
 dsm_demo_read
---
 this message is from session-1
(1 row)

Conclusion of Test-2 : a. Segment is accessible for postmaster lifetime.
 b. if user restart server, segment is
not accessible.

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


dsm_demo_v1.patch
Description: Binary data

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


Re: [HACKERS] Standalone synchronous master

2014-01-12 Thread Amit Kapila
 On 01/11/2014 08:52 PM, Amit Kapila wrote: It is better than async mode
 in a way such that in async mode it never
 waits for commits to be written to standby, but in this new mode it will
 do so unless it is not possible (all sync standby's goes down).
 Can't we use existing wal_sender_timeout, or even if user expects a
 different timeout because for this new mode, he expects master to wait
 more before it start operating like standalone sync master, we can provide
 a new parameter.

 One of the reasons that there's so much disagreement about this feature
 is that most of the folks strongly in favor of auto-degrade are thinking
 *only* of the case that the standby is completely down.  There are many
 other reasons for a sync transaction to hang, and the walsender has
 absolutely no way of knowing which is the case.  For example:

 * Transient network issues
 * Standby can't keep up with master
 * Postgres bug
 * Storage/IO issues (think EBS)
 * Standby is restarting

 You don't want to handle all of those issues the same way as far as sync
 rep is concerned.  For example, if the standby is restaring, you
 probably want to wait instead of degrading.

   I think it might be difficult to differentiate the cases except may be
   by having a separate timeout for this mode, so that it can wait more
   when server runs in this mode. OTOH why can't we define this new
   mode such that it will behave same for all cases, basically we can tell
   whenever sync standby is not available (n/w issue or m/c down), it will
   behave as master in async mode.
   Here I think the important point would be to gracefully allow resuming
   sync standby when it tries to reconnect (we can allow to reconnect if it
   can resolve all WAL differences.)


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] Standalone synchronous master

2014-01-12 Thread Rajeev rastogi

On 13th January 2013, Josh Berkus Wrote:

 I'm leading this off with a review of the features offered by the
 actual patch submitted.  My general discussion of the issues of Sync
 Degrade, which justifies my specific suggestions below, follows that.
 Rajeev, please be aware that other hackers may have different opinions
 than me on what needs to change about the patch, so you should collect
 all opinions before changing code.

Thanks for reviewing and providing the first level of comments. Surely
We'll collect all feedback to improve this patch.

 
  Add a new parameter :
 
  synchronous_standalone_master = on | off
 
 I think this is a TERRIBLE name for any such parameter.  What does
 synchronous standalone even mean?  A better name for the parameter
 would be auto_degrade_sync_replication or synchronous_timeout_action
 = error | degrade, or something similar.  It would be even better for
 this to be a mode of synchronous_commit, except that synchronous_commit
 is heavily overloaded already.

Yes we can change this parameter name. Some of the suggestion in order to 
degrade the mode
1. Auto-degrade using some sort of configuration parameter as done in 
current patch.
2. Expose the configuration variable to a new SQL-callable functions as 
suggested by Heikki.
3. Or using ALTER SYSTEM SET as suggested by others.

 Some issues raised by this log script:
 
 LOG:  standby tx0113 is now the synchronous standby with priority 1
 LOG:  waiting for standby synchronization
   -- standby wal receiver on the standby is killed (SIGKILL)
 LOG:  unexpected EOF on standby connection
 LOG:  not waiting for standby synchronization
   -- restart standby so that it connects again
 LOG:  standby tx0113 is now the synchronous standby with priority 1
 LOG:  waiting for standby synchronization
   -- standby wal receiver is first stopped (SIGSTOP) to make sure
 
 The not waiting for standby synchronization message should be marked
 something stronger than LOG.  I'd like ERROR.

Yes we can change this to ERROR.

 Second, you have the master resuming sync rep when the standby
 reconnects.  How do you determine when it's safe to do that?  You're
 making the assumption that you have a failing sync standby instead of
 one which simply can't keep up with the master, or a flakey network
 connection (see discussion below).

Yes this can be further improved so that only if we make sure that synchronous
Standby has caught up with master node (may require a better design), then only 
master can be upgraded to Synchronous mode by one of the method discussed above.

  a.   Master_to_standalone_cmd: To be executed before master
 switches to standalone mode.
 
  b.  Master_to_sync_cmd: To be executed before master switches
 from
 sync mode to standalone mode.
 
 I'm not at all clear what the difference between these two commands is.
  When would one be excuted, and when would the other be executed?  Also,
 renaming ...

There is typo mistake in above explain, meaning of two commands are:
a.Master_to_standalone_cmd: To be executed during degradation of sync mode.

 b.  Master_to_sync_cmd: To be executed before upgrade or restoration of mode.

These two commands are per the TODO item to inform DBA.

But as per Heikki suggestion, we should not use this mechanism to inform DBA 
rather
We should some have some sort of generic trap system, instead of adding this 
one 
particular extra config option specifically for this feature. 
This looks to be better idea so we can have further discussion to come with 
proper
design.


 Missing features:
 
 a) we should at least send committing clients a WARNING if they have
 commited a synchronous transaction and we are in degraded mode.

Yes it is great idea.

 One of the reasons that there's so much disagreement about this feature
 is that most of the folks strongly in favor of auto-degrade are
 thinking
 *only* of the case that the standby is completely down.  There are many
 other reasons for a sync transaction to hang, and the walsender has
 absolutely no way of knowing which is the case.  For example:
 
 * Transient network issues
 * Standby can't keep up with master
 * Postgres bug
 * Storage/IO issues (think EBS)
 * Standby is restarting
 
 You don't want to handle all of those issues the same way as far as
 sync rep is concerned.  For example, if the standby is restaring, you
 probably want to wait instead of degrading.

I think if we support to have some external SQL-callable functions as Heikki 
suggested to degrade instead of auto-degrade then user can handle at-least some 
of the above scenarios if not all based on their experience and observation. 


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] pgcrypto: implement gen_random_uuid

2014-01-12 Thread Oskari Saarenmaa

13.01.2014 04:35, Wim Lewis kirjoitti:

One comment, this:


  /* get 128 random bits */
  int err = px_get_random_bytes(buf, 16);


might be better to use px_get_pseudo_random_bytes(). UUIDs don't
need to be unguessable or have perfect entropy; they just need to
be collision-resistant. RFC4122 mentions this I think, and if you
look at the ossp-uuid function that this is replacing, it also uses
its internal PRNG for v4 UUIDs rather than strong high-entropy
randomness.

(The downside of requesting strong randomness when you don't need
it is that it can potentially cause the server to block while the
system gathers entropy.)


pgcrypto's px_get_pseudo_random_bytes is just a wrapper for 
px_get_random_bytes which itself calls system_reseed and 
fortuna_get_bytes.  system_reseed function tries to read from 
/dev/urandom, and only uses /dev/random if reading urandom fails, so it 
should never block on systems which have urandom.


That said, it may still make sense to use px_get_pseudo_random_bytes 
instead just in case it ever gets modified to do something lighter than 
px_get_random_bytes.


Thanks for the review,
Oskari



--
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] plpgsql.consistent_into

2014-01-12 Thread Pavel Stehule
2014/1/13 Gavin Flower gavinflo...@archidevsys.co.nz

 On 13/01/14 11:44, Florian Pflug wrote:

 On Jan12, 2014, at 22:37 , Pavel Stehule pavel.steh...@gmail.com wrote:

 There is  GUC for variable_conflict already too. In this case I would to
 enable this functionality everywhere (it is tool how to simply eliminate
 some kind of strange bugs) so it needs a GUC.

 We have GUC for plpgsql.variable_conflict three years and I don't know
 about any problem.

 I must say I hate behaviour-changing GUCs with quite some passion. IMHO
 they tend to cause bugs, not avoid them, in the long run. The pattern
 usually is

1) Code gets written, depends on some particular set of settings
   to work correctly

2) Code gets reused, with little further testing since it's supposed
   to be battle-proven anyway. Settings get dropped.

3) Code blows up for those corner-cases where the setting actually
   matter. Debugging is hell, because you effectively have to go
   over the code line-by-line and check if it might be affected by
   some GUC or another.

 Only a few days ago I spent more than an hour tracking down a bug
 which, as it turned out, was caused by a regex which subtly changed its
 meaning depending on whether standard_conforming_strings is on or off.

 Some GUCs are unavoidable - standard_conforming_strings, for example
 probably still was a good idea, since the alternative would have been
 to stick with the historical, non-standard behaviour forever.

 But in this case, my feeling is that the trouble such a GUC may cause
 out-weights the potential benefits. I'm all for having a directive like
 #consistent_into (though I feel that the name could convey the
 meaning better). If we *really* think that this ought to be the default
 from 9.4 onward, then we should

*) Change it to always complain, except if the function explictly
   specifies #consistent_into on or whatever.

*) Have pg_dump add that to all plpgsql functions if the server
   version is  9.4 or whatever major release this ends up in

 That's all just my opinion of course.

 best regards,
 Florian Pflug



  Possibly there should be a warning put out, whenever someone uses some
 behaviour that requires a GUC set to a non-default value?


It is a good idea. I though about it. A worry about GUC are legitimate, but
we are most static and sometimes we try to design bigger creatures, than we
try to solve.

I am thinking so dump can contain a serialized GUC values, and can raises
warnings when GUC are different (not only different from default).

Similar problems are with different FROM_COLAPS_LIMIT, JOIN_COLAPS_LIMIT,
WORK_MEM, ...

Regards

Pavel






 Cheers,
 Gavin



Re: [HACKERS] plpgsql.consistent_into

2014-01-12 Thread Pavel Stehule
2014/1/12 Florian Pflug f...@phlo.org

 On Jan12, 2014, at 22:37 , Pavel Stehule pavel.steh...@gmail.com wrote:
  There is  GUC for variable_conflict already too. In this case I would to
  enable this functionality everywhere (it is tool how to simply eliminate
  some kind of strange bugs) so it needs a GUC.
 
  We have GUC for plpgsql.variable_conflict three years and I don't know
  about any problem.

 I must say I hate behaviour-changing GUCs with quite some passion. IMHO
 they tend to cause bugs, not avoid them, in the long run. The pattern
 usually is

   1) Code gets written, depends on some particular set of settings
  to work correctly

   2) Code gets reused, with little further testing since it's supposed
  to be battle-proven anyway. Settings get dropped.

   3) Code blows up for those corner-cases where the setting actually
  matter. Debugging is hell, because you effectively have to go
  over the code line-by-line and check if it might be affected by
  some GUC or another.

 Only a few days ago I spent more than an hour tracking down a bug
 which, as it turned out, was caused by a regex which subtly changed its
 meaning depending on whether standard_conforming_strings is on or off.

 Some GUCs are unavoidable - standard_conforming_strings, for example
 probably still was a good idea, since the alternative would have been
 to stick with the historical, non-standard behaviour forever.

 But in this case, my feeling is that the trouble such a GUC may cause
 out-weights the potential benefits. I'm all for having a directive like
 #consistent_into (though I feel that the name could convey the
 meaning better). If we *really* think that this ought to be the default
 from 9.4 onward, then we should

   *) Change it to always complain, except if the function explictly
  specifies #consistent_into on or whatever.

   *) Have pg_dump add that to all plpgsql functions if the server
  version is  9.4 or whatever major release this ends up in

 That's all just my opinion of course.


I am thinking so GUC and plpgsql option can live together. If you like to
accent a some behave, then you can use a plpgsql option. On second hand, I
would to use a some functionality, that is safe, but I don't would to dirty
source code by using repeated options. But I have to check (and calculate
with risk) a GUC settings.

One idea: required GUC? Can be nice a possibility to ensure some GUC
setting, and restore ensure these values or raises warning.

Back to main topic. Required and described feature doesn't change a behave
of INTO clause. I can enable or disable this functionality and well written
code should to work without change (and problems). When check is disabled,
then execution is just less safe. So in this case, a impact of GUC is
significantly less than by you described issues. Does know anybody a use
case where this check should be disabled?

Probably we have a different experience about GUC. I had a problem with
standard_conforming_strings and bytea format some years ago. Now I prepare
document about required setting. But I can see (from my experience from
Czech area) more often  problems related to effective_cache_size or
from_collapse_limit and similar GUC. These parameters are behind knowledge
(and visibility) typical user.

Best regards

Pavel



 best regards,
 Florian Pflug