date:20131121

On Fri, Nov 22, 2013 at 1:33 AM, Alvaro Herrera
 wrote:
> Piotr Marcinczyk escribió:
>
>> 
>> + \ib > class="parameter">filename [ > class="parameter">quote_string ] 
>> + 
>> + 
>> + The \ib command appends content of file 
>> filename
>> + to current query buffer. If parameter 
>> quote_string
>> + is not set, no quotation is used. If it is set, content of file 
>> will be
>> + quoted by quote_string enclosed in 
>> $.
>> + 
>> + 
>> +   
>
> Doesn't this quoting thing seem like a usability problem?  I mean,
> there's no way you can possibly know what string to use unless you first
> verify the contents of the file yourself.  I think this is something
> that should be done automatically by psql.
>
> But, really, having to read stuff and transform into a quoted literal
> seems wrong to me.  I would like something that would read into a client
> variable that can later be used as a positional parameter to a
> parametrized query, so
>
> \ib homer ~/photos/homer.jpg
> insert into people (name, photo) values ('Homer', :homer);

 Isn't something similar already supported as mentioned in docs:

One example use of this mechanism is to copy the contents of a file
into a table column. First load the file into a variable and then
interpolate the variable's value as a quoted string:

testdb=> \set content `cat my_file.txt`
testdb=> INSERT INTO my_table VALUES (:'content');

or do you prefer an alternative without any kind of quote using \ib?

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

[HACKERS] Server is not getting started with log level as debug5 on master after commit 3147ac

In master branch, server is not getting started with log level as debug5.

Simple steps to reproduce the problem:
a. initdb -D ..\..\Data
b. change log_min_messages = debug5 in postgresql.conf
c. start server (pg_ctl start -D ..\..\Data)  -- server doesn't get started

Relevant message on server console:
DEBUG:  mapped win32 error code 2 to 2
FATAL:  could not open recovery command file "recovery.conf": No error

This problem occurs only in Windows, but the cause of problem is generic.

The problem occurs when during startup, code tries to open
recovery.conf in below function:
readRecoveryCommandFile(void)
{
..
fd = AllocateFile(RECOVERY_COMMAND_FILE, "r");
if (fd == NULL)
{
if (errno == ENOENT)
return; /* not there, so no archive recovery */
ereport(FATAL,
(errcode_for_file_access(),
errmsg("could not open recovery command file \"%s\": %m",
RECOVERY_COMMAND_FILE)));
..
}

It is expected that if file doesn't exist, errno should be ENOENT
which is correct and was working fine until commit:
3147acd63e0135aff9a6c4b01d861251925d97d9
Use improved vsnprintf calling logic in more places.

The reason is that after this commit, function appendStringInfoVA has
started calling pvsnprintf() which initializes errno to 0. As function
appendStringInfoVA gets called in error path
(errmsg_internal()->EVALUATE_MESSAGE->appendStringInfoVA), it can
reset previous errno if there is any, as it happens for function
_dosmaperr().

_dosmaperr(unsigned long e)
{
 ...
 errno = doserrors[i].doserr;
 #ifndef FRONTEND
 ereport(DEBUG5,
 (errmsg_internal("mapped win32 error code %lu to %d",
 e, errno)));
..
}

Here after setting errno, it calls errmsg_internal() which internally
resets errno to zero, so the callers of this function who are
expecting proper errno can fail which is what happens in current
issue.

I could think of below possible ways to fix the problem:
a. in function pvsnprintf(), save the errno before setting it to 0 and
then before exiting function reset it back to saved errno. I think
this is sane because in function pvsnprintf, if any error occurs due
to which errno is changed, it will not return control, so errno will
not be required by callers.
b. we can change the callers like _dosmaperr() who have responsibility
to save errno for their callers.

Patch with approach a) is attached with mail, we can change code as
per approach b) or any other better method as well, but for now I have
prepared patch with approach a), as I could not see any problem with
it.


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


pvsnprintf_issue.patch
Description: Binary data

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

Re: [HACKERS] psql should show disabled internal triggers

2013-11-21 Thread Fabrízio de Royes Mello

On Fri, Oct 25, 2013 at 3:37 PM, fabriziomello 
wrote:
>
> On 2013-09-18 15:15:55 +0200, Bernd Helmle wrote:
> > On 2013-09-18 15:15:55 +0200, Bernd Helmle wrote:
> > > --On 18. September 2013 13:52:29 +0200 Andres Freund
> > >  wrote:
> > >
> > > >If you do ALTER TABLE ... DISABLE TRIGGER ALL; and then individually
> > > >re-enable the disabled triggers it's easy to miss internal triggers.
> > > >A \d+ tablename will not show anything out of the ordinary for that
> > > >situation since we don't show internal triggers. But foreign key
checks
> > > >won't work.
> > > >So, how about displaying disabled internal triggers in psql?
> > >
> > > Hi had exactly the same concerns this morning while starting to look
at
> > the
> > > ENABLE/DISABLE constraint patch. However, i wouldn't display them as
> > > triggers, but maybe more generally as "disabled constraints" or such.
> >
> > Well, that will lead the user in the wrong direction, won't it? They
> > haven't disabled the constraint but the trigger. Especially as we
> > already have NOT VALID and might grow DISABLED for constraint
> > themselves...
> >
>
> Hi,
>
> The attached patch [1] enable PSQL to list internal disabled triggers in
\d
> only in versions >= 9.0.
>
> [1]  psql-display-all-triggers-v1.patch
> <
http://postgresql.1045698.n5.nabble.com/file/n5775954/psql-display-all-triggers-v1.patch
>
>

Hi all,

I'm just send a new WIP patch rebased from master.

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/src/bin/psql/describe.c b/src/bin/psql/describe.c
index 96322ca..f457e21 100644
--- a/src/bin/psql/describe.c
+++ b/src/bin/psql/describe.c
@@ -2079,7 +2079,7 @@ describeOneTableDetails(const char *schemaname,
 		  (pset.sversion >= 9 ? ", true" : ""),
 		  oid);
 		if (pset.sversion >= 9)
-			appendPQExpBufferStr(&buf, "NOT t.tgisinternal");
+			appendPQExpBuffer(&buf, "(NOT t.tgisinternal OR (t.tgisinternal AND t.tgenabled = 'D'))");
 		else if (pset.sversion >= 80300)
 			appendPQExpBufferStr(&buf, "t.tgconstraint = 0");
 		else

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

Re: [HACKERS] Status of FDW pushdowns

2013-11-21 Thread David Fetter

On Thu, Nov 21, 2013 at 10:46:14AM -0500, Tom Lane wrote:
> Merlin Moncure  writes:
> > On Thu, Nov 21, 2013 at 9:05 AM, Bruce Momjian  wrote:
> >> I know join pushdowns seem insignificant, but it helps to restrict what
> >> data must be passed back because you would only pass back joined rows.
> 
> > By 'insignificant' you mean 'necessary to do any non-trivial real
> > work'.   Personally, I'd prefer it if FDW was extended to allow
> > arbitrary parameterized queries like every other database connectivity
> > API ever made ever.
> 
> [ shrug... ]  So use dblink.

Not with a non-PostgreSQL data source.

> For better or worse, the FDW stuff is following the SQL standard's
> SQL/MED design, which does not do it like that.

What SQL/MED specifies along this line is purely a caution against
making a specification without a reference implementation.  If I'm
reading it correctly, it's literally impossible to make what they
suggest safe.

Given those givens, we're free to do this in a way that's not
barking-at-the-moon crazy.  At least two inter-database communication
links which work with PostgreSQL do this..

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

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

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

Re: [HACKERS] GSOC13 proposal - extend RETURNING syntax

2013-11-21 Thread David Fetter

On Fri, Oct 04, 2013 at 10:42:32AM +0200, Karol Trzcionka wrote:
> W dniu 04.10.2013 02:51, Robert Haas pisze:
> > Do you have a link to previous discussion on the mailing list?
> Sorry, most of discussion was at IRC channel.
> > I'm not positive there's enough information available
> > at that stage, but if p_target_rangetblentry is populated at that
> > point, you should be able to make AFTER.x translate to a Var
> > referencing that range table entry.
> It's not enough. Even if we know "where we are", there are more issues.
> The main question is: how should we pass information about "hello, I'm
> specific Var, don't evaluate me like others"? We can add two fields to
> Var structure (flag - normal/before/after and no. column) - however it
> needs to modify copyObject for Vars (at now it's done e.g. in
> flatten_join_alias_vars_mutator for varoattno and varnoold). If
> copyObject is modified, sure code in
> flatten_join_alias_vars_mutator/pullup_replace_vars_callback will be
> useless. I don't know if modifying pg at the low-level (changing
> structure of Var and behaviour of copyObject) is good idea. Yes if the
> community really want it but it needs more "votes". There is "medium"
> solution: changing Var structure and do the "copy" like now (in mutator
> and callback) but w/o the condition statement (for the new fields). I
> think it might need to modify more places in code because of "comparing"
> vars (maybe we'd need to include new fields while comparision).
> Regards,
> Karol Trzcionka

Karol,

Do you plan to continue this work for the current commitfest?  A lot
of people really want the feature :)

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

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


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

Re: [HACKERS] Status of FDW pushdowns

2013-11-21 Thread Shigeru Hanada

2013/11/22 Tom Lane :
> Merlin Moncure  writes:
>> On Thu, Nov 21, 2013 at 9:05 AM, Bruce Momjian  wrote:
>>> I know join pushdowns seem insignificant, but it helps to restrict what
>>> data must be passed back because you would only pass back joined rows.
>
>> By 'insignificant' you mean 'necessary to do any non-trivial real
>> work'.   Personally, I'd prefer it if FDW was extended to allow
>> arbitrary parameterized queries like every other database connectivity
>> API ever made ever.
>
> [ shrug... ]  So use dblink.  For better or worse, the FDW stuff is
> following the SQL standard's SQL/MED design, which does not do it
> like that.

Pass-through mode mentioned in SQL/MED standard might be what he wants.

-- 
Shigeru HANADA


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

Re: [HACKERS] Status of FDW pushdowns

2013-11-21 Thread Shigeru Hanada

2013/11/22 Kohei KaiGai :
> 2013/11/21 Bruce Momjian :
>> Where are we on the remaining possible pushdowns for foreign data
>> wrappers, particularly the Postgres one?  I know we do WHERE restriction
>> pushdowns in 9.3, but what about join and aggregate pushdowns?  Is
>> anyone working on those?
>>
>> I know join pushdowns seem insignificant, but it helps to restrict what
>> data must be passed back because you would only pass back joined rows.
>>
>> Do we document these missing features anywhere?
>>
> Probably, custom-scan api will provide more flexible way to push-down
> aggregate, sort or other stuff performing on regular tables, not only
> foreign tables.
> It allows extensions to offer alternative scan/join path on the planning
> stage, then executor callbacks its custom logic instead of the built-in
> one, if its cost is cheaper.

IIRC, sort push-down is already supported.  We can provide sorted
pathes by setting Pathkeys to additional ForeignPath.  postgres_fdw
doesn't support this feature because we couldn't get consensus about
how to limit sort variation. One idea was to allow to define "foreign
index" on foreign tables to indicate which column combination is
reasonably sortable.
-- 
Shigeru HANADA


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

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

I've committed this patch after some significant editorialization, but
leaving the use of TABLE( ... ) syntax in-place.  If we decide that we
don't want to risk doing that, we can change to some other syntax later.

regards, tom lane


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

Re: [HACKERS] WIP patch for updatable security barrier views

2013-11-21 Thread Craig Ringer

On 11/21/2013 10:55 PM, Dean Rasheed wrote:
> On 21 November 2013 13:15, Craig Ringer  wrote:
>> Hi all
>>
>> I have updatable security barrier views working for INSERT and DELETE,
>> so this might be a good chance to see whether the described approach is
>> acceptable in reality, not just in theory.
>>
>> I've been surprised by how well it worked out. I actually landed up
>> removing a lot of the existing updatable views code;
> 
> I fear you have removed a little too much though.

Absolutely. This is really a proof of concept to show that we can do DML
directly on a subquery by adding a new RTE for the resultRelation to the
top level of the query.

WITH CHECK OPTION was a casualty of cutting to prove the concept; I know
it needs to be fitted into the subquery based approach. I really haven't
thought about how WITH CHECK OPTION will fit into this, which may be a
mistake - I'm hoping to deal with it after I have the basics working.

There's lots of work to do, some of which will be adapting the code in
your updatable views code to work with this approach, moving them around
where appropriate.

There's also the need to inject columns for UPDATE so the whole tuple is
produced, and deal with DEFAULTs for INSERT.

> For example, something somewhere has to deal with re-mapping of
> attributes. That's pretty fundamental, otherwise it can't hope to work
> properly.
> 
> CREATE TABLE t1(a int, b text);
> CREATE VIEW v1 AS SELECT b, a FROM t1;
> INSERT INTO v1(a, b) VALUES(1, 'B');
> 
> ERROR:  table row type and query-specified row type do not match
> DETAIL:  Table has type integer at ordinal position 1, but query expects text.

Thanks. At this point I only expect it to work solidly for DELETE, and
was frankly surprised that INSERT worked at all.

The example of the problem is clear and useful, so thanks. I'm still
learning about the handling of attributes and target lists - that's the
next step in work on this.

> It sounds like it could be an interesting approach in principle, but
> you haven't yet shown how you intend to replace some of the rewriter
> code that you've removed. It feels to me that some of those things
> pretty much have to happen in the rewriter, because the planner just
> doesn't have the information it needs to be able to do it.

I'm still working a lot of that out. At this point I just wanted to
demonstrate that we can in fact do DML directly on a subquery without
view qual pull-up and view subquery flattening.

One of my main worries is that adding and re-ordering columns needs to
happen from the bottom up - for nested views, it needs to start at the
real base rel and then proceed back up the chain of nested subqueries.
View expansion happens recursively in the rewriter, so this isn't too
easy. I'm thinking of doing it when we hit a real baserel during view
expansion, walking back up the query tree doing fixups then.

-- 
 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] Patch for fail-back without fresh backup

On 2013-11-21 14:40:36 -0800, Jeff Janes wrote:
> But if the transaction would not have otherwise generated WAL (i.e. a
> select that did not have to do any HOT pruning, or an update with zero rows
> matching the where condition), doesn't it now have to flush and wait when
> it would otherwise not?

We short circuit that if there's no xid assigned. Check
RecordTransactionCommit().

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] Patch for fail-back without fresh backup

2013-11-21 Thread Jeff Janes

On Thu, Nov 21, 2013 at 12:31 PM, Alvaro Herrera
wrote:

> Bruce Momjian escribió:
> > On Thu, Oct 24, 2013 at 11:14:14PM +0300, Heikki Linnakangas wrote:
> > > On 24.10.2013 23:07, Josh Berkus wrote:
>
> > > >What kind of overhead are we talking about here?
> > >
> > > One extra WAL record whenever a hint bit is set on a page, for the
> > > first time after a checkpoint. In other words, a WAL record needs to
> > > be written in the same circumstances as with page checksums, but the
> > > WAL records are much smaller as they don't need to contain a full
> > > page image, just the block number of the changed block.
> > >
> > > Or maybe we'll write the full page image after all, like with page
> > > checksums, just without calculating the checksums. It might be
> > > tricky to skip the full-page image, because then a subsequent change
> > > of the page (which isn't just a hint-bit update) needs to somehow
> > > know it needs to take a full page image even though a WAL record for
> > > it was already written.
> >
> > Sorry to be replying late to this, but while I am not worried about the
> > additional WAL volume, does this change require the transaction to now
> > wait for a WAL sync to disk before continuing?
>
> I don't think so.  There's extra WAL written, but there's no
> flush-and-wait until end of transaction (as has always been).
>


But if the transaction would not have otherwise generated WAL (i.e. a
select that did not have to do any HOT pruning, or an update with zero rows
matching the where condition), doesn't it now have to flush and wait when
it would otherwise not?

Cheers,

Jeff

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

On 2013-11-21 17:14:17 -0500, Peter Eisentraut wrote:
> On 11/21/13, 2:35 AM, Pavel Stehule wrote:
> > I am feeling, so almost all people prefer 
> >  
> > DROP TRIGGER [ IF EXISTS ] name ON table_name [ CASCADE | RESTRICT ];
> > 
> > Can we live with it?
> 
> Fine with me.
> 
> I think it helps if you consider IF EXISTS an attribute of the command,
> not an attribute of the command parameters.
> 
> Now we should be aware that this sort of sets a precedent for ALTER
> TABLE IF EXISTS ... DROP ANYTHING ... and similar composite commands.

That already has 2 independent IF EXISTS, so I think the precedence
argument goes the other way round.

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

On 11/20/13, 8:39 PM, Rodolfo Campero wrote:
> 2013/11/20 Peter Eisentraut mailto:pete...@gmx.net>>
> 
> Updated patch
> 
> 
> I can't apply the patch; maybe I'm doing something wrong?

It looks like you are not in the right directory.



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

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

On 11/21/13, 2:35 AM, Pavel Stehule wrote:
> I am feeling, so almost all people prefer 
>  
> DROP TRIGGER [ IF EXISTS ] name ON table_name [ CASCADE | RESTRICT ];
> 
> Can we live with it?

Fine with me.

I think it helps if you consider IF EXISTS an attribute of the command,
not an attribute of the command parameters.

Now we should be aware that this sort of sets a precedent for ALTER
TABLE IF EXISTS ... DROP ANYTHING ... and similar composite commands.

If might be worth checking other SQL databases.  We stole the IF EXISTS
from somewhere, I believe.

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

On 11/21/13, 6:15 AM, Andres Freund wrote:
> On 2013-11-20 15:05:17 -0500, Robert Haas wrote:
>>> 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
> 
> Well yes. But then you need a way to store and change that random id for
> each cluster.

You can use a v3 UUID, which is globally reproducible.



-- 
Sent 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] Use MAP_HUGETLB where supported (v3)

On 2013-11-21 16:24:56 -0500, Robert Haas wrote:
> > What about
> > huge_tlb_pages={off,try}
> >
> > Or maybe
> > huge_tlb_pages={off,try,require}
> 
> I'd spell "require" as "on", or at least accept that as a synonym.

That's off,try, on is what the patch currently implements, Abhijit just
was arguing for dropping the error-out option.

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] new unicode table border styles for psql

2013-11-21 Thread Andrew Dunstan



On 11/21/2013 04:39 PM, Peter Eisentraut wrote:

On 11/21/13, 2:09 AM, Pavel Stehule wrote:

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?

Maybe make the border setting a string containing the various characters
by index.  Then everyone can create their own crazy borders.




Why not just reinvent termcap / terminfo? :-)

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] preserving forensic information when we freeze

On 2013-11-21 15:59:35 -0500, Robert Haas wrote:
> > * Should HeapTupleHeaderXminFrozen also check for FrozenTransactionId?
> >   It seems quite possible that people think they've delt with frozen
> >   xmin entirely after checking, but they still might get
> >   FrozenTransactionId back in a pg_upgraded cluster.
> 
> The reason I originally wrote the patch the way I did, rather than the
> way that you prefer, is that it minimizes the number of places where
> we might perform extra tests that are known not to be needed in
> context.  These code paths are hot.

The patch as sent shouldn't really do that in any of paths I know to be
hot - it uses *RawXmin() there.

> If you do this sort of thing,  then after macro expansion we may end up with 
> a lot of things like:
> (flags & FROZEN) || (rawxid == 2) ? 2 : rawxid.  I want to avoid that.

But in which cases would that actually be slower? There'll be no
additional code executed if the hint bits for frozen are set, and in
case not it will usually safe us an external function call to
TransactionIdPrecedes().

>  That macros is intended, specifically, to be a test for flag bits,
> and I think it should do precisely that.  If that's not what you want,
> then don't use that macro.

That's a fair argument. Although there's several HeapTupleHeader* macros
that muck with stuff besides infomask.

> > * Existing htup_details boolean checks contain an 'Is', but
> >   HeapTupleHeaderXminCommitted, HeapTupleHeaderXminInvalid,
> >   HeapTupleHeaderXminFrozen don't contain any verb. Not sure.
> 
> We could say XminIsComitted, XminIsInvalid, XminIsFrozen, etc.  I
> don't particularly care for it, but I can see the argument for it.

I don't have a clear preference either, I just noticed the inconsistency
and wasn't sure whether it was intentional.

> > * EvalPlanQualFetch() uses priorXmax like logic to find updated versions
> >   of tuples. I am not 100% sure there aren't cases where that's
> >   problematic even with the current code, but I think it's better not to
> >   use the raw xid there - priorXmax can never be FrozenTransactionId, so
> >   comparing it to the GetXmin() seems better.
> >   It also has the following wrong comment:
> >  *   (Should be safe to examine xmin without getting
> >  * buffer's content lock, since xmin never changes in an existing
> >  * tuple.)
> 
> Hmm.  The new tuple can't be frozen unless it's all-visible, and it
> can't be all-visible if our scan can still see its predecessor.  That
> would imply that if it's frozen, it must be an unrelated tuple.  I
> think.

Yes, that's my reasoning as well - and why I think we should check for
new-style frozen xids. Either via my version of GetXmin() or
HeapTupleHeaderXminFrozen() if we go with yours.

> > I think once we have this we should start opportunistically try to
> > freeze tuples during vacuum using OldestXmin instead of FreezeLimit if
> > the page is already dirty.
> 
> Separate patch, but yeah, something like that.  If we have to mark the
> page all-visible, we might as well freeze it while we're there.  We
> should think about how it interacts with Heikki's freeze-without-write
> patch though.

Definitely separate yes. And I agree, it's partially moot if Heikki's
patch gets in, but I am not sure it will make it into 9.4. There seems
to be quite some work left.

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] new unicode table border styles for psql

On 11/21/13, 2:09 AM, Pavel Stehule wrote:
> 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?

Maybe make the border setting a string containing the various characters
by index.  Then everyone can create their own crazy borders.



-- 
Sent 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 unicode table border styles for psql

On Thu, Nov 21, 2013 at 1:09 AM, Pavel Stehule  wrote:
> 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.
>
great. hm, maybe we could integrate color? (see:
http://merlinmoncure.blogspot.com/2012/09/psql-now-with-splash-of-color.html).

merlin


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

Re: [HACKERS] COMMENT ON CONSTRAINT ON DOMAIN ?

On 11/21/13, 3:29 PM, Elvis Pranskevichus wrote:
> I may be totally missing something, but there seems to be no way 
> to create a COMMENT on a domain constraint.  There is
> 
> COMMENT ON CONSTRAINT constraint_name ON table_name
> 
> but no such thing for domain constraints, which seems like a 
> weird omission.

Yeah.  Feel free to implement it.  It shouldn't be hard.



-- 
Sent 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] Use MAP_HUGETLB where supported (v3)

On Thu, Nov 21, 2013 at 4:09 PM, Alvaro Herrera
 wrote:
> Abhijit Menon-Sen wrote:
>> At 2013-11-15 15:17:32 +0200, hlinnakan...@vmware.com wrote:
>
>> > But I'm not wedded to the idea if someone objects; a log message might
>> > also be reasonable: "LOG: huge TLB pages are not supported on this
>> > platform, but huge_tlb_pages was 'on'"
>>
>> Put that way, I have to wonder if the right thing to do is just to have
>> a "try_huge_pages=on|off" setting, and log a warning if the attempt did
>> not succeed. It would be easier to document, and I don't think there's
>> much point in making it an error if the allocation fails.
>
> What about
> huge_tlb_pages={off,try}
>
> Or maybe
> huge_tlb_pages={off,try,require}

I'd spell "require" as "on", or at least accept that as a synonym.

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

On 2013-11-21 23:02:29 +0200, Heikki Linnakangas wrote:
> On 21.11.2013 22:53, Andres Freund wrote:
> >On 2013-11-21 12:51:17 -0800, Josh Berkus wrote:
> >>On 11/21/2013 12:46 PM, Andres Freund wrote:
> >>>The problem is starting with hot_standby=on on a system with
> >>>recovery.conf present. It is independent of whether you use streaming
> >>>replication, archive based recovery, or just shutdown the server and
> >>>manually copy xlog segments there.
> >>>As long as hot_standby=on, and recovery.conf is present you can hit the
> >>>bug.
> >>
> >>Oh, aha.  There have to be some transactions which are awaiting
> >>checkpoint, though, correct?  As in, if there's no activity on the
> >>master, you can't trigger the bug?
> >
> >Correct. Also, if you *start* at such a checkpoint you're not vulnerable
> >until the standby is restarted.
> 
> Keep in mind that autovacuum counts as "activity" in this case. If you're
> unlucky, that is. It's next to impossible to determine after-the-fact if
> there has been activity in the master that might've caused problems.

Well, in that case you're going to "just" loose the
pg_database/pg_class/stats updates from analyze/vacuum. That's annoying,
but not too bad.

> I wouldn't try to narrow it down any further than that, it gets too
> complicated.

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] [PATCH] Use MAP_HUGETLB where supported (v3)

On 2013-11-21 18:09:38 -0300, Alvaro Herrera wrote:
> Abhijit Menon-Sen wrote:
> > At 2013-11-15 15:17:32 +0200, hlinnakan...@vmware.com wrote:
> 
> > > But I'm not wedded to the idea if someone objects; a log message might
> > > also be reasonable: "LOG: huge TLB pages are not supported on this
> > > platform, but huge_tlb_pages was 'on'"
> > 
> > Put that way, I have to wonder if the right thing to do is just to have
> > a "try_huge_pages=on|off" setting, and log a warning if the attempt did
> > not succeed. It would be easier to document, and I don't think there's
> > much point in making it an error if the allocation fails.
> 
> What about
> huge_tlb_pages={off,try}
> 
> Or maybe
> huge_tlb_pages={off,try,require}

I'd certainly want a setting that errors out if it cannot get the memory
using hugetables. If you rely on the reduction in memory (which can be
significant on large s_b, large max_connections), it's rather annoying
not to know whether it suceeded using it.

Greetings,

Andres Freund

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


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

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


On 21.11.2013 22:55, Andres Freund wrote:

Hi,

On 2013-11-20 12:48:50 +0200, Heikki Linnakangas wrote:

On 19.11.2013 16:22, Andres Freund wrote:

On 2013-11-19 15:20:01 +0100, Andres Freund wrote:

Imo something the attached patch should be done. The description I came

g> >>up with is:


 Fix Hot-Standby initialization of clog and subtrans.


Looks ok for a back-patchable fix.


Do you plan to commit this? Or who is going to?


I can commit it tomorrow, unless someone beats me to 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

On Thu, Nov 21, 2013 at 8:14 PM, Tom Lane  wrote:
> Amit Kapila  writes:
>> Here what I have in mind is that:
>> a. In pg_dump or other internal utilities where we want to use this
>> feature, they should call PQenableStart()  or some other API before
>> calling PQConnect() which will indicate that it wants to operate
>> as a standalone mode.
>> b. In psql, if user specifies this special switch (
>> 'standalone_datadir'), then internally we will call PQenableStart()
>> and use postgres from same
>> directory.
>
> Why would you make psql behave differently from our other command-line
> clients?

   No, psql should not behave different from other clients. Sorry, I
was under assumption that for other programs we will not take backend
executable
   path.
   One other thing which is not clear to me is that how by calling
some special/new API we can ensure that the path provided by user is
   a valid path, are we going to validate file given in
'standalone_backend'  switch in some way?

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] [PATCH] Use MAP_HUGETLB where supported (v3)

Abhijit Menon-Sen wrote:
> At 2013-11-15 15:17:32 +0200, hlinnakan...@vmware.com wrote:

> > But I'm not wedded to the idea if someone objects; a log message might
> > also be reasonable: "LOG: huge TLB pages are not supported on this
> > platform, but huge_tlb_pages was 'on'"
> 
> Put that way, I have to wonder if the right thing to do is just to have
> a "try_huge_pages=on|off" setting, and log a warning if the attempt did
> not succeed. It would be easier to document, and I don't think there's
> much point in making it an error if the allocation fails.

What about
huge_tlb_pages={off,try}

Or maybe
huge_tlb_pages={off,try,require}

-- 
Á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] preserving forensic information when we freeze

Hi,

As promised I'm currently updating the patch. Some questions arose
during that:

* Should HeapTupleHeaderXminFrozen also check for FrozenTransactionId?
  It seems quite possible that people think they've delt with frozen
  xmin entirely after checking, but they still might get
  FrozenTransactionId back in a pg_upgraded cluster.
* Existing htup_details boolean checks contain an 'Is', but
  HeapTupleHeaderXminCommitted, HeapTupleHeaderXminInvalid,
  HeapTupleHeaderXminFrozen don't contain any verb. Not sure.
* EvalPlanQualFetch() uses priorXmax like logic to find updated versions
  of tuples. I am not 100% sure there aren't cases where that's
  problematic even with the current code, but I think it's better not to
  use the raw xid there - priorXmax can never be FrozenTransactionId, so
  comparing it to the GetXmin() seems better.
  It also has the following wrong comment:
 *   (Should be safe to examine xmin without getting
 * buffer's content lock, since xmin never changes in an existing
 * tuple.)
  But I don't see that causing any problems.
* ri_trigger.c did do a TransactionIdIsCurrentTransactionId() on a raw
  xmin value, the consequences are minor though.

The rewrite to introduce HeapTupleHeaderGet[Raw]Xmin() indeed somewhat
increases the patchsize - but I think it's enough increase in
expressiveness to be well worth the cost. Indeed it led me to find at
least one issue (with minor consequences).

I think once we have this we should start opportunistically try to
freeze tuples during vacuum using OldestXmin instead of FreezeLimit if
the page is already dirty.

Patch 01 is a rebased version of Robert's patch without further changes,
02 contains my suggested modifications. 

Greetings,

Andres Freund

-- 
 Andres Freund http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services
>From ac6c9a15836414208c49393d05d2d7d29e8fae4a Mon Sep 17 00:00:00 2001
From: Andres Freund 
Date: Thu, 21 Nov 2013 12:57:20 +0100
Subject: [PATCH 1/2] freeze-forensically-v1

Robert Haas
---
 src/backend/access/heap/heapam.c  | 50 +++
 src/backend/access/heap/rewriteheap.c |  1 +
 src/backend/catalog/index.c   |  1 +
 src/backend/commands/analyze.c|  1 +
 src/backend/commands/cluster.c|  1 +
 src/backend/commands/sequence.c   |  4 +--
 src/backend/commands/typecmds.c   |  3 ++-
 src/backend/commands/vacuumlazy.c |  9 +--
 src/backend/optimizer/util/plancat.c  |  1 +
 src/backend/storage/lmgr/predicate.c  |  7 -
 src/backend/utils/adt/ri_triggers.c   |  6 +++--
 src/backend/utils/cache/relcache.c|  8 --
 src/backend/utils/time/combocid.c |  8 +++---
 src/backend/utils/time/tqual.c| 31 +++---
 src/include/access/htup_details.h | 35 +++-
 15 files changed, 108 insertions(+), 58 deletions(-)

diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c
index a21f31b..eb56230 100644
--- a/src/backend/access/heap/heapam.c
+++ b/src/backend/access/heap/heapam.c
@@ -2221,13 +2221,9 @@ heap_prepare_insert(Relation relation, HeapTuple tup, TransactionId xid,
 	tup->t_data->t_infomask &= ~(HEAP_XACT_MASK);
 	tup->t_data->t_infomask2 &= ~(HEAP2_XACT_MASK);
 	tup->t_data->t_infomask |= HEAP_XMAX_INVALID;
+	HeapTupleHeaderSetXmin(tup->t_data, xid);
 	if (options & HEAP_INSERT_FROZEN)
-	{
-		tup->t_data->t_infomask |= HEAP_XMIN_COMMITTED;
-		HeapTupleHeaderSetXmin(tup->t_data, FrozenTransactionId);
-	}
-	else
-		HeapTupleHeaderSetXmin(tup->t_data, xid);
+		HeapTupleHeaderSetXminFrozen(tup->t_data);
 	HeapTupleHeaderSetCmin(tup->t_data, cid);
 	HeapTupleHeaderSetXmax(tup->t_data, 0);		/* for cleanliness */
 	tup->t_tableOid = RelationGetRelid(relation);
@@ -5120,19 +5116,15 @@ heap_freeze_tuple(HeapTupleHeader tuple, TransactionId cutoff_xid,
 	bool		changed = false;
 	TransactionId xid;
 
-	xid = HeapTupleHeaderGetXmin(tuple);
-	if (TransactionIdIsNormal(xid) &&
-		TransactionIdPrecedes(xid, cutoff_xid))
+	if (!HeapTupleHeaderXminFrozen(tuple))
 	{
-		HeapTupleHeaderSetXmin(tuple, FrozenTransactionId);
-
-		/*
-		 * Might as well fix the hint bits too; usually XMIN_COMMITTED will
-		 * already be set here, but there's a small chance not.
-		 */
-		Assert(!(tuple->t_infomask & HEAP_XMIN_INVALID));
-		tuple->t_infomask |= HEAP_XMIN_COMMITTED;
-		changed = true;
+		xid = HeapTupleHeaderGetXmin(tuple);
+		if (TransactionIdIsNormal(xid) &&
+			TransactionIdPrecedes(xid, cutoff_xid))
+		{
+			HeapTupleHeaderSetXminFrozen(tuple);
+			changed = true;
+		}
 	}
 
 	/*
@@ -5185,8 +5177,7 @@ heap_freeze_tuple(HeapTupleHeader tuple, TransactionId cutoff_xid,
 			 * Might as well fix the hint bits too; usually XMIN_COMMITTED
 			 * will already be set here, but there's a small chance not.
 			 */
-			Assert(!(tuple->t_infomask & HEAP_XMIN_INVALID));
-			tuple->t_infomask |= HEAP_XMIN_COMMITTED;
+			HeapTupleHeaderSetXminCommitted(

Re: [HACKERS] b21de4e7b32f868a23bdc5507898d36cbe146164 seems to be two bricks shy of a load

On Thu, Nov 21, 2013 at 4:22 PM, David Rowley  wrote:
> I'm not quite sure why nobody else seems to be complaining, but the changes
> to type.h in this commit seems to have broken things little.
>
> In the visual studios build I'm getting:
>
>   src\interfaces\ecpg\preproc\preproc.y(84): error C2065: 'ET_FATAL' :
> undeclared identifier [D:\Postgres\b\ecpg.vcxproj]
>   src\interfaces\ecpg\preproc\preproc.y(84): error C2051: case expression
> not constant [D:\Postgres\b\ecpg.vcxproj]
>   src\interfaces\ecpg\preproc\preproc.y(102): error C2065: 'ET_FATAL' :
> undeclared identifier [D:\Postgres\b\ecpg.vcxproj]
>   src\interfaces\ecpg\preproc\preproc.y(102): error C2051: case expression
> not constant [D:\Postgres\b\ecpg.vcxproj]
>   src\interfaces\ecpg\preproc\preproc.y(14664): error C2065: 'ET_FATAL' :
> undeclared identifier [D:\Postgres\b\ecpg.vcxproj]

Strange, I could not get this error on latest code.
I think you need to regenerate preproc.y by using command perl
mkvcbuild.pl before build.


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

On Thu, Nov 21, 2013 at 6:15 AM, Andres Freund  wrote:
>> > 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.
>
> Sure. But how often are we a) going to create such an identifier b)
> looking it up?

Never.  Make that the replication solution's problem.  Make the core
support deal only with UUIDs or pairs of 64-bit integers or something
like that, and let the replication solution decide what they mean.

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


On 21.11.2013 22:53, Andres Freund wrote:

On 2013-11-21 12:51:17 -0800, Josh Berkus wrote:

On 11/21/2013 12:46 PM, Andres Freund wrote:

The problem is starting with hot_standby=on on a system with
recovery.conf present. It is independent of whether you use streaming
replication, archive based recovery, or just shutdown the server and
manually copy xlog segments there.
As long as hot_standby=on, and recovery.conf is present you can hit the
bug.


Oh, aha.  There have to be some transactions which are awaiting
checkpoint, though, correct?  As in, if there's no activity on the
master, you can't trigger the bug?


Correct. Also, if you *start* at such a checkpoint you're not vulnerable
until the standby is restarted.


Keep in mind that autovacuum counts as "activity" in this case. If 
you're unlucky, that is. It's next to impossible to determine 
after-the-fact if there has been activity in the master that might've 
caused problems.


If you have ever set hot_standby=on in your standby server, running one 
of the affected versions, you're at risk. The standby might be corrupt, 
and should be rebuild from a base backup. The higher the transaction 
rate in the master, the higher the risk.


I wouldn't try to narrow it down any further than that, it gets too 
complicated.


- 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

On Thu, Nov 21, 2013 at 9:11 AM, Amit Kapila  wrote:
> 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?

Here what I have in mind is that:
a. In pg_dump or other internal utilities where we want to use this
feature, they should call PQenableStart()  or some other API before
calling PQConnect() which will indicate that it wants to operate
as a standalone mode.
b. In psql, if user specifies this special switch (
'standalone_datadir'), then internally we will call PQenableStart()
and use postgres from same
directory.

 So standalone_backend option will not be exposed through psql, but
other internal tools can use it.

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

On Wed, Nov 20, 2013 at 9:35 PM, Fabrízio de Royes Mello
 wrote:
>> > 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?

I dunno, but I doubt that hardcoding ext as an abbreviation for
extension is a good decision.  I also doubt that any fixed prefix is a
good decision.

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

So, create your own configuration table with a column of type regclass.

-- 
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] b21de4e7b32f868a23bdc5507898d36cbe146164 seems to be two bricks shy of a load

2013-11-21 Thread Boszormenyi Zoltan


2013-11-21 11:52 keltezéssel, David Rowley írta:
I'm not quite sure why nobody else seems to be complaining, but the changes to type.h in 
this commit seems to have broken things little.


In the visual studios build I'm getting:

  src\interfaces\ecpg\preproc\preproc.y(84): error C2065: 'ET_FATAL' : undeclared 
identifier [D:\Postgres\b\ecpg.vcxproj]
  src\interfaces\ecpg\preproc\preproc.y(84): error C2051: case expression not constant 
[D:\Postgres\b\ecpg.vcxproj]
  src\interfaces\ecpg\preproc\preproc.y(102): error C2065: 'ET_FATAL' : undeclared 
identifier [D:\Postgres\b\ecpg.vcxproj]
  src\interfaces\ecpg\preproc\preproc.y(102): error C2051: case expression not constant 
[D:\Postgres\b\ecpg.vcxproj]
  src\interfaces\ecpg\preproc\preproc.y(14664): error C2065: 'ET_FATAL' : undeclared 
identifier [D:\Postgres\b\ecpg.vcxproj]


Which I'm guessing is something to do with:

--- a/src/interfaces/ecpg/preproc/type.h 

+++ b/src/interfaces/ecpg/preproc/type.h 

@@ -186,7 
 
+186,7 
 
@@ struct assignment

 enum errortype
 {
-   ET_WARNING, ET_ERROR, ET_FATAL
+   ET_WARNING, ET_ERROR
 };


Regards

David Rowley


You should regenerate preproc.y.
"./configure --enable-depend" does the trick under Linux
but I don't know how to do it under Windows.

Best regards,
Zoltán Böszörményi

--
--
Zoltán Böszörményi
Cybertec Schönig & Schönig GmbH
Gröhrmühlgasse 26
A-2700 Wiener Neustadt, Austria
Web: http://www.postgresql-support.de
 http://www.postgresql.at/

Re: [HACKERS] new unicode table border styles for psql

2013-11-21 Thread Pavel Stehule

I should to appen list of new styles to doc. Style double6 doesnt exist,
because I renamed double5 to bold1 (and created second bold2)

I will update patch tomorrow - it will be in cf4

Regards

Pavel
Dne 21.11.2013 21:37 "Szymon Guz"  napsal(a):

> On 21 November 2013 21:15, Szymon Guz  wrote:
>
>>
>>
>>
>> On 21 November 2013 20:20, Pavel Stehule  wrote:
>>
>>> So here is patch for 9.4
>>>
>>> 7 new line styles, 2 new border styles, \pset border autocomplete
>>>
>>> Regards
>>>
>>> Pavel
>>>
>>>
>>>
>>>
>>> 2013/11/21 Szymon Guz 
>>>
 On 21 November 2013 08:09, Pavel Stehule wrote:

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

 YES!

 - Szymon

>>>
>>>
>> That's pretty cool, I'd love to see it in the core, however it doesn't
>> contain any documentation, so I'm afraid it will be hard to use for people.
>>
>> thanks,
>> Szymon
>>
>
> Hi Pavel,
> I've found two errors in the documentation at
> http://postgres.cz/wiki/Pretty_borders_in_psql
>
> 1)
>
> The unicode-double5 style looks like:
>
> x=# select * from t;
> ┌───┬───┬───┐
> │ a │ b │ t │
> ╞═══╪═══╪═══╡
> │ 1 │ 1 │ a │
> ├───┼───┼───┤
> │ 2 │ 2 │ b │
> ├───┼───┼───┤
> │ 3 │ 3 │ c │
> └───┴───┴───┘
> (3 rows)
>
> (There are horizontal lines between rows)
>
> 2) There is no unicode-double6 in psql, however it exists on the website.
>
> regards,
> Szymon
>

Re: [HACKERS] preserving forensic information when we freeze

On Thu, Nov 21, 2013 at 9:47 AM, Andres Freund  wrote:
> As promised I'm currently updating the patch. Some questions arose
> during that:
>
> * Should HeapTupleHeaderXminFrozen also check for FrozenTransactionId?
>   It seems quite possible that people think they've delt with frozen
>   xmin entirely after checking, but they still might get
>   FrozenTransactionId back in a pg_upgraded cluster.

The reason I originally wrote the patch the way I did, rather than the
way that you prefer, is that it minimizes the number of places where
we might perform extra tests that are known not to be needed in
context.  These code paths are hot.  If you do this sort of thing,
then after macro expansion we may end up with a lot of things like:
(flags & FROZEN) || (rawxid == 2) ? 2 : rawxid.  I want to avoid that.
 That macros is intended, specifically, to be a test for flag bits,
and I think it should do precisely that.  If that's not what you want,
then don't use that macro.

> * Existing htup_details boolean checks contain an 'Is', but
>   HeapTupleHeaderXminCommitted, HeapTupleHeaderXminInvalid,
>   HeapTupleHeaderXminFrozen don't contain any verb. Not sure.

We could say XminIsComitted, XminIsInvalid, XminIsFrozen, etc.  I
don't particularly care for it, but I can see the argument for it.

> * EvalPlanQualFetch() uses priorXmax like logic to find updated versions
>   of tuples. I am not 100% sure there aren't cases where that's
>   problematic even with the current code, but I think it's better not to
>   use the raw xid there - priorXmax can never be FrozenTransactionId, so
>   comparing it to the GetXmin() seems better.
>   It also has the following wrong comment:
>  *   (Should be safe to examine xmin without getting
>  * buffer's content lock, since xmin never changes in an existing
>  * tuple.)

Hmm.  The new tuple can't be frozen unless it's all-visible, and it
can't be all-visible if our scan can still see its predecessor.  That
would imply that if it's frozen, it must be an unrelated tuple.  I
think.

>   But I don't see that causing any problems.
> * ri_trigger.c did do a TransactionIdIsCurrentTransactionId() on a raw
>   xmin value, the consequences are minor though.
>
> The rewrite to introduce HeapTupleHeaderGet[Raw]Xmin() indeed somewhat
> increases the patchsize - but I think it's enough increase in
> expressiveness to be well worth the cost. Indeed it led me to find at
> least one issue (with minor consequences).
>
> I think once we have this we should start opportunistically try to
> freeze tuples during vacuum using OldestXmin instead of FreezeLimit if
> the page is already dirty.

Separate patch, but yeah, something like that.  If we have to mark the
page all-visible, we might as well freeze it while we're there.  We
should think about how it interacts with Heikki's freeze-without-write
patch though.

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

Hi,

On 2013-11-20 12:48:50 +0200, Heikki Linnakangas wrote:
> On 19.11.2013 16:22, Andres Freund wrote:
> >On 2013-11-19 15:20:01 +0100, Andres Freund wrote:
> >>Imo something the attached patch should be done. The description I came
g> >>up with is:
> >>
> >> Fix Hot-Standby initialization of clog and subtrans.
> 
> Looks ok for a back-patchable fix.

Do you plan to commit this? Or who is going to?

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

On 2013-11-21 12:51:17 -0800, Josh Berkus wrote:
> On 11/21/2013 12:46 PM, Andres Freund wrote:
> > No. Check
> > http://archives.postgresql.org/message-id/20131120234141.GI18801%40awork2.anarazel.de
> > 
> > The problem is starting with hot_standby=on on a system with
> > recovery.conf present. It is independent of whether you use streaming
> > replication, archive based recovery, or just shutdown the server and
> > manually copy xlog segments there.
> > As long as hot_standby=on, and recovery.conf is present you can hit the
> > bug.
> 
> Oh, aha.  There have to be some transactions which are awaiting
> checkpoint, though, correct?  As in, if there's no activity on the
> master, you can't trigger the bug?

Correct. Also, if you *start* at such a checkpoint you're not vulnerable
until the standby is restarted.

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] Extension Templates S03E11

Dimitri Fontaine wrote:
> Stephen Frost  writes:
> > Are there any other changes you have pending for this..?  Would be nice
> > to see the latest version which you've tested and which patches cleanly
> > against master... ;)
> 
> I just rebased now, please see attached. I had to pick new OIDs in some
> places too, but that's about it.

I think you're missing support for getObjectIdentity and
getObjectTypeDescription for the new object classes.

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

2013-11-21 Thread Josh Berkus

On 11/21/2013 12:46 PM, Andres Freund wrote:
> No. Check
> http://archives.postgresql.org/message-id/20131120234141.GI18801%40awork2.anarazel.de
> 
> The problem is starting with hot_standby=on on a system with
> recovery.conf present. It is independent of whether you use streaming
> replication, archive based recovery, or just shutdown the server and
> manually copy xlog segments there.
> As long as hot_standby=on, and recovery.conf is present you can hit the
> bug.

Oh, aha.  There have to be some transactions which are awaiting
checkpoint, though, correct?  As in, if there's no activity on the
master, you can't trigger the bug?

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

On 2013-11-21 12:39:54 -0800, Josh Berkus wrote:
> On 11/21/2013 12:36 PM, Joshua D. Drake wrote:
> > 
> > Hello,
> > 
> > This is turning into a rather large thread and I have a simple question:
> > 
> > Is a work-around to this problem as simple as disabling streaming
> > replication and enabling log shipping instead?

No. Check
http://archives.postgresql.org/message-id/20131120234141.GI18801%40awork2.anarazel.de

The problem is starting with hot_standby=on on a system with
recovery.conf present. It is independent of whether you use streaming
replication, archive based recovery, or just shutdown the server and
manually copy xlog segments there.
As long as hot_standby=on, and recovery.conf is present you can hit the
bug.

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] Add \i option to bring in the specified file as a quoted literal

Piotr Marcinczyk escribió:

> 
> + \ib  class="parameter">filename [  class="parameter">quote_string ] 
> + 
> + 
> + The \ib command appends content of file 
> filename 
> + to current query buffer. If parameter 
> quote_string 
> + is not set, no quotation is used. If it is set, content of file 
> will be
> + quoted by quote_string enclosed in 
> $.
> + 
> + 
> +   

Doesn't this quoting thing seem like a usability problem?  I mean,
there's no way you can possibly know what string to use unless you first
verify the contents of the file yourself.  I think this is something
that should be done automatically by psql.

But, really, having to read stuff and transform into a quoted literal
seems wrong to me.  I would like something that would read into a client
variable that can later be used as a positional parameter to a
parametrized query, so

\ib homer ~/photos/homer.jpg
insert into people (name, photo) values ('Homer', :homer);

and have psql turn that into 

PQexecParams("insert into people (name, photo) values ('homer', $1)",
some_array_with_homer);

so that no quoting needs to happen anywhere.

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

2013-11-21 Thread Joshua D. Drake



Hello,

This is turning into a rather large thread and I have a simple question:

Is a work-around to this problem as simple as disabling streaming 
replication and enabling log shipping instead?


Joshua D. Drake


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

2013-11-21 Thread Josh Berkus

On 11/21/2013 12:36 PM, Joshua D. Drake wrote:
> 
> Hello,
> 
> This is turning into a rather large thread and I have a simple question:
> 
> Is a work-around to this problem as simple as disabling streaming
> replication and enabling log shipping instead?

Yes, and re-cloning the replica, in case it already has corruption.


-- 
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 unicode table border styles for psql

2013-11-21 Thread Szymon Guz

On 21 November 2013 21:15, Szymon Guz  wrote:

>
>
>
> On 21 November 2013 20:20, Pavel Stehule  wrote:
>
>> So here is patch for 9.4
>>
>> 7 new line styles, 2 new border styles, \pset border autocomplete
>>
>> Regards
>>
>> Pavel
>>
>>
>>
>>
>> 2013/11/21 Szymon Guz 
>>
>>> On 21 November 2013 08:09, Pavel Stehule wrote:
>>>
 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

>>>
>>> YES!
>>>
>>> - Szymon
>>>
>>
>>
> That's pretty cool, I'd love to see it in the core, however it doesn't
> contain any documentation, so I'm afraid it will be hard to use for people.
>
> thanks,
> Szymon
>

Hi Pavel,
I've found two errors in the documentation at
http://postgres.cz/wiki/Pretty_borders_in_psql

1)

The unicode-double5 style looks like:

x=# select * from t;
┌───┬───┬───┐
│ a │ b │ t │
╞═══╪═══╪═══╡
│ 1 │ 1 │ a │
├───┼───┼───┤
│ 2 │ 2 │ b │
├───┼───┼───┤
│ 3 │ 3 │ c │
└───┴───┴───┘
(3 rows)

(There are horizontal lines between rows)

2) There is no unicode-double6 in psql, however it exists on the website.

regards,
Szymon

Re: [HACKERS] Patch for fail-back without fresh backup

Bruce Momjian escribió:
> On Thu, Oct 24, 2013 at 11:14:14PM +0300, Heikki Linnakangas wrote:
> > On 24.10.2013 23:07, Josh Berkus wrote:

> > >What kind of overhead are we talking about here?
> > 
> > One extra WAL record whenever a hint bit is set on a page, for the
> > first time after a checkpoint. In other words, a WAL record needs to
> > be written in the same circumstances as with page checksums, but the
> > WAL records are much smaller as they don't need to contain a full
> > page image, just the block number of the changed block.
> > 
> > Or maybe we'll write the full page image after all, like with page
> > checksums, just without calculating the checksums. It might be
> > tricky to skip the full-page image, because then a subsequent change
> > of the page (which isn't just a hint-bit update) needs to somehow
> > know it needs to take a full page image even though a WAL record for
> > it was already written.
> 
> Sorry to be replying late to this, but while I am not worried about the
> additional WAL volume, does this change require the transaction to now
> wait for a WAL sync to disk before continuing?

I don't think so.  There's extra WAL written, but there's no
flush-and-wait until end of transaction (as has always been).

> I thought that was the down-side to WAL logging hint bits, not the WAL
> volume itself.

I don't think this is true either.

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

[HACKERS] COMMENT ON CONSTRAINT ON DOMAIN ?

2013-11-21 Thread Elvis Pranskevichus

Hello,

I may be totally missing something, but there seems to be no way 
to create a COMMENT on a domain constraint.  There is

COMMENT ON CONSTRAINT constraint_name ON table_name

but no such thing for domain constraints, which seems like a 
weird omission.  

I couldn't find any relevant discussions on the list; is there a 
technical reason this is not implemented?

Looking at the source, get_object_address() always assumes a 
constraint is a table constraint.  It seems to me that there is 
a relatively easy fix:

COMMENT ON CONSTRAINT constraint_name ON DOMAIN domain_name,

then store constraint subject type (relation vs. domain) in 
objname list, and handle that in get_object_address().

Thoughts?

 Elvis




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

[HACKERS] MultiXact pessmization in 9.3

Hi,

While looking at the multixact truncation code (mail nearby), I've
noticed that there's a significant difference in the way multixact
members are accessed since fkey locks were introduced:

<9.3 when heap_lock_tuple finds a XMAX_IS_MULTI tuple it executes
MultiXactIdWait() which in turn uses GetMultiXactIdMembers() to get the
xids to wait for. But it skips the lookup if the mxid we lookup is older
than OldestVisibleMXactId.

9.3+ instead always looks up the members because GetMultiXactIdMembers()
is also used in cases where we need to access old xids (to check whether
members have commited in non-key updates). I've seen several profiles -
independent from this investigation - where GetMultiXactIdMembers() is
high up in the profiles. So this seems to be a significant
pessimization.
I think we need to re-introduce that optimization for the - hopefully
very common - case of LOCK_ONLY multixacts.

The file header's comment about OldestVisibleMXactId[] seems to be
out-of-date btw, since it's now perfectly normal to access mxids that
are older than what MultiXactIdSetOldestVisible() computed.

Maybe I am just missing something?

Greetings,

Andres Freund

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


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

Re: [HACKERS] new unicode table border styles for psql

2013-11-21 Thread Szymon Guz

On 21 November 2013 20:20, Pavel Stehule  wrote:

> So here is patch for 9.4
>
> 7 new line styles, 2 new border styles, \pset border autocomplete
>
> Regards
>
> Pavel
>
>
>
>
> 2013/11/21 Szymon Guz 
>
>> On 21 November 2013 08:09, Pavel Stehule  wrote:
>>
>>> 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
>>>
>>
>> YES!
>>
>> - Szymon
>>
>
>
That's pretty cool, I'd love to see it in the core, however it doesn't
contain any documentation, so I'm afraid it will be hard to use for people.

thanks,
Szymon

[HACKERS] MultiXact truncation, startup et al.

Hi,

Heikki's comments about StartupMultiXact() being executed too late in
http://archives.postgresql.org/message-id/528C9392.804%40vmware.com
made me look at how multixact truncation works, since that's where
SimpleLruTruncate() would trigger an error because of an unitialized
latest_page_number.

Turns out, we don't ever truncate pg_multixact during recovery since
9dc842f0832fd71eda826349a0c17ecf8ae93b84 because multixact truncations,
in contrast to clog, aren't WAL logged themselves. Disabling probably
was fair game back then since it wasn't too likely to remain in crash
recovery forever.
But at the very least since the addition of Hot Standby that's really
not the case anymore. If I calculate correctly currently you'd end up
with ~34GB(<9.3)/38GB of pg_multixact which seems a bit much.

I am not 100% sure, but it looks like things could actually continue to
work despite having an slru wraparound into existing data. But that's
certainly nothing I'd want to rely on and looks mostly like lucky
happenstance, especially in 9.3.

If this were a master only issue, I'd say WAL-logging mxact truncation
would be the way to go, but we can't really do that in the back branches
since multixact_redo() would throw a fit if we were to introduce a new
type of wal record and somebody would upgrade a primary first.

So, what I think we need to do is to split StartupMultiXact() into two
parts, StartupMultiXact() which only sets the offset's, members's
shared->latest_page_number and TrimMultiXact() which does the remainder
of the work, executed when finishing crash recovery at the current
location of StartupMultiXact().

At this point <9.3 and 9.3+ diverge:

<9.3 we can just re-enable doing TruncateMultiXact() in
CheckPointMultiXact() since the content of the mxacts isn't important
anymore - we just need the WAL records to be sure to have advanced the
nextMXact, nextOffset counters to be sure they are big enough.

>= 9.3 multixact checkpoints don't actually perform the truncation
anymore, but vacuum does, via vac_truncate_clog(). Which obviously isn't
executed during recovery. So we need to re-add truncation there,
possibly only during recovery (everything else should be superflous
work)?

Any comments?

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] [PERFORM] Cpu usage 100% on slave. s_lock problem.

On Thu, Nov 21, 2013 at 10:37 AM, Heikki Linnakangas
 wrote:
> On 21.11.2013 17:08, Merlin Moncure wrote:
>>
>> On Thu, Nov 21, 2013 at 9:02 AM, Andres Freund 
>> wrote:
>>>
>>> On 2013-11-21 16:25:02 +0200, Heikki Linnakangas wrote:

 Hmm. All callers of RecoveryInProgress() must be prepared to handle the
 case
 that RecoveryInProgress() returns true, but the system is no longer in
 recovery. No matter what locking we do in RecoveryInProgress(), the
 startup
 process might finish recovery just after RecoveryInProgress() has
 returned.
>>>
>>>
>>> True.
>>>
 What about the attached? It reads the shared variable without a lock or
 barrier. If it returns 'true', but the system in fact just exited
 recovery,
 that's OK. As explained above, all the callers must tolerate that
 anyway.
 But if it returns 'false', then it performs a full memory barrier, which
 should ensure that it sees any other shared variables as it is after the
 startup process cleared SharedRecoveryInProgress (notably,
 XLogCtl->ThisTimeLineID).
>>>
>>>
>>> I'd argue that we should also remove the spinlock in StartupXLOG and
>>> replace it with a write barrier. Obviously not for performance reasons,
>>> but because somebody might add more code to run under that spinlock.
>>>
>>> Looks good otherwise, although a read memory barrier ought to suffice.
>>
>>
>> This code is in a very hot code path.  Are we *sure* that the read
>> barrier is fast enough that we don't want to provide an alternate
>> function that only returns the local flag?  I don't know enough about
>> them to say either way.
>
>
> In my patch, I put the barrier inside the if (!LocalRecoveryInProgress)
> block. That codepath can only execute once in a backend, so performance is
> not an issue there. Does that look sane to you?

oh right -- certainly!

merlin


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

2013-11-21 Thread J Smith

On Thu, Nov 21, 2013 at 10:27 AM, Andres Freund  wrote:
>
> I don't think so - for one, pg_subtrans isn't really the problems with
> that bug, for another, it won't cause missing files. Also, you're not
> using replication, right?
>

Actually, this server is a master being replicated to a standby. We
haven't experienced the problem on the standby, but we are indeed
replicating the server experiencing the issue.

-- 
Sent 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 unicode table border styles for psql

2013-11-21 Thread Pavel Stehule

So here is patch for 9.4

7 new line styles, 2 new border styles, \pset border autocomplete

Regards

Pavel




2013/11/21 Szymon Guz 

> On 21 November 2013 08:09, Pavel Stehule  wrote:
>
>> 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
>>
>
> YES!
>
> - Szymon
>
commit 170c25a680a43a9bd96e2ca923cbd2492d35ceda
Author: root 
Date:   Thu Nov 21 20:16:00 2013 +0100

initial

diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c
index 638d8cb..7b2b621 100644
--- a/src/bin/psql/command.c
+++ b/src/bin/psql/command.c
@@ -2271,7 +2271,6 @@ do_pset(const char *param, const char *value, printQueryOpt *popt, bool quiet)
 			psql_error("\\pset: allowed formats are unaligned, aligned, wrapped, html, latex, troff-ms\n");
 			return false;
 		}
-
 	}
 
 	/* set table line style */
@@ -2285,12 +2284,27 @@ do_pset(const char *param, const char *value, printQueryOpt *popt, bool quiet)
 			popt->topt.line_style = &pg_asciiformat_old;
 		else if (pg_strncasecmp("unicode", value, vallen) == 0)
 			popt->topt.line_style = &pg_utf8format;
+		else if (pg_strncasecmp("unicode2", value, vallen) == 0)
+			popt->topt.line_style = &pg_utf8format2;
+		else if (pg_strncasecmp("unicode-double1", value, vallen) == 0)
+			popt->topt.line_style = &pg_utf8double1format;
+		else if (pg_strncasecmp("unicode-double2", value, vallen) == 0)
+			popt->topt.line_style = &pg_utf8double2format;
+		else if (pg_strncasecmp("unicode-double3", value, vallen) == 0)
+			popt->topt.line_style = &pg_utf8double3format;
+		else if (pg_strncasecmp("unicode-double4", value, vallen) == 0)
+			popt->topt.line_style = &pg_utf8double4format;
+		else if (pg_strncasecmp("unicode-double5", value, vallen) == 0)
+			popt->topt.line_style = &pg_utf8double5format;
+		else if (pg_strncasecmp("unicode-bold1", value, vallen) == 0)
+			popt->topt.line_style = &pg_utf8bold1format;
+		else if (pg_strncasecmp("unicode-bold2", value, vallen) == 0)
+			popt->topt.line_style = &pg_utf8bold2format;
 		else
 		{
 			psql_error("\\pset: allowed line styles are ascii, old-ascii, unicode\n");
 			return false;
 		}
-
 	}
 
 	/* set border style/width */
@@ -2298,7 +2312,6 @@ do_pset(const char *param, const char *value, printQueryOpt *popt, bool quiet)
 	{
 		if (value)
 			popt->topt.border = atoi(value);
-
 	}
 
 	/* set expanded/vertical mode */
diff --git a/src/bin/psql/print.c b/src/bin/psql/print.c
index 736225c..6f4473a 100644
--- a/src/bin/psql/print.c
+++ b/src/bin/psql/print.c
@@ -54,6 +54,8 @@ const printTextFormat pg_asciiformat =
 		{"-", "+", "+", "+"},
 		{"-", "+", "+", "+"},
 		{"-", "+", "+", "+"},
+		{"-", "+", "+", "+"},
+		{"", "|", "|", "|"},
 		{"", "|", "|", "|"}
 	},
 	"|",
@@ -75,6 +77,8 @@ const printTextFormat pg_asciiformat_old =
 		{"-", "+", "+", "+"},
 		{"-", "+", "+", "+"},
 		{"-", "+", "+", "+"},
+		{"-", "+", "+", "+"},
+		{"", "|", "|", "|"},
 		{"", "|", "|", "|"}
 	},
 	":",
@@ -97,9 +101,13 @@ const printTextFormat pg_utf8format =
 		{"\342\224\200", "\342\224\214", "\342\224\254", "\342\224\220"},
 		/* ─, ├, ┼, ┤ */
 		{"\342\224\200", "\342\224\234", "\342\224\274", "\342\224\244"},
+		/* ─, ├, ┼, ┤ */
+		{"\342\224\200", "\342\224\234", "\342\224\274", "\342\224\244"},
 		/* ─, └, ┴, ┘ */
 		{"\342\224\200", "\342\224\224", "\342\224\264", "\342\224\230"},
 		/* N/A, │, │, │ */
+		{"", "\342\224\202", "\342\224\202", "\342\224\202"},
+		/* N/A, │, │, │ */
 		{"", "\342\224\202", "\342\224\202", "\342\224\202"}
 	},
 	/* │ */
@@ -121,6 +129,253 @@ const printTextFormat pg_utf8format =
 	true
 };
 
+const printTextFormat pg_utf8format2 = {
+	"unicode2",
+	{
+		/* ─, ┌, ┬, ┐ */
+		{"\342\224\200", "\342\224\214", "\342\224\254", "\342\224\220"},
+		/* ─, ├, ┴, ┤ */
+		{"\342\224\200", "\342\224\234", "\342\224\264", "\342\224\244"},
+		/* ─, ├, ┼, ┤ */
+		{"\342\224\200", "\342\224\234", "\342\224\274", "\342\224\244"},
+		/* ─, └, ─, ┘ */
+		{"\342\224\200", "\342\224\224", "\342\224\200", "\342\224\230"},
+		/* N/A, │, │, │ */
+		{"", "\342\224\202", "\342\224\202", "\342\224\202"},
+		/* N/A, │, │, │ */
+		{"", "\342\224\202", "\342\224\202", "\342\224\202"}
+	},
+	/* │ */
+	"\342\224\202",
+	/* │ */
+	"\342\224\202",
+	/* │ */
+	"\342\224\202",
+	" ",
+	" ",
+	" ",
+	" ",
+	" ",
+	" ",
+	false
+};
+
+const printTextFormat pg_utf8double1format = {
+	"unicode-double1",
+	{
+		/* ═, ╔, ╤, ╗ */
+		{"\342\225\220", "\342\225\224", "\342\225\244", "\342\225\227"},
+		/* ─, ╟, ┼, ╢ */
+		{"\342\224\200", "\342\225\237", "\342\224\274", "\342\225\242"},
+		/* ─, ╟, ┼, ╢ */
+		{"\342\224\200", "\342\225\237", "\342\224\274", "\342\225\242"},
+		/* ═, ╚, ╧, ╝ */
+		{"\342\225\220", "\342\225\232", "\342\225\247", "\342\225\235"},
+		/* N/A, ║, │, ║ */
+		{"", "\342\2

Re: [HACKERS] WITHIN GROUP patch

2013-11-21 Thread Andrew Gierth

> "Vik" == Vik Fearing  writes:

 Vik> I certainly want it.  I do not have a copy of the SQL standard,
 Vik> but I have full faith in the Andrew Gierth's claim that this is
 Vik> part of it.

For reference, this is how I believe it matches up against the spec
(I'm working from the 2008 final):

10.9 :

   is intended to be implemented in this
  patch exactly as per spec.

  : the spec defines two of these,
  PERCENTILE_CONT and PERCENTILE_DISC:

  PERCENTILE_CONT is defined in the spec for numeric types, in which
  case it returns an approximate numeric result, and for interval, in
  which case it returns interval. Our patch defines percentile_cont
  functions for float8 and interval input types, relying on implicit
  casting to float8 to handle other numeric input types.

  As an extension to the spec, we define a percentile_cont function
  that returns an array of percentile values in one call, as suggested
  by some users.

  PERCENTILE_DISC is defined in the spec for the same types as _CONT.
  Our version on the other hand accepts any type which can be sorted,
  and returns the same type. This does mean that our version may return
  an exact numeric type rather than an approximate one, so this is a
  possible slight deviation from the spec.

  i.e. our  percentile_disc(float8) within group (order by bigint)
  returns a bigint, while the spec seems to imply it should return an
  approximate numeric type (i.e. float*).

  Again, we additionally provide an array version which is not in the
  spec.

  mode() is not in the spec, we just threw it in because it was easy.

6.10 

  The spec says that  is not allowed as a
  window function.

  The spec does not forbid other s in a window
  function call, but we have NOT attempted to implement this (largely
  for the same reasons that DISTINCT and ORDER BY are not implemented
  for aggregates as window functions).

Conformance:  all the relevant features are parts of T612, "Advanced
OLAP Operations", which we already list in the docs on the unsupported
list with the comment "some forms supported". Maybe that could be
changed now to "most forms supported", but that's a subjective call
(and one we didn't really consider doing in this patch).

-- 
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] [PERFORM] Cpu usage 100% on slave. s_lock problem.


On 21.11.2013 17:08, Merlin Moncure wrote:

On Thu, Nov 21, 2013 at 9:02 AM, Andres Freund  wrote:

On 2013-11-21 16:25:02 +0200, Heikki Linnakangas wrote:

Hmm. All callers of RecoveryInProgress() must be prepared to handle the case
that RecoveryInProgress() returns true, but the system is no longer in
recovery. No matter what locking we do in RecoveryInProgress(), the startup
process might finish recovery just after RecoveryInProgress() has returned.


True.


What about the attached? It reads the shared variable without a lock or
barrier. If it returns 'true', but the system in fact just exited recovery,
that's OK. As explained above, all the callers must tolerate that anyway.
But if it returns 'false', then it performs a full memory barrier, which
should ensure that it sees any other shared variables as it is after the
startup process cleared SharedRecoveryInProgress (notably,
XLogCtl->ThisTimeLineID).


I'd argue that we should also remove the spinlock in StartupXLOG and
replace it with a write barrier. Obviously not for performance reasons,
but because somebody might add more code to run under that spinlock.

Looks good otherwise, although a read memory barrier ought to suffice.


This code is in a very hot code path.  Are we *sure* that the read
barrier is fast enough that we don't want to provide an alternate
function that only returns the local flag?  I don't know enough about
them to say either way.


In my patch, I put the barrier inside the if (!LocalRecoveryInProgress) 
block. That codepath can only execute once in a backend, so performance 
is not an issue there. Does that look sane to you?


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

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. 

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

Well, they are for me ;-).  I'm concerned for example about how we get
ruleutils.c to reverse-list into a form that's certain to be interpreted
the same by the parser.

The whole business with the spec's reading of TABLE() seems bizarre.
AFAICS there is nothing about TABLE(foo()) that you can't get with
greater clarity by writing UNNEST(foo()) instead.  And it's not like
it's a legacy feature --- SQL99 has single-argument UNNEST() but not
TABLE(), so why'd they add TABLE() later, and why'd they make it a
strict subset of what UNNEST() can do?  I can't escape the suspicion
that I'm misreading the spec somehow ... but the text seems perfectly
clear.

Anyway, after further thought I've come up with an approach that's purely
a syntactic transformation and so less likely to cause surprise: let's
say that if we have TABLE() with a single argument, and no coldeflist
either inside or outside, then we implicitly insert UNNEST().  Otherwise
not.  This is sufficient to satisfy the case spelled out in the standard,
but it doesn't get in the way of any more-typical use of TABLE().
In particular, if you don't want the implicit UNNEST(), you can just leave
off TABLE(), because the case where we'd insert it has no features you
can't get in the old syntax.  Similarly, because ruleutils.c is already
coded not to bother with printing TABLE() if there's a single function and
no coldeflist, we needn't worry about falling foul of the implicit action
when a printed view is re-parsed.

BTW, I looked into the option of choosing a different syntax altogether,
but the results weren't too promising.  FUNCTION() doesn't work unless
we're willing to make that keyword partially reserved, which seems like
a bad thing.  (TABLE() works because TABLE is already a fully reserved
word.)  The idea with no keyword at all might work, but it seems way too
likely to cause confusion, especially if you think about parenthesized
JOIN syntax as an alternative possibility for some slightly-typoed query.

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] Logging WAL when updating hintbit

2013-11-21 Thread Sawada Masahiko

On Thu, Nov 21, 2013 at 8:59 PM, Dilip kumar  wrote:
> On 20 November 2013 22:12, Sawada Masahiko Wrote
>
>> >
>> > 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.
>
> OK, once I get it, I will review the same.
>
>
>> >
>> > 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.
>
> [Performance test on standby:]
>
> I have executed pgbench on master with WAL LEVEL "hot_stanby" and "all" 
> option, and after that run pgbench on standby with "select-only" option.
>
> WAL LEVEL (on master): hot_standby
> Select TPS (on standby)  : 4098
>
> WAL LEVEL (on master): all
> Select TPS (on standby)  : 4115
>

Thank you for testing!

It seems to have a little benefit for performance improvement on standby side.
It need to more test to write such thing into the documentation patch.

And I'm going to implement the patch as separated parameter now.
I think that this parameter should allow to use something together
with 'archive' and 'hot_standby'.
IWO not allow with 'minimal'.
Thought?

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] [PERFORM] Cpu usage 100% on slave. s_lock problem.

On Thu, Nov 21, 2013 at 9:02 AM, Andres Freund  wrote:
> On 2013-11-21 16:25:02 +0200, Heikki Linnakangas wrote:
>> Hmm. All callers of RecoveryInProgress() must be prepared to handle the case
>> that RecoveryInProgress() returns true, but the system is no longer in
>> recovery. No matter what locking we do in RecoveryInProgress(), the startup
>> process might finish recovery just after RecoveryInProgress() has returned.
>
> True.
>
>> What about the attached? It reads the shared variable without a lock or
>> barrier. If it returns 'true', but the system in fact just exited recovery,
>> that's OK. As explained above, all the callers must tolerate that anyway.
>> But if it returns 'false', then it performs a full memory barrier, which
>> should ensure that it sees any other shared variables as it is after the
>> startup process cleared SharedRecoveryInProgress (notably,
>> XLogCtl->ThisTimeLineID).
>
> I'd argue that we should also remove the spinlock in StartupXLOG and
> replace it with a write barrier. Obviously not for performance reasons,
> but because somebody might add more code to run under that spinlock.
>
> Looks good otherwise, although a read memory barrier ought to suffice.

This code is in a very hot code path.  Are we *sure* that the read
barrier is fast enough that we don't want to provide an alternate
function that only returns the local flag?  I don't know enough about
them to say either way.

merlin


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

Re: [HACKERS] Status of FDW pushdowns

Merlin Moncure  writes:
> On Thu, Nov 21, 2013 at 9:05 AM, Bruce Momjian  wrote:
>> I know join pushdowns seem insignificant, but it helps to restrict what
>> data must be passed back because you would only pass back joined rows.

> By 'insignificant' you mean 'necessary to do any non-trivial real
> work'.   Personally, I'd prefer it if FDW was extended to allow
> arbitrary parameterized queries like every other database connectivity
> API ever made ever.

[ shrug... ]  So use dblink.  For better or worse, the FDW stuff is
following the SQL standard's SQL/MED design, which does not do it
like 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] Status of FDW pushdowns

2013-11-21 Thread Kohei KaiGai

2013/11/21 Bruce Momjian :
> Where are we on the remaining possible pushdowns for foreign data
> wrappers, particularly the Postgres one?  I know we do WHERE restriction
> pushdowns in 9.3, but what about join and aggregate pushdowns?  Is
> anyone working on those?
>
> I know join pushdowns seem insignificant, but it helps to restrict what
> data must be passed back because you would only pass back joined rows.
>
> Do we document these missing features anywhere?
>
Probably, custom-scan api will provide more flexible way to push-down
aggregate, sort or other stuff performing on regular tables, not only
foreign tables.
It allows extensions to offer alternative scan/join path on the planning
stage, then executor callbacks its custom logic instead of the built-in
one, if its cost is cheaper.

Right now, it performs on relation scan or join only. However, we will be
able to apply same concept on aggregation.
For example, an aggregation node on a foreign table scan is a good
candidate to push down because it can be replaced with a custom-
logic that scans a materialized result of the remote aggregation query,
if its cost is enough cheap than local aggregation.
Probably, we need to add a hook and some logic to compare the
built-in aggregation and alternative paths provided by extensions.
It is also helpful for the people who want to implement something like
"parallel aggregate" performing on regular tables, not only foreign table.

Thanks,
-- 
KaiGai Kohei 


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

2013-11-21 Thread Tatsuo Ishii

> Well, I happen to have some pieces of such a framework: the parts which
> can automate spinning up arbitrarily complex groups of replicas and
> doing failover between them.  What we'd still need is:
> 
> a) a slightly better workload than pgbench
> b) a way to compare and test databases for data corruption of several kinds
> 
> Can someone else kick in to help with this?

Recently pgpool-II introduces its own regression test framework
written in shell script which can create a streaming replication
primary and arbitrary number of standbys in single server. The test
scenarios are written in a shell script, thus is reasonably
flexible. I have read through all the thread but I am not sure how to
reproduce the problem reliably. If I would know it, probably I can
integrate into the regression test.

> I think this last issue shows that it's critical as a community to have
> such a testing framework in place, otherwise we really need to halt all
> work on replication until we have such a thing.

Agreed. I think this is very critical for us.
--
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] Why is UPDATE with column-list syntax not implemented

2013-11-21 Thread Claudio Freire

On Thu, Nov 21, 2013 at 3:50 PM, David Johnston  wrote:
>> Why is this not implemented? Is it considered inconvenient to use, or
>> difficult to implement. or not important enough, or some other reason?
>
> I cannot answer why but I too would like to see this.  I actually asked this
> a long while back but cannot seem to find my posting or recall the response.


At least on this case, rules could implement this. Though figuring out
if whether it's the case or not might be harder.


-- 
Sent 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  writes:
> Here what I have in mind is that:
> a. In pg_dump or other internal utilities where we want to use this
> feature, they should call PQenableStart()  or some other API before
> calling PQConnect() which will indicate that it wants to operate
> as a standalone mode.
> b. In psql, if user specifies this special switch (
> 'standalone_datadir'), then internally we will call PQenableStart()
> and use postgres from same
> directory.

Why would you make psql behave differently from our other command-line
clients?  That seems bizarre.  If we're going to use a special switch
to enable standalone mode, it should be the same on every program
that supports 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] Data corruption issues using streaming replication on 9.0.14/9.2.5/9.3.1

On 2013-11-21 11:54:58 -0500, J Smith wrote:
> On Thu, Nov 21, 2013 at 10:27 AM, Andres Freund  
> wrote:
> >
> > I don't think so - for one, pg_subtrans isn't really the problems with
> > that bug, for another, it won't cause missing files. Also, you're not
> > using replication, right?
> >
> 
> Actually, this server is a master being replicated to a standby. We
> haven't experienced the problem on the standby, but we are indeed
> replicating the server experiencing the issue.

It's still not this issue in that case, but I might have an idea... Do
you have hot_standby_feedback enabled?

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] [PERFORM] Cpu usage 100% on slave. s_lock problem.

On 2013-11-21 16:25:02 +0200, Heikki Linnakangas wrote:
> Hmm. All callers of RecoveryInProgress() must be prepared to handle the case
> that RecoveryInProgress() returns true, but the system is no longer in
> recovery. No matter what locking we do in RecoveryInProgress(), the startup
> process might finish recovery just after RecoveryInProgress() has returned.

True.

> What about the attached? It reads the shared variable without a lock or
> barrier. If it returns 'true', but the system in fact just exited recovery,
> that's OK. As explained above, all the callers must tolerate that anyway.
> But if it returns 'false', then it performs a full memory barrier, which
> should ensure that it sees any other shared variables as it is after the
> startup process cleared SharedRecoveryInProgress (notably,
> XLogCtl->ThisTimeLineID).

I'd argue that we should also remove the spinlock in StartupXLOG and
replace it with a write barrier. Obviously not for performance reasons,
but because somebody might add more code to run under that spinlock.

Looks good otherwise, although a read memory barrier ought to suffice.

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

2013-11-21 Thread J Smith

On Tue, Nov 19, 2013 at 9:22 AM, Andres Freund  wrote:
> On 2013-11-19 15:20:01 +0100, Andres Freund wrote:
>> Imo something the attached patch should be done. The description I came
>> up with is:
>>
>> Fix Hot-Standby initialization of clog and subtrans.
>

G'day Andres.

This wouldn't happen to be related to the issue I was having with
missing pg_subtrans files, would it, as discussed here
http://www.postgresql.org/message-id/cadfupgc5bmtv-yg9znxv-vcfkb+jprqs7m2oesqxam_4z1j...@mail.gmail.com
? Based on my reading of the commit message and what I could glean
from the patch itself, they seem to be related.

If there's anything I can do to help test, just say the word and I can
fit in some time. Cheers.

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

Re: [HACKERS] [PERFORM] Cpu usage 100% on slave. s_lock problem.

On Thu, Nov 21, 2013 at 9:09 AM, Andres Freund  wrote:
> On 2013-11-21 09:08:05 -0600, Merlin Moncure wrote:
>> This code is in a very hot code path.  Are we *sure* that the read
>> barrier is fast enough that we don't want to provide an alternate
>> function that only returns the local flag?  I don't know enough about
>> them to say either way.
>
> A read barrier is just a compiler barrier on x86.

That's good enough for me then.

merlin


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

Re: [HACKERS] Why is UPDATE with column-list syntax not implemented

2013-11-21 Thread David Johnston

AK wrote
> 9.3 documentation says:
> 
> According to the standard, the column-list syntax should allow a list of
> columns to be assigned from a single row-valued expression, such as a
> sub-select:
> 
> UPDATE accounts SET (contact_last_name, contact_first_name) =
> (SELECT last_name, first_name FROM salesmen
>  WHERE salesmen.id = accounts.sales_id);
> This is not currently implemented — the source must be a list of
> independent expressions.
> 
> Why is this not implemented? Is it considered inconvenient to use, or
> difficult to implement. or not important enough, or some other reason?

I cannot answer why but I too would like to see this.  I actually asked this
a long while back but cannot seem to find my posting or recall the response.

David J.




--
View this message in context: 
http://postgresql.1045698.n5.nabble.com/Why-is-UPDATE-with-column-list-syntax-not-implemented-tp5779600p5779601.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] [PERFORM] Cpu usage 100% on slave. s_lock problem.

On 2013-11-21 09:08:05 -0600, Merlin Moncure wrote:
> This code is in a very hot code path.  Are we *sure* that the read
> barrier is fast enough that we don't want to provide an alternate
> function that only returns the local flag?  I don't know enough about
> them to say either way.

A read barrier is just a compiler barrier on x86.

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] Status of FDW pushdowns

On Thu, Nov 21, 2013 at 9:05 AM, Bruce Momjian  wrote:
> Where are we on the remaining possible pushdowns for foreign data
> wrappers, particularly the Postgres one?  I know we do WHERE restriction
> pushdowns in 9.3, but what about join and aggregate pushdowns?  Is
> anyone working on those?
>
> I know join pushdowns seem insignificant, but it helps to restrict what
> data must be passed back because you would only pass back joined rows.

By 'insignificant' you mean 'necessary to do any non-trivial real
work'.   Personally, I'd prefer it if FDW was extended to allow
arbitrary parameterized queries like every other database connectivity
API ever made ever.  But in lieu of that, I'll take as much push down
power as possible :-D.

merlin

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

Dean Rasheed  writes:
> On 20 November 2013 22:46, Andrew Gierth  wrote:
> "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.

> Yeah that seems preferable to option #2, which just seems to open up a
> whole can of worms.

> However, I think I would quickly find it a PITA that it kept telling
> me to wrap it in a TABLE() construct. It would seem like the "TABLE"
> was just an unnecessary noise word (from a user perspective). Could we
> simply support an alias list after the ORDINALITY, in addition to the
> coldeflist?

This seems like way too much complication to save a couple of keystrokes
in a corner case.  Two separate AS clauses applying to the same FROM
item seems mighty confusing to me ...

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

On 2013-11-21 10:25:20 -0500, J Smith wrote:
> On Tue, Nov 19, 2013 at 9:22 AM, Andres Freund  wrote:
> > On 2013-11-19 15:20:01 +0100, Andres Freund wrote:
> >> Imo something the attached patch should be done. The description I came
> >> up with is:
> >>
> >> Fix Hot-Standby initialization of clog and subtrans.
> >
> 
> G'day Andres.
> 
> This wouldn't happen to be related to the issue I was having with
> missing pg_subtrans files, would it, as discussed here
> http://www.postgresql.org/message-id/cadfupgc5bmtv-yg9znxv-vcfkb+jprqs7m2oesqxam_4z1j...@mail.gmail.com
> ? Based on my reading of the commit message and what I could glean
> from the patch itself, they seem to be related.

I don't think so - for one, pg_subtrans isn't really the problems with
that bug, for another, it won't cause missing files. Also, you're not
using replication, right?

Greetings,

Andres Freund

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


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

Re: [HACKERS] Replication Node Identifiers and crashsafe Apply Progress

On 2013-11-21 08:22:05 -0500, Robert Haas wrote:
> On Thu, Nov 21, 2013 at 6:15 AM, Andres Freund  wrote:
> >> > 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.
> >
> > Sure. But how often are we a) going to create such an identifier b)
> > looking it up?
> 
> Never.  Make that the replication solution's problem.  Make the core
> support deal only with UUIDs or pairs of 64-bit integers or something
> like that, and let the replication solution decide what they mean.

I think we're misunderstanding each other. I was commenting on your fear
that strings longer than NAMEDATALEN or something would be bad for
performance - which I don't think is very relevant because the lookups
from "public" to "internal" identifier shouldn't be in any performance
critical path.

I personally would prefer a string because it'd allow me to build an
identifier using the criterions I'd originally outlined outside of this
infrastructure.

Greetings,

Andres Freund

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


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

[HACKERS] WIP patch for updatable security barrier views

2013-11-21 Thread Craig Ringer

Hi all

I have updatable security barrier views working for INSERT and DELETE,
so this might be a good chance to see whether the described approach is
acceptable in reality, not just in theory.

I've been surprised by how well it worked out. I actually landed up
removing a lot of the existing updatable views code; once update knows
how to operate on a subquery it becomes unnecessary to duplicate the
optimiser's knowledge of how to expand and flatten a view in the rewriter.

INSERT and DELETE work. I haven't tested INSERT with defaults on the
base rel yet but suspect it'll need the same support as for update.

UPDATE isn't yet supported because of the need to inject references to
cols in the base rel that aren't selected in the view.
expand_targetlist(...) in prep/preptlist.c already does most of this
work so I hope to be able to use or adapt that.

This patch isn't subject to the replanning and invalidation issues
discussed for RLS because updatable s.b. views don't depend on the
current user or some special "bypass RLS" right like RLS would.

The regression tests die because they try to update an updatable view.

This isn't proposed for inclusion as it stands, it's a chance to comment
and say "why the heck would you do that".

-- 
 Craig Ringer   http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services
>From 93d6bb42040bc9a062d5aa345362b1f0be81670d Mon Sep 17 00:00:00 2001
From: Craig Ringer 
Date: Thu, 21 Nov 2013 21:05:39 +0800
Subject: [PATCH] First pass updatable s.b. views support - only handles DELETE
 from a view

---
 src/backend/commands/tablecmds.c   |   6 +-
 src/backend/commands/view.c|  11 +-
 src/backend/optimizer/plan/planmain.c  |  15 +++
 src/backend/optimizer/prep/preptlist.c |   2 +
 src/backend/rewrite/rewriteHandler.c   | 239 ++---
 src/include/rewrite/rewriteHandler.h   |   1 -
 6 files changed, 65 insertions(+), 209 deletions(-)

diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 0b31f55..128af98 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -8798,7 +8798,6 @@ ATExecSetRelOptions(Relation rel, List *defList, AlterTableType operation,
 		List	   *view_options = untransformRelOptions(newOptions);
 		ListCell   *cell;
 		bool		check_option = false;
-		bool		security_barrier = false;
 
 		foreach(cell, view_options)
 		{
@@ -8806,8 +8805,6 @@ ATExecSetRelOptions(Relation rel, List *defList, AlterTableType operation,
 
 			if (pg_strcasecmp(defel->defname, "check_option") == 0)
 check_option = true;
-			if (pg_strcasecmp(defel->defname, "security_barrier") == 0)
-security_barrier = defGetBoolean(defel);
 		}
 
 		/*
@@ -8817,8 +8814,7 @@ ATExecSetRelOptions(Relation rel, List *defList, AlterTableType operation,
 		if (check_option)
 		{
 			const char *view_updatable_error =
-view_query_is_auto_updatable(view_query,
-			 security_barrier, true);
+view_query_is_auto_updatable(view_query, true);
 
 			if (view_updatable_error)
 ereport(ERROR,
diff --git a/src/backend/commands/view.c b/src/backend/commands/view.c
index aca40e7..5b1457a 100644
--- a/src/backend/commands/view.c
+++ b/src/backend/commands/view.c
@@ -313,8 +313,11 @@ DefineViewRules(Oid viewOid, Query *viewParse, bool replace)
 	   list_make1(viewParse));
 
 	/*
-	 * Someday: automatic ON INSERT, etc
+	 * No automatic ON INSERT, etc is needed, since DML can operate
+	 * directly on the subquery expansion of the SELECT view with
+	 * a little massaging. See the rewriter.
 	 */
+
 }
 
 /*---
@@ -395,7 +398,6 @@ DefineView(ViewStmt *stmt, const char *queryString)
 	RangeVar   *view;
 	ListCell   *cell;
 	bool		check_option;
-	bool		security_barrier;
 
 	/*
 	 * Run parse analysis to convert the raw parse tree to a Query.  Note this
@@ -450,7 +452,6 @@ DefineView(ViewStmt *stmt, const char *queryString)
 	 * specified.
 	 */
 	check_option = false;
-	security_barrier = false;
 
 	foreach(cell, stmt->options)
 	{
@@ -458,8 +459,6 @@ DefineView(ViewStmt *stmt, const char *queryString)
 
 		if (pg_strcasecmp(defel->defname, "check_option") == 0)
 			check_option = true;
-		if (pg_strcasecmp(defel->defname, "security_barrier") == 0)
-			security_barrier = defGetBoolean(defel);
 	}
 
 	/*
@@ -469,7 +468,7 @@ DefineView(ViewStmt *stmt, const char *queryString)
 	if (check_option)
 	{
 		const char *view_updatable_error =
-			view_query_is_auto_updatable(viewParse, security_barrier, true);
+			view_query_is_auto_updatable(viewParse, true);
 
 		if (view_updatable_error)
 			ereport(ERROR,
diff --git a/src/backend/optimizer/plan/planmain.c b/src/backend/optimizer/plan/planmain.c
index 3a4e836..d3eae04 100644
--- a/src/backend/optimizer/plan/planmain.c
+++ b/src/backend/optimizer/plan/planmain.c
@@ -126,6 +126,21 @@ query_planner(PlannerInfo *root, List *tlist,
 	add_base_rels_to_query

Re: [HACKERS] b21de4e7b32f868a23bdc5507898d36cbe146164 seems to be two bricks shy of a load

2013-11-21 Thread Andrew Dunstan



On 11/21/2013 05:52 AM, David Rowley wrote:
I'm not quite sure why nobody else seems to be complaining, but the 
changes to type.h in this commit seems to have broken things little.


In the visual studios build I'm getting:

  src\interfaces\ecpg\preproc\preproc.y(84): error C2065: 'ET_FATAL' : 
undeclared identifier [D:\Postgres\b\ecpg.vcxproj]
  src\interfaces\ecpg\preproc\preproc.y(84): error C2051: case 
expression not constant [D:\Postgres\b\ecpg.vcxproj]
  src\interfaces\ecpg\preproc\preproc.y(102): error C2065: 'ET_FATAL' 
: undeclared identifier [D:\Postgres\b\ecpg.vcxproj]
  src\interfaces\ecpg\preproc\preproc.y(102): error C2051: case 
expression not constant [D:\Postgres\b\ecpg.vcxproj]
  src\interfaces\ecpg\preproc\preproc.y(14664): error C2065: 
'ET_FATAL' : undeclared identifier [D:\Postgres\b\ecpg.vcxproj]


Which I'm guessing is something to do with:

--- a/src/interfaces/ecpg/preproc/type.h 

+++ b/src/interfaces/ecpg/preproc/type.h 

@@ -186,7 
 
+186,7 
 
@@ struct assignment

 enum errortype
 {
-   ET_WARNING, ET_ERROR, ET_FATAL
+   ET_WARNING, ET_ERROR
 };






(Note, please use shortened commit hashes in your subjects - the leading 
7 hex digits of the hash are usually sufficient to identify it.)



The buildfarm animals using MSVC are not exhibiting this problem. When 
you see something like this, the buildfarm is probably the first place 
you should look.


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

2013-11-21 Thread Josh Berkus

Maciej,

Thanks for feedback -- it's helpful to get a different perspective on this.

> I used to work on a project storing large quantities of schema-less data,
> initially using MongoDB, then Postgres with JSON, and eventually I
> implemented BSON support for Postgres to get the best of both worlds:
> https://github.com/maciekgajewski/postgresbson

Huh, is that linked anywhere else?

> 
> I don't think that JSONB is a good idea. There is a lot to learn from
> MongoDB's mistakes in this area.
> 
> 1. As noted in this thread previously, JSON is a serialization format, not
> a document format.

The momentum of the industry would argue against you.

> 5. 1. and 3. are mutually exclusive. the this is one of the most valuable
> takeaways I have from working with Mongo and BSON in particular. BSON is an
> attempt to create 'binary JSON', and a failed one.

BSON was also developed part-time, by an inexperienced developer, as a
side effect of a different project.  And they froze the API to early.
And they only made a halfhearted effort to be compatible with JSON.

So the fact that BSON is a failure doesn't tell us any more than "don't
do this in a half-assed way".

> 7. It seems to me that JSONB is going to repeat all the mistakes of BSON,
> it's going to be 'neither'. If there is an agreement that Postgres needs a
> 'document' format, why not acknowledge 5., and simply adopt one of the
> existing formats. Or even better: adopt none, provide many, provide binary
> send/recv and conversion to and from JSON, let the user choose.

Well, I think others have already documented why we don't want BSON.

> The world is full of self-describing binary formats: BSON, MessagePack (
> http://msgpack.org/), protobuf, hierarchical H-Store is coming along.
> Adding another one would create confusion, and a situation similar to this:
> http://xkcd.com/927/

Apparently you missed that JSONB is just Hstore2 with different in/out
functions?

> Postgres' greatest and most under-advertised feature is it's extensibility.

I agree that we don't do enough to promote our extensions.


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

2013-11-21 Thread Fujii Masao

On Thu, Nov 21, 2013 at 6:41 AM, Robert Haas  wrote:
> 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.

Committed.

>  Then,
> you can make further changes to master in a subsequent commit.

Yeah, I will implement that patch.

Regards,

-- 
Fujii Masao


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

Re: [HACKERS] gaussian distribution pgbench


On 30.09.2013 07:12, KONDO Mitsumasa wrote:

(2013/09/27 5:29), Peter Eisentraut wrote:

This patch no longer applies.

I will try to create this patch in next commit fest.
If you have nice idea, please send me!


A few thoughts on this:

1. DBT-2 uses a non-uniform distribution. You can use that instead of 
pgbench.


2. Do we really want to add everything and the kitchen sink to pgbench? 
Every addition is small when considered alone, but we'll soon end with a 
monster. So I'm inclined to reject this patch on those grounds.


3. That said, this could be handy. But it would be even more handy if 
you could get Gaussian random numbers with \setrandom, so that you could 
use this with custom scripts. And once you implement that, do we 
actually need the -g flag anymore? If you want TPC-B transactions with 
gaussian distribution, you can write a custom script to do that. The 
documentation includes a full script that corresponds to the built-in 
TPC-B script.


So what I'd actually like to see is \setgaussian, for use in custom scripts.

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

Andrew Gierth  writes:
> "Tom" == Tom Lane  writes:
>  Tom> Anyway, after further thought I've come up with an approach
>  Tom> that's purely a syntactic transformation and so less likely to
>  Tom> cause surprise: let's say that if we have TABLE() with a single
>  Tom> argument, and no coldeflist either inside or outside, then we
>  Tom> implicitly insert UNNEST().  Otherwise not.

> This seems ugly beyond belief.

True :-(

> If there isn't a reasonable syntax alternative to TABLE(...) for the
> multiple functions case, then frankly I think we should go ahead and
> burn compatibility with a spec feature which appears to be of negative
> value.

TBH, I'm getting close to that conclusion too.  The more I look at the
spec, the more I think it must be a mistake, or else I'm somehow reading
it wrong, because it sure makes no sense for them to have invented
something that's just an alternative and less-clear syntax for a feature
they already had.

Can anyone who's following this thread check the behavior of Oracle or
DB2 to see if they interpret TABLE() the way I think the spec says?

regards, tom lane

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

Re: [HACKERS] WIP patch for updatable security barrier views

2013-11-21 Thread Dean Rasheed

On 21 November 2013 13:15, Craig Ringer  wrote:
> Hi all
>
> I have updatable security barrier views working for INSERT and DELETE,
> so this might be a good chance to see whether the described approach is
> acceptable in reality, not just in theory.
>
> I've been surprised by how well it worked out. I actually landed up
> removing a lot of the existing updatable views code;

I fear you have removed a little too much though.

For example, something somewhere has to deal with re-mapping of
attributes. That's pretty fundamental, otherwise it can't hope to work
properly.

CREATE TABLE t1(a int, b text);
CREATE VIEW v1 AS SELECT b, a FROM t1;
INSERT INTO v1(a, b) VALUES(1, 'B');

ERROR:  table row type and query-specified row type do not match
DETAIL:  Table has type integer at ordinal position 1, but query expects text.

Also, you have deleted the code that supports WITH CHECK OPTION.

 once update knows
> how to operate on a subquery it becomes unnecessary to duplicate the
> optimiser's knowledge of how to expand and flatten a view in the rewriter.
>
> INSERT and DELETE work. I haven't tested INSERT with defaults on the
> base rel yet but suspect it'll need the same support as for update.
>
> UPDATE isn't yet supported because of the need to inject references to
> cols in the base rel that aren't selected in the view.
> expand_targetlist(...) in prep/preptlist.c already does most of this
> work so I hope to be able to use or adapt that.
>
> This patch isn't subject to the replanning and invalidation issues
> discussed for RLS because updatable s.b. views don't depend on the
> current user or some special "bypass RLS" right like RLS would.
>
> The regression tests die because they try to update an updatable view.
>
> This isn't proposed for inclusion as it stands, it's a chance to comment
> and say "why the heck would you do that".
>

It sounds like it could be an interesting approach in principle, but
you haven't yet shown how you intend to replace some of the rewriter
code that you've removed. It feels to me that some of those things
pretty much have to happen in the rewriter, because the planner just
doesn't have the information it needs to be able to do it.

Regards,
Dean

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

Re: [HACKERS] Status of FDW pushdowns

Kohei KaiGai  writes:
> Right now, it performs on relation scan or join only. However, we will be
> able to apply same concept on aggregation.
> For example, an aggregation node on a foreign table scan is a good
> candidate to push down because it can be replaced with a custom-
> logic that scans a materialized result of the remote aggregation query,
> if its cost is enough cheap than local aggregation.
> Probably, we need to add a hook and some logic to compare the
> built-in aggregation and alternative paths provided by extensions.

Note that this is another thing that's blocked on Path-ifying the work
now done in grouping_planner.  We don't currently have a way to represent
a local aggregation, much less a remote one, as a Path.  We definitely
need that before we can open up any of that logic to FDWs.

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

2013-11-21 Thread J Smith

On Thu, Nov 21, 2013 at 12:23 PM, Andres Freund  wrote:
>
> It's still not this issue in that case, but I might have an idea... Do
> you have hot_standby_feedback enabled?
>

Nope, hot_standby_feedback is set to its default setting which is off.

At any rate, I'm going to try and capture a backtrace from the error
I'm experiencing as suggested in the other thread by Robert Haas and
we'll see if that provides any insight.

Cheers

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

[HACKERS] Status of FDW pushdowns

2013-11-21 Thread Bruce Momjian

Where are we on the remaining possible pushdowns for foreign data
wrappers, particularly the Postgres one?  I know we do WHERE restriction
pushdowns in 9.3, but what about join and aggregate pushdowns?  Is
anyone working on those?

I know join pushdowns seem insignificant, but it helps to restrict what
data must be passed back because you would only pass back joined rows.

Do we document these missing features anywhere?

-- 
  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] [PERFORM] Cpu usage 100% on slave. s_lock problem.


On 03.10.2013 00:14, Merlin Moncure wrote:

On Wed, Oct 2, 2013 at 3:43 PM, Ants Aasma  wrote:

On Wed, Oct 2, 2013 at 10:37 PM, Merlin Moncure  wrote:

On Wed, Oct 2, 2013 at 9:45 AM, Ants Aasma  wrote:

I haven't reviewed the code in as much detail to say if there is an
actual race here, I tend to think there's probably not, but the
specific pattern that I had in mind is that with the following actual
code:


hm.  I think there *is* a race.  2+ threads could race to the line:

LocalRecoveryInProgress = xlogctl->SharedRecoveryInProgress;

and simultaneously set the value of LocalRecoveryInProgress to false,
and both engage InitXLOGAccess, which is destructive.   The move
operation is atomic, but I don't think there's any guarantee the reads
to xlogctl->SharedRecoveryInProgress are ordered between threads
without a lock.


The backend is single-threaded, so this is moot.


InitXLOGAccess only writes backend local variables, so it can't be
destructive. In fact, it calls GetRedoRecPtr(), which does a spinlock
acquisition cycle, which should be a full memory barrier. A read
barrier after accessing SharedRecoveryInProgress is good enough, and
it seems to be necessary to avoid a race condition for
XLogCtl->ThisTimeLineID.

Admittedly the race is so unprobable that in practice it probably
doesn't matter. I just wanted to be spell out the correct way to do
unlocked accesses as it can get quite confusing. I found Herb Sutter's
"atomic<> weapons" talk very useful, thinking about the problem in
terms of acquire and release makes it much clearer to me.


If we don't care about multiple calls to InitXLOGAccess() (for a
backend) then we can get away with just a barrier.  That's a pretty
weird assumption though (even if 'improbable') and I think it should
be well documented in the code, particularly since previous versions
went though such trouble to do a proper check->set->init.

I'm still leaning on my earlier idea (recovery4.patch) since it keeps
the old semantics by exploiting the fact that the call site of
interest (only) does not require a correct answer (that is, for the
backend to think it's in recovery when it's not).  That just defers
the heap prune for a time and you don't have to mess around with
barriers at all or be concerned about present or future issues caused
by spurious extra calls to InitXLOGAccess().  The other code paths to
RecoveryInProgress() appear not to be interesting from a spinlock
perspective.


Hmm. All callers of RecoveryInProgress() must be prepared to handle the 
case that RecoveryInProgress() returns true, but the system is no longer 
in recovery. No matter what locking we do in RecoveryInProgress(), the 
startup process might finish recovery just after RecoveryInProgress() 
has returned.


What about the attached? It reads the shared variable without a lock or 
barrier. If it returns 'true', but the system in fact just exited 
recovery, that's OK. As explained above, all the callers must tolerate 
that anyway. But if it returns 'false', then it performs a full memory 
barrier, which should ensure that it sees any other shared variables as 
it is after the startup process cleared SharedRecoveryInProgress 
(notably, XLogCtl->ThisTimeLineID).


- Heikki
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
old mode 100644
new mode 100755
index a95149b..a9bfdfc
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -7370,10 +7370,7 @@ RecoveryInProgress(void)
 		/* use volatile pointer to prevent code rearrangement */
 		volatile XLogCtlData *xlogctl = XLogCtl;
 
-		/* spinlock is essential on machines with weak memory ordering! */
-		SpinLockAcquire(&xlogctl->info_lck);
 		LocalRecoveryInProgress = xlogctl->SharedRecoveryInProgress;
-		SpinLockRelease(&xlogctl->info_lck);
 
 		/*
 		 * Initialize TimeLineID and RedoRecPtr when we discover that recovery
@@ -7382,7 +7379,15 @@ RecoveryInProgress(void)
 		 * this, see also LocalSetXLogInsertAllowed.)
 		 */
 		if (!LocalRecoveryInProgress)
+		{
+			/*
+			 * If we just exited recovery, make sure we read any other global
+			 * state after SharedRecoveryInProgress (for machines with weak
+			 * memory ordering).
+			 */
+			pg_memory_barrier();
 			InitXLOGAccess();
+		}
 
 		return LocalRecoveryInProgress;
 	}

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

2013-11-21 Thread Andrew Gierth

> "Tom" == Tom Lane  writes:

 Tom> Anyway, after further thought I've come up with an approach
 Tom> that's purely a syntactic transformation and so less likely to
 Tom> cause surprise: let's say that if we have TABLE() with a single
 Tom> argument, and no coldeflist either inside or outside, then we
 Tom> implicitly insert UNNEST().  Otherwise not.

This seems ugly beyond belief.

Specifically, having TABLE(foo()) and TABLE(foo(),bar()) be radically
different constructs, and likewise  TABLE(foo()) and TABLE(foo() AS (...)),
strikes me as highly objectionable.

If there isn't a reasonable syntax alternative to TABLE(...) for the
multiple functions case, then frankly I think we should go ahead and
burn compatibility with a spec feature which appears to be of negative
value.

-- 
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] WITHIN GROUP patch

2013-11-21 Thread Vik Fearing

On 11/21/2013 11:04 AM, Atri Sharma wrote:
> Please find attached the latest patch for WITHIN GROUP. This patch is
> after fixing the merge conflicts.

I have spent quite some time on this and the previous versions. Here is
my review, following the questions on the wiki.

This patch is in context diff format and applies cleanly to today's
master. It contains several regression tests and for the most part,
good documentation. I would like to see at least one example of using
each of the two types of function (hypothetical and inverted
distribution) in section 4.2.8.

This patch implements what it says it does. We don't already have a way
to get these results without this patch that I know of, and I think we
do want it. I certainly want it. I do not have a copy of the SQL
standard, but I have full faith in the Andrew Gierth's claim that this
is part of it. Even if not, implementation details were brought up
during design and agreed upon by this list[1]. I don't see how anything
here could be dangerous. The custom ordered set functions I made
correctly passed a round-trip through dump/restore.

The code compiles without warning. All of the clean tests I did worked
as expected, and all of the dirty tests failed elegantly.

I did not find any corner cases, and I looked in as many corners as I
could think of. I didn't manage to trigger any assertion failures and I
didn't crash it.

I found no noticeable issues with performance, either directly or as
side effects.

I am not the most competent with code review so I'll be relying on
further review by another reviewer or the final committer. The patch
fixed the project comments/messages style issues I raised in my previous
review. I found the code comments lacking in some places (like
inversedistribution.c:mode_final for example) but I can't say if the
really is too terse, or if it's just me. On the other hand, I thought
the explanation blocks in the code comments were adequately descriptive.

There is some room for improvement in future versions. The query select
mode() within group (order by x) over (partition by y) from ... should
be valid but isn't. This was explained by Andrew on IRC as being
non-trivial: "specifically, we implemented WITHIN GROUP by repurposing
the infrastructure already present for agg(distinct ...) and agg(x order
by y) which are also not yet supported for
aggregate-as-window-function". I assume then that evolution on one side
will benefit the other.

All in all, I believe this is ready for committer.

[1]
http://www.postgresql.org/message-id/2b8b55b8ba82f83ef4e6070b95fb0cd0%40news-out.riddles.org.uk

--
Vik

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

On 2013-11-20 17:46:22 +0100, Andres Freund wrote:
> 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.

Nope, it wouldn't, it just uses heap_freeze_tuple() again which *only*
sets HEAP_XMIN_COMMITTED but does *not* clear HEAP_XMIN_INVALID. And if
we were to change that now, it wouldn't help already frozen tuples that
wouldn't get frozen again.

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

On 2013-11-20 15:05:17 -0500, Robert Haas wrote:
> > 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

Well yes. But then you need a way to store and change that random id for
each cluster.

Anyway, the preference is clear, so I am going to go for that in v2. I
am not sure about the type of the public identifier yet, I'll think a
bit about it.

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

Sure. But how often are we a) going to create such an identifier b)
looking it up? Hopefully both will be rather infrequent operations.

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

On 2013-11-19 14:16:04 +, Greg Stark wrote:
> On Thu, Nov 14, 2013 at 5:26 PM, Andres Freund wrote:
> 
> > But for that the receiving side needs to know up to where changes have
> > been applied. One relatively easy solution for that is that the
> > receiving side does something like:
> > UPDATE replication_progress SET lsn = '0/1600' WHERE source_id = ...;
> > before the end of every replayed transaction. But that obviously will
> > quickly cause bloat.
> >
> > Our solution to that is that a replaying process can tell the backend
> > that it is currently doing so and setup three variables for every
> > transaction:
> >
> 
> This is a pretty massive design decision to hinge on such a minor
> implementation detail of table bloat (which I don't think would actually be
> an issue anyway -- isn't that what we have HOT for?)

Not sure what HOT is going to help with? Even with HOT we can only
remove tuples that are invisible to everyone. If there are longrunning
queries on the standby - and running analytics on standbys is a rather
frequent use-case - that won't be the case for a long, long time.

> Fundamentally the question here is where to keep all the book-keeping state
> about replicas, in a central repository in the master or locally in each
> replica. At first blush it seems obvious to me that locally in each replica
> is the more flexible choice.

This really is about storing the state of apply on each replica
efficiently.

Imagine the standby just received data for a transaction x and has
replayed it locally. If it crashes in that moment, it needs to know
whether that transaction has successfully committed or not. And that has
to work even if the commit succeeded internally but hasn't yet returned
success!
So, what this provides is a facility to say 'hey, this local transaction
was at X on the source Y' and a way to get the last X for each Y at the
end of crash recovery.
Then the replication solution can restart replication from X onwards for
each Y.

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