Re: [HACKERS] Patch for CREATE RULE sgml -- Was in: [DOCS]

2014-03-22 Thread Michael Paquier
On Sat, Mar 22, 2014 at 11:18 AM, Jaime Casanova ja...@2ndquadrant.com wrote:
 On Fri, Mar 21, 2014 at 8:15 PM, Michael Paquier
 michael.paqu...@gmail.com wrote:

 On Sat, Mar 22, 2014 at 12:56 AM, Emanuel Calvo
 emanuel.ca...@2ndquadrant.com wrote:
  I realized that the output of the CREATE RULE has not a detailed
  output for the events parameter.
 
 The list of events possible is already listed in the section
 Parameters = event:

 AFAIU, the synopsis is used to build the help command (\h) in psql.
 Currently in that help the events doesn't appear.
 btw, CREATE TRIGGER already looks this way:
 http://www.postgresql.org/docs/current/static/sql-createtrigger.html
You are right, I forgot this one... In this case patch 2 could be backpatched...
-- 
Michael


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


Re: [HACKERS] ALTER TABLE lock strength reduction patch is unsafe Reply-To:

2014-03-22 Thread Simon Riggs
On 21 March 2014 23:36, Tom Lane t...@sss.pgh.pa.us wrote:
 Simon Riggs si...@2ndquadrant.com writes:
 On 21 March 2014 20:58, Noah Misch n...@leadboat.com wrote:
 It's not the behavior I would choose for a new product, but I can't see
 benefits sufficient to overturn previous decisions to keep it.

 Speechless

 The key argument for not fixing this is that it would break existing
 pg_dump files.  That's a pretty hard argument to overcome, unfortunately,
 even if you're willing to blow off the possibility that client
 applications might contain similar shortcuts.  We still do our best to
 read dump files from the 7.0 era (see ConvertTriggerToFK() for one example
 of going above and beyond for that); and every so often we do hear of
 people trying to get data out of such ancient servers.  So even if you
 went and fixed pg_dump tomorrow, it'd probably be ten or fifteen years
 before people would let you stop reading dumps from existing versions.

Noah had already convinced me, but thank you for explaining the issue
behind that.

-- 
 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] ALTER TABLE lock strength reduction patch is unsafe Reply-To:

2014-03-22 Thread Simon Riggs
On 21 March 2014 16:11, Simon Riggs si...@2ndquadrant.com wrote:

 + * Be careful to ensure this function is called for Tables and Indexes 
 only.
 + * It is not currently safe to be called for Views because security_barrier
 + * is listed as an option and so would be allowed to be set at a level 
 lower
 + * than AccessExclusiveLock, which would not be correct.

 This statement is accepted and takes only ShareUpdateExclusiveLock:

   alter table information_schema.triggers set (security_barrier = true);

 I suggest adding a LOCKMODE field to relopt_gen and adding a
 reloptions_locklevel() function to determine the required level from an
 options list.  That puts control of the lock level near the code that
 understands the implications for each option.  You can then revert the
 addition of AlterViewInternal() and some of the utility.c changes.

 Sure, that's how we code it, but I'm not sure we should introduce that
 feature. The above weirdness is not itself justification.

OK, will follow this path. It's a good idea since it encourages
authors of new options to consider properly the lock level that will
be required.

-- 
 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: Fwd: [HACKERS] Proposal: variant of regclass

2014-03-22 Thread Amit Kapila
On Wed, Feb 26, 2014 at 11:56 AM, Yugo Nagata nag...@sraoss.co.jp wrote:
 Thanks for your a lot of comments. I revised the patch according to
 comments from Robert Haas and Marti Raudsepp.

I have started looking into this patch and below are my
initial findings:

1. Dependency is not recorded when to_regclass is used,
however it is recorded when ::regclass is used. Below is
test to demonstrate this point. This change in behaviour is
neither discussed nor mentioned in docs along with patch.

usage of ::regclass

create sequence my_seq;
create table t1 (c1 int, c2 int default 'my_seq'::regclass);
insert into t1 values(1);
insert into t1 values(2);
select * from t1;
 c1 |  c2
+---
  1 | 16390
  2 | 16390
(2 rows)

drop sequence my_seq;
ERROR:  cannot drop sequence my_seq because other
objects depend on it
DETAIL:  default for table t1 column c2 depends on
sequence my_seq
HINT:  Use DROP ... CASCADE to drop the dependent
objects too.

Check pg_depend has entry for dependency of default
value of table-column on seq.

postgres=# select * from pg_depend where refobjid = 16390;
 classid | objid | objsubid | refclassid | refobjid | refobjsubid | deptype
-+---+--++--+-+-
1247 | 16391 |0 |   1259 |16390 |   0 | i
2604 | 16395 |0 |   1259 |16390 |   0 | n
(2 rows)

postgres=# select oid,adrelid from pg_attrdef;
  oid  | adrelid
---+-
 16395 |   16392
(1 row)

usage of to_regclass

create sequence my_seq;
create table t1 (c1 int, c2 int default to_regclass('my_seq'));
insert into t1 values(1);
insert into t1 values(2);
select * from t1;
 c1 |  c2
+---
  1 | 16396
  2 | 16396

select * from pg_depend where refobjid = 16396;
 classid | objid | objsubid | refclassid | refobjid | refobjsubid | deptype
-+---+--++--+-+-
1247 | 16397 |0 |   1259 |16396 |   0 | i
(1 row)

There is only one entry for pg_type which means the
dependent object on sequence is only its type, so it will
allow to drop sequence.

postgres=# drop sequence my_seq;
DROP SEQUENCE

2.
! if (!((Form_pg_type) GETSTRUCT(tup))-typisdefined)
! ereport(ERROR,
! (errcode(ERRCODE_UNDEFINED_OBJECT),
! errmsg(type \%s\ is only a shell,
! TypeNameToString(typeName)),
! parser_errposition(NULL, typeName-location)));

In this case it is not exactly same as object does not exist
so I think we should not avoid error in this case
and infact OID is valid for such a case, but in else part
you have assigned it as InvalidOid which seems to be wrong.

3.
regproc_guts()
! {
! if (!missing_ok)
! ereport(ERROR,
! (errcode(ERRCODE_AMBIGUOUS_FUNCTION),
! errmsg(more than one function named \%s\,
! pro_name_or_oid)));
! return false;
! }

In to_regproc(), this patch is trying to supress error other
object-name-not-found which as far as I could read the thread
was not the original idea and even the description in docs
says only about object-name-not-found case.
Doc Description
+ similar to the regclass, regproc, regoper and regtype casts, except
+ that they return NULL when the object is not found, instead of raising
+ an error.

Even if you want to avoid the other error('s) like
above, then I think it's better to mention the same in docs.

I am bit worried, how is user going to distinguish between
the cases when object-not-found and more-than-one-object.

I think such a problem would not arise if we write a function
for regprocedure rather than for regproc, also manual says regprocedure
is more appropriate for most usecases.
http://www.postgresql.org/docs/devel/static/datatype-oid.html

Also I think one other advantage for doing so is that we
don't need to pass missing_ok paramter to some of the functions
like FuncnameGetCandidates(), OpernameGetCandidates().

I understand that you might be using regproc/regoper or might have
a usecase for it, but is it possible for you to use regprocedure/regoperator
instead of regproc/regoper?


4.
+entrytyperegclass/type/entry
+entryget the table oid/entry

Shouldn't it be better to describe it as  get the relation oid as
it can give oid for other objects like index as well.

5.
+   para
+ to_regclass, to_regproc, to_regoper and to_regtype are functions
+ similar to the regclass, regproc, regoper and regtype casts, except

In above description, it is better to use 'object identifier types'
rather than 'casts'.

6.
!  * Guts of regoperin and to_regproc.
Here it should be regprocin

7.
* If not found and missing_ok is true, returns false instead of raising
an error.

Above line is more than 80 chars, it should be less than
80 char limit. This needs to be corrected whereever this line is used.

8.
!  * Guts of regtypein and to_regtype.
!  * If the classname is found, returns true and the OID is stored into
*typid_p.

typo in *If the classname is found*, it should be type is found.


With Regards,
Amit 

Re: [HACKERS] Review: plpgsql.extra_warnings, plpgsql.extra_errors

2014-03-22 Thread Piotr Stefaniak


 +myextra = (int *) malloc(sizeof(int));

Please consider not casting malloc(). See 
http://c-faq.com/malloc/mallocnocast.html



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


Re: [HACKERS] [RFC] What should we do for reliable WAL archiving?

2014-03-22 Thread Martijn van Oosterhout
On Sat, Mar 22, 2014 at 06:22:37AM +0900, MauMau wrote:
 From: Jeff Janes jeff.ja...@gmail.com
 Do people really just copy the files from one directory of local
 storage to
 another directory of local storage?  I don't see the point of that.
 
 It makes sense to archive WAL to a directory of local storage for
 media recovery.  Here, the local storage is a different disk drive
 which is directly attached to the database server or directly
 connected through SAN.

I'm one of those peope. They are archived into a local directory in
preparation for an rsync over ssh.

 The recommendation is to refuse to overwrite an existing file of the same
 name, and exit with failure.  Which essentially brings archiving
 to a halt,
 because it keeps trying but it will keep failing.  If we make a custom
 version, one thing it should do is determine if the existing archived file
 is just a truncated version of the attempting-to-be archived file, and if
 so overwrite it.  Because if the first archival command fails with a
 network glitch, it can leave behind a partial file.
 
 What I'm trying to address is just an alternative to cp/copy which
 fsyncs a file.  It just overwrites an existing file.

I ran into a related problem with cp, where halfway the copy the disk
was full and I was left with half a WAL file. This caused the rsync to
copy only half a file and the replication broke. This is clearly a
recoverable situation, but it didn't recover in this case.

 Yes, you're right, the failed archive attempt leaves behind a
 partial file which causes subsequent attempts to fail, if you follow
 the PG manual. That's another undesirable point in the current doc.
 To overcome this, someone on this ML recommended me to do cp %p
 /archive/dir/%f.tmp  mv /archive/dir/%f.tmp /archive/dir/%f.
 Does this solve your problem?

This would probably have handled it, but I find it odd that there's
program to handle restoring of archives properly, but on the archiving
side you have to cobble together your own shell scripts which fail in
various corner cases.

I'd love a program that just Did The Right Thing.

Have a nice day,
-- 
Martijn van Oosterhout   klep...@svana.org   http://svana.org/kleptog/
 He who writes carelessly confesses thereby at the very outset that he does
 not attach much importance to his own thoughts.
   -- Arthur Schopenhauer


signature.asc
Description: Digital signature


Re: [HACKERS] Partial index locks

2014-03-22 Thread Thom Brown
On 22 March 2014 05:32, Tom Lane t...@sss.pgh.pa.us wrote:
 Thom Brown t...@linux.com writes:
 Is it necessary for a partial index that doesn't include the row to be
 involved in locking?

 Yes.  You can't determine whether the index needs to get a new entry
 without examining its metadata, and that's what the lock is mainly about.

I see.  Why does this apply to deletes too?

 The only possible alternative would be to take the minimum possible
 lock (AccessShareLock) on each index so its metadata would hold still,
 and then upgrade that to RowExclusiveLock on the one(s) we find need
 insertions.  This is not better; it means *more* lock management traffic
 not less, and lock upgrades increase the potential for deadlocks.

Yes, I can see that wouldn't be an improvement.
-- 
Thom


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


Re: [HACKERS] Inheritance of foregn key constraints.

2014-03-22 Thread Andrzej Mazurkiewicz
Good Morning.

1. At the beginning some explanations.

I am a lazy person that tries not to reinvent a wheel.
So I try to use postgres way of automatic processing, i. e. automatic removing 
dependent objects (which I consider an elegant solution and I really like it). 
A a result, I have used the pg_depend table to force to remove dependent 
entries.

2. At the moment the following behavior is a standard one for postgres.
- a child table (inheriting form a parent table(s) no FK) can be dropped;
- a referred table (master) can be freely dropped with a CASCADE option 
(causing dropping of the FK);
- a referring table (detail) can be freely dropped (causing automatic dropping 
of the FK);
- a CHECK constraint is inherited and the inheritance can be removed freely 
although leaving the CHECK constraint (no FK);
- an inherited table with CHECK constraint can be freely dropped (no FK);
- inheritance can be added for existing tables and it can be removed (no FK).

3. The following decisions should be taken for the FK inheritance (partly 
common issues, however  I try to be precise).
- (GENERAL statement) Are modifications of a master side hierarchy (a referred 
side) allowed without dropping the FK?
- (GENERAL statement) Are modifications of a detail side hierarchy (a referred 
side) allowed without dropping the FK?
- Is detaching childs allowed in the master and detail hierarchy without 
dropping the FK?
- Is dropping tables allowed in the master and detail hierarchy without 
dropping the FK?
- Is adding inheritance allowed to the master and detail hierarchies without 
dropping the FK?
- Is creating inheriting tables allowed in the master and detail hierarchies 
without dropping the FK?

It would be good if the decisions were consistent with the existing behavior.

The consequences of the decisions are rather far going. For large databases 
adding the FK constraint might last hours or days or perhaps weeks.

For my databases, although such modification would last hours and sometimes I 
have strange and changing ideas - I can live with those hours.

Personally I would vote that the above modifications SHOULD BE ALLOWED. Simply, 
because we do not drop the whole master or detail hierarchy but modify it and 
it gives certain flexibility to manipulating the schema.

The above flexibility is similar to adding inheritance to the existing tables 
and removing inheritance for them. We do not need to create another inherited 
table and to move data into it from the existing table.

3. Perhaps , after making the above decisions, a discussion about an 
implementing changes should be continued.

4. 
  My patch need one change that might be of significance.
  A type of the depencencies (pg_depend) among the FK constraint
  (pg_constraint) and the corresponding RI_ConstraintTrigger triggers has
  to be changed from DEPENDENCY_INTERNAL to DEPENDENCY_AUTO.
 
 So in other words, somebody could (accidentally or maliciously) break the
 constraint by dropping one of its implementation triggers.  I doubt that's
 acceptable.

The present postgres behavior ALLOWS accidental or malicious break the 
constraint by dropping one of its implementation triggers. Please ref. to the 
following example.

The following script has been run by the postgres user.

CREATE DATABASE lipa;
\c lipa
CREATE TABLE master (master_a int, CONSTRAINT pk_master PRIMARY KEY 
(master_a));
CREATE TABLE detail (master_a int, detail_a int, CONSTRAINT fk0_detail FOREIGN 
KEY (master_a) REFERENCES master(master_a));
SELECT oid, tgrelid, tgname FROM pg_trigger ;
DELETE FROM pg_trigger WHERE oid = (SELECT min(oid) FROM pg_trigger WHERE 
tgname LIKE 'RI_ConstraintTrigger%' LIMIT 1);
SELECT oid, tgrelid, tgname FROM pg_trigger ;
DROP TABLE detail;
DROP TABLE master;
\c postgres
DROP DATABASE lipa;


The results of the run are as follows.

psql -f test-malicious-dropping-FK-triggers.sql postgres


CREATE DATABASE
You are now connected to database lipa as user postgres.
CREATE TABLE
CREATE TABLE
  oid  | tgrelid |tgname
---+-+--
 39898 |   39889 | RI_ConstraintTrigger_a_39898
 39899 |   39889 | RI_ConstraintTrigger_a_39899
 39900 |   39894 | RI_ConstraintTrigger_c_39900
 39901 |   39894 | RI_ConstraintTrigger_c_39901
(4 rows)

DELETE 1
  oid  | tgrelid |tgname
---+-+--
 39899 |   39889 | RI_ConstraintTrigger_a_39899
 39900 |   39894 | RI_ConstraintTrigger_c_39900
 39901 |   39894 | RI_ConstraintTrigger_c_39901
(3 rows)

psql:test-malicious-dropping-FK-triggers.sql:8: ERROR:  could not find tuple 
for trigger 39898
psql:test-malicious-dropping-FK-triggers.sql:9: ERROR:  could not find tuple 
for trigger 39898
You are now connected to database postgres as user postgres.
DROP DATABASE

 
  If this modification is not applied, the detail child table cannot be
  dropped without prevous dropping the whole FK constraint because the
  removing operation 

Re: [HACKERS] Review: plpgsql.extra_warnings, plpgsql.extra_errors

2014-03-22 Thread Tom Lane
Piotr Stefaniak postg...@piotr-stefaniak.me writes:
 +myextra = (int *) malloc(sizeof(int));

 Please consider not casting malloc(). See 

That code is per project style, and should stay that way.

 http://c-faq.com/malloc/mallocnocast.html

That argument is entirely bogus, as it considers only one possible way
in which the call could be wrong; a way that is of very low probability
in PG usage, since we include stdlib.h in our core headers.  Besides
which, as noted in the page itself, most modern compilers would warn
anyway if you forgot the inclusion.

On the other side, coding with the explicit cast helps guard against far
more dangerous coding errors, which the compiler will *not* help you with.
What if myextra is actually of type int64 *?  In that case you probably
meant to make enough space for an int64 not an int.  But without the cast,
you won't be told you did anything wrong.  This is a particular hazard if
you change your mind later on about the type of myextra.  (A colleague
at Salesforce got burnt in exactly that way, just a couple days ago.)

So, general policy around here is that malloc and palloc calls should look
like

ptr = (foo *) malloc(n * sizeof(foo));

so that there's a direct, visible connection between the size calculation
and the type of the resulting pointer.

I'm aware that there are some places in the code that fail to do this,
but they are not models to emulate.

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] Partial index locks

2014-03-22 Thread Tom Lane
Thom Brown t...@linux.com writes:
 On 22 March 2014 05:32, Tom Lane t...@sss.pgh.pa.us wrote:
 Yes.  You can't determine whether the index needs to get a new entry
 without examining its metadata, and that's what the lock is mainly about.

 I see.  Why does this apply to deletes too?

The executor doesn't take locks on indexes for a delete.  I think the
planner probably does, though, since it wants to look at all the indexes
to see if any can be used to satisfy WHERE searches.

Possibly it would be worth hacking the planner to only take
AccessShareLock not RowExclusiveLock on target indexes in DELETE.
I can't get very excited about that though; in what circumstances
would it actually make a difference?

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] Partial index locks

2014-03-22 Thread Thom Brown
On 22 March 2014 15:04, Tom Lane t...@sss.pgh.pa.us wrote:
 Thom Brown t...@linux.com writes:
 On 22 March 2014 05:32, Tom Lane t...@sss.pgh.pa.us wrote:
 Yes.  You can't determine whether the index needs to get a new entry
 without examining its metadata, and that's what the lock is mainly about.

 I see.  Why does this apply to deletes too?

 The executor doesn't take locks on indexes for a delete.  I think the
 planner probably does, though, since it wants to look at all the indexes
 to see if any can be used to satisfy WHERE searches.

 Possibly it would be worth hacking the planner to only take
 AccessShareLock not RowExclusiveLock on target indexes in DELETE.
 I can't get very excited about that though; in what circumstances
 would it actually make a difference?

Well I wasn't looking for things to optimise, so much as trying to
understand the logic behind the existing behaviour.  But thanks for
the explanation.

-- 
Thom


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


Re: [HACKERS] Inheritance of foregn key constraints.

2014-03-22 Thread Tom Lane
Andrzej Mazurkiewicz andr...@mazurkiewicz.org writes:
 So in other words, somebody could (accidentally or maliciously) break the
 constraint by dropping one of its implementation triggers.  I doubt that's
 acceptable.

 The present postgres behavior ALLOWS accidental or malicious break the 
 constraint by dropping one of its implementation triggers. Please ref. to the 
 following example.
 The following script has been run by the postgres user.

Well, right there you lost me, because superusers are exempt from all
permissions checks by definition; and in particular, direct manipulations
of the system catalogs by superusers are always out of scope for
discussions of what the system should try to protect itself against.
(Try delete from pg_proc; in a scratch database sometime.)

My point is that without the internal dependency, a normal user could do
standard SQL commands (ie DROP TRIGGER) and break the FK that way.
That's the case that's not acceptable.

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] Review: plpgsql.extra_warnings, plpgsql.extra_errors

2014-03-22 Thread Piotr Stefaniak

On 03/22/2014 04:00 PM, Tom Lane wrote:

That argument is entirely bogus, as it considers only one possible way
in which the call could be wrong; a way that is of very low probability
in PG usage, since we include stdlib.h in our core headers.  Besides
which, as noted in the page itself, most modern compilers would warn
anyway if you forgot the inclusion.

Apart from what the page says, I also think of casting malloc() as bad 
style and felt the need to bring this up. But since you pointed out why 
you don't want to remove the cast, I withdraw my previous suggestion.



On the other side, coding with the explicit cast helps guard against far
more dangerous coding errors, which the compiler will *not* help you with.
What if myextra is actually of type int64 *?  In that case you probably
meant to make enough space for an int64 not an int.  But without the cast,
you won't be told you did anything wrong.  This is a particular hazard if
you change your mind later on about the type of myextra.  (A colleague
at Salesforce got burnt in exactly that way, just a couple days ago.)


So perhaps this alternative:
  myextra = malloc(sizeof *myextra);


PS.
Coding style matters to me, but I was and still am far from insisting on 
anything.



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


Re: [HACKERS] Review: plpgsql.extra_warnings, plpgsql.extra_errors

2014-03-22 Thread Tom Lane
Piotr Stefaniak postg...@piotr-stefaniak.me writes:
 Apart from what the page says, I also think of casting malloc() as bad 
 style and felt the need to bring this up.

Well, that's a value judgement I don't happen to agree with.  Yeah, it'd
be better if the language design were such that we could avoid explicit
casting everywhere, but in this context casting is less risky than not
casting.

 So perhaps this alternative:
myextra = malloc(sizeof *myextra);

[ shrug... ]  That's about a wash for this exact use case, but it gets
messy as soon as the lefthand side is anything more complicated than a
simple variable name.  And it doesn't scale to cases where the malloc
result isn't directly assigned to anything --- for example, what if
you want to pass the result of palloc() directly to some other
function, or return it from the current function?

The bigger picture though is that the style with the explicit cast is
already extremely widely used in Postgres.  That being the case,
conforming to project style is better than using some inconsistent
convention, regardless of your personal views about whether there's a
better way to do 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] Review: plpgsql.extra_warnings, plpgsql.extra_errors

2014-03-22 Thread Piotr Stefaniak

On 03/22/2014 04:00 PM, Tom Lane wrote:

On the other side, coding with the explicit cast helps guard against far
more dangerous coding errors, which the compiler will*not*  help you with.
What if myextra is actually of type int64 *?
Indeed, neither gcc -Wall -Wextra -std=c89 -pedantic nor clang 
-Weverything -Wno-shadow -std=c89 -pedantic issues a warning in such 
case. clang --analyze, however, does. Perhaps TenDRA would, if it ever 
worked.


This message is meant to be merely informative, since I've put some 
effort into this test. I'm not trying to argue.
#include stdlib.h

typedef long int int64;

int main(void)
{
  int  *myextra;

  /* with explicit casting */
  myextra = (int *) malloc(sizeof (int));
free(myextra);

  /* with no explicit casting */
  myextra = malloc(sizeof (int));
free(myextra);
  
  /* myextra now becomes int64 */
  {
int64 *myextra;

/* with explicit casting */
myextra = (int *) malloc(sizeof (int)); /* [1], [2]. and [3] warn here */
  free(myextra);

/* with no explicit casting */
myextra = malloc(sizeof (int)); /* Only [3] warns here */
  free(myextra);
  }

  return 0;
}

/*
 1: gcc 4.8.2: gcc -Wall -Wextra -std=c89 -pedantic /tmp/test.c
 2: clang 3.5.0: clang -Weverything -Wno-shadow -std=c89 -pedantic /tmp/test.c
 3: clang 3.5.0: clang --analyze /tmp/test.c
 */

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


Re: [HACKERS] psql blows up on BOM character sequence

2014-03-22 Thread Jim Nasby

On 3/21/14, 8:13 PM, David E. Wheeler wrote:

On Mar 21, 2014, at 2:16 PM, Andrew Dunstan and...@dunslane.net wrote:


Surely if it were really a major annoyance, someone would have sent code to fix 
it during the last 4 years and more since the above.

I suspect it's a minor annoyance :-)

But by all means add it to the TODO list if it's not there already.


I have cleaned up many a BOM added to files that made psql blow up. I think 
PGAdmin III was a culprit, though I’m not sure (I don’t use, it, cleaned up 
after coworkers who do).


Yes, my coworker that figured out what the problem was said the culprit here is 
actually pgAdmin. :(
--
Jim C. Nasby, Data Architect   j...@nasby.net
512.569.9461 (cell) http://jim.nasby.net


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


Re: [HACKERS] Partial index locks

2014-03-22 Thread Jim Nasby

On 3/21/14, 7:59 PM, Vik Fearing wrote:

On 03/22/2014 01:43 AM, Thom Brown wrote:

Hi,

I've created a table with 1000 partial indexes.  Each one matches
exactly one row based on the predicate WHERE id = value.

However, when I perform an UPDATE of a single row in a transaction,
I've noticed that all those partial indexes show up in pg_locks with
RowExclusiveLock.

Only 2 of those indexes have a reference to the row: the primary key
and a single partial index.

Is it necessary for a partial index that doesn't include the row to be
involved in locking?


What if the update puts the row into one of the other indexes?


Also, why are you doing this in the first place? I'm guessing you measured some 
non-trivial performance improvement from doing this; could you share that with 
us?
--
Jim C. Nasby, Data Architect   j...@nasby.net
512.569.9461 (cell) http://jim.nasby.net


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


Re: [HACKERS] psql blows up on BOM character sequence

2014-03-22 Thread Jim Nasby

On 3/21/14, 4:54 PM, Tom Lane wrote:

Merlin Moncure mmonc...@gmail.com writes:

There is no way for psql to handle that case though unless you'd strip
*all* BOMs encountered.  Compounding this problem is that there's no
practical way AFAIK to send multiple file to psql via single command
line invocation.  If you pass multiple -f arguments all but one is
ignored.


Well, that seems like a solvable but rather independent problem.
I guess one issue is how you'd define the meaning of --single ...
one transaction per run, or one per file?


Well, if you're catting multiple files into psql -1, you'd get all the files in 
one transaction, right? So I'd say that's what should happen.
--
Jim C. Nasby, Data Architect   j...@nasby.net
512.569.9461 (cell) http://jim.nasby.net


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


Re: [HACKERS] ALTER TABLE lock strength reduction patch is unsafe

2014-03-22 Thread Jim Nasby

On 2/26/14, 9:15 AM, Simon Riggs wrote:

On 26 February 2014 13:38, Andres Freundand...@2ndquadrant.com  wrote:

Hi,

On 2014-02-26 07:32:45 +, Simon Riggs wrote:

 * This definitely should include isolationtester tests actually
performing concurrent ALTER TABLEs. All that's currently there is
tests that the locklevel isn't too high, but not that it actually works.


There is no concurrent behaviour here, hence no code that would be
exercised by concurrent tests.


Huh? There's most definitely new concurrent behaviour. Previously no
other backends could have a relation open (and locked) while it got
altered (which then sends out relcache invalidations). That's something
that should be tested.

It has been. High volume concurrent testing has been performed, per
Tom's original discussion upthread, but that's not part of the test
suite.
For other tests I have no guide as to how to write a set of automated
regression tests. Anything could cause a failure, so I'd need to write
an infinite set of tests to prove there is no bug*somewhere*. How
many tests are required? 0, 1, 3, 30?


ISTM that we don't want hand-written tests here, but rather generated tests 
that actually hit all potential cases. Obviously we'd never run that as part of 
normal reqression, but farm animals certainly could.
--
Jim C. Nasby, Data Architect   j...@nasby.net
512.569.9461 (cell) http://jim.nasby.net


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


Re: [HACKERS] Partial index locks

2014-03-22 Thread Thom Brown
On 22 March 2014 16:28, Jim Nasby j...@nasby.net wrote:
 On 3/21/14, 7:59 PM, Vik Fearing wrote:

 On 03/22/2014 01:43 AM, Thom Brown wrote:

 Hi,

 I've created a table with 1000 partial indexes.  Each one matches
 exactly one row based on the predicate WHERE id = value.

 However, when I perform an UPDATE of a single row in a transaction,
 I've noticed that all those partial indexes show up in pg_locks with
 RowExclusiveLock.

 Only 2 of those indexes have a reference to the row: the primary key
 and a single partial index.

 Is it necessary for a partial index that doesn't include the row to be
 involved in locking?


 What if the update puts the row into one of the other indexes?


 Also, why are you doing this in the first place? I'm guessing you measured
 some non-trivial performance improvement from doing this; could you share
 that with us?

Heh, no.  I was just experimenting with various things, and also
trying to break stuff.

-- 
Thom


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


Re: [HACKERS] jsonb status

2014-03-22 Thread Bruce Momjian
On Sat, Mar 22, 2014 at 01:53:06PM -0700, Peter Geoghegan wrote:
 Attached is v12. I think I've brought this as far as I can.
 
 This is mostly just bug fixes, and some additional refactoring. I've
 incorporated Andres' feedback. The only points that I think worth
 noting are:
 
 * The documentation has been significantly expanded to discuss
 containment further, since it's a significant part of the feature.
 This could probably use some polishing and general scrutiny, which is
 something that Andrew may consider. I didn't have time to go over it
 to the extent that I'd prefer.
 
 * I altered containment semantics slightly. Now, it is not possible
 for a raw scalar to contain a single-element array of the same
 scalar, while the inverse is still possible (raw scalars still contain
 themselves too). This made sense to me, and is consistent with the
 behavior of the B-Tree operator class, where a raw scalar is not equal
 to a single-element array of the same scalar. Rather, array is greater
 than the scalar on the basis of its type alone, as at every other
 nesting level. The fact that an array can contain a raw scalar is
 explicitly presented as an exception to the general principle that
 containment needs to be at the same nesting level.
 
 I'm not going to go into the details of the bugs fixed, since no one
 reported them here, and since they're trivial in nature. For full
 details, you can review the log of our publicly accessible feature
 branch.

What did you decide about hashing values in indexes vs. putting them in
literally?

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

  + Everyone has their own god. +


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


Re: [HACKERS] jsonb status

2014-03-22 Thread Peter Geoghegan
On Sat, Mar 22, 2014 at 5:22 PM, Bruce Momjian br...@momjian.us wrote:
 What did you decide about hashing values in indexes vs. putting them in
 literally?

There are two GIN opclasses supplied. There is a default, which
supports more operators (various existence operators - see the
documentation). There is an alternative called jsonb_hash_ops that
only supports containment, and performs considerably better than the
default. Containment *is* the compelling operator to support, though -
you can do rather a lot with it. This must be what you're referring
to, since I recall you blogged about the response it got at pgConf.EU.
Both are available.


-- 
Peter Geoghegan


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


Re: [HACKERS] Useless Replica Identity: NOTHING noise from psql \d

2014-03-22 Thread Bruce Momjian
On Tue, Dec 17, 2013 at 09:37:09AM -0500, Robert Haas wrote:
  Patch attached.
 
 I vote for showing it only with +, but regardless of whether the value
 matches the expected default.  I'd keep the relkind test, though,
 because I think I noticed that it currently shows up for indexes,
 which is dumb.

Is this the patch you had in mind?  I kept the pg_catalog filter.  Do we
want to always show the replica identity line for \d+?

test= \d+ test
 Table public.test
 Column |  Type   | Modifiers | Storage | Stats target | Description
+-+---+-+--+-
 x  | integer |   | plain   |  |
Replica Identity: full
Has OIDs: no

I used lower-case for the value, rather than all-caps.

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

  + Everyone has their own god. +
diff --git a/src/bin/psql/describe.c b/src/bin/psql/describe.c
new file mode 100644
index a194ce7..a75fc82
*** a/src/bin/psql/describe.c
--- b/src/bin/psql/describe.c
*** describeOneTableDetails(const char *sche
*** 2345,2358 
  			printTableAddFooter(cont, buf.data);
  		}
  
! 		if ((tableinfo.relkind == 'r' || tableinfo.relkind == 'm') 
! 			tableinfo.relreplident != 'd'  tableinfo.relreplident != 'i')
  		{
  			const char *s = _(Replica Identity);
  
  			printfPQExpBuffer(buf, %s: %s,
  			  s,
! 			  tableinfo.relreplident == 'n' ? NOTHING : FULL);
  			printTableAddFooter(cont, buf.data);
  		}
  
--- 2345,2358 
  			printTableAddFooter(cont, buf.data);
  		}
  
! 		if (verbose  (tableinfo.relkind == 'r' || tableinfo.relkind == 'm') 
! 			strcmp(schemaname, pg_catalog) != 0)
  		{
  			const char *s = _(Replica Identity);
  
  			printfPQExpBuffer(buf, %s: %s,
  			  s,
! 			  tableinfo.relreplident == 'n' ? nothing : full);
  			printTableAddFooter(cont, buf.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: Fwd: [HACKERS] Proposal: variant of regclass

2014-03-22 Thread Amit Kapila
On Sat, Mar 22, 2014 at 1:17 PM, Amit Kapila amit.kapil...@gmail.com wrote:
 On Wed, Feb 26, 2014 at 11:56 AM, Yugo Nagata nag...@sraoss.co.jp wrote:
 Thanks for your a lot of comments. I revised the patch according to
 comments from Robert Haas and Marti Raudsepp.

 I have started looking into this patch and below are my
 initial findings:

 1. Dependency is not recorded when to_regclass is used,
 however it is recorded when ::regclass is used. Below is
 test to demonstrate this point. This change in behaviour is
 neither discussed nor mentioned in docs along with patch.

I think this is expected as per current design, because for
functions, it will create dependency on function (funcid), but
not on it's argument values. So I think for this behaviour, we might
need to mention in docs that to_regclass() and other newly added
functions will behave differently for creation of dependencies.

Anyone has any objection for this behaviour difference between
usage of ::regclass and to_regclass()?


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


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