Re: [HACKERS] pg_retainxlog for inclusion in 9.3?

[Magnus Hagander]

On Mon, Jan 14, 2013 at 5:56 PM, Robert Haas  wrote:
> On Fri, Jan 4, 2013 at 4:55 PM, Dimitri Fontaine  
> wrote:
>> Robert Haas  writes:
>>> Mostly that it seems like a hack, and I suspect we may come up with a
>>> better way to do this in the future.
>>
>> Do you have the specs of such better way? Would it be a problem to have
>> both pg_retainxlog and the new way?
>
> Well, I think in the long term we are likely to want the master to
> have some kind of ability to track the positions of its slaves, even
> when they are disconnected.  And, optionally, to retain the WAL that
> they need, again even when they are disconnected.  If such an ability
> materializes, this will be moot (even as I think that pg_standby is
> now largely moot, at least for new installations, now that we have
> standby_mode=on).

I agree. But just as we had pg_standby for quite a while before we got
standby_mode=on, I believe we should have pg_retainxlog (or something
like it) until we have something more integrated.

--
 Magnus Hagander
 Me: http://www.hagander.net/
 Work: http://www.redpill-linpro.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] unlogged tables vs. GIST

[Jeevan Chalke]

Hi Robert / Tom

I think to support GiST with unlogged table we need to do three things:

1. Teach the buffer manager that the LSN of a page not marked
BM_PERMANENT can be ignored

2. Teach GetXLogRecPtrForTemp() to allocate fake LSNs for GiST buffers
using a 64-bit, counter that is persisted across clean shutdowns

3. Remove the error checks that prevent creating an unlogged GiST
index and update the documentation accordingly

PFA, patch which try to do above things.

For (2), I have added a new function called, GetXLogRecPtrForUnloogedRel()
which returns a fake LSN for GiST indexes. However, I have removed
GetXLogRecPtrForTemp() function and added its functionality inside this new
function itself to avoid complexity.

With this patch I am able to create GiST for unlogged tables.
It works well with my examples.
Also make check has no issues.

Please have a look and let me know your views.

Thanks



On Sat, Dec 18, 2010 at 7:20 AM, Robert Haas  wrote:

> On Fri, Dec 17, 2010 at 4:17 PM, Tom Lane  wrote:
> >> Since these bits will only be set/cleared when the buffer mapping is
> >> changed, can we examine this bit without taking the spinlock?
> >
> > Only if you're willing for the result to be unreliable.
>
> If we read the bits while someone else is writing them, we'll get
> either the old or the new value, but the BM_FLUSH_XLOG bit will be the
> same either way.  When FlushBuffer() is called, a shared content log
> and a pin are held, so the buffer can't be renamed under us.  If
> that's really unacceptable, we can do something like the attached, but
> I think this is unnecessarily gross.  We already assume in other
> places that the read or write of an integer is atomic.  In this case
> we don't even need it to be fully atomic - we just need the particular
> byte that contains this bit not to go through some intermediate state
> where the bit is unset.  Is there really a memory architecture out
> there that's crazy enough to let such a thing happen?  If so, I'd
> expect things to be breaking right and left on that machine anyway.
>
> --
> 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
>
>


-- 
Jeevan B Chalke
Senior Software Engineer, R&D
EnterpriseDB Corporation
The Enterprise PostgreSQL Company

Phone: +91 20 30589500

Website: www.enterprisedb.com
EnterpriseDB Blog: http://blogs.enterprisedb.com/
Follow us on Twitter: http://www.twitter.com/enterprisedb

This e-mail message (and any attachment) is intended for the use of the
individual or entity to whom it is addressed. This message contains
information from EnterpriseDB Corporation that may be privileged,
confidential, or exempt from disclosure under applicable law. If you are
not the intended recipient or authorized to receive this for the intended
recipient, any use, dissemination, distribution, retention, archiving, or
copying of this communication is strictly prohibited. If you have received
this e-mail in error, please notify the sender immediately by reply e-mail
and delete this message.


support_unlogged_gist_indexes.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] logical changeset generation v4

[Mark Kirkwood]

On 15/01/13 17:37, Mark Kirkwood wrote:

On 15/01/13 14:38, Andres Freund wrote:

Hi everyone,

Here is the newest version of logical changeset generation.






I'm quite interested in this feature - so tried applying the 19 
patches to the latest 9.3 checkout. Patch and compile are good.


However portals seem busted:

bench=# BEGIN;
BEGIN
bench=# DECLARE c1 CURSOR FOR SELECT * FROM pgbench_accounts;
DECLARE CURSOR
bench=# FETCH 2 FROM c1;
 aid | bid | abalance | filler

-+-+--+- 


-
   1 |   1 |0 |

   2 |   1 |0 |

(2 rows)

bench=# DELETE FROM pgbench_accounts WHERE CURRENT OF c1;
The connection to the server was lost. Attempting reset: Failed.



Sorry - forgot to add: assert and debug build, and it is an assertion 
failure that is being picked up:


TRAP: FailedAssertion("!(htup->t_tableOid != ((Oid) 0))", File: 
"tqual.c", Line: 940)




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

Re: [HACKERS] logical changeset generation v4

[Mark Kirkwood]

On 15/01/13 14:38, Andres Freund wrote:

Hi everyone,

Here is the newest version of logical changeset generation.






I'm quite interested in this feature - so tried applying the 19 patches 
to the latest 9.3 checkout. Patch and compile are good.


However portals seem busted:

bench=# BEGIN;
BEGIN
bench=# DECLARE c1 CURSOR FOR SELECT * FROM pgbench_accounts;
DECLARE CURSOR
bench=# FETCH 2 FROM c1;
 aid | bid | abalance | filler

-+-+--+-
-
   1 |   1 |0 |

   2 |   1 |0 |

(2 rows)

bench=# DELETE FROM pgbench_accounts WHERE CURRENT OF c1;
The connection to the server was lost. Attempting reset: Failed.



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

Re: [HACKERS] count(*) of zero rows returns 1

[Gurjeet Singh]

On Mon, Jan 14, 2013 at 11:03 PM, Alvaro Herrera
wrote:

> Gurjeet Singh escribió:
>
> > Interesting to note that SELECT * FROM table_with_zero_cols does not
> > complain of anything.
> >
> > postgres=# select * from test1;
> > --
> > (0 rows)
> >
> > This I believe result of the fact that we allow user to drop all columns
> of
> > a table.
> >
> > On a side note, Postgres allows me to do this (which I don't think is a
> bug
> > or useless): I inserted some rows into a table, and then dropped the
> > columns. The resulting table has no columns, but live rows.
> >
> > postgres=# select * from test_0_col_table ;
> > --
> > (20 rows)
>
> Yeah.
>
> alvherre=# create table foo ();
> CREATE TABLE
> alvherre=# insert into foo default values;
> INSERT 0 1
> alvherre=# insert into foo default values;
> INSERT 0 1
> alvherre=# insert into foo default values;
> INSERT 0 1
> alvherre=# insert into foo default values;
> INSERT 0 1
> alvherre=# insert into foo select * from foo;
> INSERT 0 4
> alvherre=# insert into foo select * from foo;
> INSERT 0 8
> alvherre=# insert into foo select * from foo;
> INSERT 0 16
> alvherre=# insert into foo select * from foo;
> INSERT 0 32
> alvherre=# insert into foo select * from foo;
> INSERT 0 64
> alvherre=# select count(*) from foo;
>  count
> ---
>128
> (1 fila)
>
> alvherre=# select * from foo;
> --
> (128 filas)
>
> If you examine the ctid system column you can even see that those empty
> rows consume some storage space.


I was trying to build a case and propose that we issue a TRUNCATE on the
table after the last column is dropped. But then realized that the rows may
have become invisible, but they can be brought back to visibility by simply
adding a new column. These rows with get the new column's DEFAULT value
(NULL by default), and then the result of a SELECT * will show all the rows
again.

-- 
Gurjeet Singh

http://gurjeet.singh.im/

Re: [HACKERS] Patches for TODO item: Avoid truncating empty OCDR temp tables on COMMIT

[Gurjeet Singh]

On Mon, Jan 14, 2013 at 10:33 PM, Tom Lane  wrote:

> Gurjeet Singh  writes:
> > Please find attached two patches, implementing two different approaches
> to
> > fix the issue of COMMIT truncating empty TEMP tables that have the ON
> > COMMIT DELETE ROWS attribute.
>
> > v2.patch: This approach introduces a boolean 'rd_rows_inserted' in
> > RelationData struct, and sets this struct to true for every TEMP table in
> > which a row is inserted. During commit, we avoid truncating those OCDR
> temp
> > tables that haven't been inserted into in this transaction.
>
> I think this is unacceptable on its face.  It essentially supposes that
> relcache entries are reliable storage.  They are not.  There are some
> places where we rely on relcache entries preserving state information
> for optimization purposes --- but in this case, discarding a relcache
> entry would result in visibly incorrect behavior, not just some
> performance loss.
>

Would it be acceptable if we inverted the meaning of the struct member, and
named it to  rd_rows_not_inserted. When registering an ON COMMIT action, we
can set this member to true, and set it to false when inserting a row into
it. The pre-commit hook will truncate any relation that doesn't have this
member set to true.

With that in place, even if the relcache entry is discarded midway through
the transaction, the cleanup code will truncate the relation, preserving
the correct behaviour.
-- 
Gurjeet Singh

http://gurjeet.singh.im/

Re: [HACKERS] count(*) of zero rows returns 1

[Alvaro Herrera]

Gurjeet Singh escribió:

> Interesting to note that SELECT * FROM table_with_zero_cols does not
> complain of anything.
> 
> postgres=# select * from test1;
> --
> (0 rows)
> 
> This I believe result of the fact that we allow user to drop all columns of
> a table.
> 
> On a side note, Postgres allows me to do this (which I don't think is a bug
> or useless): I inserted some rows into a table, and then dropped the
> columns. The resulting table has no columns, but live rows.
> 
> postgres=# select * from test_0_col_table ;
> --
> (20 rows)

Yeah.

alvherre=# create table foo ();
CREATE TABLE
alvherre=# insert into foo default values;
INSERT 0 1
alvherre=# insert into foo default values;
INSERT 0 1
alvherre=# insert into foo default values;
INSERT 0 1
alvherre=# insert into foo default values;
INSERT 0 1
alvherre=# insert into foo select * from foo;
INSERT 0 4
alvherre=# insert into foo select * from foo;
INSERT 0 8
alvherre=# insert into foo select * from foo;
INSERT 0 16
alvherre=# insert into foo select * from foo;
INSERT 0 32
alvherre=# insert into foo select * from foo;
INSERT 0 64
alvherre=# select count(*) from foo;
 count 
---
   128
(1 fila)

alvherre=# select * from foo;
--
(128 filas)

If you examine the ctid system column you can even see that those empty
rows consume some storage space.

-- 
Á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] logical changeset generation v4

[Alvaro Herrera]

Andres Freund wrote:

I've been giving a couple of these parts a look.  In particular

> [03] Split out xlog reading into its own module called xlogreader

Cleaned this one up a bit last week.  I will polish it some more,
publish for some final comments, and commit.

> [08] wal_decoding: Introduce InvalidCommandId and declare that to be the new 
> maximum for CommandCounterIncrement

This seems reasonable.  Mainly it has the effect that a transaction can
have exactly one less command than before.  I don't think this is a
problem for anyone in practice.

> [09] Adjust all *Satisfies routines to take a HeapTuple instead of a 
> HeapTupleHeader

Seemed okay when I looked at it.


> Second, I don't think the test_logical_replication functions should live
> in core as they shouldn't be used for a production replication scenario
> (causes longrunning transactions, requires polling) , but I have failed
> to find a neat way to include a contrib extension in the plain
> regression tests.

I think this would work if you make a "stamp" file in the contrib
module, similar to how doc/src/sgml uses those.



-- 
Á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] count(*) of zero rows returns 1

[Gurjeet Singh]

On Mon, Jan 14, 2013 at 4:15 PM, Tom Lane  wrote:

> David Johnston  writes:
> > Tom Lane-2 wrote
> >> For that to return zero, it would also be necessary for "SELECT 2+2"
> >> to return zero rows.  Which would be consistent with some views of the
> >> universe, but not particularly useful.
>
> > Given that:
>
> > SELECT *;
> > Results in:
> > SQL Error: ERROR:  SELECT * with no tables specified is not valid
>
> That has nothing to do with the number of rows, though.  That's
> complaining that there are no columns for the * to refer to.
>

Interesting to note that SELECT * FROM table_with_zero_cols does not
complain of anything.

postgres=# select * from test1;
--
(0 rows)

This I believe result of the fact that we allow user to drop all columns of
a table.

On a side note, Postgres allows me to do this (which I don't think is a bug
or useless): I inserted some rows into a table, and then dropped the
columns. The resulting table has no columns, but live rows.

postgres=# select * from test_0_col_table ;
--
(20 rows)


> I get that the horse has already left the barn on this one but neither "0"
> > nor "1" seem particularly sound answers to the question "SELECT
> count(*)".
>
> Yeah, it's more about convenience than principle.  AFAICS there are three
> defensible answers to what an omitted FROM clause ought to mean:
>
> 1. It's not legal (the SQL spec's answer).
> 2. It implicitly means a table of no columns and 1 row (PG's answer).
> 3. It implicitly means a table of no columns and 0 rows (which is what
>I take Gurjeet to be advocating for).
>

I wasn't advocating it, but was trying to wrap my head around why Postgres
would do something like count(*) of nothing == 1.

-- 
Gurjeet Singh

http://gurjeet.singh.im/

Re: [HACKERS] Patches for TODO item: Avoid truncating empty OCDR temp tables on COMMIT

[Tom Lane]

Gurjeet Singh  writes:
> Please find attached two patches, implementing two different approaches to
> fix the issue of COMMIT truncating empty TEMP tables that have the ON
> COMMIT DELETE ROWS attribute.

> v2.patch: This approach introduces a boolean 'rd_rows_inserted' in
> RelationData struct, and sets this struct to true for every TEMP table in
> which a row is inserted. During commit, we avoid truncating those OCDR temp
> tables that haven't been inserted into in this transaction.

I think this is unacceptable on its face.  It essentially supposes that
relcache entries are reliable storage.  They are not.  There are some
places where we rely on relcache entries preserving state information
for optimization purposes --- but in this case, discarding a relcache
entry would result in visibly incorrect behavior, not just some
performance loss.

regards, tom lane


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

Re: [HACKERS] [PATCH] Compile without warning with gcc's -Wtype-limits, -Wempty-body

[Tom Lane]

Andres Freund  writes:
> On 2013-01-14 20:39:05 -0500, Peter Eisentraut wrote:
>> On Tue, 2013-01-15 at 00:29 +0100, Andres Freund wrote:
>>> Independently from this patch, should we add -Wtype-limits to the
>>> default parameters?

>> I think we have had a discussion along this line before.  I am against
>> fixing warnings from this option, because those changes would hide
>> errors if a variable's type changed from signed to unsigned or vice
>> versa, which could happen because of refactoring or it might be
>> dependent on system headers.

> Well, I already found a bug (although with very limited consequences) in
> the walsender code and one with graver consequences in code I just
> submitted. So I don't really see that being on-par with some potential
> future refactoring...

FWIW, I agree with Peter --- in particular, warning against "x >= MIN"
just because MIN happens to be zero and x happens to be unsigned is the
sort of nonsense up with which we should not put.  Kowtowing to that
kind of warning makes the code less robust, not more so.

It's a shame that the compiler writers have not figured this out and
separated misguided pedantry from actually-useful warnings.  If I assign
-1 to an unsigned variable, by all means tell me about *that*.  Don't
tell me your opinion of whether an assertion check is necessary.

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] count(*) of zero rows returns 1

[Gurjeet Singh]

On Mon, Jan 14, 2013 at 3:09 PM, David Johnston  wrote:

> What does "SELECT * FROM dual" in Oracle yield?
>

AFAICR, 'dual' table has one column named 'DUMMY' and one row with value,
single character X.

-- 
Gurjeet Singh

http://gurjeet.singh.im/

Re: [HACKERS] logical changeset generation v4

[Abhijit Menon-Sen]

At 2013-01-15 02:38:45 +0100, and...@2ndquadrant.com wrote:
>
> 2) Currently the logical replication infrastructure assigns a
> 'slot-id' when a new replica is setup. That slot id isn't really
> nice (e.g. "id-321578-3"). It also requires that [18] keeps state
> in a global variable to make writing regression tests easy.
> 
> I think it would be better to make the user specify those replication
> slot ids, but I am not really sure about it.

I agree, it would be better to let the user name the slot (and report an
error if the given name is already in use).

> 3) Currently no options can be passed to an output plugin. I am
> thinking about making "INIT_LOGICAL_REPLICATION 'plugin'" accept the
> now widely used ('option' ['value'], ...) syntax and pass that to the
> output plugin's initialization function.

Sounds good.

> 4) Does anybody object to:
> -- allocate a permanent replication slot
> INIT_LOGICAL_REPLICATION 'plugin' 'slotname' (options);
> 
> -- stream data
> START_LOGICAL_REPLICATION 'slotname' 'recptr';
> 
> -- deallocate a permanent replication slot
> FREE_LOGICAL_REPLICATION 'slotname';

That looks fine, but I think it should be:

INIT_LOGICAL_REPLICATION 'slotname' 'plugin' (options);

i.e., swap 'plugin' and 'slotname' in your proposal to make the slotname
come first for all three commands. Not important, but a wee bit nicer.

-- Abhijit


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

Re: [HACKERS] Curious buildfarm failures

[Tom Lane]

Heikki Linnakangas  writes:
> The problem seems to be when the the old and the key hash to the same 
> bucket. In that case, hash_update_hash_key() tries to link the entry to 
> itself. The attached patch fixes it for me.

Doh!  I had a feeling that that needed a special case, but didn't think
hard enough.  Thanks.

I think the patch could do with more than no comment, but will fix
that and apply.

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] Get current query in a trigger function

[Vlad Arkhipov]

On 01/15/2013 01:45 AM, Robert Haas wrote:

On Fri, Jan 11, 2013 at 4:47 AM, Vlad Arkhipov  wrote:

Is there any simple way of getting a query for which a trigger was executed?
debug_query_string and ActivePortal->sourceText return the top query when
there are nested triggers.

I believe - only if the trigger is written in C.

Yes, the trigger is written in C. But I didn't find any way to get 
access to the current EState or QueryDesc from a trigger function. The 
only common place of a trigger and the corresponding EState/QueryDesc 
structs seems to be CurrentMemoryContext in a trigger function, which 
ancestor has to be (?) EState->es_query_cxt. It's an ugly solution of 
course.


P.S. Is it a mistype EState->es_query_cxt? Should it be es_query_ctx?


--
Sent 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] COPY .. COMPRESSED

[Stephen Frost]

* Peter Eisentraut (pete...@gmx.net) wrote:
> On 1/14/13 11:28 AM, Stephen Frost wrote:
> > While there is no option currently for having the server do the
> > compression before sending the data over the wire.
> 
> OpenSSL?

To be honest, I expected that to come up earlier in this discussion.
It'd be redundant to use OpenSSL for compression and then ALSO do
compression on the client side to save into a custom format dump.
There's also plenty of reasons to not want to deal with OpenSSL just to
have compression support.  Now, protocol-level on-the-wire compression
is another option, but there's quite a few drawbacks to that and quite a
bit of work involved.  Having support for COPY-based compression could
be an answer for many cases where on-the-wire compression is desirable.

Being able to use pipe's for the backend-side of COPY is a good
solution, for that.  I'm looking forward to having it and plan to review
the patch.  That said, I'd like to find an answer to some of these other
use cases, if possible.

Thanks,

Stephen


signature.asc
Description: Digital signature

Re: [HACKERS] logical changeset generation v4

[Abhijit Menon-Sen]

At 2013-01-14 18:15:39 -0800, j...@agliodbs.com wrote:
>
> Is there a git fork for logical replication somewhere?

git://git.postgresql.org/git/users/andresfreund/postgres.git, branch
xlog-decoding-rebasing-cf4 (and xlogreader_v4).

-- Abhijit


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

Re: [HACKERS] logical changeset generation v4

[anara...@anarazel.de]

Josh Berkus  schrieb:

>Andreas,
>
>Is there a git fork for logical replication somewhere?

Check the bottom of the email ;)

--- 
Please excuse brevity and formatting - I am writing this on my mobile phone.


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

Re: [HACKERS] logical changeset generation v4

[Josh Berkus]

Andreas,

Is there a git fork for logical replication somewhere?

-- 
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] [PATCH] Compile without warning with gcc's -Wtype-limits, -Wempty-body

[Andres Freund]

On 2013-01-14 20:39:05 -0500, Peter Eisentraut wrote:
> On Tue, 2013-01-15 at 00:29 +0100, Andres Freund wrote:
> > Independently from this patch, should we add -Wtype-limits to the
> > default parameters?
> 
> I think we have had a discussion along this line before.  I am against
> fixing warnings from this option, because those changes would hide
> errors if a variable's type changed from signed to unsigned or vice
> versa, which could happen because of refactoring or it might be
> dependent on system headers.

Well, I already found a bug (although with very limited consequences) in
the walsender code and one with graver consequences in code I just
submitted. So I don't really see that being on-par with some potential
future refactoring...

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] Compile without warning with gcc's -Wtype-limits, -Wempty-body

[Peter Eisentraut]

On Tue, 2013-01-15 at 00:29 +0100, Andres Freund wrote:
> Independently from this patch, should we add -Wtype-limits to the
> default parameters?

I think we have had a discussion along this line before.  I am against
fixing warnings from this option, because those changes would hide
errors if a variable's type changed from signed to unsigned or vice
versa, which could happen because of refactoring or it might be
dependent on system headers.

FWIW, clang has the same warning on by default.  There, it's called
-Wtautological-compare.

I'm less concerned about -Wempty-body, but I can't see the practical use
either way.



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

Re: [HACKERS] json api WIP patch

[Andrew Dunstan]

On 01/14/2013 07:36 PM, Merlin Moncure wrote:

While testing this I noticed that integer based 'get' routines are
zero based -- was this intentional?  Virtually all other aspects of
SQL are 1 based:

postgres=# select json_get('[1,2,3]', 1);
  json_get
--
  2
(1 row)

postgres=# select json_get('[1,2,3]', 0);
  json_get
--
  1
(1 row)




Yes. it's intentional. SQL arrays might be 1-based by default, but 
JavaScript arrays are not. JsonPath and similar gadgets treat the arrays 
as zero-based. I suspect the Json-using community would not thank us for 
being overly SQL-centric on this - and I say that as someone who has 
always thought zero based arrays were a major design mistake, 
responsible for countless off-by-one errors.


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] json api WIP patch

[Merlin Moncure]

On Thu, Jan 10, 2013 at 5:42 PM, Andrew Dunstan  wrote:
>
> On 01/04/2013 03:23 PM, Andrew Dunstan wrote:
>>
>>
>> On 01/02/2013 05:51 PM, Andrew Dunstan wrote:
>>>
>>>
>>> On 01/02/2013 04:45 PM, Robert Haas wrote:

 On Wed, Dec 26, 2012 at 3:33 PM, Andrew Dunstan 
 wrote:
>
> Here is a patch for the first part of the JSON API that was recently
> discussed. It includes the json  parser hook infrastructure and
> functions
> for json_get and friends, plus json_keys.
>>
>>
>>
>> Udated patch that contains most of the functionality I'm after. One piece
>> left is populate_recordset (populate a set of records from a single json
>> datum which is an array of objects, in one pass). That requires a bit of
>> thought.
>>
>> I hope most of the whitespace issues are fixed.
>>
>>
>
>
> This updated patch contains all the intended functionality, including
> operators for the json_get_path functions, so you can say things like
>
> select jsonval->array['f1','0','f2] ...
>
> It also removes any requirement to copy the json value before setting up the
> lexer by removing the lexer's requirement to have a nul terminated string.
> Instead the lexer is told the input length and relies on that. For this
> reason, json_in() now calls cstring_get_text() before rather than after
> calling the validation routine, but that's really not something worth
> bothering about.
>
> A couple of points worth noting: it's a pity that we have to run CREATE OR
> REPLACE FUNCTION in system_views.sql in order to set up default values for
> builtin functions. That feels very kludgy. Also, making operators for
> variadic functions is a bit of a pain. I had to set up non-variadic version
> of the same functions (see json_get_path_op and json_get_path_as_text_op)
> just so I could set up the operators. Neither of these are exactly
> showstopper items, just mild annoyances.
>
> I will continue hunting memory leaks, but when Merlin gets done with docco I
> think we'll be far enough advanced to add this to the commitfest.

While testing this I noticed that integer based 'get' routines are
zero based -- was this intentional?  Virtually all other aspects of
SQL are 1 based:

postgres=# select json_get('[1,2,3]', 1);
 json_get
--
 2
(1 row)

postgres=# select json_get('[1,2,3]', 0);
 json_get
--
 1
(1 row)

merlin


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

[HACKERS] [PATCH] Compile without warning with gcc's -Wtype-limits, -Wempty-body

[Andres Freund]

Hi,

the attached trivial patch allows to compile with -Wtype-limits
-Wempty-body (or -Wextra -Wno-unused-parameter -Wno-sign-compare
-Wno-missing-field-initializers).

As the two fixes seem harmless, that seems to be a good idea. And the
recent "bug" (its not really that, but ...) in walsender.c shows that at
least -Wtype-limits is a sensible warning.

Independently from this patch, should we add -Wtype-limits to the
default parameters?

Greetings,

Andres Freund

-- 
 Andres Freund http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services
diff --git a/src/backend/storage/lmgr/lock.c b/src/backend/storage/lmgr/lock.c
index f2cf5c6..263fe5a 100644
--- a/src/backend/storage/lmgr/lock.c
+++ b/src/backend/storage/lmgr/lock.c
@@ -331,8 +331,8 @@ PROCLOCK_PRINT(const char *where, const PROCLOCK *proclockP)
 }
 #else			/* not LOCK_DEBUG */
 
-#define LOCK_PRINT(where, lock, type)
-#define PROCLOCK_PRINT(where, proclockP)
+#define LOCK_PRINT(where, lock, type) (void)0
+#define PROCLOCK_PRINT(where, proclockP) (void)0
 #endif   /* not LOCK_DEBUG */
 
 
diff --git a/src/backend/tsearch/wparser_def.c b/src/backend/tsearch/wparser_def.c
index cb2f8eb..d8c6be5 100644
--- a/src/backend/tsearch/wparser_def.c
+++ b/src/backend/tsearch/wparser_def.c
@@ -1811,7 +1811,7 @@ TParserGet(TParser *prs)
 pg_mblen(prs->str + prs->state->posbyte);
 
 		Assert(prs->state->posbyte + prs->state->charlen <= prs->lenstr);
-		Assert(prs->state->state >= TPS_Base && prs->state->state < TPS_Null);
+		Assert(prs->state->state < TPS_Null);
 		Assert(Actions[prs->state->state].state == prs->state->state);
 
 		if (prs->state->pushedAtAction)

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

Re: [HACKERS] s/size_t/off_t/ in sendTimeLineHistory

[Heikki Linnakangas]

On 15.01.2013 00:27, Andres Freund wrote:

When looking at the report of probable fallout from the elog stuff I
noticed this warning:

walsender.c(366): warning #186: pointless comparison of unsigned integer with 
zero

Which is accurate, the rather unlikely problem of a seek error could not
be noticed atm and would probably send strange stuff over the wire.


Thanks, applied.

- 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] Curious buildfarm failures

[Heikki Linnakangas]

On 15.01.2013 00:14, Heikki Linnakangas wrote:

On 14.01.2013 23:35, Tom Lane wrote:

Since commit 2065dd2834e832eb820f1fbcd16746d6af1f6037, there have been
a few buildfarm failures along the lines of

-- Commit table drop
COMMIT PREPARED 'regress-two';
! PANIC: failed to re-find shared proclock object
! PANIC: failed to re-find shared proclock object
! connection to server was lost

Evidently I bollixed something, but what? I've been unable to reproduce
this locally so far. Anybody see what's wrong?


I was able to reproduce this by setting max_locks_per_transaction and
max_connections to the minimum. My assumption is that there's something
wrong in the way hash_update_hash_key() handles collisions.


The problem seems to be when the the old and the key hash to the same 
bucket. In that case, hash_update_hash_key() tries to link the entry to 
itself. The attached patch fixes it for me.


- Heikki
*** a/src/backend/utils/hash/dynahash.c
--- b/src/backend/utils/hash/dynahash.c
***
*** 1022,1027  hash_update_hash_key(HTAB *hashp,
--- 1022,1028 
  	uint32		newhashvalue;
  	Size		keysize;
  	uint32		bucket;
+ 	uint32		newbucket;
  	long		segment_num;
  	long		segment_ndx;
  	HASHSEGMENT segp;
***
*** 1078,1087  hash_update_hash_key(HTAB *hashp,
  	 */
  	newhashvalue = hashp->hash(newKeyPtr, hashp->keysize);
  
! 	bucket = calc_bucket(hctl, newhashvalue);
! 
! 	segment_num = bucket >> hashp->sshift;
! 	segment_ndx = MOD(bucket, hashp->ssize);
  
  	segp = hashp->dir[segment_num];
  
--- 1079,1087 
  	 */
  	newhashvalue = hashp->hash(newKeyPtr, hashp->keysize);
  
! 	newbucket = calc_bucket(hctl, newhashvalue);
! 	segment_num = newbucket >> hashp->sshift;
! 	segment_ndx = MOD(newbucket, hashp->ssize);
  
  	segp = hashp->dir[segment_num];
  
***
*** 1115,1126  hash_update_hash_key(HTAB *hashp,
  
  	currBucket = existingElement;
  
! 	/* OK to remove record from old hash bucket's chain. */
! 	*oldPrevPtr = currBucket->link;
  
! 	/* link into new hashbucket chain */
! 	*prevBucketPtr = currBucket;
! 	currBucket->link = NULL;
  
  	/* copy new key into record */
  	currBucket->hashvalue = newhashvalue;
--- 1115,1129 
  
  	currBucket = existingElement;
  
! 	if (bucket != newbucket)
! 	{
! 		/* OK to remove record from old hash bucket's chain. */
! 		*oldPrevPtr = currBucket->link;
  
! 		/* link into new hashbucket chain */
! 		*prevBucketPtr = currBucket;
! 		currBucket->link = NULL;
! 	}
  
  	/* copy new key into record */
  	currBucket->hashvalue = newhashvalue;

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

Re: [HACKERS] recent ALTER whatever .. SET SCHEMA refactoring

[Alvaro Herrera]

Kohei KaiGai escribió:

> I'm probably saying same idea. It just adds invocation of external
> functions to check naming conflicts of functions or collation; that
> takes additional 4-lines for special case handling
> in AlterObjectNamespace_internal().

Okay, I can agree with this implementation plan.  I didn't like your
patch very much though; I'm not sure that the handling of collations was
sane.  And the names of the AlterFooNamespace_oid() functions is now
completely misleading, because they no longer do that.  So I renamed
them, and while at it I noticed that the collation case can share code
with the RENAME code path; the function case could probably do likewise,
but it becomes uglier.

Note that after this patch, the error message now cites the function
signature, not just the name.  This is more consistent with other cases.

9.2 and HEAD:
alvherre=# alter function foo.f(int,int) set schema bar;
ERROR:  function "f" already exists in schema "bar"
(this error is misleading, because there can validly be a f(int)
function in schema bar and that wouldn't cause a problem for this SET
SCHEMA command).

After my patch:
alvherre=# alter function foo.f(int,int) set schema bar;
ERROR:  function f(integer, integer) already exists in schema "bar"

-- 
Álvaro Herrerahttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
*** a/src/backend/commands/alter.c
--- b/src/backend/commands/alter.c
***
*** 19,27 
--- 19,29 
  #include "catalog/dependency.h"
  #include "catalog/indexing.h"
  #include "catalog/namespace.h"
+ #include "catalog/pg_collation.h"
  #include "catalog/pg_largeobject.h"
  #include "catalog/pg_largeobject_metadata.h"
  #include "catalog/pg_namespace.h"
+ #include "catalog/pg_proc.h"
  #include "commands/alter.h"
  #include "commands/collationcmds.h"
  #include "commands/conversioncmds.h"
***
*** 146,185  ExecAlterObjectSchemaStmt(AlterObjectSchemaStmt *stmt)
  {
  	switch (stmt->objectType)
  	{
- 		case OBJECT_AGGREGATE:
- 			return AlterFunctionNamespace(stmt->object, stmt->objarg, true,
- 		  stmt->newschema);
- 
- 		case OBJECT_COLLATION:
- 			return AlterCollationNamespace(stmt->object, stmt->newschema);
- 
  		case OBJECT_EXTENSION:
  			return AlterExtensionNamespace(stmt->object, stmt->newschema);
  
! 		case OBJECT_FUNCTION:
! 			return AlterFunctionNamespace(stmt->object, stmt->objarg, false,
! 		  stmt->newschema);
! 
  		case OBJECT_SEQUENCE:
  		case OBJECT_TABLE:
  		case OBJECT_VIEW:
- 		case OBJECT_FOREIGN_TABLE:
  			return AlterTableNamespace(stmt);
  
- 		case OBJECT_TYPE:
  		case OBJECT_DOMAIN:
  			return AlterTypeNamespace(stmt->object, stmt->newschema,
  	  stmt->objectType);
  
  			/* generic code path */
  		case OBJECT_CONVERSION:
  		case OBJECT_OPERATOR:
  		case OBJECT_OPCLASS:
  		case OBJECT_OPFAMILY:
! 		case OBJECT_TSPARSER:
  		case OBJECT_TSDICTIONARY:
  		case OBJECT_TSTEMPLATE:
- 		case OBJECT_TSCONFIGURATION:
  			{
  Relation	catalog;
  Relation	relation;
--- 148,179 
  {
  	switch (stmt->objectType)
  	{
  		case OBJECT_EXTENSION:
  			return AlterExtensionNamespace(stmt->object, stmt->newschema);
  
! 		case OBJECT_FOREIGN_TABLE:
  		case OBJECT_SEQUENCE:
  		case OBJECT_TABLE:
  		case OBJECT_VIEW:
  			return AlterTableNamespace(stmt);
  
  		case OBJECT_DOMAIN:
+ 		case OBJECT_TYPE:
  			return AlterTypeNamespace(stmt->object, stmt->newschema,
  	  stmt->objectType);
  
  			/* generic code path */
+ 		case OBJECT_AGGREGATE:
+ 		case OBJECT_COLLATION:
  		case OBJECT_CONVERSION:
+ 		case OBJECT_FUNCTION:
  		case OBJECT_OPERATOR:
  		case OBJECT_OPCLASS:
  		case OBJECT_OPFAMILY:
! 		case OBJECT_TSCONFIGURATION:
  		case OBJECT_TSDICTIONARY:
+ 		case OBJECT_TSPARSER:
  		case OBJECT_TSTEMPLATE:
  			{
  Relation	catalog;
  Relation	relation;
***
*** 253,270  AlterObjectNamespace_oid(Oid classId, Oid objid, Oid nspOid,
  break;
  			}
  
- 		case OCLASS_PROC:
- 			oldNspOid = AlterFunctionNamespace_oid(objid, nspOid);
- 			break;
- 
  		case OCLASS_TYPE:
  			oldNspOid = AlterTypeNamespace_oid(objid, nspOid, objsMoved);
  			break;
  
  		case OCLASS_COLLATION:
- 			oldNspOid = AlterCollationNamespace_oid(objid, nspOid);
- 			break;
- 
  		case OCLASS_CONVERSION:
  		case OCLASS_OPERATOR:
  		case OCLASS_OPCLASS:
--- 247,258 
  break;
  			}
  
  		case OCLASS_TYPE:
  			oldNspOid = AlterTypeNamespace_oid(objid, nspOid, objsMoved);
  			break;
  
+ 		case OCLASS_PROC:
  		case OCLASS_COLLATION:
  		case OCLASS_CONVERSION:
  		case OCLASS_OPERATOR:
  		case OCLASS_OPCLASS:
***
*** 380,385  AlterObjectNamespace_internal(Relation rel, Oid objid, Oid nspOid)
--- 368,385 
   errmsg("%s already exists in schema \"%s\"",
  		getObjectDescriptionOids(classId, objid),
  		get_namespace_name(nspOid;
+ 	else if (classId == ProcedureRelationId)
+ 		IsThereFunctionInNames

[HACKERS] s/size_t/off_t/ in sendTimeLineHistory

[Andres Freund]

Hi,

When looking at the report of probable fallout from the elog stuff I
noticed this warning:

walsender.c(366): warning #186: pointless comparison of unsigned integer with 
zero

Which is accurate, the rather unlikely problem of a seek error could not
be noticed atm and would probably send strange stuff over the wire.

Greetings,

Andres Freund

-- 
 Andres Freund http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services
diff --git a/src/backend/replication/walsender.c b/src/backend/replication/walsender.c
index 5408b14..e4700da 100644
--- a/src/backend/replication/walsender.c
+++ b/src/backend/replication/walsender.c
@@ -315,8 +315,8 @@ SendTimeLineHistory(TimeLineHistoryCmd *cmd)
 	char		histfname[MAXFNAMELEN];
 	char		path[MAXPGPATH];
 	int			fd;
-	size_t		histfilelen;
-	size_t		bytesleft;
+	off_t		histfilelen;
+	off_t		bytesleft;
 
 	/*
 	 * Reply with a result set with one row, and two columns. The first col

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

Re: [HACKERS] Curious buildfarm failures

[Heikki Linnakangas]

On 14.01.2013 23:35, Tom Lane wrote:

Since commit 2065dd2834e832eb820f1fbcd16746d6af1f6037, there have been
a few buildfarm failures along the lines of

   -- Commit table drop
   COMMIT PREPARED 'regress-two';
! PANIC:  failed to re-find shared proclock object
! PANIC:  failed to re-find shared proclock object
! connection to server was lost

Evidently I bollixed something, but what?  I've been unable to reproduce
this locally so far.  Anybody see what's wrong?


I was able to reproduce this by setting max_locks_per_transaction and 
max_connections to the minimum. My assumption is that there's something 
wrong in the way hash_update_hash_key() handles collisions.


- 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] Curious buildfarm failures

[Andres Freund]

On 2013-01-14 22:50:16 +0100, Andres Freund wrote:
> On 2013-01-14 16:35:48 -0500, Tom Lane wrote:
> > Since commit 2065dd2834e832eb820f1fbcd16746d6af1f6037, there have been
> > a few buildfarm failures along the lines of
> >
> >   -- Commit table drop
> >   COMMIT PREPARED 'regress-two';
> > ! PANIC:  failed to re-find shared proclock object
> > ! PANIC:  failed to re-find shared proclock object
> > ! connection to server was lost
> >
> > Evidently I bollixed something, but what?  I've been unable to reproduce
> > this locally so far.  Anybody see what's wrong?
> >
> > Another thing is that dugong has been reproducibly failing with
> >
> >  drop cascades to table testschema.atable
> >   -- Should succeed
> >   DROP TABLESPACE testspace;
> > + ERROR:  tablespace "testspace" is not empty
> >
> > since the elog-doesn't-return patch (b853eb97) went in.  Maybe this is
> > some local problem there but I'm suspicious that there's a connection.
> > But what?
> >
> > Any insights out there?
>
> It also has:
>
> FATAL:  could not open file "base/16384/28182": No such file or directory
> CONTEXT:  writing block 6 of relation base/16384/28182
> TRAP: FailedAssertion("!(PrivateRefCount[i] == 0)", File: "bufmgr.c", Line: 
> 1743)

> #3  0x40b4b510 in ExceptionalCondition (
> conditionName=0x40d76390 "!(PrivateRefCount[i] == 0)",
> errorType=0x40d06500 "FailedAssertion",
> fileName=0x40d76260 "bufmgr.c", lineNumber=1743) at assert.c:54
> #4  0x407a7d20 in AtProcExit_Buffers (code=1, arg=59) at bufmgr.c:1743
> #5  0x407c4e50 in shmem_exit (code=1) at ipc.c:221
>
> in the log. So it seems like it also could be related to locking
> changes although I don't immediately see why.

No such "luck" though, the animal only applied the elog commits:
http://buildfarm.postgresql.org/cgi-bin/show_stage_log.pl?nm=dugong&dt=2013-01-14%2000%3A00%3A02&stg=SCM-checkout

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] COPY .. COMPRESSED

[Peter Eisentraut]

On 1/14/13 11:28 AM, Stephen Frost wrote:
> While there is no option currently for having the server do the
> compression before sending the data over the wire.

OpenSSL?


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

Re: [HACKERS] Curious buildfarm failures

[Andres Freund]

On 2013-01-14 16:35:48 -0500, Tom Lane wrote:
> Since commit 2065dd2834e832eb820f1fbcd16746d6af1f6037, there have been
> a few buildfarm failures along the lines of
>   
>   -- Commit table drop
>   COMMIT PREPARED 'regress-two';
> ! PANIC:  failed to re-find shared proclock object
> ! PANIC:  failed to re-find shared proclock object
> ! connection to server was lost
> 
> Evidently I bollixed something, but what?  I've been unable to reproduce
> this locally so far.  Anybody see what's wrong?
> 
> Another thing is that dugong has been reproducibly failing with
> 
>  drop cascades to table testschema.atable
>   -- Should succeed
>   DROP TABLESPACE testspace;
> + ERROR:  tablespace "testspace" is not empty
> 
> since the elog-doesn't-return patch (b853eb97) went in.  Maybe this is
> some local problem there but I'm suspicious that there's a connection.
> But what?
> 
> Any insights out there?

It also has:

LOG:  received fast shutdown request
LOG:  aborting any active transactions
LOG:  autovacuum launcher shutting down
LOG:  shutting down
FATAL:  could not open file "base/16384/28182": No such file or directory
CONTEXT:  writing block 6 of relation base/16384/28182
TRAP: FailedAssertion("!(PrivateRefCount[i] == 0)", File: "bufmgr.c", Line: 
1743)
LOG:  checkpointer process (PID 30366) was terminated by signal 6: Aborted
LOG:  terminating any other active server processes
LOG:  abnormal database system shutdown


== stack trace: pgsql.9958/src/test/regress/tmp_check/data/core 
==
Using host libthread_db library "/lib/tls/libthread_db.so.1".

warning: Can't read pathname for load map: Input/output error.
Core was generated by `postgres: checkpointer process   
 '.
Program terminated with signal 6, Aborted.

#0  0xa0010620 in __kernel_syscall_via_break ()
#0  0xa0010620 in __kernel_syscall_via_break ()
#1  0x20953bb0 in raise () from /lib/tls/libc.so.6.1
#2  0x20956df0 in abort () from /lib/tls/libc.so.6.1
#3  0x40b4b510 in ExceptionalCondition (
conditionName=0x40d76390 "!(PrivateRefCount[i] == 0)", 
errorType=0x40d06500 "FailedAssertion", 
fileName=0x40d76260 "bufmgr.c", lineNumber=1743) at assert.c:54
#4  0x407a7d20 in AtProcExit_Buffers (code=1, arg=59) at bufmgr.c:1743
#5  0x407c4e50 in shmem_exit (code=1) at ipc.c:221
#6  0x407c4fa0 in proc_exit_prepare (code=1) at ipc.c:181
#7  0x407c4ab0 in proc_exit (code=1) at ipc.c:96
#8  0x40b5d390 in errfinish (dummy=0) at elog.c:518
#9  0x40823380 in _mdfd_getseg (reln=0x60155420, 
forknum=1397792, blkno=6, skipFsync=0 '\0', behavior=EXTENSION_FAIL)
at md.c:577
#10 0x4081e5c0 in mdwrite (reln=0x60155420, 
forknum=MAIN_FORKNUM, blocknum=6, buffer=0x21432ea0 "", 
skipFsync=0 '\0') at md.c:735
#11 0x40824690 in smgrwrite (reln=0x60155420, 
forknum=MAIN_FORKNUM, blocknum=6, buffer=0x21432ea0 "", 
skipFsync=0 '\0') at smgr.c:534
#12 0x4079e510 in FlushBuffer (buf=0x1, reln=0x60155420)
at bufmgr.c:1941
#13 0x407a10b0 in SyncOneBuffer (buf_id=0, skip_recently_used=0 '\0')
at bufmgr.c:1677
#14 0x407a0c00 in CheckPointBuffers (flags=5) at bufmgr.c:1284
#15 0x401fcbf0 in CheckPointGuts (checkPointRedo=80827000, flags=5)
at xlog.c:7391
#16 0x401fb2a0 in CreateCheckPoint (flags=5) at xlog.c:7240
#17 0x401f6820 in ShutdownXLOG (code=14699520, 
arg=4611686018440093920) at xlog.c:6823
#18 0x4072d780 in _setjmp_lpad_CheckpointerMain_0$0$18 ()
at checkpointer.c:413
#19 0x40235810 in AuxiliaryProcessMain (argc=496536, 
argv=0x6f80e520) at bootstrap.c:433
#20 0x407172b0 in StartChildProcess (type=508288) at postmaster.c:4956
#21 0x40713f50 in reaper (postgres_signal_arg=30365)
at postmaster.c:2568
#22 
#23 0xa0010620 in __kernel_syscall_via_break ()
#24 0x20953f70 in sigprocmask () from /lib/tls/libc.so.6.1
#25 0x40720480 in ServerLoop () at postmaster.c:1521
#26 0x4071d9d0 in PostmasterMain (argc=6, argv=0x600d85e0)
at postmaster.c:1244
#27 0x40577a30 in main (argc=6, argv=0x600d8010) at main.c:197

in the log. So it seems like it also could be related to locking
changes although I don't immediately see why.

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] Curious buildfarm failures

[Tom Lane]

Since commit 2065dd2834e832eb820f1fbcd16746d6af1f6037, there have been
a few buildfarm failures along the lines of
  
  -- Commit table drop
  COMMIT PREPARED 'regress-two';
! PANIC:  failed to re-find shared proclock object
! PANIC:  failed to re-find shared proclock object
! connection to server was lost

Evidently I bollixed something, but what?  I've been unable to reproduce
this locally so far.  Anybody see what's wrong?

Another thing is that dugong has been reproducibly failing with

 drop cascades to table testschema.atable
  -- Should succeed
  DROP TABLESPACE testspace;
+ ERROR:  tablespace "testspace" is not empty

since the elog-doesn't-return patch (b853eb97) went in.  Maybe this is
some local problem there but I'm suspicious that there's a connection.
But what?

Any insights out there?

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] count(*) of zero rows returns 1

[Tom Lane]

David Johnston  writes:
> Tom Lane-2 wrote
>> For that to return zero, it would also be necessary for "SELECT 2+2"
>> to return zero rows.  Which would be consistent with some views of the
>> universe, but not particularly useful.

> Given that:

> SELECT *;
> Results in: 
> SQL Error: ERROR:  SELECT * with no tables specified is not valid

That has nothing to do with the number of rows, though.  That's
complaining that there are no columns for the * to refer to.
(Note that "count(*)" is an unrelated idiom -- the * there really has
nothing to do with its usage in SELECT *.)

> I get that the horse has already left the barn on this one but neither "0"
> nor "1" seem particularly sound answers to the question "SELECT count(*)".

Yeah, it's more about convenience than principle.  AFAICS there are three
defensible answers to what an omitted FROM clause ought to mean:

1. It's not legal (the SQL spec's answer).
2. It implicitly means a table of no columns and 1 row (PG's answer).
3. It implicitly means a table of no columns and 0 rows (which is what
   I take Gurjeet to be advocating for).

Only #2 allows the "SELECT " idiom to do anything useful.
But once you accept that, the behaviors of the aggregates fall out of
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] [Pgbuildfarm-members] Version 4.10 of buildfarm client released.

[Kevin Grittner]

Andrew Dunstan wrote:

>> For Linux, perhaps some form of lsof with the +D option?

> This actually won't help. In most cases the relevant data directory has 
> long disappeared out from under the rogue postmaster as part of 
> buildfarm cleanup. Also, lsof is not universally available. We try to 
> avoid creating new dependencies if possible.

Well, I did say "for Linux" and the reason I suggested lsof is that
it does show deleted files which are being held open (or in the
suggested command, the pids of processes holding open files in or
under the requested directory). However, if you want a solution
which works for all OSs, lsof obviously doesn't do the job; and if
the directory itself is deleted, +D doesn't help -- you would need
to grep the full output

Anyway, I guess we don't really need to do anything anyway, so the
point is moot.

-Kevin


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

Re: [HACKERS] fix SQL example syntax in file comment

[Tom Lane]

=?UTF-8?B?SmFuIFVyYmHFhHNraQ==?=  writes:
> Here's a trivial patch that fixes a comment in execProcNode.c

Applied, thanks.

> For archeological interest, that comment dates back to when it was 
> written in POSTQUEL... The cleanup in 
> a9b1ff4c1d699c8aa615397d47bb3071275c64ef changed RETRIEVE to SELECT, but 
> forgot to add a FROM clause :)

Actually, that was perfectly legal back then.

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] Timing events WIP v1

[Greg Smith]

On 1/14/13 11:19 AM, Peter Geoghegan wrote:

I noticed that when !log_checkpoints, control never reaches the site
where the hook is called, and thus the checkpoint info is not stored.
Is that the intended behaviour of the patch?


I was aware and considered it a defensible situation--that turning off 
checkpoint logging takes that data out everywhere.  But it was not 
necessarily the right thing to do.



Currently, explain.c
manages to generate JSON representations of plans with very little
fuss, and without using any of the JSON datatype stuff. Doing this
with hstore is just way too controversial, and not particularly worth
it, IMHO.


I wasn't optimistic after seeing the number of bugs that scurried out 
when the hstore rock was turned over for this job.  On the 
implementation side, the next round of code I've been playing with here 
has struggled with the problem of rendering to strings earlier than I'd 
like.  I'd like to delay that as long as possible; certainly not do it 
during storage, and preferably it only happens when someone asks for the 
timing event.


> With a "where event_type = x" in the query predicate, the
> JSON datums would have predictable, consistent structure, facilitating
> machine reading and aggregation.

Filtering on a range of timestamps or on the serial number field is the 
main thing that I imagined, as something that should limit results 
before even producing tuples.  The expected and important case where 
someone wants "all timing events after #123" after persisting #123 to 
disk, I'd like that to be efficient.  All of the fields I'd want to see 
filtering on are part of the fixed set of columns every entry will have.


To summarize, your suggestion is to build an in-memory structure capable 
of holding the timing event data.  The Datum approach will be used to 
cope with different types of events having different underlying types. 
The output format for queries against this data set will be JSON, 
rendered directly from the structure similarly to how EXPLAIN (FORMAT 
JSON) outputs query trees.  The columns every row contains, like a 
serial number, timestamp, and pid, can be filtered on by something 
operating at the query executor level.  Doing something useful with the 
more generic, "dynamic schema" parts will likely require parsing the 
JSON output.


Those are all great ideas I think people could live with.

It looks to me like the hook definition itself would need the entire 
data structure defined, and known to work, before its API could be 
nailed down.  I was hoping that we might get a hook for diverting this 
data committed into 9.3 even if the full extension to expose it wasn't 
nailed down.  That was based on similarity to the generic logging hook 
that went into 9.2.  This new implementation idea reminds me more of the 
query length decoration needed for normalized pg_stat_statements 
though--something that wasn't easy to just extract out from the consumer 
at all.


--
Greg Smith   2ndQuadrant USg...@2ndquadrant.com   Baltimore, MD
PostgreSQL Training, Services, and 24x7 Support www.2ndQuadrant.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] Validation in to_date()

[Tom Lane]

Hitoshi Harada  writes:
> to_date() doesn't check the date range, which results in unreadable
> data like this.

> foo=# create table t as select to_date('-12-10 BC', '-MM-DD
> BC')::timestamp;
> SELECT 1
> foo=# table t;
> ERROR:  timestamp out of range

> Attached is to add IS_VALID_JULIAN() as we do like in date_in().  I
> think to_date() should follow it because it is the entrance place to
> check sanity.

This looks like a good idea, will commit.

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] count(*) of zero rows returns 1

[David Johnston]

Tom Lane-2 wrote
> Gurjeet Singh <

> singh.gurjeet@

> > writes:
>> Can somebody explain why a standalone count(*) returns 1?
>> postgres=# select count(*);
>>  count
>> ---
>>  1
>> (1 row)
> 
> The Oracle equivalent of that would be "SELECT count(*) FROM dual".
> Does it make more sense to you thought of that way?
> 
>> I agree it's an odd thing for someone to query, but I feel it should
>> return
>> 0, and not 1.
> 
> For that to return zero, it would also be necessary for "SELECT 2+2"
> to return zero rows.  Which would be consistent with some views of the
> universe, but not particularly useful.  Another counterexample is
> 
> regression=# select sum(42);
>  sum 
> -
>   42
> (1 row)
> 
> which by your argument would need to return NULL, since that would be
> SUM's result over zero rows.

Given that:

SELECT *;

Results in: 

SQL Error: ERROR:  SELECT * with no tables specified is not valid

then an aggregate over an error should not magically cause the error to go
away.

I am curious on some points:

Is there something in the standard that makes "SELECT count(*)" valid?
What does "SELECT * FROM dual" in Oracle yield?
Is there a meaningful use case for "SELECT sum(42)", or more specifically
any aggregate query where there are no table/value inputs?  

I get the "SELECT 2+2" and its ilk as there needs to be some way to evaluate
constants.

I get that the horse has already left the barn on this one but neither "0"
nor "1" seem particularly sound answers to the question "SELECT count(*)".

David J.





--
View this message in context: 
http://postgresql.1045698.n5.nabble.com/count-of-zero-rows-returns-1-tp5739973p5740160.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] Hash twice

[Simon Riggs]

On 14 January 2013 19:12, Robert Haas  wrote:
> On Sat, Jan 12, 2013 at 11:39 AM, Simon Riggs  wrote:
>> Lock code says it calculates "hash value once and then pass around as 
>> needed".
>>
>> But it actually calculates it twice for new locks.
>>
>> Trivial patch attached to make it follow the comments in
>> LockTagHashCode and save a few cycles.
>
> Hmm.  This is a nice idea, but it doesn't look right to me, because
> you're searching LockMethodLocalHash with a hash code intended to be
> used in LockMethodLockHash, and the two hashing schemes are not
> compatible, because the former is hashing a LOCALLOCKTAG, and the
> latter is hashing a LOCKTAG, and both are just using tag_hash.

You're right. At local level we need to refcount requests, whereas we
only ever pass first request through to main table. That means the
unique key is different.

> On the flip side if I'm wrong and the hashing schemes are compatible,
> there are other places in the file where the same trick could be
> employed.

But having said that, we already make ProcLockHash use a variation of
the LockHash to avoid recalculation.

So we should just make
 LocalLockTagHashCode = LockTagHashCode() + mode;

Then we can use LockTagHashCode everywhere, which is easier to do than
documenting why we don't.


Anyway, just an idle thought while looking into something else.

-- 
 Simon Riggs   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] Hash twice

[Robert Haas]

On Sat, Jan 12, 2013 at 11:39 AM, Simon Riggs  wrote:
> Lock code says it calculates "hash value once and then pass around as needed".
>
> But it actually calculates it twice for new locks.
>
> Trivial patch attached to make it follow the comments in
> LockTagHashCode and save a few cycles.

Hmm.  This is a nice idea, but it doesn't look right to me, because
you're searching LockMethodLocalHash with a hash code intended to be
used in LockMethodLockHash, and the two hashing schemes are not
compatible, because the former is hashing a LOCALLOCKTAG, and the
latter is hashing a LOCKTAG, and both are just using tag_hash.

On the flip side if I'm wrong and the hashing schemes are compatible,
there are other places in the file where the same trick could be
employed.

-- 
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] erroneous restore into pg_catalog schema

[Alvaro Herrera]

Tom Lane escribió:
> Alvaro Herrera  writes:

> > alvherre=# create extension adminpack;
> > ERROR:  permission denied for schema pg_catalog
> 
> Um.  I knew that that module's desire to shove stuff into pg_catalog
> would bite us someday.  But now that I think about it, I'm pretty sure
> I recall discussions to the effect that there are other third-party
> modules doing similar things.

How about we provide a superuser-only function that an extension can
call which will set enableSystemTableMods?  It would get back
automatically to the default value on transaction end.  That way,
extensions that wish to install stuff in pg_catalog can explicitely
declare it, i, and the rest of the world enjoys consistent protection.

-- 
Á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] Re: Privileges for INFORMATION_SCHEMA.SCHEMATA (was Re: [DOCS] Small clarification in "34.41. schemata")

[Tom Lane]

Casey Allen Shobe  writes:
> On Wed, Jan 9, 2013 at 8:56 PM, Tom Lane  wrote:
>> However, it seems to me that this behavior is actually wrong for our
>> purposes, as it represents a too-literal reading of the spec.  The SQL
>> standard has no concept of privileges on schemas, only ownership.
>> We do have privileges on schemas, so it seems to me that the consistent
>> thing would be for this view to show any schema that you either own or
>> have some privilege on.

> IMHO, schemata should follow the standard as it does today.  Other
> platforms have privileges on schemas as well, and this sort of thing seems
> to fall into the same bucket as other platform compatibilities outside the
> scope of what the standard thinks about, which means you use pg_catalog to
> access that information rather than information_schema, which should be
> expected to work consistently on all platforms that implement it.

Meh.  To me, standards compliance requires that if you have created a
SQL-compliant database, you'd better see spec-compliant output from the
information schema.  As soon as you do something outside the standard
(in this instance, grant some privileges on a schema), it becomes a
judgment call whether and how that should affect what you see in the
information schema.

It may be that the current behavior of this view is actually the best
thing, but a standards-compliance argument doesn't do anything to
convince 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] bugfix: --echo-hidden is not supported by \sf statements

[Tom Lane]

Robert Haas  writes:
> On Mon, Jan 14, 2013 at 11:56 AM, Tom Lane  wrote:
>> So far as I can tell, get_create_function_cmd (and lookup_function_oid
>> too) were intentionally designed to not show their queries, and for that
>> matter they go out of their way to produce terse error output if they
>> fail.  I'm not sure why we should revisit that choice.  In any case
>> it seems silly to change one and not the other.

> Agreed on the second point, but I think I worked on that patch, and I
> don't think that was intentional on my part.  You worked on it, too,
> IIRC, so I guess you'll have to comment on your intentions.

> Personally I think -E is one of psql's finer features, so +1 from me
> for making it apply across the board.

Well, fine, but then it should fix both of them and remove
minimal_error_message altogether.  I would however suggest eyeballing
what happens when you try "\ef nosuchfunction" (with or without -E).
I'm pretty sure that the reason for the special error handling in these
functions is that the default error reporting was unpleasant for this
use-case.

regards, tom lane


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

Re: [HACKERS] [PERFORM] Slow query: bitmap scan troubles

[Tom Lane]

Robert Haas  writes:
> On Mon, Jan 14, 2013 at 12:23 PM, Tom Lane  wrote:
>> The old code definitely had an unreasonably large charge for indexes
>> exceeding 1e8 or so tuples.  This wouldn't matter that much for simple
>> single-table lookup queries, but I could easily see it putting the
>> kibosh on uses of an index on the inside of a nestloop.

> The reported behavior was that the planner would prefer to
> sequential-scan the table rather than use the index, even if
> enable_seqscan=off.  I'm not sure what the query looked like, but it
> could have been something best implemented as a nested loop w/inner
> index-scan.

Remember also that "enable_seqscan=off" merely adds 1e10 to the
estimated cost of seqscans.  For sufficiently large tables this is not
exactly a hard disable, just a thumb on the scales.  But I don't know
what your definition of "extremely large indexes" is.

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] json api WIP patch

[Andrew Dunstan]

On 01/14/2013 11:32 AM, Robert Haas wrote:


So, how much performance does this lose on json_in() on a large
cstring, as compared with master?


That's a good question. I'll try to devise a test.



I can't shake the feeling that this is adding a LOT of unnecessary
data copying.  For one thing, instead of copying every single lexeme
(including the single-character ones?) out of the original object, we
could just store a pointer to the offset where the object starts and a
length, instead of copying it.


In the pure pares case (json_in, json_reccv) nothing extra should be 
copied. On checking this after reading the above I found that wasn't 
quite the case, and some lexemes (scalars and field names, but not 
punctuation) were being copied when not needed. I have made a fix (see 
) 
which I will include in the next version I publish.


In the case of string lexemes, we are passing back a de-escaped version, 
so just handing back pointers to the beginning and end in the input 
string doesn't work.




This is also remarkably thin on comments.


Fair criticism. I'll work on that.

Thanks for looking at this.

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] bugfix: --echo-hidden is not supported by \sf statements

[Robert Haas]

On Mon, Jan 14, 2013 at 11:56 AM, Tom Lane  wrote:
> So far as I can tell, get_create_function_cmd (and lookup_function_oid
> too) were intentionally designed to not show their queries, and for that
> matter they go out of their way to produce terse error output if they
> fail.  I'm not sure why we should revisit that choice.  In any case
> it seems silly to change one and not the other.

Agreed on the second point, but I think I worked on that patch, and I
don't think that was intentional on my part.  You worked on it, too,
IIRC, so I guess you'll have to comment on your intentions.

Personally I think -E is one of psql's finer features, so +1 from me
for making it apply across the board.

-- 
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] [PERFORM] Slow query: bitmap scan troubles

[Robert Haas]

On Mon, Jan 14, 2013 at 12:23 PM, Tom Lane  wrote:
> Robert Haas  writes:
>> I'm not sure I have anything intelligent to add to this conversation -
>> does that make me the wisest of all the Greeks? - but I do think it
>> worth mentioning that I have heard occasional reports within EDB of
>> the query planner refusing to use extremely large indexes no matter
>> how large a hammer was applied.  I have never been able to obtain
>> enough details to understand the parameters of the problem, let alone
>> reproduce it, but I thought it might be worth mentioning anyway in
>> case it's both real and related to the case at hand.  Basically I
>> guess that boils down to: it would be good to consider whether the
>> costing model is correct for an index of, say, 1TB.
>
> Well, see the cost curves at
> http://www.postgresql.org/message-id/13967.1357866...@sss.pgh.pa.us
>
> The old code definitely had an unreasonably large charge for indexes
> exceeding 1e8 or so tuples.  This wouldn't matter that much for simple
> single-table lookup queries, but I could easily see it putting the
> kibosh on uses of an index on the inside of a nestloop.

The reported behavior was that the planner would prefer to
sequential-scan the table rather than use the index, even if
enable_seqscan=off.  I'm not sure what the query looked like, but it
could have been something best implemented as a nested loop w/inner
index-scan.

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

[HACKERS] Re: Privileges for INFORMATION_SCHEMA.SCHEMATA (was Re: [DOCS] Small clarification in "34.41. schemata")

[Casey Allen Shobe]

On Wed, Jan 9, 2013 at 8:56 PM, Tom Lane  wrote:

> However, it seems to me that this behavior is actually wrong for our
> purposes, as it represents a too-literal reading of the spec.  The SQL
> standard has no concept of privileges on schemas, only ownership.
> We do have privileges on schemas, so it seems to me that the consistent
> thing would be for this view to show any schema that you either own or
> have some privilege on.


IMHO, schemata should follow the standard as it does today.  Other
platforms have privileges on schemas as well, and this sort of thing seems
to fall into the same bucket as other platform compatibilities outside the
scope of what the standard thinks about, which means you use pg_catalog to
access that information rather than information_schema, which should be
expected to work consistently on all platforms that implement it.

-- 
Casey Allen Shobe
ca...@shobe.info

Re: [HACKERS] erroneous restore into pg_catalog schema

[Dimitri Fontaine]

Tom Lane  writes:
> Um.  I knew that that module's desire to shove stuff into pg_catalog
> would bite us someday.  But now that I think about it, I'm pretty sure
> I recall discussions to the effect that there are other third-party
> modules doing similar things.

Yes, and some more have been starting to do that now that they have
proper DROP EXTENSION support to clean themselves up. At least that's
what I think the reason for doing so is…


-- 
Dimitri Fontaine
http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support


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

Re: [HACKERS] erroneous restore into pg_catalog schema

[Tom Lane]

Alvaro Herrera  writes:
> The attached patch seems to work:

> alvherre=# create table pg_catalog.foo (a int);
> ERROR:  permission denied for schema pg_catalog

> I notice that contrib/adminpack now fails, though (why doesn't this
> module have a regression test?):

> alvherre=# create extension adminpack;
> ERROR:  permission denied for schema pg_catalog

Um.  I knew that that module's desire to shove stuff into pg_catalog
would bite us someday.  But now that I think about it, I'm pretty sure
I recall discussions to the effect that there are other third-party
modules doing similar things.

Anyway, this seems to answer Robert's original question about why
relations were special-cased: there are too many special cases around
this behavior.  I think we should seriously consider just reverting
a475c6036.

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] [PERFORM] Slow query: bitmap scan troubles

[Tom Lane]

Robert Haas  writes:
> I'm not sure I have anything intelligent to add to this conversation -
> does that make me the wisest of all the Greeks? - but I do think it
> worth mentioning that I have heard occasional reports within EDB of
> the query planner refusing to use extremely large indexes no matter
> how large a hammer was applied.  I have never been able to obtain
> enough details to understand the parameters of the problem, let alone
> reproduce it, but I thought it might be worth mentioning anyway in
> case it's both real and related to the case at hand.  Basically I
> guess that boils down to: it would be good to consider whether the
> costing model is correct for an index of, say, 1TB.

Well, see the cost curves at
http://www.postgresql.org/message-id/13967.1357866...@sss.pgh.pa.us

The old code definitely had an unreasonably large charge for indexes
exceeding 1e8 or so tuples.  This wouldn't matter that much for simple
single-table lookup queries, but I could easily see it putting the
kibosh on uses of an index on the inside of a nestloop.

It's possible that the new code goes too far in the other direction:
we're now effectively assuming that all inner btree pages stay in cache
no matter how large the index is.  At some point it'd likely be
appropriate to start throwing in some random_page_cost charges for inner
pages beyond the third/fourth/fifth(?) level, as Simon speculated about
upthread.  But I thought we could let that go until we start seeing
complaints traceable to 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] PL/perl should fail on configure, not make

[Tom Lane]

pgbuildf...@jdrake.com writes:
> On Thu, 10 Jan 2013, Andrew Dunstan wrote:
>> Without access to the machine it's pretty hard to know, so I was just
>> speculating fairly wildly. If Jeremy can find out what the problem is that
>> would be good, or if he can give us access to the machine it shouldn't be
>> terribly hard to get to the bottom.

> I'll see what I can do.  For now, I can tell you that it works with GCC
> and fails with ICC.

BTW, just to close out this thread on the public list --- Andrew found
out that the problem goes away if one removes "-parallel" from the list
of compiler options.  That makes it a pretty blatant compiler bug, and
one that we've not seen elsewhere, so it's probably specific to the
(old) icc version that Jeremy is running on this buildfarm animal.

Given that no other buildfarm members are showing a problem, I don't
intend to revert the configure change.  Jeremy should update his icc
or remove the -parallel switch on that animal.

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] bugfix: --echo-hidden is not supported by \sf statements

[Pavel Stehule]

2013/1/14 Tom Lane :
> Pavel Stehule  writes:
>> this is very simple patch -  it enables hidden_queries  for commands
>> \sf and \ef to be consistent with other describing commands.
>
> So far as I can tell, get_create_function_cmd (and lookup_function_oid
> too) were intentionally designed to not show their queries, and for that
> matter they go out of their way to produce terse error output if they
> fail.  I'm not sure why we should revisit that choice.  In any case
> it seems silly to change one and not the other.

Motivation for this patch is consistency with other backslash
statements. Some people uses --echo-hidden option like "tutorial" or
"navigation" over system tables or builtin functionality.  With this
patch these people should be navigated to function pg_get_functiondef.
There are no other motivation - jut it should to help to users that
has no necessary knowledges look to psql source code.

I am not sure about --echo-hidden option - if it is limited just to
system tables or  complete functionality. Probably both designs are
valid. So first we should to decide if this behave is bug or not. I am
co-author \sf and I didn't calculate with  --echo-hidden options. Now
I am inclined so it is bug. This bug is not significant - it is just
one detail - but for some who will work with psql this can be pleasant
feature if psql will be consistent in all.

After decision I can recheck this patch and enhance it for all statements.

Regards

Pavel

>
> A purely stylistic gripe is that you have get_create_function_cmd taking
> a PGconn parameter that now has nothing to do with its behavior.
>



> regards, tom lane


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

Re: [HACKERS] pg_retainxlog for inclusion in 9.3?

[Robert Haas]

On Fri, Jan 4, 2013 at 4:55 PM, Dimitri Fontaine  wrote:
> Robert Haas  writes:
>> Mostly that it seems like a hack, and I suspect we may come up with a
>> better way to do this in the future.
>
> Do you have the specs of such better way? Would it be a problem to have
> both pg_retainxlog and the new way?

Well, I think in the long term we are likely to want the master to
have some kind of ability to track the positions of its slaves, even
when they are disconnected.  And, optionally, to retain the WAL that
they need, again even when they are disconnected.  If such an ability
materializes, this will be moot (even as I think that pg_standby is
now largely moot, at least for new installations, now that we have
standby_mode=on).

-- 
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] bugfix: --echo-hidden is not supported by \sf statements

[Tom Lane]

Pavel Stehule  writes:
> this is very simple patch -  it enables hidden_queries  for commands
> \sf and \ef to be consistent with other describing commands.

So far as I can tell, get_create_function_cmd (and lookup_function_oid
too) were intentionally designed to not show their queries, and for that
matter they go out of their way to produce terse error output if they
fail.  I'm not sure why we should revisit that choice.  In any case
it seems silly to change one and not the other.

A purely stylistic gripe is that you have get_create_function_cmd taking
a PGconn parameter that now has nothing to do with its behavior.

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] erroneous restore into pg_catalog schema

[Alvaro Herrera]

Tom Lane wrote:
> Alvaro Herrera  writes:
> > Tom Lane escribi�:
> >> I will bet that this is more breakage from the DDL-code refactoring that
> >> has been going on.  I am getting closer and closer to wanting that
> >> reverted.  KaiGai-san seems to have been throwing out lots of special
> >> cases that were there for good reasons.
> 
> > Isn't this just a475c6036?
> 
> Ah ... well, at least it was intentional.  But still wrongheaded,
> as this example shows.  What we should have done was what the commit
> message suggests, ie place a replacement check somewhere "upstream"
> where it would apply to all object types.  First thought that comes to
> mind is to add a hack to pg_namespace_aclcheck, or maybe at some call
> site(s).

The attached patch seems to work:

alvherre=# create table pg_catalog.foo (a int);
ERROR:  permission denied for schema pg_catalog

It passes regression tests for both core and contrib.

I notice that contrib/adminpack now fails, though (why doesn't this
module have a regression test?):

alvherre=# create extension adminpack;
ERROR:  permission denied for schema pg_catalog

It sounds hard to support that without some other special hack.  Not
sure what to do here.  Have adminpack set allowSystemTableMods somehow?

I grepped for other occurences of "pg_catalog" in contrib SQL scripts,
and all other modules seem to work (didn't try sepgsql):

$ rgrep -l pg_catalog */*sql  | cut -d/ -f1 | while read module; do echo 
module: $module; psql -c "create extension $module"; done

module: adminpack
ERROR:  permission denied for schema pg_catalog
module: btree_gist
CREATE EXTENSION
module: btree_gist
ERROR:  extension "btree_gist" already exists
module: citext
CREATE EXTENSION
module: citext
ERROR:  extension "citext" already exists
module: intarray
CREATE EXTENSION
module: isn
CREATE EXTENSION
module: lo
CREATE EXTENSION
module: pg_trgm
CREATE EXTENSION
module: pg_trgm
ERROR:  extension "pg_trgm" already exists
module: sepgsql
ERROR:  could not open extension control file
"/home/alvherre/Code/pgsql/install/HEAD/share/extension/sepgsql.control":
No such file or directory
module: tcn
CREATE EXTENSION
module: test_parser
CREATE EXTENSION
module: tsearch2
CREATE EXTENSION
module: tsearch2
ERROR:  extension "tsearch2" already exists

-- 
Alvaro Herrerahttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
diff --git a/src/backend/catalog/aclchk.c b/src/backend/catalog/aclchk.c
index 0bf5356..3738cf5 100644
--- a/src/backend/catalog/aclchk.c
+++ b/src/backend/catalog/aclchk.c
@@ -4445,6 +4445,11 @@ pg_largeobject_aclcheck_snapshot(Oid lobj_oid, Oid roleid, AclMode mode,
 AclResult
 pg_namespace_aclcheck(Oid nsp_oid, Oid roleid, AclMode mode)
 {
+	if (mode == ACL_CREATE && !allowSystemTableMods &&
+		(IsSystemNamespace(nsp_oid) || IsToastNamespace(nsp_oid)) &&
+		IsNormalProcessingMode())
+		return ACLCHECK_NO_PRIV;
+
 	if (pg_namespace_aclmask(nsp_oid, roleid, mode, ACLMASK_ANY) != 0)
 		return ACLCHECK_OK;
 	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] GIN over array of ENUMs

[Robert Haas]

On Thu, Jan 10, 2013 at 12:08 PM, Rod Taylor  wrote:
> I wish to create this data structure but GIN does not currently support an
> array of ENUM. Is intarray() a good place to look into adding ENUM support
> or is there already an operator class for working supports enums that I
> simply don't see at the moment.

What does intarray have to do with enums?

If you're talking about contrib/intarray, I suspect that's not a
particularly good place to do anything, except maybe rm -rf .

-- 
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] pg_ctl idempotent option

[Boszormenyi Zoltan]

2013-01-14 16:22 keltezéssel, Tom Lane írta:

Peter Eisentraut  writes:

Here is a patch to add an option -I/--idempotent to pg_ctl, the result
of which is that pg_ctl doesn't error on start or stop if the server is
already running or already stopped.

Idempotent is a ten-dollar word.  Can we find something that average
people wouldn't need to consult a dictionary to understand?


--do-nothing-if-same-state?



Also, should the option affect *only* the result code as you have it,
or should it also change the printed messages to reflect that the
case is considered expected?

Also it appears to me that the hunk at lines 812ff is changing the
default behavior, which is not what the patch is advertised to do.

regards, tom lane





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



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

Re: [HACKERS] Get current query in a trigger function

[Robert Haas]

On Fri, Jan 11, 2013 at 4:47 AM, Vlad Arkhipov  wrote:
> Is there any simple way of getting a query for which a trigger was executed?
> debug_query_string and ActivePortal->sourceText return the top query when
> there are nested triggers.

I believe - only if the trigger is written in C.

-- 
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] [PERFORM] Slow query: bitmap scan troubles

[Robert Haas]

On Thu, Jan 10, 2013 at 8:07 PM, Tom Lane  wrote:
> Comments?

I'm not sure I have anything intelligent to add to this conversation -
does that make me the wisest of all the Greeks? - but I do think it
worth mentioning that I have heard occasional reports within EDB of
the query planner refusing to use extremely large indexes no matter
how large a hammer was applied.  I have never been able to obtain
enough details to understand the parameters of the problem, let alone
reproduce it, but I thought it might be worth mentioning anyway in
case it's both real and related to the case at hand.  Basically I
guess that boils down to: it would be good to consider whether the
costing model is correct for an index of, say, 1TB.

-- 
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] json api WIP patch

[Robert Haas]

On Thu, Jan 10, 2013 at 6:42 PM, Andrew Dunstan  wrote:
>> Udated patch that contains most of the functionality I'm after. One piece
>> left is populate_recordset (populate a set of records from a single json
>> datum which is an array of objects, in one pass). That requires a bit of
>> thought.
>>
>> I hope most of the whitespace issues are fixed.
>
>
> This updated patch contains all the intended functionality, including
> operators for the json_get_path functions, so you can say things like
>
> select jsonval->array['f1','0','f2] ...
>
> It also removes any requirement to copy the json value before setting up the
> lexer by removing the lexer's requirement to have a nul terminated string.
> Instead the lexer is told the input length and relies on that. For this
> reason, json_in() now calls cstring_get_text() before rather than after
> calling the validation routine, but that's really not something worth
> bothering about.
>
> A couple of points worth noting: it's a pity that we have to run CREATE OR
> REPLACE FUNCTION in system_views.sql in order to set up default values for
> builtin functions. That feels very kludgy. Also, making operators for
> variadic functions is a bit of a pain. I had to set up non-variadic version
> of the same functions (see json_get_path_op and json_get_path_as_text_op)
> just so I could set up the operators. Neither of these are exactly
> showstopper items, just mild annoyances.
>
> I will continue hunting memory leaks, but when Merlin gets done with docco I
> think we'll be far enough advanced to add this to the commitfest.

So, how much performance does this lose on json_in() on a large
cstring, as compared with master?

I can't shake the feeling that this is adding a LOT of unnecessary
data copying.  For one thing, instead of copying every single lexeme
(including the single-character ones?) out of the original object, we
could just store a pointer to the offset where the object starts and a
length, instead of copying it.

This is also remarkably thin on comments.

-- 
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] [PATCH] COPY .. COMPRESSED

[Stephen Frost]

* Tom Lane (t...@sss.pgh.pa.us) wrote:
> Stephen Frost  writes:
> > I do like the idea of a generalized answer which just runs a
> > user-provided command on the server but that's always going to require
> > superuser privileges.
> 
> The design that was being kicked around allowed pipes to be used on the
> client side too, ie \copy foo to '| gzip ...'.  That form would not
> require any special privileges, and might be thought preferable for
> another reason too: it offloads the work from the server.

It's a different use-case which, imv, is really already trivially
covered:

psql -c 'COPY foo TO STDOUT;' | gzip > myfile.gz

While there is no option currently for having the server do the
compression before sending the data over the wire.

Thanks,

Stephen


signature.asc
Description: Digital signature

Re: [HACKERS] Proposal for Allow postgresql.conf values to be changed via SQL [review]

[Amit kapila]

On Monday, January 14, 2013 6:48 PM Boszormenyi Zoltan wrote:
2013-01-14 07:47 keltezéssel, Amit kapila írta:
> On Sunday, January 13, 2013 2:45 PM Boszormenyi Zoltan wrote:
> 2013-01-13 05:49 keltezéssel, Amit kapila írta:
>> On Sunday, January 13, 2013 12:41 AM Tom Lane wrote:
>> Boszormenyi Zoltan  writes:

 I can think of below ways to generate tmp file
 1. Generate session specific tmp file name during operation. This will be 
 similar to what is
  currently being done in OpenTemporaryFileinTablespace() to generate a 
 tempfilename.
 2. Generate global tmp file name. For this we need to maintain global 
 counter.
 3. Always generate temp file with same name "postgresql.auto.conf.tmp", as 
 at a time only one
  session is allowed to operate.
>>> What's wrong with mkstemp("config_dir/postgresql.auto.conf.XX")
>>> that returns   a file descriptor for an already created file with a unique 
>>> name?
>> I think for Windows exactly same behavior API might not exist, we might need 
>> to use GetTempFileName.
>> It will generate some different kind of name, which means cleanup also need 
>> to take care of different kind of names.
>> So if this is right, I think it might not be very appropriate to use it.

> In that case (and also because the LWLock still serialize the whole procedure)
> you can use GetTempFileName() on Windows and tempnam(3) for non-Windows.
> GetTempFileName() and tempnam() return the constructed temporary file name
> out of the directory and the filename prefix components.

Okay, I shall work on this based on your suggestion.

With Regards,
Amit Kapila.

-- 
Sent 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] COPY .. COMPRESSED

[Stephen Frost]

* Claudio Freire (klaussfre...@gmail.com) wrote:
> On Mon, Jan 14, 2013 at 1:01 PM, Stephen Frost  wrote:
> > I do like the idea of a generalized answer which just runs a
> > user-provided command on the server but that's always going to require
> > superuser privileges.
> 
> Unless it's one of a set of superuser-authorized compression tools.

Which would require a new ACL system for handling that, as I mentioned..
That certainly isn't what the existing patch does.

What would that look like?  How would it operate?  How would a user
invoke it or even know what options are available?  Would we provide
anything by default?  It's great to consider that possibility but
there's a lot of details involved.

I'm a bit nervous about having a generalized system which can run
anything on the system when called by a superuser but when called by a
regular user we're on the hook to verify the request against a
superuser-provided list and to then make sure nothing goes wrong.

Thanks,

Stephen


signature.asc
Description: Digital signature

Re: [HACKERS] Timing events WIP v1

[Peter Geoghegan]

I decided to take a look at this.

On 15 November 2012 09:56, Greg Smith  wrote:
> -I modeled the hook here on the logging one that went into 9.2.  It's
> defined in its own include file and it gets initialized by the logging
> system.  No strong justification for putting it there, it was just similar
> to the existing hook and that seemed reasonable.  This is in some respects
> an alternate form of logging after all.  The data sent by the hook itself
> needs to be more verbose in order to handle the full spec, right now I'm
> just asking about placement.

I noticed that when !log_checkpoints, control never reaches the site
where the hook is called, and thus the checkpoint info is not stored.
Is that the intended behaviour of the patch? I don't feel strongly
either way, but it did give me pause for a second. pg_stat

I don't like the design of this patch. It seems like we should be
moving as far away from an (internal) textual representation as
possible - that's basically the main reason why logging is bad.
Textual representations make easy, correct querying hard, are bloated,
and are really no better than manual logging.

The first question I'd ask is "what is true of all timing events?".
You address this in your original RFC [1]. However, this isn't
reflected in the datastructures that the patch uses, and I suspect
that it should be.

What I'm envisioning is new NodeTag infrastructure. You'd have an
"abstract base class". This is sort of like the one used by plan nodes
(i.e. the Plan struct). This would contain the information that is
essential to all timing events (in the case of Plan, this includes
startup and total costs for the node in question). You'd refactor
structs like CheckpointStatsData, to compose them such that their
first field was this parent struct (i.e. logically, they'd inherit
from, say, TimingEvent, which itself would inherit from Node, in the
style of Plan's children). That'd entail normalising things like
inconsistent use of datatypes to represent time intervals and whatnot
among these sorts of structs in the extant code, which is painful, but
long term it seems like the way forward.

With this commonality and variability analysis performed, and having
refactored the code, you'd be able to use type punning to pass the
essential information to your new hook. You'd have a way to usefully
restrict statistics to only one class of timing event, because you
could expose the NodeTag in your view (or perhaps a prettified version
thereof), by putting something in a query predicate. You can perform
aggregation, too. Maybe this abstract base class would look something
like:

typedef struct TimingEvent
{
NodeTag type;

/*
 * Common structural data for all TimingEvent types.
 */
double duration;  /* Duration of event in milliseconds (per project
policy for views) */
double naffected; /* Number of objects (of variable type) affected */
char   *string;  /* Non-standardised event string */
} TimingEvent;

However, my failure to come up with more fields here leads me to
believe that we'd be better off with a slightly different approach.
Let's forget about storing text altogether, and make the unstructured
"union" field JSON (while making fields common to all timing events
plain-old scalar datums, for ease of use).

It would become the job of PGTE_memsize() to make each entry in the
shared hashtable wide enough to accommodate the widest of all possible
TimingEvent-inheriting structs. We'd just store heterogeneous
TimingEvent-inheriting structs in the hash table. It would be
pg_timing_events job to call a JSON conversion routine directly, with
baked-in knowledge, within a NodeTag case statement.

It actually wouldn't be hard to have the view spit out some JSON from
the internal, compact struct representation. Currently, explain.c
manages to generate JSON representations of plans with very little
fuss, and without using any of the JSON datatype stuff. Doing this
with hstore is just way too controversial, and not particularly worth
it, IMHO. With a "where event_type = x" in the query predicate, the
JSON datums would have predictable, consistent structure, facilitating
machine reading and aggregation.

[1] Original RFC e-mail:
http://www.postgresql.org/message-id/509300f7.5000...@2ndquadrant.com

-- 
Peter Geoghegan   http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training and 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] COPY .. COMPRESSED

[Tom Lane]

Stephen Frost  writes:
> I do like the idea of a generalized answer which just runs a
> user-provided command on the server but that's always going to require
> superuser privileges.

The design that was being kicked around allowed pipes to be used on the
client side too, ie \copy foo to '| gzip ...'.  That form would not
require any special privileges, and might be thought preferable for
another reason too: it offloads the work from the server.

regards, tom lane


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

Re: [HACKERS] erroneous restore into pg_catalog schema

[Tom Lane]

Alvaro Herrera  writes:
> Tom Lane escribió:
>> I will bet that this is more breakage from the DDL-code refactoring that
>> has been going on.  I am getting closer and closer to wanting that
>> reverted.  KaiGai-san seems to have been throwing out lots of special
>> cases that were there for good reasons.

> Isn't this just a475c6036?

Ah ... well, at least it was intentional.  But still wrongheaded,
as this example shows.  What we should have done was what the commit
message suggests, ie place a replacement check somewhere "upstream"
where it would apply to all object types.  First thought that comes to
mind is to add a hack to pg_namespace_aclcheck, or maybe at some call
site(s).

regards, tom lane


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

Re: [HACKERS] [PATCH] COPY .. COMPRESSED

[Claudio Freire]

On Mon, Jan 14, 2013 at 1:01 PM, Stephen Frost  wrote:
> I do like the idea of a generalized answer which just runs a
> user-provided command on the server but that's always going to require
> superuser privileges.

Unless it's one of a set of superuser-authorized compression tools.


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

Re: [HACKERS] erroneous restore into pg_catalog schema

[Alvaro Herrera]

Tom Lane escribió:
> "Erik Rijkers"  writes:
> > On Sun, January 13, 2013 22:09, Tom Lane wrote:
> >> BTW, although Erik claimed this behaved more sanely in 9.2, a closer
> >> look at the commit logs says that the bogus commit shipped in 9.2,
> >> so AFAICS it's broken there too.
> 
> > [ not so ]
> 
> Hm, you are right, there's another problem that's independent of
> search_path.  In 9.2,
> 
> regression=# create table pg_catalog.t(f1 int);
> ERROR:  permission denied to create "pg_catalog.t"
> DETAIL:  System catalog modifications are currently disallowed.
> 
> but HEAD is missing that error check:
> 
> regression=# create table pg_catalog.t(f1 int);
> CREATE TABLE
> 
> I will bet that this is more breakage from the DDL-code refactoring that
> has been going on.  I am getting closer and closer to wanting that
> reverted.  KaiGai-san seems to have been throwing out lots of special
> cases that were there for good reasons.

Isn't this just a475c6036?

-- 
Á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] [PATCH] COPY .. COMPRESSED

[Stephen Frost]

* Simon Riggs (si...@2ndquadrant.com) wrote:
> There is a new option being added to pre/post process data, so it
> seems like the best way to add new features - in general.

That structure appears to have no option for passing compressed data to
or from a client connection.  Instead, it actually overloads the typical
meaning for options sent to \copy (which, imv, is "run COPY on the server
with these options and have the results stored locally") to mean
something different (run part of the COPY command on the server and part
of it locally).

> Specifically, we do support compressed output so a simple patch to
> allow re-loading of the compressed data we generate does seem sensible
> and reasonable.

Right, we're already using gzip for pg_dump/pg_restore.  This just gives
an option to move that compression over to the server side.  Also, I'd
be happy to add support for other compression options.

I do like the idea of a generalized answer which just runs a
user-provided command on the server but that's always going to require
superuser privileges.

Thanks,

Stephen


signature.asc
Description: Digital signature

Re: [HACKERS] pg_ctl idempotent option

[Thom Brown]

On 14 January 2013 15:29, Alvaro Herrera  wrote:

> Tom Lane wrote:
> > Peter Eisentraut  writes:
> > > Here is a patch to add an option -I/--idempotent to pg_ctl, the result
> > > of which is that pg_ctl doesn't error on start or stop if the server is
> > > already running or already stopped.
> >
> > Idempotent is a ten-dollar word.  Can we find something that average
> > people wouldn't need to consult a dictionary to understand?
>
> --no-error perhaps?
>

Couldn't that be misconstrued as to suppress any type of error?

I personally can't think of something terse enough to put into a
descriptive long-word option that would adequately describe the change in
behaviour it provides.

--suppress-error-when-target-status-already-present   ... bit too wordy. ;)

-- 
Thom

Re: [HACKERS] pg_ctl idempotent option

[Tom Lane]

Alvaro Herrera  writes:
> Tom Lane wrote:
>> Peter Eisentraut  writes:
>>> Here is a patch to add an option -I/--idempotent to pg_ctl, the result
>>> of which is that pg_ctl doesn't error on start or stop if the server is
>>> already running or already stopped.

>> Idempotent is a ten-dollar word.  Can we find something that average
>> people wouldn't need to consult a dictionary to understand?

> --no-error perhaps?

Meh, that's probably going too far in the direction of imprecision.
The point of this patch is that only very specific errors are
suppressed.

I don't have a better idea though.  It'd be easier if there were
separate switches for the two cases, then you could call them
--ok-if-running and --ok-if-stopped.  But that's not very workable,
if only because both would want the same single-letter abbreviation.

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] erroneous restore into pg_catalog schema

[Alvaro Herrera]

Tom Lane escribió:

> I will bet that this is more breakage from the DDL-code refactoring that
> has been going on.  I am getting closer and closer to wanting that
> reverted.  KaiGai-san seems to have been throwing out lots of special
> cases that were there for good reasons.

I will have a look.

-- 
Á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] pg_ctl idempotent option

[Alvaro Herrera]

Tom Lane wrote:
> Peter Eisentraut  writes:
> > Here is a patch to add an option -I/--idempotent to pg_ctl, the result
> > of which is that pg_ctl doesn't error on start or stop if the server is
> > already running or already stopped.
> 
> Idempotent is a ten-dollar word.  Can we find something that average
> people wouldn't need to consult a dictionary to understand?

--no-error perhaps?

-- 
Á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] find libxml2 using pkg-config

[Tom Lane]

Peter Eisentraut  writes:
> The attached patch looks for pkg-config first, and finds libxml2 using
> that if available.  Otherwise it falls back to using xml2-config.

What happens if pkg-config is installed but doesn't know anything about
xml2?  I'd expect the code to fall back to the old method, but this
patch doesn't appear to have any sanity check whatsoever on pkg-config's
output.

regards, tom lane


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

Re: [HACKERS] pg_ctl idempotent option

[Tom Lane]

Peter Eisentraut  writes:
> Here is a patch to add an option -I/--idempotent to pg_ctl, the result
> of which is that pg_ctl doesn't error on start or stop if the server is
> already running or already stopped.

Idempotent is a ten-dollar word.  Can we find something that average
people wouldn't need to consult a dictionary to understand?

Also, should the option affect *only* the result code as you have it,
or should it also change the printed messages to reflect that the
case is considered expected?

Also it appears to me that the hunk at lines 812ff is changing the
default behavior, which is not what the patch is advertised to do.

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] common fe/be library (was Re: [PATCH] binary heap implementation)

[Merlin Moncure]

On Mon, Jan 14, 2013 at 4:55 AM, Abhijit Menon-Sen  wrote:
> At 2012-11-15 12:08:12 -0500, robertmh...@gmail.com wrote:
>>
>> Still, maybe we could have a src/framework directory that uses the
>> same trick to produce a libpgframework that frontend code can use,
>> and a lib pgframework_srv that can be linked into the backend.
>> That's might not actually be that much work; we'd just need to
>> clone all the existing src/port logic.
>
> Does anyone have further comments about Robert's idea above? Or
> alternative suggestions about how to structure a library of routines
> that can be used in either the backend or in client code (like the
> binary heap implementation)?

A couple observations:
*) Not sure about the name: something like "libcommon"?

*) This is useful and potentially good for many things.  For example,
maybe type manipulation code could be pushed into the common library
at some point.

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] [PATCH] COPY .. COMPRESSED

[Simon Riggs]

On 14 January 2013 13:43, Stephen Frost  wrote:
> Tom,
>
> * Tom Lane (t...@sss.pgh.pa.us) wrote:
>> Stephen Frost  writes:
>> >   Attached is a patch to add a 'COMPRESSED' option to COPY which will
>> >   cause COPY to expect a gzip'd file on input and which will output a
>> >   gzip'd file on output.  Included is support for backend COPY, psql's
>> >   \copy, regression tests for both, and documentation.
>>
>> I don't think it's a very good idea to invent such a specialized option,
>> nor to tie it to gzip, which is widely considered to be old news.
>
> We're already using gzip/zlib for pg_dump/pg_restore, so it was simple
> and straight-forward to add and would allow utilizing this option while
> keeping the custom dump format the same.

Both thoughts are useful, I think.

There is a new option being added to pre/post process data, so it
seems like the best way to add new features - in general.

Specifically, we do support compressed output so a simple patch to
allow re-loading of the compressed data we generate does seem sensible
and reasonable.

-- 
 Simon Riggs   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] COPY .. COMPRESSED

[Stephen Frost]

Tom,

* Tom Lane (t...@sss.pgh.pa.us) wrote:
> Stephen Frost  writes:
> >   Attached is a patch to add a 'COMPRESSED' option to COPY which will
> >   cause COPY to expect a gzip'd file on input and which will output a
> >   gzip'd file on output.  Included is support for backend COPY, psql's
> >   \copy, regression tests for both, and documentation.
> 
> I don't think it's a very good idea to invent such a specialized option,
> nor to tie it to gzip, which is widely considered to be old news.

We're already using gzip/zlib for pg_dump/pg_restore, so it was simple
and straight-forward to add and would allow utilizing this option while
keeping the custom dump format the same.  It also happens to match what
I need.  While gzip might be 'old hat' it's still extremely popular.
I'd be happy to add support for bzip2 or something else that people are
interested in, and support compression options for zlib if necessary
too.  This was intended to get the ball rolling on something as the last
discussion that I had seen while hunting through the archives was from
2006, obviously I missed the boat on the last set of patches.

> There was discussion (and, I think, a patch in the queue) for allowing
> COPY to pipe into or out of an arbitrary shell pipe.  Why would that not
> be enough to cover this use-case?  That is, instead of a hard-wired
> capability, people would do something like COPY TO '| gzip >file.gz'.
> Or they could use bzip2 or whatever struck their fancy.

Sounds like a nice idea, but I can't imagine it'd be available to anyone
except for superusers, and looking at that patch, that's exactly the
restriction which is in place for it.  In addition, that patch's support
for "\copy" implements everything locally, making it little different
from "zcat mycsv.csv.gz | psql".  The patch that I proposed actually
sent the compressed stream across the wire, reducing bandwidth
utilization.

All that said, I've nothing against having the pipe option for the
backend COPY command; a bit annoyed with myself for somehow missing that
patch.  I don't like what it's doing with psql's \copy command and would
rather we figure out a way to support PROGRAM .. TO STDOUT, but that
still would require superuser privileges.  I don't see any easy way to
support compressed data streaming to/from the server for COPY w/o
defining what methods are available or coming up with some ACL system
for what programs can be called by the backend.

Thanks,

Stephen


signature.asc
Description: Digital signature

Re: [HACKERS] pgcrypto seeding problem when ssl=on

[Marko Kreen]

On Mon, Jan 14, 2013 at 3:00 PM, Noah Misch  wrote:
> On Mon, Jan 14, 2013 at 02:21:00PM +0200, Marko Kreen wrote:
>> Note: reading from /dev/urandom does not affect /dev/random.
>
> Reading from /dev/urandom drains the pool that serves /dev/random:
>
> $ cat /proc/sys/kernel/random/entropy_avail
> 3596
> $ dd iflag=nonblock bs=100 count=1 if=/dev/random of=/dev/null
> 1+0 records in
> 1+0 records out
> 100 bytes (100 B) copied, 0.000174798 s, 572 kB/s
> $ cat /proc/sys/kernel/random/entropy_avail
> 2839
> $ head -c1000 /dev/urandom >/dev/null
> $ cat /proc/sys/kernel/random/entropy_avail
> 212
> $ dd iflag=nonblock bs=100 count=1 if=/dev/random of=/dev/null
> 0+1 records in
> 0+1 records out
> 38 bytes (38 B) copied, 0.000101439 s, 375 kB/s

This slightly weakens my argument, but not completely - any
/dev/urandom-using processes are still unaffected, but
indeed processes using /dev/random can block.  But what are those?

So it's still problem only on systems that do not have /dev/urandom.

Btw, observe fun behaviour:

$ cat /proc/sys/kernel/random/entropy_avail
3451
$ cat /proc/sys/kernel/random/entropy_avail
3218
$ cat /proc/sys/kernel/random/entropy_avail
3000
$ cat /proc/sys/kernel/random/entropy_avail
2771
$ cat /proc/sys/kernel/random/entropy_avail
2551
$ cat /proc/sys/kernel/random/entropy_avail
2321
$ cat /proc/sys/kernel/random/entropy_avail
2101
$ cat /proc/sys/kernel/random/entropy_avail
1759
$ cat /proc/sys/kernel/random/entropy_avail
1527
$ cat /proc/sys/kernel/random/entropy_avail
1319
$ cat /proc/sys/kernel/random/entropy_avail
1080
$ cat /proc/sys/kernel/random/entropy_avail
844
$ cat /proc/sys/kernel/random/entropy_avail
605
$ cat /proc/sys/kernel/random/entropy_avail
366
$ cat /proc/sys/kernel/random/entropy_avail
128
$ cat /proc/sys/kernel/random/entropy_avail
142
$ cat /proc/sys/kernel/random/entropy_avail

Each exec() eats from the pool on modern system.

-- 
marko


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

Re: [HACKERS] Proposal for Allow postgresql.conf values to be changed via SQL [review]

[Boszormenyi Zoltan]

2013-01-14 07:47 keltezéssel, Amit kapila írta:

On Sunday, January 13, 2013 2:45 PM Boszormenyi Zoltan wrote:
2013-01-13 05:49 keltezéssel, Amit kapila írta:

On Sunday, January 13, 2013 12:41 AM Tom Lane wrote:
Boszormenyi Zoltan  writes:

No, I mean the reaper(SIGNAL_ARGS) function in
src/backend/postmaster/postmaster.c where your patch has this:
*** a/src/backend/postmaster/postmaster.c
--- b/src/backend/postmaster/postmaster.c
***
*** 2552,2557  reaper(SIGNAL_ARGS)

I have not been following this thread, but I happened to see this bit,
and I have to say that I am utterly dismayed that anything like this is
even on the table.  This screams overdesign.  We do not need a global
lock file, much less postmaster-side cleanup.  All that's needed is a
suitable convention about temp file names that can be written and then
atomically renamed into place.  If we're unlucky enough to crash before
a temp file can be renamed into place, it'll just sit there harmlessly.

I can think of below ways to generate tmp file
1. Generate session specific tmp file name during operation. This will be 
similar to what is
 currently being done in OpenTemporaryFileinTablespace() to generate a 
tempfilename.
2. Generate global tmp file name. For this we need to maintain global counter.
3. Always generate temp file with same name "postgresql.auto.conf.tmp", as at a 
time only one
 session is allowed to operate.

What's wrong with mkstemp("config_dir/postgresql.auto.conf.XX")
that returns   a file descriptor for an already created file with a unique name?

I think for Windows exactly same behavior API might not exist, we might need to 
use GetTempFileName.
It will generate some different kind of name, which means cleanup also need to 
take care of different kind of names.
So if this is right, I think it might not be very appropriate to use it.


In that case (and also because the LWLock still serialize the whole procedure)
you can use GetTempFileName() on Windows and tempnam(3) for non-Windows.
GetTempFileName() and tempnam() return the constructed temporary file name
out of the directory and the filename prefix components.

The differences are:
1. For GetTempFileName(), you have to provide a buffer and it uses 3 bytes from
the prefix string. Always give you this path: dir\pfx.TMP

2. tempnam() gives you a malloc()ed result buffer and uses at most 5 bytes from
the prefix string. According to the NOTES section in the man page, the specified
directory is only considered if $TMPDIR is not set in the environment or the
directory is not usable, like write is not permitted or the directory is 
missing.
In this case. If TMPDIR not only is present and usable but on a different 
filesystem
as the target config_dir/postgresql.auto.conf, rename() would return the EXDEV
error:

   EXDEV  The  links  named  by new and old are on different file systems and the 
implementation does not support links

  between file systems.

Maybe mktemp(3) (not mk*s*temp!) instead of tempnam() is better here
because it always uses the specified template. You have to use you own
buffer for mktemp(). The BUGS section in man 3 mktemp says:

BUGS
   Never use mktemp().  Some implementations follow 4.3BSD and replace XX by the 
current process ID  and  a  single
   letter,  so that at most 26 different names can be returned. Since on the one hand 
the names are easy to guess, and
   on the other hand there is a race between testing whether the name  exists  and  
opening  the  file,  every  use  of

   mktemp() is a security risk.  The race is avoided by mkstemp(3).

I think we don't really care about the security risk about the file name
being easy to guess and there is no race condition because of the LWLock.

With GetTempFileName() and mktemp(), you can have a lot of common code,
like pass the same 3-letter prefix and preallocate or statically declare the 
output buffer.


Please point me if I am wrong as I have not used it.


Note that the working directory of PostgreSQL is $PGDATA so "config_dir"
and files under that can be accessed with a relative path.
But a cleanup somewhere is needed to remove the stale temp files.
I think it's enough at postmaster startup. A machine that's crashing
so often that the presence of the stale temp files causes out of disk
errors would surely be noticed much earlier.

In PostmasterMain(), we already call RemovePGTempFiles(); So I think we can 
delete this tmp file at
that place in PostmasterMain().
I shall do this, if nobody raise objection about it.


No objection.



With Regards,
Amit Kapila.




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



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

Re: [HACKERS] pgcrypto seeding problem when ssl=on

[Noah Misch]

On Mon, Jan 14, 2013 at 02:21:00PM +0200, Marko Kreen wrote:
> Note: reading from /dev/urandom does not affect /dev/random.

Reading from /dev/urandom drains the pool that serves /dev/random:

$ cat /proc/sys/kernel/random/entropy_avail 
3596
$ dd iflag=nonblock bs=100 count=1 if=/dev/random of=/dev/null
1+0 records in
1+0 records out
100 bytes (100 B) copied, 0.000174798 s, 572 kB/s
$ cat /proc/sys/kernel/random/entropy_avail 
2839
$ head -c1000 /dev/urandom >/dev/null
$ cat /proc/sys/kernel/random/entropy_avail 
212
$ dd iflag=nonblock bs=100 count=1 if=/dev/random of=/dev/null
0+1 records in
0+1 records out
38 bytes (38 B) copied, 0.000101439 s, 375 kB/s


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

Re: [HACKERS] pgcrypto seeding problem when ssl=on

[Marko Kreen]

On Mon, Jan 14, 2013 at 12:46 AM, Tom Lane  wrote:
> Marko Kreen  writes:
>> On Fri, Dec 21, 2012 at 10:27 PM, Noah Misch  wrote:
>>> How about instead calling RAND_cleanup() after each backend fork?
>
>> Attached is a patch that adds RAND_cleanup() to fork_process().
>
> I remain unconvinced that this is the best solution.  Anybody else have
> an opinion?

Do you have knowledge about systems that have /dev/random (blocking)
but not /dev/urandom (non-blocking)?  The only argument I see against
RAND_cleanup() is that postgres might eat entropy from /dev/random (blocking)
and cause both other programs and itself block, waiting for more entropy.

But this can only happen on systems that don't have /dev/urandom.

Note: reading from /dev/urandom does not affect /dev/random.

-- 
marko


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

Re: [HACKERS] passing diff options to pg_regress

[Andres Freund]

On 2013-01-14 06:57:52 -0500, Peter Eisentraut wrote:
> I sometimes find it useful to view a regression test difference using
> other diff options, such as -u -w or more context.  There is currently
> no easy way to accomplish that.
> 
> I suggest allowing to override the diff options using an environment
> variable, such as PG_REGRESS_DIFF_OPTS.  The patch is very small.
> 
+ERANGE

I usually do this by manually doing the diff, but thats pretty annoying.

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] Bug: Windows installer modifies ACLs on the whole volume

[Sandeep Thakkar]

Alex, how much of your D:\ is occupied or what is the total size of the
files and folders in D:\ ?

Can you just try the following command manually to see if it returns quick?

icacls  D:\ /grant "UserName":RX

On Fri, Jan 11, 2013 at 9:28 AM, Craig Ringer  wrote:

> On 01/09/2013 05:10 PM, emergency.sho...@gmail.com wrote:
> > One click installer postgresql-9.2.2-1-windows-x64.exe from
> > http://www.postgresql.org/download/windows/.
> >
> > Start the installer from "D:\downloads", choose "C:\Program
> > Files\PostgreSQL\9.2" as binary directory, and
> > "D:\db\postgresql\9.2\data" as the data directory.
> >
> > The installer runs for > 1 hour (actually it had not finished yet).
> > While doing so, it starts ICACLS child processes that modify ACLs of
> > every directory (and file?) on the D:\ volume:
> >
> > icacls  D:\ /grant "UserName":(NP)(RX)
> > ...
> > icacls  D:\ /grant "NT AUTHORITY\NetworkService":(NP)(RX)
> > ...
> I see periodic reports of this issue, but I don't think it's ever been
> conclusively solved. I seem to recall discussion suggesting the cause
> had been found a while ago, but you're on the latest release so this
> isn't just an old in-installer-version issue.
>
> It'd be helpful to have a full list of installed software, service
> packs, etc, along with the exact Windows version.
>
> I've CC'd Sandeep at EnterpriseDB, who IIRC is looking after the
> installer at the moment.
>
> --
>  Craig Ringer   http://www.2ndQuadrant.com/
>  PostgreSQL Development, 24x7 Support, Training & Services
>
>


-- 
Sandeep Thakkar
Senior Software Engineer
EnterpriseDB Corporation
The Enterprise Postgres Company
Phone: +91.20.30589523

Website: www.enterprisedb.com
EnterpriseDB Blog: http://blogs.enterprisedb.com/
Follow us on Twitter: http://www.twitter.com/enterprisedb

This e-mail message (and any attachment) is intended for the use of the
individual or entity to whom it is addressed. This message contains
information from EnterpriseDB Corporation that may be privileged,
confidential, or exempt from disclosure under applicable law. If you are
not the intended recipient or authorized to receive this for the intended
recipient, any use, dissemination, distribution, retention, archiving, or
copying of this communication is strictly prohibited. If you have received
this e-mail in error, please notify the sender immediately by reply e-mail
and delete this message.

[HACKERS] passing diff options to pg_regress

[Peter Eisentraut]

I sometimes find it useful to view a regression test difference using
other diff options, such as -u -w or more context.  There is currently
no easy way to accomplish that.

I suggest allowing to override the diff options using an environment
variable, such as PG_REGRESS_DIFF_OPTS.  The patch is very small.

diff --git a/src/test/regress/pg_regress.c b/src/test/regress/pg_regress.c
index 1668119..96ba79c 100644
--- a/src/test/regress/pg_regress.c
+++ b/src/test/regress/pg_regress.c
@@ -1970,6 +1970,9 @@
 	 */
 	ifunc();
 
+	if (getenv("PG_REGRESS_DIFF_OPTS"))
+		pretty_diff_opts = getenv("PG_REGRESS_DIFF_OPTS");
+
 	while ((c = getopt_long(argc, argv, "hV", long_options, &option_index)) != -1)
 	{
 		switch (c)

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

[HACKERS] find libxml2 using pkg-config

[Peter Eisentraut]

In multi-arch OS installations, using a single foo-config script to find
libraries is problematic, because you don't know which architecture it
will point to, and you can't choose which one you want.  Using
pkg-config is better in that situation, because you can use its
environment variables to point to your preferred version
of /usr/lib*/pkgconfig or similar.

In configure, we use xml2-config and pthread-config.  The latter doesn't
exist on my system, so I'm just dealing with xml2-config now.

The attached patch looks for pkg-config first, and finds libxml2 using
that if available.  Otherwise it falls back to using xml2-config.

diff --git a/configure.in b/configure.in
index 2dee4b3..9f8b0f9 100644
--- a/configure.in
+++ b/configure.in
@@ -706,18 +706,24 @@ PGAC_ARG_BOOL(with, libxml, no, [build with XML support],
   [AC_DEFINE([USE_LIBXML], 1, [Define to 1 to build with XML support. (--with-libxml)])])
 
 if test "$with_libxml" = yes ; then
-  AC_CHECK_PROGS(XML2_CONFIG, xml2-config)
-  if test -n "$XML2_CONFIG"; then
-for pgac_option in `$XML2_CONFIG --cflags`; do
-  case $pgac_option in
--I*|-D*) CPPFLAGS="$CPPFLAGS $pgac_option";;
-  esac
-done
-for pgac_option in `$XML2_CONFIG --libs`; do
-  case $pgac_option in
--L*) LDFLAGS="$LDFLAGS $pgac_option";;
-  esac
-done
+  AC_CHECK_PROGS(PKG_CONFIG, pkg-config)
+  if test -n "$PKG_CONFIG"; then
+CPPFLAGS="$CPPFLAGS "`$PKG_CONFIG libxml-2.0 --cflags-only-I`
+LDFLAGS="$LDFLAGS "`$PKG_CONFIG libxml-2.0 --libs-only-L`
+  else
+AC_CHECK_PROGS(XML2_CONFIG, xml2-config)
+if test -n "$XML2_CONFIG"; then
+  for pgac_option in `$XML2_CONFIG --cflags`; do
+case $pgac_option in
+  -I*|-D*) CPPFLAGS="$CPPFLAGS $pgac_option";;
+esac
+  done
+  for pgac_option in `$XML2_CONFIG --libs`; do
+case $pgac_option in
+  -L*) LDFLAGS="$LDFLAGS $pgac_option";;
+esac
+  done
+fi
   fi
 fi
 
diff --git a/doc/src/sgml/installation.sgml b/doc/src/sgml/installation.sgml
index 6e43bcb..ef32bca 100644
--- a/doc/src/sgml/installation.sgml
+++ b/doc/src/sgml/installation.sgml
@@ -892,11 +892,18 @@ Configuration
 
 
 
- Libxml installs a program xml2-config that
- can be used to detect the required compiler and linker
- options.  PostgreSQL will use it automatically if found.  To
- specify a libxml installation at an unusual location, you can
- either set the environment variable
+ To detect the required compiler and linker options, PostgreSQL will
+ query pkg-config, if it is installed.  Otherwise
+ the program xml2-config, which is installed by
+ libxml, will be used if it is found.  Use
+ of pkg-config is preferred, because it can deal
+ with multi-arch installations better.
+
+
+
+ To specify a libxml installation at an unusual location, you can
+ either set pkg-config-related environment variables
+ (see its documentation), or set the environment variable
  XML2_CONFIG to point to the
  xml2-config program belonging to the
  installation, or use the options

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

[HACKERS] pg_ctl idempotent option

[Peter Eisentraut]

Here is a patch to add an option -I/--idempotent to pg_ctl, the result
of which is that pg_ctl doesn't error on start or stop if the server is
already running or already stopped.

The previous discussion was at
.

diff --git a/contrib/start-scripts/linux b/contrib/start-scripts/linux
index b950cf5..03c6e50 100644
--- a/contrib/start-scripts/linux
+++ b/contrib/start-scripts/linux
@@ -84,17 +84,17 @@ case $1 in
 	echo -n "Starting PostgreSQL: "
 	test x"$OOM_SCORE_ADJ" != x && echo "$OOM_SCORE_ADJ" > /proc/self/oom_score_adj
 	test x"$OOM_ADJ" != x && echo "$OOM_ADJ" > /proc/self/oom_adj
-	su - $PGUSER -c "$DAEMON -D '$PGDATA' &" >>$PGLOG 2>&1
+	su - $PGUSER -c "$DAEMON -I -D '$PGDATA' &" >>$PGLOG 2>&1
 	echo "ok"
 	;;
   stop)
 	echo -n "Stopping PostgreSQL: "
-	su - $PGUSER -c "$PGCTL stop -D '$PGDATA' -s -m fast"
+	su - $PGUSER -c "$PGCTL stop -I -D '$PGDATA' -s -m fast"
 	echo "ok"
 	;;
   restart)
 	echo -n "Restarting PostgreSQL: "
-	su - $PGUSER -c "$PGCTL stop -D '$PGDATA' -s -m fast -w"
+	su - $PGUSER -c "$PGCTL stop -I -D '$PGDATA' -s -m fast -w"
 	test x"$OOM_SCORE_ADJ" != x && echo "$OOM_SCORE_ADJ" > /proc/self/oom_score_adj
 	test x"$OOM_ADJ" != x && echo "$OOM_ADJ" > /proc/self/oom_adj
 	su - $PGUSER -c "$DAEMON -D '$PGDATA' &" >>$PGLOG 2>&1
diff --git a/doc/src/sgml/ref/pg_ctl-ref.sgml b/doc/src/sgml/ref/pg_ctl-ref.sgml
index 3107514..549730d 100644
--- a/doc/src/sgml/ref/pg_ctl-ref.sgml
+++ b/doc/src/sgml/ref/pg_ctl-ref.sgml
@@ -39,6 +39,7 @@
-o options
-p path
-c
+   -I
   
 
   
@@ -55,6 +56,7 @@
i[mmediate]
  

+   -I
   
 
   
@@ -271,6 +273,25 @@ Options
  
 
  
+  -I
+  --idempotent
+  
+   
+When used with the start or stop
+actions, return exit code 0 if the server is already running or
+stopped, respectively.  Otherwise, an error is raised and a nonzero
+exit code is returned in these cases.
+   
+
+   
+This option is useful for System-V-style init scripts, which require
+the start and stop actions to be
+idempotent.
+   
+  
+ 
+
+ 
   -l filename
   --log filename
   
diff --git a/src/bin/pg_ctl/pg_ctl.c b/src/bin/pg_ctl/pg_ctl.c
index 2a00cd2..d3c78e5 100644
--- a/src/bin/pg_ctl/pg_ctl.c
+++ b/src/bin/pg_ctl/pg_ctl.c
@@ -86,6 +86,7 @@
 static char *pgdata_opt = NULL;
 static char *post_opts = NULL;
 static const char *progname;
+static bool idempotent = false;
 static char *log_file = NULL;
 static char *exec_path = NULL;
 static char *register_servicename = "PostgreSQL";		/* FIXME: + version ID? */
@@ -812,9 +813,17 @@
 	{
 		old_pid = get_pgpid();
 		if (old_pid != 0)
-			write_stderr(_("%s: another server might be running; "
-		   "trying to start server anyway\n"),
-		 progname);
+		{
+			if (idempotent)
+write_stderr(_("%s: another server might be running; "
+			   "trying to start server anyway\n"),
+			 progname);
+			else
+			{
+write_stderr(_("%s: another server might be running\n"), progname);
+exit(1);
+			}
+		}
 	}
 
 	read_post_opts();
@@ -900,7 +909,10 @@
 	{
 		write_stderr(_("%s: PID file \"%s\" does not exist\n"), progname, pid_file);
 		write_stderr(_("Is server running?\n"));
-		exit(1);
+		if (idempotent)
+			return;
+		else
+			exit(1);
 	}
 	else if (pid < 0)			/* standalone backend, not postmaster */
 	{
@@ -1793,9 +1805,9 @@
 	printf(_("%s is a utility to initialize, start, stop, or control a PostgreSQL server.\n\n"), progname);
 	printf(_("Usage:\n"));
 	printf(_("  %s init[db]   [-D DATADIR] [-s] [-o \"OPTIONS\"]\n"), progname);
-	printf(_("  %s start   [-w] [-t SECS] [-D DATADIR] [-s] [-l FILENAME] [-o \"OPTIONS\"]\n"), progname);
-	printf(_("  %s stop[-W] [-t SECS] [-D DATADIR] [-s] [-m SHUTDOWN-MODE]\n"), progname);
-	printf(_("  %s restart [-w] [-t SECS] [-D DATADIR] [-s] [-m SHUTDOWN-MODE]\n"
+	printf(_("  %s start   [-w] [-t SECS] [-D DATADIR] [-s] [-I] [-l FILENAME] [-o \"OPTIONS\"]\n"), progname);
+	printf(_("  %s stop[-W] [-t SECS] [-D DATADIR] [-s] [-I] [-m SHUTDOWN-MODE]\n"), progname);
+	printf(_("  %s restart [-w] [-t SECS] [-D DATADIR] [-s]  [-m SHUTDOWN-MODE]\n"
 			 " [-o \"OPTIONS\"]\n"), progname);
 	printf(_("  %s reload  [-D DATADIR] [-s]\n"), progname);
 	printf(_("  %s status  [-D DATADIR]\n"), progname);
@@ -1828,6 +1840,8 @@
 	printf(_("  -o OPTIONS command line options to pass to postgres\n"
 	 " (PostgreSQL server executable) or initdb\n"));
 	printf(_("  -p PATH-TO-POSTGRESnormally not necessary\n"));
+	printf(_("\nOptions for start or stop:\n"));
+	printf(_("  -I, --idempotent   don't error if server already running or stopped\n"));
 	printf(_("\nOptions for stop or restart:\n"));
 	printf(_("  -m, --mode=MODEMODE can be \"smart\", \"fast\", or \"immediate\"\n"));
 
@@ -2006,6 +2020,7 

[HACKERS] common fe/be library (was Re: [PATCH] binary heap implementation)

[Abhijit Menon-Sen]

At 2012-11-15 12:08:12 -0500, robertmh...@gmail.com wrote:
>
> Still, maybe we could have a src/framework directory that uses the
> same trick to produce a libpgframework that frontend code can use,
> and a lib pgframework_srv that can be linked into the backend.
> That's might not actually be that much work; we'd just need to
> clone all the existing src/port logic.

Does anyone have further comments about Robert's idea above? Or
alternative suggestions about how to structure a library of routines
that can be used in either the backend or in client code (like the
binary heap implementation)?

-- Abhijit


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

Re: [HACKERS] pg_retainxlog for inclusion in 9.3?

[Magnus Hagander]

On Sat, Jan 5, 2013 at 3:11 PM, Magnus Hagander  wrote:
> On Fri, Jan 4, 2013 at 7:13 PM, Peter Eisentraut  wrote:
>> On 1/3/13 12:30 PM, Robert Haas wrote:
>>> On Thu, Jan 3, 2013 at 11:32 AM, Magnus Hagander  
>>> wrote:
 Any particular reason? It goes pretty tightly together with
 pg_receivexlog, which is why I'd prefer putting it alongside that one.
 But if you have a good argument against it, I can change my mind :)
>>>
>>> Mostly that it seems like a hack, and I suspect we may come up with a
>>> better way to do this in the future.
>>
>> It does seem like a hack.  Couldn't this be implemented with a backend
>> switch instead?
>
> It definitely is a bit of a hack.
>
> I assume by backend switch you mean guc, right? If so, no, not easily
> so. Because it's the archiver process that does the deleting. And this
> process does not have access to a "full backend interface", e.g. the
> ability to run a query. We could make it look at the same data that's
> currently shown in pg_stat_replicatoin through shared memory, but thta
> would *only* work in the very most simple cases (e.g. a single
> pg_receivexlog and no other replication). The ability to run a custom
> SQL query is going to be necessary for anything a bit more advanced.
>
>
>> Also, as a small practical matter, since this is a server-side program
>> (since it's being used as archive_command), we shouldn't put it into the
>> pg_basebackup directory, because that would blur the lines about what to
>> install where, in particular for the translations.
>
> Good argument. That along with the being a hack, and the comment from
> Robert, means that maybe contrib/ is a better place for it, yes.

Here's a version for inclusion in /contrib.

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


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

[HACKERS] Thinking about WITH CHECK OPTION for views

[Dean Rasheed]

I've been thinking about WITH CHECK OPTION for auto-updatable views.
Given the timing I doubt if this will be ready for 9.3, since I only
get occasional evenings and weekends to hack on postgres, but I think
it's probably worth kicking off a discussion, starting with a
description of what the feature actually is.

>From the SQL spec:

 ::=
CREATE [ RECURSIVE ] VIEW  
AS  [ WITH [  ] CHECK OPTION ]

 ::= CASCADED | LOCAL

If WITH CHECK OPTION is specified and  is not specified,
then a  of CASCADED is implicit.

In other words there are 3 possible levels of check, which are:

1). NO CHECK - i.e., what we do now.

2). LOCAL CHECK - new tuples are validated against the WHERE quals
specified locally on this view, but not against any quals from any
underlying views (unless they also specify the WITH CHECK OPTION).

3). CASCADED CHECK - new tuples are validated against the WHERE quals
of this view and all underlying views (regardless of whether or not
those underlying views specify the WITH CHECK OPTION).


The SQL spec is also very specific about how and when the checks
should be applied:

* Each view's quals should be checked *after* inserting into the
underlying base relation.

* Checks should be applied starting with the innermost views and
working out to the outermost (top-level) view.

* Obviously the checks apply to INSERT and UPDATE statements only (not DELETE).

In code terms, this suggests that the new check should go in
ExecInsert/ExectUpdate after firing any AFTER ROW triggers and before
processing the RETURNING list. The check itself is very similar to a
regular CHECK constraint, but with a couple of differences:

* if the qual evaluates to NULL it should be a failure since the new
row won't be visible in the view. That's the opposite logic from a
CHECK constraint where NULL is OK.

* it probably wants to support sub-queries in the qual (we allow such
views to be auto-updatable). This doesn't seem so hard to do, and
makes more logical sense than supporting sub-queries in CHECK
constraints, since the contents of the view are dynamic and the
consistency of the constraint is automatically preserved when data in
referenced tables is modified.


Finally, the SQL spec is very strict about which kinds of view are
allowed to specify the WITH CHECK OPTION.

* WITH LOCAL CHECK OPTION can only be specified on a view that is
auto-updatable, not one that is trigger-updatable.

* WITH CASCADED CHECK OPTION can only be specified on a view if it is
auto-updatable, and all its underlying views are also auto-updatable,
not trigger-updatable.

In order to enforce this, the SQL spec says that there should be an
additional check in CREATE TRIGGER to ensure that an INSTEAD OF
trigger isn't defined on a view that has the WITH CHECK OPTION, or a
view that is
part of a hierarchy of views where any view higher up has the CASCADED
CHECK OPTION.

These restrictions seem pretty strict, but on reflection I think that
they make sense, because the query resulting from a trigger-updatable
view isn't necessarily going to have access to all the data needed to
check that view's quals (which may refer to columns in the base
relation that aren't exposed in the view, and hence aren't returned by
the trigger).


There are similar problems with INSTEAD rules, except I think they are
worse because the rule could completely rewrite the query, and so even
a LOCAL WCO won't work in general, if any of the relations underlying
the view have an INSTEAD rule.

Attached is an initial proof-of-concept patch. It's still missing a
number of things, in particular it currently doesn't do any of these
checks for INSTEAD OF triggers or DO INSTEAD rules. The last few
examples of the
regression tests show the sorts of things that go wrong without these checks.

Regards,
Dean


with-check-option.patch.gz
Description: GNU Zip compressed data

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

Re: [HACKERS] Re: logical changeset generation v3 - comparison to Postgres-R change set format

[Markus Wanner]

On 01/13/2013 09:04 PM, Hannu Krosing wrote:
> I'd just start with what send() and recv() on each type produces
> now using GCC on 64bit Intel and move towards adjusting others
> to match. For a period anything else would still be allowed, but
> be "non-standard"

Intel being little endian seems like a bad choice to me, given that
send/recv kind of implies network byte ordering. I'd rather not tie this
to any particular processor architecture at all (at least not solely on
the ground that it's the most common one at the time).

I have no strong opinion on "sameness" of NULLs and could also imagine
that to throw some kind of invalid operation error. Based on the ground
that neither is a value and it's unclear what send() method to use at all.

FWIW, trying to determine the length of a sent NULL gives an interesting
result that I don't currently understand.

> psql (9.2.2)
> Type "help" for help.
> 
> postgres=# SELECT length(int4send(NULL));
>  length 
> 
>
> (1 row)
> 
> postgres=# SELECT length(float4send(NULL));
>  length 
> 
>
> (1 row)
> 
> postgres=# SELECT length(textsend(NULL));
>  length 
> 
>
> (1 row)
>
> postgres=# SELECT length(textsend(NULL) || '\000'::bytea);
>  length 
> 
>
> (1 row)


Regards

Markus Wanner


-- 
Sent 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: logical changeset generation v3 - comparison to Postgres-R change set format

[Hannu Krosing]

On 01/13/2013 08:06 PM, Dimitri Fontaine wrote:

Hannu Krosing  writes:

Does this hint that postgreSQL also needs an sameness operator
( "is" or "===" in same languages).

How do people feel about adding a real sameness operator ?

Well. I would prefer it if we can bypass the need for it.

What is actually sufficient for current problem is sameness
which compares outputs of type output functions and also
considers NULLs to be the same.

The reason for not providing equality for xml was not that two xml
files which compare equal as text could be considered unequal in
any sense but that there are some other textual representations
of the same xml which could also be considered to be equal, like
 different whitespace between tag and attribute

Then Do we need the full range of eq, eql, equal and equalp predicates,
and would all of them allow overriding or just some?

I consider sameness as basic thing as IS NULL, so the sameness
should not be overridable. Extending IS NOT DISTINCT FROM to
do this comparison instead of current '=' seems reasonable.

That is

SELECT ''::xml IS DISTINCT FROM ''::xml

should return TRUE as long as the internal representation of the
two differ and even after you add equality operator to xml
which compares some canonic form of xml and thus would make

SELECT ''::xml = ''::xml ;

be TRUE.


Regards,
Hannu



   http://www.cs.cmu.edu/Groups/AI/html/cltl/clm/node74.html

Regards,




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