Re: [HACKERS] Re: [BUGS] BUG #7873: pg_restore --clean tries to drop tables that don't exist

[Pavel Stehule]

Hello


2013/11/19 Robert Haas 

> On Tue, Nov 19, 2013 at 3:53 AM, Dean Rasheed 
> wrote:
> > 1). Keep the existing syntax:
> >
> > DROP TRIGGER [ IF EXISTS ] name ON table_name [ CASCADE | RESTRICT ];
> >
> > but make it tolerate a non-existent table when "IF EXISTS" is specified.
>
> I don't love this option, but I like it better than the other proposals.
>

we are in agreement, so we want this feature. How we can decide about
syntax?

I am feeling, so almost all people prefer

DROP TRIGGER [ IF EXISTS ] name ON table_name [ CASCADE | RESTRICT ];

Can we live with it?

Regards

Pavel


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

[HACKERS] new unicode table border styles for psql

[Pavel Stehule]

Hello

I wrote new styles for  psql table borders.

http://postgres.cz/wiki/Pretty_borders_in_psql

This patch is simply and I am think so some styles can be interesting for
final presentation.

Do you think so this feature is generally interesting and should be in core?

Regards

Pavel

Re: [HACKERS] New option for pg_basebackup, to specify a different directory for pg_xlog

[Haribabu kommi]

on 20 November 2013 23:44 Gavin Flower wrote:
>On 20/11/13 23:43, Haribabu kommi wrote:

>>I tried using of stat'ing in two directories, which is having a problem in 
>>windows.

>>So modified old approach to detect limited errors. Updated patch attached.

>>This will detect and throw an error in the following scenarios.

>>1. pg_basebackup -D /home/data --xlogdir=/home/data

>>2. pg_basebackup -D data --xlogdir=/home/data -- home is the CWD

>>3. pg_basebackup -D ../data --xlogdir=/data -- home is the CWD


>I don't think Postgres on other systems should be hobbled by the limitations 
>of Microsoft software!
>If certain features of Postgres are either not available, or are available in 
>a reduced form on Microsoft platforms, then this should be documented - might 
>provide subtle hints to upgrade to Linux.

The patch which sent in earlier mail provides the detection of base and xlog 
directories are same or not
In both windows and linux with a different way of identifying the same without 
stat'ing.
If other also agrees to add stat'ing then I will change the patch accordingly 
and document the behavior
Change for windows.

Regards,
Hari babu.

Re: [HACKERS] COPY table FROM STDIN doesn't show count tag

[Amit Khandekar]

On 21 November 2013 10:13, Tom Lane  wrote:

> Amit Khandekar  writes:
> > Rather than a behaviour change, it is a bug that we are fixing. User
> > already expects to see copy status printed, so as per user there would be
> > no behaviour change.
>
> This is arrant nonsense.  It's a behavior change.  You can't make it
> not that by claiming something about user expectations.  Especially
> since this isn't exactly a corner case that nobody has seen in
> the fifteen years or so that it's worked like that.  People do know
> how this works.
>

Yes, I agree that this is not a corner case, so users may already know the
current behaviour.

>
> I don't object to changing it, but I do agree with Robert that it's
> important to quantize such changes, ie, try to get a set of related
> changes to appear in the same release.  People don't like repeatedly
> revising their code for such things.
>

Ok. we will then first fix the \COPY TO issue where it does not revert back
the overriden psql output file handle. Once this is solved, fix for both
COPY FROM and COPY TO, like how it is done in the patch earlier (
copydefectV2.patch).

>
> regards, tom lane
>

Re: [HACKERS] COPY table FROM STDIN doesn't show count tag

[Tom Lane]

Amit Khandekar  writes:
> Rather than a behaviour change, it is a bug that we are fixing. User
> already expects to see copy status printed, so as per user there would be
> no behaviour change.

This is arrant nonsense.  It's a behavior change.  You can't make it
not that by claiming something about user expectations.  Especially
since this isn't exactly a corner case that nobody has seen in
the fifteen years or so that it's worked like that.  People do know
how this works.

I don't object to changing it, but I do agree with Robert that it's
important to quantize such changes, ie, try to get a set of related
changes to appear in the same release.  People don't like repeatedly
revising their code for such things.

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] Extra functionality to createuser

[Amit Kapila]

On Wed, Nov 20, 2013 at 9:53 PM, Christopher Browne  wrote:
> Wait, that doesn't work if more than one role is added, as they get
> merged together by the quoting.
>
> A somewhat ugly amount of quoting can be done at the shell level to
> induce double quotes.
>
> $ createuser -g "\"test_rol'e_3\"" usequoted3
>
> I note that similar (with not quite identical behaviour) issues apply
> to the user name.  Perhaps the
> resolution to this is to leave quoting issues to the administrator.
> That simplifies the problem away.

We are already doing something similar for username, refer below line:
printfPQExpBuffer(&sql, "CREATE ROLE %s", fmtId(newuser));

Here fmtId() is doing handling for quotes. Now for new syntax IN ROLE,
the difference is that it can have multiple strings which needs
additional handling. I think some similar handling should be there in
server, pg_dump as well.

> I suspect that the apparatus needed to do a thorough solution (e.g. -
> parse the string, and do something
> "smarter") may be larger than is worth getting into.

I think if it needs some bigger solution then we can leave it by
having small note in documentation, but if it's just a matter of
calling some existing functions or mimic some handling done at other
place, then it is worth trying.

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] noob's query

[Amit Kapila]

On Thu, Nov 21, 2013 at 7:44 AM, Asit Mahato  wrote:
> Hi All,
>
> I would like to contribute to postgresql. But when i find a bug or issue
> which i would like to fix then a lot of questions arise to me. Those are
> below:
>
> 1)This one (the bug or issue in to do list i have selected) may be to hard
> to me. so from which one should i start?

This you have to only decide, but there are features marked as [E]
in the todo list which you can consider as your starting point.

> 2)which files are related to this bug?

Most (at least those which are marked as [E]) of the features are
enhancements, so you can debug the base feature and try to analyse how
to extend it for new feature. After your initial analysis, if you
want, you can even post on this list to get feedback or you can
continue to prepare the patch and upload it to CommitFest for
feedback.


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] Proof of concept: standalone backend with full FE/BE protocol

[Amit Kapila]

On Thu, Nov 21, 2013 at 2:14 AM, Tom Lane  wrote:
> Peter Eisentraut  writes:
>> The argument elsewhere in this thread was that the reason for putting
>> this in the connection options was so that you do *not* have to patch up
>> every client to be able to use this functionality.  If you have to add
>> separate options everywhere, then you might as well just have a separate
>> libpq function to initiate the session.
>
> Right, Andres was saying that we had to do both (special switches that
> lead to calling a special connection function).

   Doesn't the new option 'standalone_datadir' (which is already in
patch) a good candidate for special switch?
   How does having one more new switch helps better?

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] Easily reading debug_print_plan

[Alvaro Herrera]

Craig Ringer escribió:
> On 11/20/2013 04:22 PM, Antonin Houska wrote:
> > vim editor. The '%' shortcut can be used to jump between opening and
> > closing brackets and thus skip smaller or bigger parts of the output.
> > IMO, this output is primarily for hackers (as opposed to application
> > developers or users) and hacker should know at least a few vim shortcuts
> > anyway :-)
> 
> That's what I'm currently doing, I just wanted something that makes it
> quicker and easier. Jumping around the tree is good, but easy
> collapse/expand would be much better.

I think teaching Vim to fold (see ":help foldmethod") using the syntax
might be useful.  I would assume it's just a matter of writing a syntax
colorizer for outfuncs.c output, and set the foldmethod to syntax ...?

-- 
Á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] COPY table FROM STDIN doesn't show count tag

[Amit Khandekar]

On 20 November 2013 18:11, Robert Haas  wrote:

> On Wed, Nov 20, 2013 at 4:56 AM, Amit Khandekar
>  wrote:
> > So I think it is best to solve this as a different issue, and we should ,
> > for this commitfest,  fix only COPY FROM. Once the \COPY existing issue
> is
> > solved, only then we can start printing the \COPY TO status as well.
>
> I actually think that we should probably fix the \COPY issue first.
> Otherwise, we may end up (for example) changing COPY FROM in one
> release and COPY TO in the next release, and that would be annoying.
> It does cause application compatibility problems to some degree when
> we change things like this, so it's useful to avoid doing it multiple
> times.  And I can't really see a principled reason for COPY FROM and
> COPY TO to behave differently, either.
>

Rather than a behaviour change, it is a bug that we are fixing. User
already expects to see copy status printed, so as per user there would be
no behaviour change.

So the idea is to fix it in two places independently. Whatever fix we are
doing for COPY FROM , we would not revert or change that fix when we fix
the COPY TO issue. The base changes will go in COPY FROM fix, and then we
will be extending (not rewriting) the fix for COPY TO after fixing the
\COPY-TO issue.

Also, we already have a fix ready for COPY FROM, so I thought we better
commit this first.


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

Re: [HACKERS] Proof of concept: standalone backend with full FE/BE protocol

[Bruce Momjian]

On Wed, Nov 20, 2013 at 05:38:14PM -0500, Gurjeet Singh wrote:
> On Wed, Nov 20, 2013 at 3:44 PM, Tom Lane  wrote:
> 
> 
> To my mind, the "create a socket and hope nobody else can get to it"
> approach is exactly one of the main things we're trying to avoid here.
> If you'll recall, awhile back we had a big discussion about how pg_upgrade
> could positively guarantee that nobody messed with the source database
> while it was working, and we still don't have a bulletproof guarantee
> there.  I would like to fix that by making pg_upgrade use only standalone
> backends to talk to the source database, never starting a real postmaster
> at all.  But if the standalone-pg_dump mode goes through a socket, we're
> back to square one on that concern.
> 
> 
> (I couldn't find the pg_upgrade-related thread mentioned above).
> 
> I am not sure of the mechanics of this, but can we not launch the postmaster
> with a random magic-cookie, and use that cookie while initiating the 
> connection
> from libpq. The postmaster will then reject any connections that don't provide
> the cookie.
> 
> We do something similar to enable applications to send cancellation signals
> (postmaster.c:Backend.cancel_key), just that it's establishing trust in the
> opposite direction.

The magic cookie can be tha application_name.  I had pg_upgrade code to
prevent anyone from connecting unless their application_name was
"pg_upgrade", but the idea was rejected.

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

  + Everyone has their own god. +


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

Re: [HACKERS] UNNEST with multiple args, and TABLE with multiple funcs

[David Johnston]

Tom Lane-2 wrote
> We could conceivably say that we'll implicitly UNNEST() if the function
> returns array, and not otherwise --- but that seems pretty inconsistent
> and surprise-making to me. 

The use-cases for putting a scalar array returning function call into a
TABLE construct, and NOT wanting the array to be un-nested, are likely few
and far between.

Neither the inconsistency nor surprise-making are serious deal-breakers for
me.

And if we do go with the "screw the standard" approach then we should just
state right now that we will never adhere to standard on "inconsistency
grounds" and not even encourage others to make it work.  If "TABLE(
array_scalar_func() )" ends up only returning a single row then nothing can
be done to make it unnest the array and conform with the syntax without
breaking backward compatibility.

I'd rather change "TABLE" to "FUNCTION" and leave the implementation of
TABLE open for future standards-compliance - which maybe you do as well and
just haven't carried that sentiment to your more recent responses

David J.







--
View this message in context: 
http://postgresql.1045698.n5.nabble.com/UNNEST-with-multiple-args-and-TABLE-with-multiple-funcs-tp5767280p5779518.html
Sent from the PostgreSQL - hackers mailing list archive at Nabble.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] UNNEST with multiple args, and TABLE with multiple funcs

[Tom Lane]

David Johnston  writes:
> Tom Lane-2 wrote
>> We could conceivably say that we'll implicitly UNNEST() if the function
>> returns array, and not otherwise --- but that seems pretty inconsistent
>> and surprise-making to me.  I'm not too sure what to do if a function
>> returns setof array, either.

> If a function returns a scalar array (RETURNS text[]) we would unnest the
> array per-spec.  If it returns a set (RETURN setof anything {including a
> single array}) we would not unnest it since set returning functions are
> non-spec - instead we'd use our SRF processing routine.  If the function
> returns a scalar non-array the implicit single-row returned by the function
> would be output.

I find that way too inconsistent to be a sane specification.

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] UNNEST with multiple args, and TABLE with multiple funcs

[David Johnston]

Tom Lane-2 wrote
> Andrew Gierth <

> andrew@.org

> > writes:
>> "Tom" == Tom Lane <

> tgl@.pa

> > writes:
>>  Tom> and this would result in producing the array elements as a table
>>  Tom> column.  There is nothing in there about a function returning
>>  Tom> set.
> 
>> In the spec, there is no such thing as a function returning a set of
>> rows in the sense that we use.
> 
> Right, but they do have a concept of arrays that's similar to ours,
> and AFAICS the spec demands different behavior for an array-returning
> function than what we've got here.
> 
> We could conceivably say that we'll implicitly UNNEST() if the function
> returns array, and not otherwise --- but that seems pretty inconsistent
> and surprise-making to me.  I'm not too sure what to do if a function
> returns setof array, either.

If a function returns a scalar array (RETURNS text[]) we would unnest the
array per-spec.  If it returns a set (RETURN setof anything {including a
single array}) we would not unnest it since set returning functions are
non-spec - instead we'd use our SRF processing routine.  If the function
returns a scalar non-array the implicit single-row returned by the function
would be output.

How would the spec interpret:

CREATE FUNCTION f(IN text, OUT text[]) RETURNS record AS $$ ...

TABLE( f('id_123') )

If that is illegal because the result is not just a single array value then
we would not unnest the component array and would also output the implicit
single-row.

My $0.02, quickly gathered

David J.




--
View this message in context: 
http://postgresql.1045698.n5.nabble.com/UNNEST-with-multiple-args-and-TABLE-with-multiple-funcs-tp5767280p5779515.html
Sent from the PostgreSQL - hackers mailing list archive at Nabble.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] UNNEST with multiple args, and TABLE with multiple funcs

[Tom Lane]

Robert Haas  writes:
> The original post on this thread includes this example, which mixes
> SRFs and arrays by running the array through UNNEST:

> select * from table(generate_series(10,20,5), unnest(array['fred','jim']));

> But if we think the spec calls for things to be implicitly unnested,
> you could still get the same effect by adjusting the query.  You'd
> just get rid of the UNNEST from the argument that had it and wrap
> ARRAY(SELECT ...) around the other one:

> select * from table(array(select generate_series(10,20,5)),
> array['fred','jim']);

> It's not clear to me whether that's likely to be inefficient in
> practical cases,

Yeah, it would be :-(.  Maybe we could hack something to translate
unnest(array(...)) into a no-op, but ugh.  You really want to make
people use a syntax like that, even if it weren't inefficient?
It's verbose, ugly, unintuitive, and redundant given that you could
just write UNNEST() instead of TABLE().  But more: we *know* what
the common case is going to be, based on existing usage of SRFs,
and forced-unnest ain't it.  So I'm thinking benign neglect of the
spec's syntax is the way to go.  If anyone does come along and say
they want the spec's semantics, let them implement it, and the
syntax to go with it.

regards, tom lane


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

Re: [HACKERS] [PATCH] Store Extension Options

[Fabrízio de Royes Mello]

On Thu, Nov 21, 2013 at 12:05 AM, Robert Haas  wrote:
>
> On Wed, Nov 20, 2013 at 1:52 PM, Fabrízio de Royes Mello
>  wrote:
> > The main goal of this patch is enable to an user the capability to store
> > options
> > (relations and attributes) related to extensions by using a fixed prefix
> > called 'ext' in
> > the defined name. It's cant be useful for replication solutions.
> >
> > So, with this patch we can do that:
> >
> > ALTER TABLE foo
> >SET (ext.somext.do_replicate=true);
> >
> > When 'ext' is the fixed prefix, 'somext' is the extension name,
> > 'do_replicate' is the
> > extension option and 'true' is the value.
>
> This doesn't seem like a particular good choice of syntax;

What's your syntax suggestion?


> and I also have my doubts about the usefulness of the feature.
>

This feature can be used for replication solutions, but also can be used
for any extension that need do some specific configuration about tables,
attributes and/or indexes.

Regards,

--
Fabrízio de Royes Mello
Consultoria/Coaching PostgreSQL
>> Timbira: http://www.timbira.com.br
>> Blog sobre TI: http://fabriziomello.blogspot.com
>> Perfil Linkedin: http://br.linkedin.com/in/fabriziomello
>> Twitter: http://twitter.com/fabriziomello

Re: [HACKERS] UNNEST with multiple args, and TABLE with multiple funcs

[Tom Lane]

Andrew Gierth  writes:
> "Tom" == Tom Lane  writes:
>  Tom> and this would result in producing the array elements as a table
>  Tom> column.  There is nothing in there about a function returning
>  Tom> set.

> In the spec, there is no such thing as a function returning a set of
> rows in the sense that we use.

Right, but they do have a concept of arrays that's similar to ours,
and AFAICS the spec demands different behavior for an array-returning
function than what we've got here.

We could conceivably say that we'll implicitly UNNEST() if the function
returns array, and not otherwise --- but that seems pretty inconsistent
and surprise-making to me.  I'm not too sure what to do if a function
returns setof array, either.

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] UNNEST with multiple args, and TABLE with multiple funcs

[David Johnston]

Robert Haas wrote
> select * from table(array(select generate_series(10,20,5)),
> array['fred','jim']);

Can we have our arrays and eat our functions too? (and is someone willing to
bake such a complicated cake...)

select * from table ( ARRAY | FUNCTION/SET [, ARRAY | FUNCTION/SET ]* )

The standard-compliant case is handled as required - and those who want to
write compliant code can use the array(select function) trick - while others
can avoid straining their eyes and fingers.

Since we would have to invent implicit unnesting anyway to conform, and the
function version is working currently, the suggested behavior would seem to
be the ideal target.


David J.




--
View this message in context: 
http://postgresql.1045698.n5.nabble.com/UNNEST-with-multiple-args-and-TABLE-with-multiple-funcs-tp5767280p5779512.html
Sent from the PostgreSQL - hackers mailing list archive at Nabble.com.


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

[HACKERS] noob's query

[Asit Mahato]

Hi All,

I would like to contribute to postgresql. But when i find a bug or issue
which i would like to fix then a lot of questions arise to me. Those are
below:

1)This one (the bug or issue in to do list i have selected) may be to hard
to me. so from which one should i start?

2)which files are related to this bug?


Thanks and Regards,
*Asit Mahato*

Re: [HACKERS] Can we trust fsync?

[Joshua D. Drake]

On 11/20/2013 03:45 PM, Craig Ringer wrote:


I'm really concerned by this post on Linux's fsync and disk flush behaviour:

http://milek.blogspot.com.au/2010/12/linux-osync-and-write-barriers.html

and seeking opinions from folks here who've been deeply involved in
write reliability work.

The amount of change in write reliablity behaviour in Linux across
kernel versions, file systems and storage abstraction layers is worrying
- different results for LVM vs !LVM, md vs !md, ext3 vs other, etc.

If this isn't something that's already been seen and dealt with then
I'll see if I can take a look into it once the RLS work is dealt with.



I thought Greg did some testing on this a while back and determined 
which versions were safe... (/me looks for post)


JD

--
Command Prompt, Inc. - http://www.commandprompt.com/  509-416-6579
PostgreSQL Support, Training, Professional Services and Development
High Availability, Oracle Conversion, Postgres-XC, @cmdpromptinc
For my dreams of your image that blossoms
   a rose in the deeps of my heart. - W.B. Yeats


--
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] UNNEST with multiple args, and TABLE with multiple funcs

[Andrew Gierth]

> "Tom" == Tom Lane  writes:

 Tom> and this would result in producing the array elements as a table
 Tom> column.  There is nothing in there about a function returning
 Tom> set.

In the spec, there is no such thing as a function returning a set of
rows in the sense that we use.

Functions can return arrays or multisets of simple or composite types,
but a multiset is a single value like an array (just with slightly
different semantics), not a set of rows. (And in particular it's not
ordered.)

-- 
Andrew (irc:RhodiumToad)


-- 
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] Store Extension Options

[Robert Haas]

On Wed, Nov 20, 2013 at 1:52 PM, Fabrízio de Royes Mello
 wrote:
> The main goal of this patch is enable to an user the capability to store
> options
> (relations and attributes) related to extensions by using a fixed prefix
> called 'ext' in
> the defined name. It's cant be useful for replication solutions.
>
> So, with this patch we can do that:
>
> ALTER TABLE foo
>SET (ext.somext.do_replicate=true);
>
> When 'ext' is the fixed prefix, 'somext' is the extension name,
> 'do_replicate' is the
> extension option and 'true' is the value.

This doesn't seem like a particular good choice of syntax; and I also
have my doubts about the usefulness of the feature.

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


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

Re: [HACKERS] UNNEST with multiple args, and TABLE with multiple funcs

[Robert Haas]

On Wed, Nov 20, 2013 at 3:07 PM, Tom Lane  wrote:
> Andrew Gierth  wrote:
>> The spec syntax for table function calls, 
>> in , looks like TABLE(func(args...)) AS ...
>
>> This patch implements that, plus an extension: it allows multiple
>> functions, TABLE(func1(...), func2(...), func3(...)) [WITH ORDINALITY]
>> and defines this as meaning that the functions are to be evaluated in
>> parallel.
>
> I went back and looked at the spec, and so far as I can tell, the claim
> that this is spec syntax plus an extension is a falsehood.  What
> I read in SQL:2008 7.6  is
>
>  ::=
>   TABLE   
>
> where  is elsewhere defined to be an
> expression returning an array or multiset value, and then syntax rule 2
> says:
>
> * the  shall be a 
>
> * this construct is equivalent to UNNEST (  )
>
> So unless I'm misreading it, the spec's idea is that you could write
>
>SELECT ... FROM TABLE( function_returning_array(...) )
>
> and this would result in producing the array elements as a table column.
> There is nothing in there about a function returning set.  You could argue
> that that leaves us with the freedom to define what the construct does
> for functions returning set --- but as this patch stands, if a function
> doesn't return set but does return an array, the behavior will not be what
> the spec plainly demands.

The original post on this thread includes this example, which mixes
SRFs and arrays by running the array through UNNEST:

select * from table(generate_series(10,20,5), unnest(array['fred','jim']));

But if we think the spec calls for things to be implicitly unnested,
you could still get the same effect by adjusting the query.  You'd
just get rid of the UNNEST from the argument that had it and wrap
ARRAY(SELECT ...) around the other one:

select * from table(array(select generate_series(10,20,5)),
array['fred','jim']);

It's not clear to me whether that's likely to be inefficient in
practical cases, but there's no real loss of functionality.  IOW, I'm
not sure we really need to invent a new syntax here; maybe we can just
implement the spec, assuming your interpretation thereof is correct.

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


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

Re: [HACKERS] Can we trust fsync?

[Tom Lane]

Craig Ringer  writes:
> The amount of change in write reliablity behaviour in Linux across
> kernel versions, file systems and storage abstraction layers is worrying
> - different results for LVM vs !LVM, md vs !md, ext3 vs other, etc.

Well, we pretty much *have to* trust fsync --- there's not a lot we can
do if the kernel doesn't get this right.  My takeaway is that you don't
want to be running a production database on bleeding-edge kernels or
filesystem stacks.  If you want to use Linux, use a distro from a vendor
with a track record for caring about stability.  (I'll omit the commercial
for my former employers, but ...)

Also, it's not that hard to do plug-pull testing to verify that your
system is telling the truth about fsync.  This really ought to be part
of acceptance testing for any new DB server.

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] information schema parameter_default implementation

[Rodolfo Campero]

2013/11/20 Peter Eisentraut 

> Updated patch
>

I can't apply the patch; maybe I'm doing something wrong?

$ git apply v2-0001-Implement-information_schema.parameters.parameter.patch
v2-0001-Implement-information_schema.parameters.parameter.patch:49:
trailing whitespace.
   CAST((ss.x).n AS sql_identifier) AS dtd_identifier,
v2-0001-Implement-information_schema.parameters.parameter.patch:50:
trailing whitespace.
   CAST(
v2-0001-Implement-information_schema.parameters.parameter.patch:51:
trailing whitespace.
 CASE WHEN pg_has_role(proowner, 'USAGE')
v2-0001-Implement-information_schema.parameters.parameter.patch:52:
trailing whitespace.
  THEN pg_get_function_arg_default(p_oid, (ss.x).n)
v2-0001-Implement-information_schema.parameters.parameter.patch:53:
trailing whitespace.
  ELSE NULL END
error: patch failed: doc/src/sgml/information_schema.sgml:3323
error: doc/src/sgml/information_schema.sgml: patch does not apply
error: patch failed: src/backend/catalog/information_schema.sql:1133
error: src/backend/catalog/information_schema.sql: patch does not apply
error: patch failed: src/backend/utils/adt/ruleutils.c:2266
error: src/backend/utils/adt/ruleutils.c: patch does not apply
error: patch failed: src/include/catalog/catversion.h:53
error: src/include/catalog/catversion.h: patch does not apply
error: patch failed: src/include/catalog/pg_proc.h:1973
error: src/include/catalog/pg_proc.h: patch does not apply
error: patch failed: src/include/utils/builtins.h:665
error: src/include/utils/builtins.h: patch does not apply
error: patch failed: src/test/regress/expected/create_function_3.out:425
error: src/test/regress/expected/create_function_3.out: patch does not apply
error: patch failed: src/test/regress/sql/create_function_3.sql:138
error: src/test/regress/sql/create_function_3.sql: patch does not apply

Re: [HACKERS] Data corruption issues using streaming replication on 9.0.14/9.2.5/9.3.1

[Andres Freund]

On 2013-11-20 16:36:46 -0800, Christophe Pettus wrote:
> 
> On Nov 20, 2013, at 3:57 PM, Andres Freund  wrote:
> 
> > On 2013-11-20 15:52:22 -0800, Josh Berkus wrote:
> >> Oh, so this doesn't just happen when the base backup is first taken;
> >> *any* time the standby is restarted, it can happen. (!!!)
> > 
> > Yes.
> 
> So, to be completely clear, any secondary running the affected
> versions which is started with hot_standby=on could potentially be
> corrupted even if it never connects to a primary?

Yes.

Greetings,

Andres Freund

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


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

Re: [HACKERS] Can we trust fsync?

[Tatsuo Ishii]

> On 11/21/2013 07:45 AM, Craig Ringer wrote:
>> I'm really concerned by this post on Linux's fsync and disk flush behaviour:
>> 
>> http://milek.blogspot.com.au/2010/12/linux-osync-and-write-barriers.html
> 
> ... and yes, I realise that's partly why we have the "fsync" param to
> control different sync modes. Just concerned it's even more variable
> than I thought.

So on linux, we don't have any safe option for wal_sync_method?
--
Tatsuo Ishii
SRA OSS, Inc. Japan
English: http://www.sraoss.co.jp/index_en.php
Japanese: http://www.sraoss.co.jp


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

Re: [HACKERS] Data corruption issues using streaming replication on 9.0.14/9.2.5/9.3.1

[Christophe Pettus]

On Nov 20, 2013, at 3:57 PM, Andres Freund  wrote:

> On 2013-11-20 15:52:22 -0800, Josh Berkus wrote:
>> Oh, so this doesn't just happen when the base backup is first taken;
>> *any* time the standby is restarted, it can happen. (!!!)
> 
> Yes.

So, to be completely clear, any secondary running the affected versions which 
is started with hot_standby=on could potentially be corrupted even if it never 
connects to a primary?

--
-- Christophe Pettus
   x...@thebuild.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] Can we trust fsync?

[Craig Ringer]

On 11/21/2013 07:45 AM, Craig Ringer wrote:
> I'm really concerned by this post on Linux's fsync and disk flush behaviour:
> 
> http://milek.blogspot.com.au/2010/12/linux-osync-and-write-barriers.html

... and yes, I realise that's partly why we have the "fsync" param to
control different sync modes. Just concerned it's even more variable
than I thought.


-- 
 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] Easily reading debug_print_plan

[Craig Ringer]

On 11/20/2013 10:36 PM, Tom Lane wrote:
> Craig Ringer  writes:
>> I'm spending a lot of time staring at parse and plan trees at the
>> moment, and I'm finding reading them rather cumbersome.
> 
> Is there a particular reason you're doing that rather than looking at
> EXPLAIN output?  Only the latter is meant to be at all user-friendly.

Because I'm working on updatable security_barrier views using the
approach outlined to the list earlier. EXPLAIN really doesn't do the
trick for working on the guts of the rewriter.

>> The same representation is used for storing rules. So it can't be
>> changed for BC reasons and compactness/performance.
> 
> We could in principle change to a different text representation for
> stored rules.  Compactness would be an issue if it were materially
> bigger than the existing formatting, but offhand it seems like JSON
> is morally equivalent to what we do now, no?
> 
> If you think this is worthwhile, you might care to take a look at
> outfuncs.c/readfuncs.c and figure out what it'd take to switch to
> json-compatible formatting.

I do think it might be worthwhile at some point, but once I remembered
it was about more than just debug_print_ output - that DB performance is
impacted - I realised it was not a topic for a quick and simple change.
Benchmarking required, etc.


-- 
 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] Data corruption issues using streaming replication on 9.0.14/9.2.5/9.3.1

[Andres Freund]

On 2013-11-20 15:52:22 -0800, Josh Berkus wrote:
> Andres,
> 
> > Everytime the server in HS mode allows connections ("consistent recovery 
> > state
> > reached at ..." and "database system is ready to accept read only
> > connections" in the log), the bug can be triggered. If there weren't too
> > many transactions at that point, the problem won't occur until the
> > standby is restarted.
> 
> Oh, so this doesn't just happen when the base backup is first taken;
> *any* time the standby is restarted, it can happen. (!!!)

Yes.

> If you have any ideas for how we'd write code to scan for this kind of
> corruption, please post them.

I don't really have one. Current corruption would be somewhat easy to
detect (walk through the clog, check if all commit bits match), but that
doesn't detect wether already truncated clog was corrupted.

Greetings,

Andres Freund

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


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

Re: [HACKERS] Data corruption issues using streaming replication on 9.0.14/9.2.5/9.3.1

[Josh Berkus]

Andres,

> Everytime the server in HS mode allows connections ("consistent recovery state
> reached at ..." and "database system is ready to accept read only
> connections" in the log), the bug can be triggered. If there weren't too
> many transactions at that point, the problem won't occur until the
> standby is restarted.

Oh, so this doesn't just happen when the base backup is first taken;
*any* time the standby is restarted, it can happen. (!!!)

>> If someone is doing PITR based on a snapshot taken with pg_basebackup,
>> that will only trip this corruption bug if the user has hot_standby=on
>> in their config *while restoring*?  Or is it critical if they have
>> hot_standby=on while backing up?
> 
> hot_standby=on only has an effect while starting up with a recovery.conf
> present. So, if you have an old base backup around and all WAL files,
> you can start from that.
> 
> Does that answer your questsions?

Yeah, thanks.

If you have any ideas for how we'd write code to scan for this kind of
corruption, please post them.


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

[HACKERS] Can we trust fsync?

[Craig Ringer]

I'm really concerned by this post on Linux's fsync and disk flush behaviour:

http://milek.blogspot.com.au/2010/12/linux-osync-and-write-barriers.html

and seeking opinions from folks here who've been deeply involved in
write reliability work.

The amount of change in write reliablity behaviour in Linux across
kernel versions, file systems and storage abstraction layers is worrying
- different results for LVM vs !LVM, md vs !md, ext3 vs other, etc.

If this isn't something that's already been seen and dealt with then
I'll see if I can take a look into it once the RLS work is dealt with.

-- 
 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] Data corruption issues using streaming replication on 9.0.14/9.2.5/9.3.1

[Andres Freund]

On 2013-11-20 10:48:41 -0800, Josh Berkus wrote:
> > Presumably a replica created while all traffic was halted on the master
> > would be clean, correct?  This bug can only be triggered if there's
> > heavy write load on the master, right?

Kinda. It's unfortunately necessary to understand how HS works to some
degree:
Everytime a server is (re-)started with a recovery.conf present and
hot_standby=on (be it streaming, archive based replication or PITR) the
Hot Standby code is used.
(Crash|Replication)-Recovery starts by reading the last checkpoint (from
pg_control or, if present, backup.label) and then replays WAL from the
'redo' point included in the checkpoint. The bug then occurs when it
first (or, in some case second time) replays a 'xl_running_xacts'
record. That's used to reconstruct information needed to allow queries.

Everytime the server in HS mode allows connections ("consistent recovery state
reached at ..." and "database system is ready to accept read only
connections" in the log), the bug can be triggered. If there weren't too
many transactions at that point, the problem won't occur until the
standby is restarted.

> If someone is doing PITR based on a snapshot taken with pg_basebackup,
> that will only trip this corruption bug if the user has hot_standby=on
> in their config *while restoring*?  Or is it critical if they have
> hot_standby=on while backing up?

hot_standby=on only has an effect while starting up with a recovery.conf
present. So, if you have an old base backup around and all WAL files,
you can start from that.

Does that answer your questsions?

Greetings,

Andres Freund

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


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

Re: [HACKERS] Storing pg_stat_statements query texts externally, pg_stat_statements in core

[Peter Geoghegan]

With a pg_stat_statements.max of 1,000, on my system with the patch
applied the additional amount of shared memory required is only
192KiB. That compares with about 1.17MiB for the same setting in
master's version of pg_stat_statements. Surely with this huge
reduction, we should revisit that default - I think that a default
setting of 5,000 for pg_stat_statements.max makes more sense.

entry_dealloc() requires an exclusive lock, locking all queries out of
simply recording their costs. With a lot of cache pressure this could
be really expensive. In my opinion that risk alone justifies the
change.

Without adding another GUC, we might even go so far as to have a
mechanism like checkpoint_warning warn that entry_dealloc() calls
occur too frequently...

-- 
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] Storage formats for JSON WAS: additional json functionality

[Andres Freund]

On 2013-11-20 10:01:28 -0800, Josh Berkus wrote:
> Greg,
> 
> > Not being super familiar with either BSON our JSONB what advantages are we
> > gaining from the difference?
> 
> We have the JSONB/Hstore2 format *now*, and it can go into 9.4.

That patch needs a *fair* amount of work though.

Greetings,

Andres Freund

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


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

Re: [HACKERS] Proof of concept: standalone backend with full FE/BE protocol

[Andres Freund]

On 2013-11-20 15:44:03 -0500, Tom Lane wrote:
> In practice, as long as psql and pg_dump and pg_upgrade can do it, I
> think we've covered most of the interesting bases.

I'd say vacuumdb/reindexdb should be added to that list. In my
experience xid wraparound and corrupted system indexes are the most
frequent use-case of single user mode.

Greetings,

Andres Freund

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


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

Re: [HACKERS] WITH ORDINALITY versus column definition lists

[Andrew Gierth]

> "Tom" == Tom Lane  writes:

 Tom> 1. Reinsert HEAD's prohibition against directly combining WITH
 Tom> ORDINALITY with a coldeflist (with a better error message and a
 Tom> HINT suggesting that you can get what you want via the TABLE
 Tom> syntax).

That gets my vote.

-- 
Andrew (irc:RhodiumToad)


-- 
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] Proof of concept: standalone backend with full FE/BE protocol

[Gurjeet Singh]

On Wed, Nov 20, 2013 at 3:44 PM, Tom Lane  wrote:

>
> To my mind, the "create a socket and hope nobody else can get to it"
> approach is exactly one of the main things we're trying to avoid here.
> If you'll recall, awhile back we had a big discussion about how pg_upgrade
> could positively guarantee that nobody messed with the source database
> while it was working, and we still don't have a bulletproof guarantee
> there.  I would like to fix that by making pg_upgrade use only standalone
> backends to talk to the source database, never starting a real postmaster
> at all.  But if the standalone-pg_dump mode goes through a socket, we're
> back to square one on that concern.
>

(I couldn't find the pg_upgrade-related thread mentioned above).

I am not sure of the mechanics of this, but can we not launch the
postmaster with a random magic-cookie, and use that cookie while initiating
the connection from libpq. The postmaster will then reject any connections
that don't provide the cookie.

We do something similar to enable applications to send cancellation signals
(postmaster.c:Backend.cancel_key), just that it's establishing trust in the
opposite direction.

Best regards,
-- 
Gurjeet Singh http://gurjeet.singh.im/

EnterprsieDB Inc. www.enterprisedb.com

Re: [HACKERS] additional json functionality

[Robert Haas]

On Wed, Nov 20, 2013 at 12:50 PM, Greg Stark  wrote:
>> For one thing, our storage format is different from theirs (better,
>> frankly), and as a result is not compliant with their "standard".
>
> Not being super familiar with either BSON our JSONB what advantages are we
> gaining from the difference?

BSON assumes, for example, that all integers fit in 64-bits and all
floating point values can be accurately represented as float8.  So not
all JSON objects can be represented as BSON without loss of
information.

BSON also adds a bunch of extra types that are not part of JSON, like
timestamps, regular expressions, and embedded documents.  So not all
BSON objects can be represented as JSON without loss of information.

While it's tempting to think that BSON is a serialization format for
JSON, and the name is meant to suggest that, it really isn't.  It's
just a serialization format for approximately whatever the authors
thought would be useful, which happens to be kinda like JSON.  Sorta.

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


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

Re: [HACKERS] Suggestion: Issue warning when calling SET TRANSACTION outside transaction block

[Kevin Grittner]

Robert Haas  wrote:

> Well, Tom and I are on opposite sides of this, I suppose.  I
> prefer ERROR for everything other than the top-level transaction
> commands, and see no benefit from opting for a wishy-washy
> warning.

+1

If the user issued a local command outside of a transaction there
is an extremely high probability that they intended to either set
session state or affect the next transaction.  Either way,
modifying the database with the wrong setting could lead to data
corruption -- at least for some of these settings.  IMO it would be
foolish to insist on consistency with prior releases rather than on
giving people the best shot at preventing, or at least promptly
identifying the cause of, data corruption.

Obviously, changing the level of these is not material for back-
patching.

--
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] [PATCH] Add transforms feature

[Robert Haas]

On Wed, Nov 20, 2013 at 11:51 AM, Peter Eisentraut  wrote:
> This is a transition problem.  Nobody is required to install the
> transforms into their existing databases.  They probably shouldn't.

Sure, but that's like saying "nobody's required to use this
behavior-changing GUC, so it's OK to have a behavior-changing GUC".

The point I think Dimitri is making, which IMHO is entirely valid, is
that the feature as currently designed is database-wide.  You either
get this behavior for all of your functions, or you get it for none of
them, and that might well not be what you want.  For example, it's
easy to imagine that you might want to install extensions A and B.  A
expects that a certain transform is loaded into the database, and B
expects that it isn't.  You now have created a situation where
extensions A and B can't be used together.  That sucks.

If the transform were a property of particular function argument
positions, this wouldn't be a problem.  You could declare, in effect,
that a certain function takes a transformed hstore, and this other one
takes a non-transformed hstore.  Now life is good.  But that's not
what is being proposed.

You could argue that such a level of granularity is overkill, but
frankly I've never had a real good feeling about the likelihood of
these transforms getting installed in the first place.  In theory, if
you're using hstore and you're using plperl, you ought to also install
hstore-plperl-transform, but I bet a significant percentage of people
won't.  So I don't foresee a finite transition period after which
databases without transforms go away and all code is written with the
assumption that transforms are available; instead, I foresee different
people assuming different things and ending up with mutually
incompatible code bases.

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


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

Re: [HACKERS] -d option for pg_isready is broken

[Robert Haas]

On Wed, Nov 20, 2013 at 4:55 AM, Fujii Masao  wrote:
>> Not that I can see, but it's not very future-proof.  If libpq changes
>> its idea of what will provoke database-name expansion, this will again
>> be broken.
>
> Yeah, I agree that we should make the logic of pg_isready more future-proof
> in 9.4dev. One idea is to expose internal_ping() as a libpq function. Then,
> instead of just calling PQpingParams(), we can do PQconnectStartParams(),
> libpq-function-version-of-internal_ping(), PQhost(), PQport(), and PQfinish().
> That is, we get to know the host and port information by calling
> PQhost() and PQport(),
> after trying to connect to the server.

Hmm, that sounds like a possibly promising approach.

> But we cannot use this idea in 9.3 because it's too late to add new
> libpq function there.
> Also I don't think that the minor version release would change that
> libpq's logic in 9.3.
> So, barring any objections, I will commit the latest version of the
> patch in 9.3.

I think you should commit it to both master and REL9_3_STABLE.  Then,
you can make further changes to master in a subsequent commit.

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


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

Re: [HACKERS] Suggestion: Issue warning when calling SET TRANSACTION outside transaction block

[Bruce Momjian]

On Wed, Nov 20, 2013 at 04:31:12PM -0500, Robert Haas wrote:
> On Wed, Nov 20, 2013 at 4:19 PM, Bruce Momjian  wrote:
> > On Wed, Nov 20, 2013 at 10:16:00AM -0500, Bruce Momjian wrote:
> >> > > The attached patch changes ABORT from NOTICE to WARNING, and documents
> >> > > that all other are errors.  This "top-level" logic idea came from 
> >> > > Robert
> >> > > Haas, and it has some level of consistency.
> >> >
> >> > This patch utterly fails to address my complaint.
> >> >
> >> > More to the point, I think it's a waste of time to make these sorts of
> >> > adjustments.  The only thanks you're likely to get for it is complaints
> >> > about cross-version behavioral changes.  Also, you're totally ignoring
> >> > the thought that these different message levels might've been selected
> >> > intentionally, back when the code was written.  Since there have been
> >> > no field complaints about the inconsistency, what's your hurry to
> >> > change it?  See Emerson.
> >>
> >> My problem was that they issued _no_ message at all.  I am fine with
> >> them issuing a warning if that's what people prefer.  As they are all
> >> SET commands, they will be consistent.
> >
> > OK, here is a patch which changes ABORT from NOTICE to WARNING, and SET
> > from ERROR (which is new in 9.4) to WARNING.
> 
> Well, Tom and I are on opposite sides of this, I suppose.  I prefer
> ERROR for everything other than the top-level transaction commands,
> and see no benefit from opting for a wishy-washy warning.

Well, the only way I can process this is to think of psql with
ON_ERROR_STOP enabled.  Would we want a no-op command to abort psql?  I
can see logic that top-level transaction commands and SET to not, but
other commands do.  I can also see them all aborting psql, or none of
them.  :-(

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

  + Everyone has their own god. +


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

Re: [HACKERS] Autoconf 2.69 update

[Robert Haas]

On Wed, Nov 20, 2013 at 4:31 AM, Oskari Saarenmaa  wrote:
> ISTM autoconf has been better with backwards compatibility lately. Maybe the
> fatal error could be changed to a warning and/or the check for version ==
> 2.63 be replaced with a check for version >= 2.63?  Without a strict
> requirement for a certain autoconf version it would make sense to also drop
> the built autoconf artifacts from the git repository which would make diffs
> shorter and easier to review when touching configure.in.

-1 from me.  Dropping configure from the repository would
significantly increase the burden to compile and install PostgreSQL
from source.  Not everyone has autoconf installed.

-1 also for loosening the version check.  I guarantee that if we do
that, people will use varying versions when regenerating configure,
and we'll have a mess.  Even if we standardize the version committers
are supposed to use, someone will foul it up at least twice a year.
The last thing I need is to have more things that I can accidentally
screw up while committing; the list is too long already.

I realize that those checks are a bit of a nuisance, but if they
bother you you can just whack them out locally and proceed.  Or else
you can download and compile the right version of autoconf.  If you're
doing sufficiently serious development that you need to touch
configure.in, the 5 minutes it takes you to build a local install of
the right autoconf version should be the least of your concerns.  It's
not hard; I've done it multiple times, and have multiple versions of
autoconf installed on those systems where I need to be able to re-run
autoconf on any supported branch.

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


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

Re: [HACKERS] Suggestion: Issue warning when calling SET TRANSACTION outside transaction block

[Robert Haas]

On Wed, Nov 20, 2013 at 4:19 PM, Bruce Momjian  wrote:
> On Wed, Nov 20, 2013 at 10:16:00AM -0500, Bruce Momjian wrote:
>> > > The attached patch changes ABORT from NOTICE to WARNING, and documents
>> > > that all other are errors.  This "top-level" logic idea came from Robert
>> > > Haas, and it has some level of consistency.
>> >
>> > This patch utterly fails to address my complaint.
>> >
>> > More to the point, I think it's a waste of time to make these sorts of
>> > adjustments.  The only thanks you're likely to get for it is complaints
>> > about cross-version behavioral changes.  Also, you're totally ignoring
>> > the thought that these different message levels might've been selected
>> > intentionally, back when the code was written.  Since there have been
>> > no field complaints about the inconsistency, what's your hurry to
>> > change it?  See Emerson.
>>
>> My problem was that they issued _no_ message at all.  I am fine with
>> them issuing a warning if that's what people prefer.  As they are all
>> SET commands, they will be consistent.
>
> OK, here is a patch which changes ABORT from NOTICE to WARNING, and SET
> from ERROR (which is new in 9.4) to WARNING.

Well, Tom and I are on opposite sides of this, I suppose.  I prefer
ERROR for everything other than the top-level transaction commands,
and see no benefit from opting for a wishy-washy warning.

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


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

Re: [HACKERS] Suggestion: Issue warning when calling SET TRANSACTION outside transaction block

[Bruce Momjian]

On Wed, Nov 20, 2013 at 10:16:00AM -0500, Bruce Momjian wrote:
> > > The attached patch changes ABORT from NOTICE to WARNING, and documents
> > > that all other are errors.  This "top-level" logic idea came from Robert
> > > Haas, and it has some level of consistency.
> > 
> > This patch utterly fails to address my complaint.
> > 
> > More to the point, I think it's a waste of time to make these sorts of
> > adjustments.  The only thanks you're likely to get for it is complaints
> > about cross-version behavioral changes.  Also, you're totally ignoring
> > the thought that these different message levels might've been selected
> > intentionally, back when the code was written.  Since there have been
> > no field complaints about the inconsistency, what's your hurry to
> > change it?  See Emerson.
> 
> My problem was that they issued _no_ message at all.  I am fine with
> them issuing a warning if that's what people prefer.  As they are all
> SET commands, they will be consistent.

OK, here is a patch which changes ABORT from NOTICE to WARNING, and SET
from ERROR (which is new in 9.4) to WARNING.

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

  + Everyone has their own god. +
diff --git a/doc/src/sgml/ref/abort.sgml b/doc/src/sgml/ref/abort.sgml
new file mode 100644
index 246e8f8..f3a2fa8
*** a/doc/src/sgml/ref/abort.sgml
--- b/doc/src/sgml/ref/abort.sgml
*** ABORT [ WORK | TRANSACTION ]
*** 63,70 

  

!Issuing ABORT when not inside a transaction does
!no harm, but it will provoke a warning message.

   
  
--- 63,69 

  

!Issuing ABORT outside of a transaction block has no effect.

   
  
diff --git a/doc/src/sgml/ref/rollback.sgml b/doc/src/sgml/ref/rollback.sgml
new file mode 100644
index b265545..4f79621
*** a/doc/src/sgml/ref/rollback.sgml
--- b/doc/src/sgml/ref/rollback.sgml
*** ROLLBACK [ WORK | TRANSACTION ]
*** 59,66 

  

!Issuing ROLLBACK when not inside a transaction does
!no harm, but it will provoke a warning message.

   
  
--- 59,66 

  

!Issuing ROLLBACK outside of a transaction
!block has no effect.

   
  
diff --git a/doc/src/sgml/ref/set.sgml b/doc/src/sgml/ref/set.sgml
new file mode 100644
index 6290c9d..5a84f69
*** a/doc/src/sgml/ref/set.sgml
--- b/doc/src/sgml/ref/set.sgml
*** SET [ SESSION | LOCAL ] TIME ZONE { 
Specifies that the command takes effect for only the current
transaction.  After COMMIT or ROLLBACK,
!   the session-level setting takes effect again.
!   PostgreSQL reports an error if
!   SET LOCAL is used outside a transaction block.
   
  
 
--- 110,117 
   
Specifies that the command takes effect for only the current
transaction.  After COMMIT or ROLLBACK,
!   the session-level setting takes effect again.  This has no effect
!   outside of a transaction block.
   
  
 
diff --git a/doc/src/sgml/ref/set_constraints.sgml b/doc/src/sgml/ref/set_constraints.sgml
new file mode 100644
index 895a5fd..a33190c
*** a/doc/src/sgml/ref/set_constraints.sgml
--- b/doc/src/sgml/ref/set_constraints.sgml
*** SET CONSTRAINTS { ALL | 
 This command only alters the behavior of constraints within the
!current transaction. Thus, if you execute this command outside of a
!transaction block
!(BEGIN/COMMIT pair), it will
!generate an error.

   
  
--- 99,105 
  

 This command only alters the behavior of constraints within the
!current transaction.  This has no effect outside of a transaction block.

   
  
diff --git a/doc/src/sgml/ref/set_transaction.sgml b/doc/src/sgml/ref/set_transaction.sgml
new file mode 100644
index 391464a..e90ff4a
*** a/doc/src/sgml/ref/set_transaction.sgml
--- b/doc/src/sgml/ref/set_transaction.sgml
*** SET SESSION CHARACTERISTICS AS TRANSACTI
*** 185,191 

 If SET TRANSACTION is executed without a prior
 START TRANSACTION or BEGIN,
!it will generate an error.

  

--- 185,191 

 If SET TRANSACTION is executed without a prior
 START TRANSACTION or BEGIN,
!it will have no effect.

  

diff --git a/src/backend/access/transam/xact.c b/src/backend/access/transam/xact.c
new file mode 100644
index 0591f3f..2cdcc98
*** a/src/backend/access/transam/xact.c
--- b/src/backend/access/transam/xact.c
*** PreventTransactionChain(bool isTopLevel,
*** 2961,2972 
   *	use of the current statement's results.  Likewise subtransactions.
   *	Thus this is an inverse for PreventTransactionChain.
   *
   *	isTopLevel: passed down from ProcessUtility to determine whether we are
   *	inside a function.
   *	stmtType: statement type name, for error messages.
   */
  void
! RequireTransactionChain(bool isTopLevel, const char *stmtType)
  {
  	/*
  	 * xact block already started?

Re: [HACKERS] WITH ORDINALITY versus column definition lists

[David Johnston]

Tom Lane-2 wrote
> David Johnston <

> polobo@

> > writes:
>> Just to clarify we are still allowing simple aliasing:
> 
>> select * from generate_series(1,2) with ordinality as t(f1,f2); 
> 
> Right, that works (and is required by spec, I believe).  It's what to
> do with our column-definition-list extension that's at issue.
> 
>> Not sure if this is possible at this point but really the alias for the
>> ordinality column would be attached directly to the ordinality keyword.
> 
>> e.g., ...) with ordinality{alias} as t(a1, a2)
> 
> This has no support in the standard.

Now I'm just spinning some thoughts:

) with ordinality AS t(a1 text, a2 text | ord1)  -- type-less, but a
different separator

) with ordinality AS t(a1 text, a2 text)(ord1) -- stick it in its own
section, type-less

) with ordinality AS t(a1 text, a2 text) ordinal(ord1) --name the section
too

would probably want to extend the alias syntax to match...

Is there any precedent in other RDBMS to consider?

I don't see any obvious alternatives to the ones you listed and syntax is
really not a huge barrier.  If the implementation of an optionally specified
alias is a barrier then either someone needs to feel strongly enough to
implement it or just default to #1 for the time being.

But others really haven't had a chance to read and respond yet so I'm gonna
get off this train for a while.


David J.





--
View this message in context: 
http://postgresql.1045698.n5.nabble.com/WITH-ORDINALITY-versus-column-definition-lists-tp5779443p5779473.html
Sent from the PostgreSQL - hackers mailing list archive at Nabble.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] WITH ORDINALITY versus column definition lists

[Tom Lane]

David Johnston  writes:
> Just to clarify we are still allowing simple aliasing:

> select * from generate_series(1,2) with ordinality as t(f1,f2); 

Right, that works (and is required by spec, I believe).  It's what to
do with our column-definition-list extension that's at issue.

> Not sure if this is possible at this point but really the alias for the
> ordinality column would be attached directly to the ordinality keyword.

> e.g., ...) with ordinality{alias} as t(a1, a2)

This has no support in the standard.

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] Proof of concept: standalone backend with full FE/BE protocol

[Tom Lane]

Peter Eisentraut  writes:
> The argument elsewhere in this thread was that the reason for putting
> this in the connection options was so that you do *not* have to patch up
> every client to be able to use this functionality.  If you have to add
> separate options everywhere, then you might as well just have a separate
> libpq function to initiate the session.

Right, Andres was saying that we had to do both (special switches that
lead to calling a special connection function).  I'm not terribly happy
about that, because it will greatly constrain the set of programs that are
able to connect to standalone backends --- but I think that there are some
in this discussion who want that, anyway.  In practice, as long as psql
and pg_dump and pg_upgrade can do it, I think we've covered most of the
interesting bases.

To my mind, the "create a socket and hope nobody else can get to it"
approach is exactly one of the main things we're trying to avoid here.
If you'll recall, awhile back we had a big discussion about how pg_upgrade
could positively guarantee that nobody messed with the source database
while it was working, and we still don't have a bulletproof guarantee
there.  I would like to fix that by making pg_upgrade use only standalone
backends to talk to the source database, never starting a real postmaster
at all.  But if the standalone-pg_dump mode goes through a socket, we're
back to square one on that concern.

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] WITH ORDINALITY versus column definition lists

[David Johnston]

Tom Lane-2 wrote
> David Johnston <

> polobo@

> > writes:
>> Tom Lane-2 wrote
>>> It seems to me that we don't really want this behavior of the coldeflist
>>> not including the ordinality column.  It's operating as designed, maybe,
>>> but it's unexpected and confusing.  We could either
>>> 
>>> 1. Reinsert HEAD's prohibition against directly combining WITH
>>> ORDINALITY
>>> with a coldeflist (with a better error message and a HINT suggesting
>>> that
>>> you can get what you want via the TABLE syntax).
>>> 
>>> 2. Change the parser so that the coldeflist is considered to include the
>>> ordinality column, for consistency with the bare-alias case.  We'd
>>> therefore insist that the last coldeflist item be declared as int8, and
>>> then probably have to strip it out internally.
> 
>> Two options I came up with:
> 
>> 1) disallow any type specifier on the last item:  t(f1 int, f2 text, o1)
>> 2) add a new pseudo-type, "ord":  t(f1 int, f2 text, o1 ord)
> 
>> I really like option #2.
> 
> I don't.  Pseudo-types have a whole lot of baggage.  #1 is a mess too.
> And in either case, making coldef list items optional increases the number
> of ways to make a mistake, if you accidentally omit some other column for
> instance.

I'll have to trust on the baggage/mess conclusion but if you can distinctly
and un-ambigiously identify the coldeflist item that is to be used for
ordinality column aliasing then the mistakes related to the
function-record-coldeflist are the same as now.  There may be more (be still
quite few I would think) ways for the user to make a mistake but the syntax
ones are handled anyway and so if the others can be handled reasonably well
the UI for the feature becomes more friendly.

IOW, instead of adding int8 and ignoring it we poll the last item,
conditionally discard it (like the int8 case), then handle the possibly
modified structure as planned.


> Basically the problem here is that it's not immediately obvious whether
> the coldef list ought to include the ordinality column or not.  The user
> would probably guess not (since the system knows what type ordinality
> should be).  

Yes, if the column is not made optional somehow then I dislike option #2


> The TABLE syntax is really a vastly better solution for this.  So I'm
> thinking my #1 is the best answer, assuming we can come up with a good
> error message.  My first attempt would be
> 
> ERROR: WITH ORDINALITY cannot be used with a column definition list
> HINT: Put the function's column definition list inside TABLE() syntax.
> 
> Better ideas?

Works for me if #1 is implemented.



Just to clarify we are still allowing simple aliasing:

select * from generate_series(1,2) with ordinality as t(f1,f2); 

Its only when the output of the function is "record" does the restriction of
placing the record-returning function call into TABLE (if you want ordinals)
come into play.


select * from table(array_to_set(array['one', 'two']) as (f1 int,f2 text))
with ordinality as t(a1,a2,a3); 

If we could do away with having to re-specify the record-aliases in the
outer layer (a1, a2) then I'd be more understanding but I'm thinking that is
not possible unless you force a single-column alias definition attached to
WITH ORDINALITY to mean alias the ordinality column only.


On the plus side: anyone using record-returning functions is already dealing
with considerable verbosity so this extra bit doesn't seem to be adding that
much overhead; and since the alias - t(a1,a2,a3) - is optional if you don't
care about aliasing the with ordinal column the default case is not that
verbose (just add the surrounding TABLE).

I feel like I need a flow-chart for #1...

With #2 (w/ optional) you can add in an alias for the ordinality column
anyplace you would be specifying a coldeflist OR alias list.  Favoring the
pseudo-type solution is the fact that given the prior sentence if you place
"o1 ord" in the wrong place it is possible to generate an error like "with
ordinality not present for aliasing".

#1 is simpler to implement and does not preclude #2 in the future.

Possible #3?

Not sure if this is possible at this point but really the alias for the
ordinality column would be attached directly to the ordinality keyword.

e.g., ...) with ordinality{alias} as t(a1, a2)

David J.








--
View this message in context: 
http://postgresql.1045698.n5.nabble.com/WITH-ORDINALITY-versus-column-definition-lists-tp5779443p5779468.html
Sent from the PostgreSQL - hackers mailing list archive at Nabble.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] Proof of concept: standalone backend with full FE/BE protocol

[Robert Haas]

On Wed, Nov 20, 2013 at 3:32 PM, Peter Eisentraut  wrote:
> On 11/20/13, 3:24 PM, Robert Haas wrote:
>> The point is that client applications should expose whether or not to
>> set this function as a command-line switch separate from whatever they
>> accept in terms of connection strings.  So pg_dump should have a flag
>> called --standalone-server or something like, and it should all
>> PQenableStartServer() only when that flag is used.
>
> The argument elsewhere in this thread was that the reason for putting
> this in the connection options was so that you do *not* have to patch up
> every client to be able to use this functionality.  If you have to add
> separate options everywhere, then you might as well just have a separate
> libpq function to initiate the session.

Well, that's fair enough.  I don't care much what the syntax is for
invoking the postmaster this way, as long as it's reasonably
convenient.  I just want there to be one.

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


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

Re: [HACKERS] Easily reading debug_print_plan

[Robert Haas]

On Wed, Nov 20, 2013 at 9:36 AM, Tom Lane  wrote:
> We could in principle change to a different text representation for
> stored rules.  Compactness would be an issue if it were materially
> bigger than the existing formatting, but offhand it seems like JSON
> is morally equivalent to what we do now, no?

Yeah, but it gains a little.

{FROB :zot 3}
would become something like
{"type": "FROB", "zot": 3}

You could minimize the damage by using a single character name, like
an underscore, for the node type, and emitting all whitespace:

{"_":"FROB","zot":3}

...but it's still more.  Possibly not enough to matter, but more.

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


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

Re: [HACKERS] Proof of concept: standalone backend with full FE/BE protocol

[Peter Eisentraut]

On 11/20/13, 3:24 PM, Robert Haas wrote:
> The point is that client applications should expose whether or not to
> set this function as a command-line switch separate from whatever they
> accept in terms of connection strings.  So pg_dump should have a flag
> called --standalone-server or something like, and it should all
> PQenableStartServer() only when that flag is used.

The argument elsewhere in this thread was that the reason for putting
this in the connection options was so that you do *not* have to patch up
every client to be able to use this functionality.  If you have to add
separate options everywhere, then you might as well just have a separate
libpq function to initiate the session.


-- 
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] Proof of concept: standalone backend with full FE/BE protocol

[Robert Haas]

On Wed, Nov 20, 2013 at 10:13 AM, Peter Eisentraut  wrote:
> On 11/14/13, 1:41 AM, Amit Kapila wrote:
>>Security Concern
>>-
>>If a user can specify libpq  connection options, he can now execute
>> any file he wants by passing it as standalone_backend.
>>
>>Method to resolve Security concern
>>
>>If an application wants to allow these connection parameters to be
>> used, it would need to do PQenableStartServer() first. If it doesn't,
>> those connection parameters will be rejected.
>
> I don't think this really helps.  You can't tell with reasonable effort
> or certainty whether a given program is calling PQenableStartServer(),
> so you cannot audit this from the outside.  Also, someone could,
> depending on circumstances, dynamically load a module that calls
> PQenableStartServer(), thus circumventing this check.

What?!  The complaint is that somebody who only has access to set
connection parameters could cause a server to be started.  There's a
tremendous gulf between "I can set the connection string" and "I can
set LD_PRELOAD".  If you can set LD_PRELOAD to a value of your choice,
I'm pretty sure you can do things that are far more entertaining than
calling a hypothetical PQenableStartServer() function.

> And maybe before
> long someone will patch up all drivers to call PQenableStartServer()
> automatically, because why shouldn't I be able to run a standalone
> backend from PHP or Ruby?  Also, at some point at least, something like
> phpPgAdmin called pg_dump internally, so you could imagine that in
> situations like that, assuming that pg_dump called
> PQenableStartServer(), with a little bit craftiness, you could expose
> the execute-any-file hole through a web server.

The point is that client applications should expose whether or not to
set this function as a command-line switch separate from whatever they
accept in terms of connection strings.  So pg_dump should have a flag
called --standalone-server or something like, and it should all
PQenableStartServer() only when that flag is used.  So if the user has
a shell script that invokes pg_dump -d "$1", the user cannot contrive
a server.  If they write the script as pg_dump --standalone-server -d
"$1", then they can, but by putting that option in there you pretty
much bought the farm.  Any program that calls that function
unconditionally while at the same time accepting untrusted user input
will be insecure, but chmod -R u+s /bin is insecure, too.  That's why
we don't do that.

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


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

Re: [HACKERS] Proof of concept: standalone backend with full FE/BE protocol

[Peter Eisentraut]

On 11/20/13, 10:48 AM, Tom Lane wrote:
> Perhaps more to the point, I think this approach actually breaks one of
> the principal good-thing-in-emergencies attributes of standalone mode,
> namely being sure that nobody but you can connect.  With this, you're
> right back to having a race condition as to whether your psql command
> gets to the socket before somebody else.

I don't disagree, except maybe about the relative gravity of the various
competing concerns.  But I want to see if we can split the proposed
patch into smaller, more acceptable parts.

There is elegance in being able to start a standalone backend from libpq
connection parameters.  But there are also security concerns and some
general concerns about promoting an embedded database mode.

If we allow single-user backends to speak protocol over sockets, then we
have at least solved the problem of being able to use standard tools in
emergency mode.  And I don't think it precludes adding some of the other
functionality later.


-- 
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] GIN improvements part2: fast scan

[Alexander Korotkov]

On Wed, Nov 20, 2013 at 3:06 AM, Alexander Korotkov wrote:

> On Fri, Nov 15, 2013 at 11:19 AM, Alexander Korotkov  > wrote:
>
>> On Fri, Nov 15, 2013 at 12:34 AM, Heikki Linnakangas <
>> hlinnakan...@vmware.com> wrote:
>>
>>> On 14.11.2013 19:26, Alexander Korotkov wrote:
>>>
 On Sun, Jun 30, 2013 at 3:00 PM, Heikki Linnakangas <
 hlinnakan...@vmware.com

> wrote:
>

  On 28.06.2013 22:31, Alexander Korotkov wrote:
>
>  Now, I got the point of three state consistent: we can keep only one
>> consistent in opclasses that support new interface. exact true and
>> exact
>> false values will be passed in the case of current patch consistent;
>> exact
>> false and unknown will be passed in the case of current patch
>> preConsistent. That's reasonable.
>>
>>
> I'm going to mark this as "returned with feedback". For the next
> version,
> I'd like to see the API changed per above. Also, I'd like us to do
> something about the tidbitmap overhead, as a separate patch before
> this, so
> that we can assess the actual benefit of this patch. And a new test
> case
> that demonstrates the I/O benefits.
>


 Revised version of patch is attached.
 Changes are so:
 1) Patch rebased against packed posting lists, not depends on additional
 information now.
 2) New API with tri-state logic is introduced.

>>>
>>> Thanks! A couple of thoughts after a 5-minute glance:
>>>
>>> * documentation
>>>
>>
>> Will provide documented version this week.
>>
>>
>>> * How about defining the tri-state consistent function to also return a
>>> tri-state? True would mean that the tuple definitely matches, false means
>>> the tuple definitely does not match, and Unknown means it might match. Or
>>> does return value true with recheck==true have the same effect? If I
>>> understood the patch, right, returning Unknown or True wouldn't actually
>>> make any difference, but it's conceivable that we might come up with more
>>> optimizations in the future that could take advantage of that. For example,
>>> for a query like "foo OR (bar AND baz)", you could immediately return any
>>> tuples that match foo, and not bother scanning for bar and baz at all.
>>
>>
>> The meaning of recheck flag when input contains unknown is undefined now.
>> :)
>> For instance, we could define it in following ways:
>> 1) Like returning Unknown meaning that consistent with true of false
>> instead of input Unknown could return either true or false.
>> 2) Consistent with true of false instead of input Unknown could return
>> recheck. This meaning is probably logical, but I don't see any usage of it.
>>
>> I'm not against idea of tri-state returning value for consisted, because
>> it's logical continuation of its tri-state input. However, I don't see
>> usage of distinguish True and Unknown in returning value for now :)
>>
>> In example you give we can return foo immediately, but we have to create
>> full bitmap. So we anyway will have to scan (bar AND baz). We could skip
>> part of trees for bar and baz. But it's possible only when foo contains
>> large amount of sequential TIDS so we can be sure that we didn't miss any
>> TIDs. This seems to be very narrow use-case for me.
>>
>> Another point is that one day we probably could immediately return tuples
>> in gingettuple. And with LIMIT clause and no sorting we can don't search
>> for other tuples. However, gingettuple was removed because of reasons of
>> concurrency. And my patches for index-based ordering didn't return it in
>> previous manner: it collects all the results and then returns them
>> one-by-one.
>>
>
> I'm trying to make fastscan work with GinFuzzySearchLimit. Then I figure
> out that I don't understand how GinFuzzySearchLimit works. Why with
> GinFuzzySearchLimit startScan can return without doing startScanKey? Is it
> a bug?
>

Revised version of patch is attached. Changes are so:
1) Support for GinFuzzySearchLimit.
2) Some documentation.
Question about GinFuzzySearchLimit is still relevant.

--
With best regards,
Alexander Korotkov.


gin-fast-scan.8.patch.gz
Description: GNU Zip compressed data

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

Re: [HACKERS] Proof of concept: standalone backend with full FE/BE protocol

[Peter Eisentraut]

On 11/20/13, 11:31 AM, Stephen Frost wrote:
> Couldn't that be an issue for people who have multiple major versions of
> binaries installed?  In particular, the "default" on the system for psql
> might be 9.3 while the cluster you're trying to recover may be 9.2.  Of
> course, in that case you might say to use the 9.2 psql, which would be
> fair, but what if you're looking to get the data out of the 9.2 DB and
> into the 9.3?  In that case, we'd recommend using the 9.3 pg_dump.

Right.  And also, in emergency situations you might have a custom built
postgres binary lying around in a separate path that includes a patch
from a mailing list you're supposed to test or something.  Best not to
make that even more difficult.



-- 
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] UNNEST with multiple args, and TABLE with multiple funcs

[Tom Lane]

Andrew Gierth  wrote:
> The spec syntax for table function calls, 
> in , looks like TABLE(func(args...)) AS ...

> This patch implements that, plus an extension: it allows multiple
> functions, TABLE(func1(...), func2(...), func3(...)) [WITH ORDINALITY]
> and defines this as meaning that the functions are to be evaluated in
> parallel.

I went back and looked at the spec, and so far as I can tell, the claim
that this is spec syntax plus an extension is a falsehood.  What
I read in SQL:2008 7.6  is

 ::=
  TABLE   

where  is elsewhere defined to be an
expression returning an array or multiset value, and then syntax rule 2
says:

* the  shall be a 

* this construct is equivalent to UNNEST (  )

So unless I'm misreading it, the spec's idea is that you could write

   SELECT ... FROM TABLE( function_returning_array(...) )

and this would result in producing the array elements as a table column.
There is nothing in there about a function returning set.  You could argue
that that leaves us with the freedom to define what the construct does
for functions returning set --- but as this patch stands, if a function
doesn't return set but does return an array, the behavior will not be what
the spec plainly demands.

I do like the basic concept of this syntax, but I think it's a serious
error to appropriate the TABLE() spelling for something that doesn't
agree with the spec's semantics for that spelling.  We need to spell it
some other way.

I've not experimented to see what's practical in bison, but a couple
of ideas that come to mind are:

1. Use FUNCTION instead of TABLE.

2. Don't use any keyword, just parens.  Right now you get a syntax error
from that:

regression=# select * from (foo(), bar()) s;
ERROR:  syntax error at or near ","
LINE 1: select * from (foo(), bar()) s;
^

which implies that it's syntax space we could commandeer.  On the other
hand, I'm a bit worried about the future-proof-ness of such a choice.
It's uncomfortably close to one of the ways to write a row expression,
so it's not too hard to foresee the SQL committee someday defining
something like this in FROM clauses.  It's also hard to see what you'd
call the construct in documentation or error messages --- no keyword means
no easy name to apply.

Thoughts, other ideas?

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] Replication Node Identifiers and crashsafe Apply Progress

[Robert Haas]

On Tue, Nov 19, 2013 at 1:20 PM, Andres Freund  wrote:
> On 2013-11-19 12:47:29 -0500, Robert Haas wrote:
>> On Tue, Nov 19, 2013 at 11:57 AM, Andres Freund  
>> wrote:
>> > Agreed. As an alternative we could just have a single - probably longer
>> > than NAMEDATALEN - string to identify replication progress and rely on
>> > the users of the facility to build the identifier automatically
>> > themselves using components that are helpful in their system.
>>
>> I tend to feel like a generic identifier would be better.  I'm not
>> sure why something like a UUID wouldn't be enough, though.
>> Arbitrary-length identifiers will be bad for performance, and 128 bits
>> ought to be enough for anyone.
>
> That's what I had suggested to some people originally and the response
> was, well, somewhat unenthusiastic. It's not that easy to assign them in
> a meaningful automated manner. How do you automatically assign a pg
> cluster an id?

/dev/urandom

> But yes, maybe the answer is to balk on that part, let the users figure
> out what's best, and then only later implement more policy based on that
> experience.
>
> WRT performance: I agree that fixed-width identifiers are more
> performant, that's why I went for them, but I am not sure it's that
> important. The performance sensitive parts should all be done using the
> internal id the identifier maps to, not the public one.

But I thought the internal identifier was exactly what we're creating.

I think we should also take note of Steve Singer's comments.  Perhaps
this entire endeavor is premature.

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


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

Re: [HACKERS] Dynamic Shared Memory stuff

[Robert Haas]

On Wed, Nov 20, 2013 at 8:32 AM, Heikki Linnakangas
 wrote:
> I'm trying to catch up on all of this dynamic shared memory stuff. A bunch
> of random questions and complaints:
>
> What kind of usage are we trying to cater with the dynamic shared memory?

Parallel sort, and then parallel other stuff.  Eventually general
parallel query.

I have recently updated https://wiki.postgresql.org/wiki/Parallel_Sort
and you may find that interesting/helpful as a statement of intent.

> How many allocations? What size will they have have typically, minimum and
> maximum?

The facility is intended to be general, so the answer could vary
widely by application.  The testing that I have done so far suggests
that for message-passing, relatively small queue sizes (a few kB,
perhaps 1 MB at the outside) should be sufficient.  However,
applications such as parallel sort could require vast amounts of
shared memory.  Consider a machine with 1TB of memory performing a
512GB internal sort.  You're going to need 512GB of shared memory for
that.

> I looked at the message queue implementation you posted, but I
> wonder if that's the use case you're envisioning for this, or if you have
> more things in mind.

I consider that to be the first application of dynamic shared memory
and expect it to be used for (1) returning errors from background
workers to the user backend and (2) funneling tuples from one portion
of a query tree that has been split off to run in a background worker
back to the user backend.  However, I expect that many clients of the
dynamic shared memory system will want to roll their own data
structures.  Parallel internal sort (or external sort) is obviously
one, and in addition to that we might have parallel construction of
in-memory hash tables for a hash join or hash agg, or, well, anything
else you'd like to parallelize.  I expect that many of those case will
result in much larger allocations than what we need just for message
passing.

> * dsm_handle is defined in dsm_impl.h, but it's exposed in the function
> signatures in dsm.h. ISTM it should be moved to dsm.h

Well, dsm_impl.h is the low-level stuff, and dsm.h is intended as the
user API.  Unfortunately, whichever file declares that will have to be
included by the other one, so the separation is not as clean as I
would like, but I thought it made more sense for the high-level stuff
to depend on the low-level stuff rather than the other way around.

> * The DSM API contains functions for resizing the segment. That's not
> exercised by the MQ or TOC facilities. Is that going to stay dead code, or
> do you envision a user for it?

I dunno.  It certainly seems like a useful thing to be able to do - if
we run out of memory, go get some more.  It'd obviously be more useful
if we had a full-fledged allocator for dynamic shared memory, which is
something that I'd like to build but haven't built yet.  However,
after discovering that it doesn't work either on Windows or with
System V shared memory, I'm less sanguine about the chances of finding
good uses for it.  I haven't completely given up hope, but I don't
have anything concrete in mind at the moment.  It'd be a little more
plausible if we adjusted things so that the mmap() implementation
works on Windows.

> * dsm_impl_can_resize() incorrectly returns false for DSM_IMPL_MMAP. The
> mmap() implementation can resize.

Oops, that's a bug.

> * This is an issue I've seen for some time with git master, while working on
> various things. Sometimes, when I kill the server with CTRL-C, I get this in
> the log:
>
> ^CLOG:  received fast shutdown request
> LOG:  aborting any active transactions
> FATAL:  terminating connection due to administrator command
> LOG:  autovacuum launcher shutting down
> LOG:  shutting down
> LOG:  database system is shut down
> LOG:  could not remove shared memory segment "/PostgreSQL.1804289383":
> Tiedostoa tai hakemistoa ei ole
>
> (that means ENOENT)
>
> And I just figured out why that happens: If you take a base backup of a
> running system, the pg_dynshmem/state file is included in the backup. If you
> now start up a standby from the backup on the same system, it will "clean
> up" and reuse the dynshmem segment still used by the master system. Now,
> when you shut down the master, you get that message in the log. If the
> segment was actually used for something, the master would naturally crash.

Ooh.  Well, pg_basebackup can be fixed not to copy that, but there's
still going to be a problem with old-style base backups.  We could try
to figure out some additional sanity check for the dsm code to use, to
determine whether or not it belongs to the same cluster, like storing
the port number or the system identifier or some other value in the
shared memory segment and then comparing it to verify whether we've
got the same one.  Or perhaps we could store the PID of the creating
postmaster in there and check whether that PID is still alive,
although we might get confused if the PID has been rec

Re: [HACKERS] WITH ORDINALITY versus column definition lists

[Tom Lane]

David Johnston  writes:
> Tom Lane-2 wrote
>> It seems to me that we don't really want this behavior of the coldeflist
>> not including the ordinality column.  It's operating as designed, maybe,
>> but it's unexpected and confusing.  We could either
>> 
>> 1. Reinsert HEAD's prohibition against directly combining WITH ORDINALITY
>> with a coldeflist (with a better error message and a HINT suggesting that
>> you can get what you want via the TABLE syntax).
>> 
>> 2. Change the parser so that the coldeflist is considered to include the
>> ordinality column, for consistency with the bare-alias case.  We'd
>> therefore insist that the last coldeflist item be declared as int8, and
>> then probably have to strip it out internally.

> Two options I came up with:

> 1) disallow any type specifier on the last item:  t(f1 int, f2 text, o1)
> 2) add a new pseudo-type, "ord":  t(f1 int, f2 text, o1 ord)

> I really like option #2.

I don't.  Pseudo-types have a whole lot of baggage.  #1 is a mess too.
And in either case, making coldef list items optional increases the number
of ways to make a mistake, if you accidentally omit some other column for
instance.

Basically the problem here is that it's not immediately obvious whether
the coldef list ought to include the ordinality column or not.  The user
would probably guess not (since the system knows what type ordinality
should be).  Unless he's trying to specify a column name for the ordinality
column, in which case he'll realize the syntax forces it to be there.
Any way you slice it, that's going to lead to confusion and bug reports.

The TABLE syntax is really a vastly better solution for this.  So I'm
thinking my #1 is the best answer, assuming we can come up with a good
error message.  My first attempt would be

ERROR: WITH ORDINALITY cannot be used with a column definition list
HINT: Put the function's column definition list inside TABLE() syntax.

Better ideas?

regards, tom lane


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

[HACKERS] [PATCH] Store Extension Options

[Fabrízio de Royes Mello]

Hi all,

The main goal of this patch is enable to an user the capability to store
options
(relations and attributes) related to extensions by using a fixed prefix
called 'ext' in
the defined name. It's cant be useful for replication solutions.

So, with this patch we can do that:

ALTER TABLE foo
   SET (ext.somext.do_replicate=true);

When 'ext' is the fixed prefix, 'somext' is the extension name,
'do_replicate' is the
extension option and 'true' is the value.

Also we can use this form to define storage options to indexes and
per-attribute
options.


Regards,

--
Fabrízio de Royes Mello
Consultoria/Coaching PostgreSQL
>> Timbira: http://www.timbira.com.br
>> Blog sobre TI: http://fabriziomello.blogspot.com
>> Perfil Linkedin: http://br.linkedin.com/in/fabriziomello
>> Twitter: http://twitter.com/fabriziomello
diff --git a/doc/src/sgml/ref/alter_index.sgml b/doc/src/sgml/ref/alter_index.sgml
index d210077..bf4e196 100644
--- a/doc/src/sgml/ref/alter_index.sgml
+++ b/doc/src/sgml/ref/alter_index.sgml
@@ -82,6 +82,15 @@ ALTER INDEX [ IF EXISTS ] name RESE
   
   to get the desired effects.
  
+ 
+   
+ A special prefix called 'ext.' can be 
+ used to define storage parameter. The storage parameters with this prefix
+ will be used by 'extensions'. Storage option nomenclature: ext.name.option=value
+ (ext=fixed prefix, name=extension name, option=option name and value=option value).
+ See example bellow.
+   
+ 
 

 
@@ -202,6 +211,17 @@ ALTER INDEX distributors SET (fillfactor = 75);
 REINDEX INDEX distributors;
 
 
+  
+   To set a storage parameter to be used by extensions:
+
+ALTER INDEX distributors
+  SET (ext.somext.do_replicate=true);
+
+   (ext=fixed prefix, somext=extension name, do_replicate=option name and 
+   true=option value)
+
+
+
  
 
  
diff --git a/doc/src/sgml/ref/alter_table.sgml b/doc/src/sgml/ref/alter_table.sgml
index 89649a2..4756a58 100644
--- a/doc/src/sgml/ref/alter_table.sgml
+++ b/doc/src/sgml/ref/alter_table.sgml
@@ -213,6 +213,17 @@ ALTER TABLE [ IF EXISTS ] name
   of statistics by the PostgreSQL query
   planner, refer to .
  
+
+ 
+  
+   A special prefix called 'ext.' can be used to 
+   define per-attribute options. The attribute options with this prefix will be
+   used by 'extensions'. The attribute option nomenclature: ext.name.option=value
+   (ext=fixed prefix, name=extension name, option=option name and value=option value).
+   See the example bellow.
+  
+ 
+
 

 
@@ -476,6 +487,11 @@ ALTER TABLE [ IF EXISTS ] name
ALTER TABLE does not treat OIDS as a
storage parameter.  Instead use the SET WITH OIDS
and SET WITHOUT OIDS forms to change OID status.
+   A special prefix called 'ext.' can be 
+   used to define storage parameter. The storage parameters with this prefix
+   will be used by 'extensions'. Storage option nomenclature: ext.name.option=value
+   (ext=fixed prefix, name=extension name, option=option name and value=option value).
+   See example bellow.
   
  
 
@@ -1112,6 +1128,26 @@ ALTER TABLE distributors DROP CONSTRAINT distributors_pkey,
 ADD CONSTRAINT distributors_pkey PRIMARY KEY USING INDEX dist_id_temp_idx;
 
 
+  
+   To set a per-attribute option to be used by extensions:
+
+ALTER TABLE distributors
+  ALTER COLUMN dist_id SET (ext.somext.do_replicate=true);
+
+   (ext=fixed prefix, somext=extension name, do_replicate=option name and 
+   true=option value)
+
+
+  
+   To set a storage parameter to be used by extensions:
+
+ALTER TABLE distributors
+  SET (ext.somext.do_replicate=true);
+
+   (ext=fixed prefix, somext=extension name, do_replicate=option name and 
+   true=option value)
+
+
  
 
  
diff --git a/src/backend/access/common/reloptions.c b/src/backend/access/common/reloptions.c
index b5fd30a..06c2b3a 100644
--- a/src/backend/access/common/reloptions.c
+++ b/src/backend/access/common/reloptions.c
@@ -275,6 +275,8 @@ static void initialize_reloptions(void);
 static void parse_one_reloption(relopt_value *option, char *text_str,
 	int text_len, bool validate);
 
+static bool is_extension_namespace(char *namespace);
+
 /*
  * initialize_reloptions
  *		initialization routine, must be called before parsing
@@ -602,13 +604,15 @@ transformRelOptions(Datum oldOptions, List *defList, char *namspace,
 			/* Search for a match in defList */
 			foreach(cell, defList)
 			{
-DefElem*def = (DefElem *) lfirst(cell);
+DefElem	   *def = (DefElem *) lfirst(cell);
 int			kw_len;
+char	   *text_compare;
 
 /* ignore if not in the same namespace */
 if (namspace == NULL)
 {
-	if (def->defnamespace != NULL)
+	if (def->defnamespace != NULL &&
+		!is_extension_namespace(def->defnamespace))
 		continue;
 }
 else if (def->defnamespace == NULL)
@@ -617,8 +621,17 @@ transformRelOptions(Datum oldOptions, List *defLi

Re: [HACKERS] additional json functionality

[Andrew Dunstan]

On 11/20/2013 01:36 PM, Tom Lane wrote:


You'd have to make the data self-identifying (which I think was the plan
already), and ensure that *every* function taking "json" could cope with
both formats on input.  The typmod could only be expected to be enforced
when storing or explicitly casting to one subformat, much like operations
on "numeric" pay little attention to the original precision/scale if any.

I agree that this solution isn't terribly workable, mainly because it'd
break any third-party C functions that take json today.





Yeah, I had come to this conclusion. I don't think we can bolt on 
typmods after the event.


I don't think having a separate "jsonb" type will be a tragedy.

I'm already planning on overloading the existing json functions and 
operators.



cheers

andrew


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

Re: [HACKERS] Data corruption issues using streaming replication on 9.0.14/9.2.5/9.3.1

[Josh Berkus]

On 11/20/2013 10:30 AM, Josh Berkus wrote:
> Andrews, Kevin:

Andres, that is.

> 
> Presumably a replica created while all traffic was halted on the master
> would be clean, correct?  This bug can only be triggered if there's
> heavy write load on the master, right?
> 

Also, just to verify:

If someone is doing PITR based on a snapshot taken with pg_basebackup,
that will only trip this corruption bug if the user has hot_standby=on
in their config *while restoring*?  Or is it critical if they have
hot_standby=on while backing up?

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

[David E. Wheeler]

On Nov 20, 2013, at 6:19 AM, Peter Eisentraut  wrote:

> openjade:hstore.sgml:206:16:E: document type does not allow element 
> "VARLISTENTRY" here; assuming missing "VARIABLELIST" start-tag

Thanks, I fixed this one.

David

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

Re: [HACKERS] Add CREATE support to event triggers

[Christopher Browne]

On Fri, Nov 8, 2013 at 10:33 AM, Alvaro Herrera
 wrote:
> Hello,
>
> Attached you can find a very-much-WIP patch to add CREATE info support
> for event triggers (normalized commands).  This patch builds mainly on
> two things:
>
> 1. Dimitri's "DDL rewrite" patch he submitted way back, in
>http://www.postgresql.org/message-id/m2zk1j9c44@2ndquadrant.fr
>
> I borrowed the whole ddl_rewrite.c code, and tweaked it a bit.  There
> are several things still wrong with it and which will need to be fixed
> before a final patch can even be contemplated; but there are some
> questions that require a consensus answer before I go and fix it all,
> because what it will look like will depend on said answers.

I have tried this out; the patch applies fine.

Note that it induces (modulo my environment) a failure in "make check".

The "opr_sanity" test fails.

postgres@cbbrowne ~/p/s/t/regress> diff expected/opr_sanity.out
results/opr_sanity.out
348,350c348,351
<  oid | proname
< -+-
< (0 rows)
---
>  oid  | proname
> --+--
>  3567 | pg_event_trigger_get_normalized_commands
> (1 row)

That's a minor problem; the trouble there is that the new function is not
yet documented.  Not a concern at this stage.

> 2. The ideas we used to build DROP support.  Mainly, the interesting
>thing here is the fact that we use a SRF to report, at
>ddl_command_end, all the objects that were created during execution
>of that command.  We do this by collecting them in a list in some raw
>form somewhere during ProcessUtility, and then spitting them out if
>the SRF is called.  I think the general idea is sound, although of
>course I admit there might be bugs in the implementation.
>
> Note this patch doesn't try to add any kind of ALTER support.  I think
> this is fine in principle, because we agreed that we would attack each
> kind of command separately (divide to conquer and all that); but there
> is a slight problem for some kind of objects that are represented partly
> as ALTER state during creation; for example creating a table with a
> sequence uses ALTER SEQ/OWNED BY internally at some point.  There might
> be other cases I'm missing, also.  (The REFRESH command is nominally
> also supported.)

I imagine that the things we create in earlier stages may help with later
stages, so it's worth *some* planning so we can hope not to build bits
now that push later enhancements into corners that they can't get out of.

But I'm not disagreeing at all.

> Now about the questions I mentioned above:
>
> a) It doesn't work to reverse-parse the statement nodes in all cases;
> there are several unfixable bugs if we only do that.  In order to create
> always-correct statements, we need access to the catalogs for the
> created objects.  But if we are doing catalog access, then it seems to
> me that we can do away with the statement parse nodes completely and
> just reconstruct the objects from catalog information.  Shall we go that
> route?

Here's a case where it doesn't work.

testevent@localhost->  create schema foo;
CREATE SCHEMA
testevent@localhost->  create domain foo.bar integer;
CREATE DOMAIN
testevent@localhost->  CREATE OR REPLACE FUNCTION snitch() RETURNS
event_trigger LANGUAGE plpgsql AS $$
testevent$# DECLARE
testevent$# r RECORD;
testevent$# BEGIN
testevent$# FOR r IN SELECT * FROM
pg_event_trigger_get_normalized_commands()
testevent$# LOOP
testevent$# RAISE NOTICE 'object created: id %, statement %',
testevent$# r.identity, r.command;
testevent$# END LOOP;
testevent$# END;
testevent$# $$;
CREATE FUNCTION
testevent@localhost->  CREATE EVENT TRIGGER snitch ON ddl_command_end
EXECUTE PROCEDURE snitch();
CREATE EVENT TRIGGER
testevent@localhost->  set search_path to public, foo;
SET
testevent@localhost->  create table foo.foo2 (acolumn bar);
NOTICE:  object created: id foo.foo2, statement CREATE TABLE foo.foo2
(acolumn bar)
CREATE TABLE

The trouble is that you have only normalized the table name.  The
domain, bar, needs its name normalized as well.

> b) What's the best design of the SRF output?  This patch proposes two
> columns, object identity and create statement.  Is there use for
> anything else?  Class/object OIDs perhaps, schema OIDs for objects types
> that have it?  I don't see any immediate need to that info, but perhaps
> someone does.

Probably an object type is needed as well, to know if it's a table or
a domain or a sequence or whatever.

I suspect that what will be needed to make it all usable is some sort of
"structured" form.  That is in keeping with Robert Haas' discomfort with
the normalized form.

My "minor" gripe is that you haven't normalized enough (e.g. - it should be
CREATE TABLE foo.foo2 (acolumn foo.bar), capturing the normalization of
data types that are referenced).

But Robert's quite right that users may want more than just to capture that
literally;

Re: [HACKERS] additional json functionality

[Tom Lane]

David Johnston  writes:
>> On 11/18/2013 06:13 AM, Peter Eisentraut wrote:
>>> We could do something like SQL/XML and specify the level of "validity"
>>> in a typmod, e.g., json(loose), json(strict), etc.

> Three things:

> 1) How would this work in the face of functions that erase typemod
> information?

You'd have to make the data self-identifying (which I think was the plan
already), and ensure that *every* function taking "json" could cope with
both formats on input.  The typmod could only be expected to be enforced
when storing or explicitly casting to one subformat, much like operations
on "numeric" pay little attention to the original precision/scale if any.

I agree that this solution isn't terribly workable, mainly because it'd
break any third-party C functions that take json today.

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] WITH ORDINALITY versus column definition lists

[David Johnston]

Tom Lane-2 wrote
> It seems to me that we don't really want this behavior of the coldeflist
> not including the ordinality column.  It's operating as designed, maybe,
> but it's unexpected and confusing.  We could either
> 
> 1. Reinsert HEAD's prohibition against directly combining WITH ORDINALITY
> with a coldeflist (with a better error message and a HINT suggesting that
> you can get what you want via the TABLE syntax).
> 
> 2. Change the parser so that the coldeflist is considered to include the
> ordinality column, for consistency with the bare-alias case.  We'd
> therefore insist that the last coldeflist item be declared as int8, and
> then probably have to strip it out internally.

#2 but I am hoping to be able to make the definition of the column optional. 
One possibility is that if you do want to provide an alias you have to make
it clear that the coldeflist item in question is only valid for a with
ordinality column alias.  Otherwise the entire coldeflist is used to alias
the record-type output and the ordinality column is provided its default
name.

Two options I came up with:

1) disallow any type specifier on the last item:  t(f1 int, f2 text, o1)
2) add a new pseudo-type, "ord":  t(f1 int, f2 text, o1 ord)

I really like option #2.  It makes it perfectly clear, entirely within the
coldeflist SQL, that the last column is different and in this case optional
both in the sense of providing an alias and also the user can drop the whole
ordinality aspect of the call as well.  The system does not need to be told,
by the user, the actual type of the ordinality column.  And given that I
would supposed most people would think to use "int" or "bigint" before using
"int8" the usability there is improved once they need and then learn that to
alias the ordinality column they use the "ord" type which would internally
resolve to the necessary output type.

Option one is somewhat simpler but the slight added verbosity makes reading
the SQL coldeflist easier, IMO, since you are already scanning name-type
pairs and recognizing the missing type is, for me, harder than reading off
"ord" and recalling its meaning.

David J.





--
View this message in context: 
http://postgresql.1045698.n5.nabble.com/WITH-ORDINALITY-versus-column-definition-lists-tp5779443p5779449.html
Sent from the PostgreSQL - hackers mailing list archive at Nabble.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] Proof of concept: standalone backend with full FE/BE protocol

[Tom Lane]

Andres Freund  writes:
> On 2013-11-20 11:08:33 -0500, Tom Lane wrote:
>> Andres Freund  writes:
>>> Something like PQstartSingleUser(dsn) returning a established connection
>>> seems better to me.

>> That just pushes the problem up a level --- how are you going to tell
>> psql, pg_dump, or other programs that they should do that?

> An explicit parameter. A program imo explicitly needs to be aware that a
> PQconnect() suddenly starts forking and such. What if it is using
> threads? What if it has it's own SIGCHLD handler for other business it's
> doing?

Hm.  That's a fair point.  I don't especially buy your other argument
about additional connections --- if the program tries such, they'll
just fail, which can hardly be said to be unexpected.  But it's reasonable
to worry that programs might need to be aware that they now have a child
process.  (It occurs to me that we'll need to provide a way to get the
PID of the child, too.)

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] Data corruption issues using streaming replication on 9.0.14/9.2.5/9.3.1

[Josh Berkus]

Andrews, Kevin:

Presumably a replica created while all traffic was halted on the master
would be clean, correct?  This bug can only be triggered if there's
heavy write load on the master, right?

-- 
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] New option for pg_basebackup, to specify a different directory for pg_xlog

[Gavin Flower]

On 20/11/13 23:43, Haribabu kommi wrote:

On 19 November 2013 19:12 Fujii Masao wrote:

On Tue, Nov 19, 2013 at 9:14 PM, Haribabu kommi
 wrote:

On 18 November 2013 23:30 Fujii Masao wrote:

On Tue, Nov 19, 2013 at 12:01 AM, Haribabu kommi
 wrote:

Thanks for newer version of the patch!

I found that the empty base directory is created and remains even
when the same directory is specified in both -D and --xlogdir and

the

error occurs.
I think that it's
better to throw an error in that case before creating any new

directory.

Thought?

By creating the base directory only the patch finds whether provided
base and Xlog directories are same or not? To solve this problem
following options are possible

1. No problem as it is just an empty base directory, so it can be

reused in the next time

Leave it as it is.
2. Once the error is identified, the base directory can be deleted.
3. write a new function to remove the parent references from the

provided path to identify

the absolute path used for detecting base and Xlog directories are

same or not?

Please provide your suggestions.


+xlogdir = get_absolute_path(xlog_dir);

xlog_dir must be an absolute path. ISTM we don't do the above. No?

It is required. As user can provide the path as

/home/installation/bin/../bin/data.

The provided path is considered as absolute path only but while
comparing the same With data directory path it will not match.

Okay, maybe I understand you. In order to know the real absolute path,
basically we need to create the directory specified in --xlogdir,
change the working directory to it and calculate the parent path.
You're worried about the cases as follows, for example.
Right?

* path containing ".." like /aaa/bbb/../ccc is specified in --xlogdir
* symbolic link is specified in --xlogdir

On the second thought, I'm thinking that it might be overkill to add
such not simple code for that small benefit.

I tried using of stat'ing in two directories, which is having a problem in 
windows.
So modified old approach to detect limited errors. Updated patch attached.
This will detect and throw an error in the following scenarios.
1. pg_basebackup -D /home/data --xlogdir=/home/data
2. pg_basebackup -D data --xlogdir=/home/data -- home is the CWD
3. pg_basebackup -D ../data --xlogdir=/data -- home is the CWD

Please let me know your suggestions.

Regards,
Hari babu.


I don't think Postgres on other systems should be hobbled by the 
limitations of Microsoft software!


If certain features of Postgres are either not available, or are 
available in a reduced form on Microsoft platforms, then this should be 
documented - might provide subtle hints to upgrade to Linux.



Cheers,
Gavin

Re: [HACKERS] additional json functionality

[Andrew Dunstan]

On 11/20/2013 12:50 PM, Greg Stark wrote:


On Sat, Nov 16, 2013 at 12:32 AM, Josh Berkus > wrote:


On 11/15/2013 04:00 PM, David Johnston wrote:
> Looking at this a different way: could we just implement BSON
and leave json
> alone?
>
> http://bsonspec.org/

In short?  No.

For one thing, our storage format is different from theirs (better,
frankly), and as a result is not compliant with their "standard".


Not being super familiar with either BSON our JSONB what advantages 
are we gaining from the difference?


It might be interesting if we supported the same binary representation 
so we could have a binary send/recv routines that don't need to do any 
serialization/deserialization. Especially since a standard format 
would potentially be skipping the serialization/deserialization on 
both the server and client.







To start with, it doesn't support arbitrary precision numerics. That 
means that right off the bat it's only accepting a subset of what the 
JSON spec allows. 'Nuff said, I think.


cheers

andrew



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

Re: [HACKERS] Storage formats for JSON WAS: additional json functionality

[Josh Berkus]

Greg,

> Not being super familiar with either BSON our JSONB what advantages are we
> gaining from the difference?

We have the JSONB/Hstore2 format *now*, and it can go into 9.4.  This
makes it inherently superior to any theoretical format.  So any further
discussion (below) is strictly academic, for the archives.

> It might be interesting if we supported the same binary representation so
> we could have a binary send/recv routines that don't need to do any
> serialization/deserialization. Especially since a standard format would
> potentially be skipping the serialization/deserialization on both the
> server and client.

Leaving aside that we don't want to implement 10gen's spec (because of
major omissions like decimal numbers), the fundamental issue with
binary-update-in-place is that nobody (certainly not 10gen) has devised
a way to do it without having ginormous amounts of bloat in value
storage.  The way BSON allows for binary-update-in-place, as I
understand it, is to pad all values with lots of zero bytes and to
prohibit compression, either of which are much larger losses for
performance than serialization is.

In other words, binary-update-in-place seems like a clear win for
heirarchical data storage, but practically it's not.

Of course, an investigation into this by someone with much more
knowledge of low-level storage than me (most of this list) is welcome.

-- 
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] additional json functionality

[Josh Berkus]

On 11/20/2013 04:52 AM, Robert Haas wrote:
> I confess to being a bit perplexed by why we want hstore and json to
> share a common binary format.  hstore doesn't store hierarchical data;
> json does.  If we design a binary format for json, doesn't that just
> obsolete store?  Why go to a lot of trouble to extend our home-grown
> format if we've got a standard format to plug into?

See hstore2 patch from Teodor.  That's what we're talking about, not
hstore1, which as you point out is non-heirarchical.

-- 
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] -d option for pg_isready is broken

[Josh Berkus]

On 11/20/2013 01:55 AM, Fujii Masao wrote:
> Yeah, I agree that we should make the logic of pg_isready more future-proof
> in 9.4dev. One idea is to expose internal_ping() as a libpq function. Then,
> instead of just calling PQpingParams(), we can do PQconnectStartParams(),
> libpq-function-version-of-internal_ping(), PQhost(), PQport(), and PQfinish().
> That is, we get to know the host and port information by calling
> PQhost() and PQport(),
> after trying to connect to the server.

+1

This would allow writers of client drivers to implement their own
pg_ping() functions instead of needing to go through the shell (as I
currently do with pg_isready).

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

[HACKERS] WITH ORDINALITY versus column definition lists

[Tom Lane]

Consider the following case of a function that requires a column
definition list (example borrowed from the regression tests):

create function array_to_set(anyarray) returns setof record as $$
  select i AS "index", $1[i] AS "value" from generate_subscripts($1, 1) i
$$ language sql strict immutable;

select * from array_to_set(array['one', 'two']) as t(f1 int,f2 text);

What if you want to add ordinality to that?  In HEAD you get:

regression=# select * from array_to_set(array['one', 'two']) with ordinality as 
t(f1 int,f2 text);
ERROR:  WITH ORDINALITY is not supported for functions returning "record"
LINE 1: select * from array_to_set(array['one', 'two']) with ordinal...
  ^

which is a restriction imposed by the original WITH ORDINALITY patch.
The currently-submitted patch removes this restriction (although not the
documentation about it :-(), and what you get is

regression=# select * from array_to_set(array['one', 'two']) with ordinality as 
t(f1 int,f2 text);
 f1 | f2  | ordinality 
+-+
  1 | one |  1
  2 | two |  2
(2 rows)

Notice that the coldef list doesn't include the ordinality column, so in
this syntax there is no way to choose a different name for the ordinality
column.  The new TABLE syntax provides an arguably-saner solution:

regression=# select * from table(array_to_set(array['one', 'two']) as (f1 
int,f2 text)) with ordinality;
 f1 | f2  | ordinality 
+-+
  1 | one |  1
  2 | two |  2
(2 rows)

regression=# select * from table(array_to_set(array['one', 'two']) as (f1 
int,f2 text)) with ordinality as t(a1,a2,a3);
 a1 | a2  | a3 
+-+
  1 | one |  1
  2 | two |  2
(2 rows)

Now, it seems to me that putting WITH ORDINALITY on the same syntactic
level as the coldeflist is pretty confusing, especially since it behaves
differently than WITH ORDINALITY with a simple alias list:

regression=# select * from generate_series(1,2) with ordinality as t(f1,f2);
 f1 | f2 
+
  1 |  1
  2 |  2
(2 rows)

Here, the alias list does extend to the ordinality column.

It seems to me that we don't really want this behavior of the coldeflist
not including the ordinality column.  It's operating as designed, maybe,
but it's unexpected and confusing.  We could either

1. Reinsert HEAD's prohibition against directly combining WITH ORDINALITY
with a coldeflist (with a better error message and a HINT suggesting that
you can get what you want via the TABLE syntax).

2. Change the parser so that the coldeflist is considered to include the
ordinality column, for consistency with the bare-alias case.  We'd
therefore insist that the last coldeflist item be declared as int8, and
then probably have to strip it out internally.

Thoughts?

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] additional json functionality

[Greg Stark]

On Sat, Nov 16, 2013 at 12:32 AM, Josh Berkus  wrote:

> On 11/15/2013 04:00 PM, David Johnston wrote:
> > Looking at this a different way: could we just implement BSON and leave
> json
> > alone?
> >
> > http://bsonspec.org/
>
> In short?  No.
>
> For one thing, our storage format is different from theirs (better,
> frankly), and as a result is not compliant with their "standard".


Not being super familiar with either BSON our JSONB what advantages are we
gaining from the difference?

It might be interesting if we supported the same binary representation so
we could have a binary send/recv routines that don't need to do any
serialization/deserialization. Especially since a standard format would
potentially be skipping the serialization/deserialization on both the
server and client.



-- 
greg

Re: [HACKERS] GIN improvements part 1: additional information

[Heikki Linnakangas]

On 06.11.2013 17:36, Alvaro Herrera wrote:

Just for my own illumination, can someone explain this bit?

+ If a posting list is too large to store in-line in a key entry, a posting tree
+ is created. A posting tree is a B-tree structure, where the ItemPointer is
+ used as the key. At the leaf-level, item pointers are stored compressed, in
+ "varbyte encoding".

I think the first ItemPointer mentioned (the key) refers to a TID
pointing to the index, and "item pointers stored compressed" refers to
the TIDs pointing to the heap (the data).  Is that correct?


No, they both refer to TIDs pointing to the heap.


I'm also interested in the "FIXME explain varbyte encoding" explanation
currently missing, if somebody can write it down ...


Alexander's latest version filled in that explanation (haven't read it 
myself yet)


- Heikki


--
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] Add transforms feature

[Peter Eisentraut]

On 11/15/13, 11:04 AM, Dimitri Fontaine wrote:
>   - Documentation style seems to be to be different from the "man page"
> or "reference docs" style that we use elsewhere, and is instead
> deriving the general case from examples. Reads strange.

Which specific section do you have in mind?  It's hard to explain this
feature in abstract terms, I think.

>   - The internal datatype argument and return type discussion for
> function argument looks misplaced, but I don't have a better
> proposition for that.

OK, maybe I'll put that in parentheses or a separate paragraph.

>   - Do we need an ALTER TRANSFORM command?
> 
> Usually we have at least an Owner for the new objects and a command
> to change the owner. Then should we be able to change the
> function(s) used in a transform?

We don't have ALTER CAST either, and no one's been too bothered about
that.  It's possible, of course.

>   - Should transform live in a schema?
> 
> At first sight, no reason why, but see next point about a use case
> that we might be able to solve doing that.

Transforms don't have a name, so I don't quite see what you mean here.

>   - SQL Standard has something different named the same thing,
> targetting client side types apparently. Is there any reason why we
> would want to stay away from using the same name for something
> really different in PostgreSQL?

Let's review that, as there as been some confusion about that.  The SQL
standard syntax is

CREATE TRANSFORM FOR   (...details...);

and then there is

SET DEFAULT TRANSFORM GROUP 
SET TRANSFORM GROUP FOR TYPE  

This is essentially an elaborate way to have custom input/output
formats, like DateStyle or bytea_output.

(You can find examples of this in the IBM DB2 documentation.  Some of
their clients apparently set a certain transform group automatically,
allowing you to set per-interface output formats.)

The proposed syntax in the other hand is

CREATE TRANSFORM FOR  LANGUAGE  (...details...);

So you could consider LANGUAGE  to be the implicit transform group
of language , if you like.

Or you could consider that this is a situation like VIEW vs.
MATERERIALIZED VIEW: they sound the same, they are a bit alike, but the
implementation details are different.

All obvious synonyms of "transform" (conversion, translation, etc.) are
already in use.

> On the higher level design, the big question here is about selective
> behavior. As soon as you CREATE TRANSFORM FOR hstore LANGUAGE plperl
> then any plperl function will now receive its hstore arguments as a
> proper perl hash rather than a string.
> 
> Any pre-existing plperl function with hstore arguments or return type
> then needs to be upgraded to handle the new types nicely, and some of
> those might not be under the direct control of the DBA running the
> CREATE TRANSFORM command, when using some plperl extensions for example.

I had proposed disallowing installing a transform that would affect
existing functions.  That was rejected or deemed unnecessary.  You can't
have it both ways. ;-)

> A mechanism allowing for the transform to only be used in some functions
> but not others might be useful. The simplest such mechanism I can think
> of is modeled against the PL/Java classpath facility as specified in the
> SQL standard: you attach a classpath per schema.

Anything that's a problem per-database would also be a problem per schema.

> Should using the schema to that ends be frowned upon, then we need a way
> to register each plperl function against using or not using the
> transform facility, defaulting to not using anything. Maybe something
> like the following:
> 
>   CREATE FUNCTION foo(hash hstore, x ltree)
>  RETURNS hstore
>  LANGUAGE plperl
>  USING TRANSFORM FOR hstore, ltree
>   AS $$ … $$;

This is a transition problem.  Nobody is required to install the
transforms into their existing databases.  They probably shouldn't.

How many people actually use hstore with PL/Perl or PL/Python now?
Probably not many, because it's weird.

I like to think about how this works for new development:  Here is my
extension type, here is how it interfaces with languages.  Once you have
established that, you don't want to have to repeat that every time you
write a function.  That's error prone and cumbersome.  And anything
that's set per schema or higher is a dependency tracking and caching mess.

Also, extension types should work the same as built-in types.
Eventually, I'd like to rip out the hard-coded data type support in
PL/Python and replace it with built-in transforms.  Even if we don't
actually do it, conceptually it should be possible.  Now if we require
"USING TRANSFORM FOR int, bytea" every time, we'd have taken a big step
back.  Effectively, we already have built-in transforms in PL/Python.
We have added a few more over the years.  It's been a bit of a pain from
time to time.  At least, with this feature we'd be moving this decision
into user space and give pe

Re: [HACKERS] Logging WAL when updating hintbit

[Sawada Masahiko]

On Wed, Nov 20, 2013 at 9:19 PM, Dilip kumar  wrote:
> On 19 November 2013 22:19, Sawada Masahiko Wrote
>>> >
>>
>> Thanks!
>> I took it wrong.
>> I think that there are quite a few difference amount of WAL.
>>
>> > Did you test about amount of WAL size in your patch?
>>
>> Not yet. I will do that.
>
> 1. Patch applies cleanly to master HEAD.
> 2. No Compilation Warning.
> 3. It works as per the patch expectation.
>
> Some Suggestion:
> 1. Add new WAL level ("all") in comment in postgresql.conf
>wal_level = hot_standby # minimal, archive, or 
> hot_standby

Thank you for reviewing the patch.
Yes, I will do that. And I'm going to implement documentation patch.

>
> Performance Test Result:
> Run with pgbench for 300 seconds
>
> WAL level : hot_standby
> WAL Size  :   111BF8A8
> TPS   :   125
>
> WAL level : all
> WAL Size  :   11DB5AF8
> TPS   :   122
>
> * TPS is almost constant but WAL size is increased around 11M.
>
> This is the first level of observation, I will continue to test few more 
> scenarios including performance test on standby.

Thank you for performance testing.
According of test result, TPS of 'all'  lower than 'hot_standby',but
WAL size is increased.
I think that it should be separate parameter.
And TPS on master side is is almost constant. so this patch might have
several benefit for performance
improvement on standby side if the result of performance test on
standby side is improved.

Regards,

---
Sawada Masahiko


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

Re: [HACKERS] Data corruption issues using streaming replication on 9.0.14/9.2.5/9.3.1

[Andres Freund]

On 2013-11-20 18:25:56 +0200, Heikki Linnakangas wrote:
> Isn't it possible that the standby has already incorrectly set
> HEAP_XMIN_INVALID hint bit on a page? The full page images generated by
> VACUUM FREEZE will correct the damage, but if not, e.g. because
> full_page_writes=off, strange things will happen.

The xlog_heap_freeze records should repair that afaics.

> Personally, I wouldn't trust anything less than a new base backup.

But I still tend to agree.

Greetings,

Andres Freund

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


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

Re: [HACKERS] Proof of concept: standalone backend with full FE/BE protocol

[Andres Freund]

On 2013-11-20 17:19:42 +0100, Andres Freund wrote:
> > That just pushes the problem up a level --- how are you going to tell
> > psql, pg_dump, or other programs that they should do that?
> 
> An explicit parameter. A program imo explicitly needs to be aware that a
> PQconnect() suddenly starts forking and such. What if it is using
> threads? What if it has it's own SIGCHLD handler for other business it's
> doing?

Just as an example, consider what happens if somebody does pg_dump -j?
Or somebody specifies such a connection for primary_conninfo?

I am also not sure whether vacuumdb -a/reindexdb -a (both not unlikely
commands to use for single user mode) are careful enough not to have
parallel connections open?

Greetings,

Andres Freund

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


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

Re: [HACKERS] additional json functionality

[David Johnston]

Hannu Krosing-3 wrote
> On 11/18/2013 06:49 PM, Josh Berkus wrote:
>> On 11/18/2013 06:13 AM, Peter Eisentraut wrote:
>>> On 11/15/13, 6:15 PM, Josh Berkus wrote:
 Thing is, I'm not particularly concerned about *Merlin's* specific use
 case, which there are ways around. What I am concerned about is that we
 may have users who have years of data stored in JSON text fields which
 won't survive an upgrade to binary JSON, because we will stop allowing
 certain things (ordering, duplicate keys) which are currently allowed
 in
 those columns.  At the very least, if we're going to have that kind of
 backwards compatibilty break we'll want to call the new version 10.0.
>>> We could do something like SQL/XML and specify the level of "validity"
>>> in a typmod, e.g., json(loose), json(strict), etc.
>> Doesn't work; with XML, the underlying storage format didn't change.
>> With JSONB, it will ... so changing the typemod would require a total
>> rewrite of the table.  That's a POLS violation if I ever saw one
> We do rewrites on typmod changes already.
> 
> To me having json(string) and json(hstore) does not seem too bad.

Three things:

1) How would this work in the face of functions that erase typemod
information?
2) json [no type mod] would have to effectively default to json(string)?
3) how would #1 and #2 interact?

I pondered the general idea but my (admittedly limited) gut feeling is that
using typemod would possibly be technically untenable and from an end-user
perspective would be even more confusing than having two types.

David J.






--
View this message in context: 
http://postgresql.1045698.n5.nabble.com/additional-json-functionality-tp5777975p5779428.html
Sent from the PostgreSQL - hackers mailing list archive at Nabble.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] review: autovacuum_work_mem

[Nigel Heron]

On Mon, Nov 18, 2013 at 11:36 PM, Peter Geoghegan  wrote:
> Please reply to the original thread in future (even if the Reply-to
> Message-ID is the same, I see this as a separate thread).
>
sorry about that, when i added "review" to the subject gmail removed
the thread info.
for reference the original thread started here:


>
> Revision attached.
> --
> Peter Geoghegan

Review for Peter Geoghegan's v2 patch in CF 2013-11:
https://commitfest.postgresql.org/action/patch_view?id=1262

Submission review
-
* Is the patch in a patch format which has context?
Yes

* Does it apply cleanly to the current git master
(04eee1fa9ee80dabf7cf4b8b9106897272e9b291)?
patching file src/backend/commands/vacuumlazy.c
Hunk #2 succeeded at 1582 (offset 1 line).

* Does it include reasonable tests, necessary doc patches, etc?
Documentation patches included.
No additional tests.

Usability review
-
* Does the patch actually implement that?
Yes.

* Do we want that?
Yes. The original thread has references, see


* Do we already have it?
No.

* Does it follow SQL spec, or the community-agreed behavior?
SQL spec: n/a
community: Yes. The original thread has references, see


* Does it include pg_dump support (if applicable)?
n/a

Feature test
-
* Does the feature work as advertised?
Yes.

* Are there corner cases the author has failed to consider?
None that i can see.

* Are there any assertion failures or crashes?
No.

Performance review
-
* Does the patch slow down simple tests?
No.

* If it claims to improve performance, does it?
n/a

* Does it slow down other things?
No.

Coding review
-
* Does it follow the project coding guidelines?
Yes.

* Are there portability issues?
None that i can see.

* Will it work on Windows/BSD etc?
None that i can see. (I only tested it on linux though)

* Does it do what it says, correctly?
Yes.

* Does it produce compiler warnings?
No.

* Can you make it crash?
No.

Architecture review
-
* Is everything done in a way that fits together coherently with other
features/modules?
Yes.

* Are there interdependencies that can cause problems?
No.

-nigel.


-- 
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] Proof of concept: standalone backend with full FE/BE protocol

[Stephen Frost]

* Tom Lane (t...@sss.pgh.pa.us) wrote:
> I think we'd be better off trying to fix the security issue by
> constraining what can be executed as a "standalone backend".  Would
> it work to insist that psql/pg_dump launch the program named postgres
> from the same bin directory they're in, rather than accepting a path
> from the connection string?

Couldn't that be an issue for people who have multiple major versions of
binaries installed?  In particular, the "default" on the system for psql
might be 9.3 while the cluster you're trying to recover may be 9.2.  Of
course, in that case you might say to use the 9.2 psql, which would be
fair, but what if you're looking to get the data out of the 9.2 DB and
into the 9.3?  In that case, we'd recommend using the 9.3 pg_dump.

Basically, I'd suggest that we try and avoid things like "the binaries
have to be in the same directory"..  With regard to access to the
socket, perhaps we create our own socket w/ 0600 and use that?  Seems
like it'd be sufficient to prevent the 'normal' users from getting into
the DB while we're working on it.  If there's two different individuals
gettings into the same system and trying to start the same cluster as
the same unix user, well..  I'm not convinced we'd be able to come up
with a perfect solution to that anyway.

Thanks,

Stephen


signature.asc
Description: Digital signature

Re: [HACKERS] Data corruption issues using streaming replication on 9.0.14/9.2.5/9.3.1

[Heikki Linnakangas]

On 20.11.2013 17:06, Kevin Grittner wrote:

Andres Freund  wrote:

On 2013-11-20 06:21:13 -0800, Kevin Grittner wrote:




  So as long as there are no open transactions or prepared
  transactions on the master which started before the release with
  the fix is applied, VACUUM FREEZE would be guaranteed to work?
  Since I don't see how a non-prepared transaction would be running
  from before a minor release upgrade, that just means we have to
  make sure there are no prepared transactions from before the
  upgrade?


That's not a bad point. So the way to fix it would be:

1) Restart the standby to the new minor release, wait for catchup
2) Restart the primary (fast or smart) to the new minor release
3) Acquire enough new xids to make sure we cross a clog page (?)
4) Jot down a new xid:  SELECT txid_current()::bigint % (1::bigint<<33-1)
5) vacuumdb -z -a
6) Ensure that there are no prepared xacts older than 3) around
SELECT *
FROM pg_prepared_xacts
ORDER BY age(transaction) DESC LIMIT 1;
7) Ensure the xmin horizon is above the one from: 3:
SELECT datname, datfrozenxid
FROM pg_database
WHERE datname != 'template0'
ORDER BY age(datfrozenxid) DESC LIMIT 1;
8) Get the current lsn: SELECT pg_current_xlog_location();
9) verify on each standby that SELECT pg_last_xlog_receive_location() is
past 7)
10) be happy

I am not sure how we can easily compute that 6) and 7) are past 3) in
the presence of xid wraparounds.



I may well be missing something here, but wouldn't it be sufficient to?:
1) Restart the standby to the new minor release, wait for catchup
2) Restart the primary (fast or smart) to the new minor release
3) Run VACUUM FREEZE (optionally with ANALYZE) in each database on primary
4) Run CHECKPOINT command on primary, or just wait for one to run
5) Wait for standby to process to the checkpoint
6) Be happy


Isn't it possible that the standby has already incorrectly set 
HEAP_XMIN_INVALID hint bit on a page? The full page images generated by 
VACUUM FREEZE will correct the damage, but if not, e.g. because 
full_page_writes=off, strange things will happen.


Personally, I wouldn't trust anything less than a new base backup.

- Heikki


--
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] Extra functionality to createuser

[Christopher Browne]

Wait, that doesn't work if more than one role is added, as they get
merged together by the quoting.

A somewhat ugly amount of quoting can be done at the shell level to
induce double quotes.

$ createuser -g "\"test_rol'e_3\"" usequoted3

I note that similar (with not quite identical behaviour) issues apply
to the user name.  Perhaps the
resolution to this is to leave quoting issues to the administrator.
That simplifies the problem away.
I suspect that the apparatus needed to do a thorough solution (e.g. -
parse the string, and do something
"smarter") may be larger than is worth getting into.


-- 
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] additional json functionality

[Hannu Krosing]

On 11/18/2013 06:49 PM, Josh Berkus wrote:
> On 11/18/2013 06:13 AM, Peter Eisentraut wrote:
>> On 11/15/13, 6:15 PM, Josh Berkus wrote:
>>> Thing is, I'm not particularly concerned about *Merlin's* specific use
>>> case, which there are ways around. What I am concerned about is that we
>>> may have users who have years of data stored in JSON text fields which
>>> won't survive an upgrade to binary JSON, because we will stop allowing
>>> certain things (ordering, duplicate keys) which are currently allowed in
>>> those columns.  At the very least, if we're going to have that kind of
>>> backwards compatibilty break we'll want to call the new version 10.0.
>> We could do something like SQL/XML and specify the level of "validity"
>> in a typmod, e.g., json(loose), json(strict), etc.
> Doesn't work; with XML, the underlying storage format didn't change.
> With JSONB, it will ... so changing the typemod would require a total
> rewrite of the table.  That's a POLS violation if I ever saw one
We do rewrites on typmod changes already.

To me having json(string) and json(hstore) does not seem too bad.

Cheers
Hannu


-- 
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] Proof of concept: standalone backend with full FE/BE protocol

[Andres Freund]

On 2013-11-20 11:08:33 -0500, Tom Lane wrote:
> Andres Freund  writes:
> > On 2013-11-20 10:48:20 -0500, Tom Lane wrote:
> >> constraining what can be executed as a "standalone backend".  Would
> >> it work to insist that psql/pg_dump launch the program named postgres
> >> from the same bin directory they're in, rather than accepting a path
> >> from the connection string?
> 
> > But why do we want to start the server through the connection string
> > using PQconnectb() in the first place? That doesn't really seem right to
> > me.
> > Something like PQstartSingleUser(dsn) returning a established connection
> > seems better to me.
> 
> That just pushes the problem up a level --- how are you going to tell
> psql, pg_dump, or other programs that they should do that?

An explicit parameter. A program imo explicitly needs to be aware that a
PQconnect() suddenly starts forking and such. What if it is using
threads? What if it has it's own SIGCHLD handler for other business it's
doing?

Greetings,

Andres Freund

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


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

Re: [HACKERS] Extra functionality to createuser

[Christopher Browne]

On Tue, Nov 19, 2013 at 11:54 PM, Amit Kapila  wrote:
> On further tests, I found inconsistency in behavior when some special
> characters are used in role names.
>
> 1. Test for role name containing quotes
> a. In psql, create a role containing quotes in role name.
>create role amitk in role "test_ro'le_3";
>
> b. Now if we try to make a new role member of this role using
> createuser utility, it gives error
> try-1
> createuser.exe -g test_ro'le_3 -p 5446 amitk_2
> createuser: creation of new role failed: ERROR:  unterminated quoted
> string at or near "'le_3;"
> LINE 1: ... NOCREATEDB NOCREATEROLE INHERIT LOGIN IN ROLE test_ro'le_3;
> try-2
> createuser.exe -g "test_ro'le_3" -p 5446 amitk
> createuser: creation of new role failed: ERROR:  unterminated quoted
> string at or near "'le_3;"
> LINE 1: ... NOCREATEDB NOCREATEROLE INHERIT LOGIN IN ROLE test_ro'le_3;
>
> c. If I try quoted string in new role to be created, it works fine.
> createuser.exe -p 5446 am'itk_2
>
> As quoted strings work well for role names, I think it should work
> with -g option as well.
>
> 2. Test for role name containing special character ';' (semicolon)
> a. create role "test;_1";
>
> b. Now if we try to make a new role member of this role using
> createuser utility, it gives error
> try-1
> createuser.exe -g test;_1 -p 5446 amitk_4
> createuser: creation of new role failed: ERROR:  syntax error at or near "_1"
> LINE 1: ...RUSER NOCREATEDB NOCREATEROLE INHERIT LOGIN IN ROLE test;_1;
> try-2^
> createuser.exe -g "test;_1" -p 5446 amitk_4
> createuser: creation of new role failed: ERROR:  syntax error at or near "_1"
> LINE 1: ...RUSER NOCREATEDB NOCREATEROLE INHERIT LOGIN IN ROLE test;_1;
> ^
> try-3
> createuser.exe -g 'test;_1' -p 5446 amitk_4
> createuser: creation of new role failed: ERROR:  syntax error at or
> near "'test;_1'"
> LINE 1: ...SER NOCREATEDB NOCREATEROLE INHERIT LOGIN IN ROLE 'test;_1';
>
> c. If I try semicolon in new role to be created, it works fine.
> createuser.exe -p 5446 amit;k_3
>
> As semicolon work well for role names, I think it should work with -g
> option as well.

I was not unconscious of there being the potential for issue here; there is an
easy answer of double quoting the string, thus:

diff --git a/src/bin/scripts/createuser.c b/src/bin/scripts/createuser.c
index 88b8f2a..04ec324 100644
--- a/src/bin/scripts/createuser.c
+++ b/src/bin/scripts/createuser.c
@@ -308,7 +308,7 @@ main(int argc, char *argv[])
if (conn_limit != NULL)
appendPQExpBuffer(&sql, " CONNECTION LIMIT %s", conn_limit);
if (roles != NULL)
-   appendPQExpBuffer(&sql, " IN ROLE %s", roles);
+   appendPQExpBuffer(&sql, " IN ROLE \"%s\"", roles);
appendPQExpBufferStr(&sql, ";\n");

if (echo)
(END)

I was conscious of not quoting it.  Note that other parameters are not quoted
either, so I imagined I was being consistent with that.

I have added the above change, as well as rebasing, per Peter's recommendation.
-- 
When confronted by a difficult problem, solve it by reducing it to the
question, "How would the Lone Ranger handle this?"
diff --git a/doc/src/sgml/ref/createuser.sgml b/doc/src/sgml/ref/createuser.sgml
index 2f1ea2f..5a38d2e 100644
--- a/doc/src/sgml/ref/createuser.sgml
+++ b/doc/src/sgml/ref/createuser.sgml
@@ -131,6 +131,16 @@ PostgreSQL documentation
  
 
  
+  -g roles
+  --roles=roles
+  
+   
+Indicates roles to which this role will be added immediately as a new 
member.
+   
+  
+ 
+
+ 
   -i
   --inherit
   
diff --git a/src/bin/scripts/createuser.c b/src/bin/scripts/createuser.c
index 83623ea..04ec324 100644
--- a/src/bin/scripts/createuser.c
+++ b/src/bin/scripts/createuser.c
@@ -24,6 +24,7 @@ main(int argc, char *argv[])
{"host", required_argument, NULL, 'h'},
{"port", required_argument, NULL, 'p'},
{"username", required_argument, NULL, 'U'},
+   {"roles", required_argument, NULL, 'g'},
{"no-password", no_argument, NULL, 'w'},
{"password", no_argument, NULL, 'W'},
{"echo", no_argument, NULL, 'e'},
@@ -57,6 +58,7 @@ main(int argc, char *argv[])
char   *host = NULL;
char   *port = NULL;
char   *username = NULL;
+   char   *roles = NULL;
enum trivalue prompt_password = TRI_DEFAULT;
boolecho = false;
boolinteractive = false;
@@ -83,7 +85,7 @@ main(int argc, char *argv[])
 
handle_help_version_opts(argc, argv, "createuser", help);
 
-   while ((c = getopt_long(argc, argv, "h:p:U:wWedDsSaArRiIlLc:PEN",
+   while ((c = getopt_long(argc, argv, "h:p:U:g:wWedDsSaArRiIlLc:PEN",

Re: [HACKERS] Handling GIN incomplete splits

[Heikki Linnakangas]

On 19.11.2013 14:48, Michael Paquier wrote:

Here is a review of the first three patches:
1) Further gin refactoring:
make check passes (core tests and contrib tests).
Code compiles without warnings.


Committed.


Then... About the patch... Even if I got little experience with code of
gin, moving the flag for search mode out of btree, as well as removing the
logic of PostingTreeScan really makes the code lighter and easier to follow.
Just wondering, why not simplifying as well ginTraverseLock:ginbtree.c at
the same time to something similar to that?
if (!GinPageIsLeaf(page) || searchMode == TRUE)
return access;

/* we should relock our page */
LockBuffer(buffer, GIN_UNLOCK);
LockBuffer(buffer, GIN_EXCLUSIVE);

/* But root can become non-leaf during relock */
if (!GinPageIsLeaf(page))
{
/* restore old lock type (very rare) */
LockBuffer(buffer, GIN_UNLOCK);
LockBuffer(buffer, GIN_SHARE);
 }
else
access = GIN_EXCLUSIVE;
 return access;
Feel free to discard as I can imagine that changing such code would make
back-branch maintenance more difficult and it would increase conflicts with
patches currently in development.


Yeah, might be more readable to write it that way. There's a lot of 
cleanup that could be done to the gin code, these patches are by no 
means the end of it.



2) Refactoring of internal gin btree (needs patch 1 applied first):
make check passes (core tests and contrib tests).
Code compiles without warnings.


Committed.


Yep, removing ginPageGetLinkItup makes sense. Just to be picky, I would
have put the arguments of GinFormInteriorTuple replacing ginPageGetLinkItup
in 3 separate lines just for lisibility.


Ok, did that.


In dataPrepareDownlink:gindatapage.c, depending on if lpage is a leaf page
or not, isn't it inconsistent with the older code not to use
GinDataPageGetItemPointer and GinDataPageGetPostingItem to set
btree->pitem.key.


Hmm. The old code in dataSplitPage() didn't use 
GinDataPageGetItemPointer/PostingItem either.


The corresponding code in ginContinueSplit did, though. There was 
actually an inconsistency there: the ginContinueSplit function took the 
downlink's key from the last item on the page (using maxoff), while 
dataSplitPage took it from the right bound using 
GinDataPageGetRightBound(). Both are the same, dataSplitPage copies the 
value from the last item to the right bound, so it doesn't make a 
difference. They would diverge if the last item on the page is deleted, 
though, so the old coding in ginContinueSplit was actually a bit suspicious.



In ginContinueSplit:ginxlog.c, could it be possible to remove this code? It
looks that its deletion has been forgotten:
/*

  * elog(NOTICE,"ginContinueSplit root:%u l:%u r:%u",  split->rootBlkno,
  * split->leftBlkno, split->rightBlkno);
  */


Yeah, that's just leftover debug code. But again, I'll leave that for 
another patch (in fact, the whole function will go away with the fourth 
patch, anyway).



3) More refactoring (needs patches 1 and 2):
make check passes (core tests and contrib tests).
Code compiles without warnings.
Perhaps this patch would have been easier to read with context diffs :) It
just moves code around so nothing to say.


Committed.

Thanks for the review! I'll let you finish the review of the fourth 
patch. Meanwhile, I'll take another look at Alexander's gin packed 
posting items patch, and see how badly these commits bitrotted it.

- Heikki


--
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] Proof of concept: standalone backend with full FE/BE protocol

[Tom Lane]

Andres Freund  writes:
> On 2013-11-20 10:48:20 -0500, Tom Lane wrote:
>> constraining what can be executed as a "standalone backend".  Would
>> it work to insist that psql/pg_dump launch the program named postgres
>> from the same bin directory they're in, rather than accepting a path
>> from the connection string?

> But why do we want to start the server through the connection string
> using PQconnectb() in the first place? That doesn't really seem right to
> me.
> Something like PQstartSingleUser(dsn) returning a established connection
> seems better to me.

That just pushes the problem up a level --- how are you going to tell
psql, pg_dump, or other programs that they should do that?

regards, tom lane


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

Re: [HACKERS] Proof of concept: standalone backend with full FE/BE protocol

[Andres Freund]

On 2013-11-20 10:48:20 -0500, Tom Lane wrote:
> constraining what can be executed as a "standalone backend".  Would
> it work to insist that psql/pg_dump launch the program named postgres
> from the same bin directory they're in, rather than accepting a path
> from the connection string?

But why do we want to start the server through the connection string
using PQconnectb() in the first place? That doesn't really seem right to
me.
Something like PQstartSingleUser(dsn) returning a established connection
seems better to me.

Greetings,

Andres Freund

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


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

Re: [HACKERS] Proof of concept: standalone backend with full FE/BE protocol

[Tom Lane]

Peter Eisentraut  writes:
> I would consider sidestepping this entire issue by having the
> stand-alone backend create a Unix-domain socket and have a client
> connect to that in the normal way.

Hmm.  But that requires the "stand-alone backend" to take on at least
some properties of a postmaster; at the very least, it would need to
accept some form of shutdown signal (not just EOF on its stdin).

Perhaps more to the point, I think this approach actually breaks one of
the principal good-thing-in-emergencies attributes of standalone mode,
namely being sure that nobody but you can connect.  With this, you're
right back to having a race condition as to whether your psql command
gets to the socket before somebody else.

I think we'd be better off trying to fix the security issue by
constraining what can be executed as a "standalone backend".  Would
it work to insist that psql/pg_dump launch the program named postgres
from the same bin directory they're in, rather than accepting a path
from the connection string?

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] Autoconf 2.69 update

[Andrew Dunstan]

On 11/20/2013 10:28 AM, Magnus Hagander wrote:


On Wed, Nov 20, 2013 at 3:58 PM, Andres Freund > wrote:



On 2013-11-20 09:53:53 -0500, Tom Lane wrote:
> As a rule, you're not supposed to bother including the configure
output
> script in a submitted diff anyway.  Certainly any committer
worth his
> commit bit is going to ignore it and redo autoconf for himself.

The committer maybe, but it's a PITA for reviewers on machines without
the matching autoconf version around. Which at least currently
frequently isn't packaged anymore...


That's going to be a PITA whichever way you go, though, because there 
is not one standard about which autoconf version distros have. It's 
certainly not all that have 2.69. I frequently do my builds on Ubuntu 
12.04 for example, which has 2.15,  2.59, 2.64 and 2.68 (don't ask me 
how they ended up with that combination).


The point is - regardless of which version you chose, reviewers and 
committers are going to have to deal with a local installation in many 
cases anyway. So we might be better off just documenting that in a 
more clear way.






And it only matters if you're reviewing things that touch the configure 
setup. That's a tiny minority of patches.


cheers

andrew


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

Re: [HACKERS] Autoconf 2.69 update

[Magnus Hagander]

On Wed, Nov 20, 2013 at 3:58 PM, Andres Freund wrote:

>
> On 2013-11-20 09:53:53 -0500, Tom Lane wrote:
> > As a rule, you're not supposed to bother including the configure output
> > script in a submitted diff anyway.  Certainly any committer worth his
> > commit bit is going to ignore it and redo autoconf for himself.
>
> The committer maybe, but it's a PITA for reviewers on machines without
> the matching autoconf version around. Which at least currently
> frequently isn't packaged anymore...
>
>
That's going to be a PITA whichever way you go, though, because there is
not one standard about which autoconf version distros have. It's certainly
not all that have 2.69. I frequently do my builds on Ubuntu 12.04 for
example, which has 2.15,  2.59, 2.64 and 2.68 (don't ask me how they ended
up with that combination).

The point is - regardless of which version you chose, reviewers and
committers are going to have to deal with a local installation in many
cases anyway. So we might be better off just documenting that in a more
clear way.

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

Re: [HACKERS] Shave a few instructions from child-process startup sequence

[Peter Eisentraut]

On 11/5/13, 2:47 AM, Gurjeet Singh wrote:
> On Mon, Nov 4, 2013 at 12:20 AM, Tom Lane  > wrote:
> 
> But we're not buying much.  A few instructions during postmaster
> shutdown
> is entirely negligible.
> 
> 
> The patch is for ClosePostmasterPorts(), which is called from every
> child process startup sequence (as $subject also implies), not in
> postmaster shutdown. I hope that adds some weight to the argument.

If there is a concern about future maintenance, you could add assertions
(in appropriate compile mode) that the rest of the array is indeed
PGINVALID_SOCKET.  I think that could be a win for both performance and
maintainability.


-- 
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] Suggestion: Issue warning when calling SET TRANSACTION outside transaction block

[Bruce Momjian]

On Wed, Nov 20, 2013 at 10:04:22AM -0500, Tom Lane wrote:
> Bruce Momjian  writes:
> > On Tue, Nov 19, 2013 at 10:21:47PM -0500, Tom Lane wrote:
> >> My personal standpoint is that I don't care much whether these messages
> >> are NOTICE or WARNING.  What I'm not happy about is promoting cases that
> >> have been non-error conditions for years into ERRORs.
> 
> > I don't remember any cases where that was suggested.
> 
> Apparently you've forgotten the commit that was the subject of this
> thread.  You took a bunch of SET cases that we've always accepted
> without any complaint whatsoever, and made them into ERRORs, thereby
> breaking any applications that might've expected such usage to be
> harmless.  I would be okay if that patch had issued WARNINGs, which
> as you can see from the thread title was the original suggestion.

Oh, those changes.  I thought we were just looking at _additional_
changes.

> > The attached patch changes ABORT from NOTICE to WARNING, and documents
> > that all other are errors.  This "top-level" logic idea came from Robert
> > Haas, and it has some level of consistency.
> 
> This patch utterly fails to address my complaint.
> 
> More to the point, I think it's a waste of time to make these sorts of
> adjustments.  The only thanks you're likely to get for it is complaints
> about cross-version behavioral changes.  Also, you're totally ignoring
> the thought that these different message levels might've been selected
> intentionally, back when the code was written.  Since there have been
> no field complaints about the inconsistency, what's your hurry to
> change it?  See Emerson.

My problem was that they issued _no_ message at all.  I am fine with
them issuing a warning if that's what people prefer.  As they are all
SET commands, they will be consistent.

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

  + Everyone has their own god. +


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

Re: [HACKERS] Data corruption issues using streaming replication on 9.0.14/9.2.5/9.3.1

[Andres Freund]

On 2013-11-20 07:06:04 -0800, Kevin Grittner wrote:
> > That's not a bad point. So the way to fix it would be:
> > 
> > 1) Restart the standby to the new minor release, wait for catchup
> > 2) Restart the primary (fast or smart) to the new minor release
> > 3) Acquire enough new xids to make sure we cross a clog page (?)
> > 4) Jot down a new xid:  SELECT txid_current()::bigint % (1::bigint<<33-1)
> > 5) vacuumdb -z -a
> > 6) Ensure that there are no prepared xacts older than 3) around
> > SELECT *
> > FROM pg_prepared_xacts
> > ORDER BY age(transaction) DESC LIMIT 1;
> > 7) Ensure the xmin horizon is above the one from: 3:
> > SELECT datname, datfrozenxid
> > FROM pg_database
> > WHERE datname != 'template0'
> > ORDER BY age(datfrozenxid) DESC LIMIT 1;
> > 8) Get the current lsn: SELECT pg_current_xlog_location();
> > 9) verify on each standby that SELECT pg_last_xlog_receive_location() is
> >    past 7)
> > 10) be happy
> > 
> > I am not sure how we can easily compute that 6) and 7) are past 3) in
> > the presence of xid wraparounds.
> 
> 
> I may well be missing something here, but wouldn't it be sufficient to?:
> 1) Restart the standby to the new minor release, wait for catchup
> 2) Restart the primary (fast or smart) to the new minor release
> 3) Run VACUUM FREEZE (optionally with ANALYZE) in each database on primary
> 4) Run CHECKPOINT command on primary, or just wait for one to run
> 5) Wait for standby to process to the checkpoint
> 6) Be happy

Well, in some cases it might. But what if there's a prepared xact
around? Or a transaction started directly after 2) preventing
FreezeLimit to go up? Or vacuum_defer_cleanup_age is set? Or there's
another bug like 4c697d8f4845823a8af67788b219ffa4516ad14c?

Greetings,

Andres Freund

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


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