Re: [HACKERS] Double sorting split patch

2011-10-06 Thread Heikki Linnakangas

On 05.10.2011 15:59, Alexander Korotkov wrote:

Path without allocating extra bytes is attached.
I run some more detailed tests on geonames and two smaller datasets from
rtreeportal.org.


Ok, thanks. Looks good to me now, so committed.

--
  Heikki Linnakangas
  EnterpriseDB   http://www.enterprisedb.com

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


Re: [HACKERS] Double sorting split patch

2011-10-06 Thread Alexander Korotkov
On Thu, Oct 6, 2011 at 11:06 AM, Heikki Linnakangas 
heikki.linnakan...@enterprisedb.com wrote:

 On 05.10.2011 15:59, Alexander Korotkov wrote:

 Path without allocating extra bytes is attached.
 I run some more detailed tests on geonames and two smaller datasets from
 rtreeportal.org.


 Ok, thanks. Looks good to me now, so committed.

Thanks. I'm going to continue work on application of this split method in
following areas:
1) range types
2) seg contrib module
3) cube contrib module (there situation is not so easy, probably some
heuristic of split method selection would be required)
Do you think that separation of some common parts of algorithm implemetation
is resonable in order to avoid code duplication? For example, different
application of algorithm could share function of splits enumeration along
axis which takes pointer to consider_split as an argument.

--
With best regards,
Alexander Korotkov.


Re: [HACKERS] Review: Non-inheritable check constraints

2011-10-06 Thread Nikhil Sontakke
Hi Alex,

I didn't care for the changes to gram.y so I reworked it a bit so we
 now pass is_only to AddRelationNewConstraint() (like we do with
 is_local). Seemed simpler but maybe I missed something. Comments?


Hmmm, your patch checks for a constraint being only via:

  !recurse  !recursing

I hope that is good enough to conclusively conclude that the constraint is
'only'. This check was not too readable in the existing code for me anyways
;). If we check at the grammar level, we can be sure. But I remember not
being too comfortable about the right position to ascertain this
characteristic.


 I also moved the is_only check in AtAddCheckConstraint() to before we
 grab and loop through any children. Seemed a bit wasteful to loop
 through the new constraints just to set a flag so that we could bail
 out while looping through the children.


Ditto comment for this function. I thought this function went to great
lengths to spit out a proper error in case of inconsistencies between parent
and child. But if your check makes it simpler, that's good!


 You also forgot to bump Natts_pg_constraint.


Ouch. Thanks for the catch.


 PFA the above changes as well as being rebased against master.


Thanks Alex, appreciate the review!

Regards,
Nikhils


Re: [HACKERS] WIP: SP-GiST, Space-Partitioned GiST

2011-10-06 Thread Oleg Bartunov

We are working on the hackers documentation, hope to submit it before my
himalaya track.

Oleg
On Sun, 2 Oct 2011, Tom Lane wrote:


Heikki Linnakangas heikki.linnakan...@enterprisedb.com writes:

On 06.09.2011 20:34, Oleg Bartunov wrote:

Here is the latest spgist patch, which has all planned features as well as
all overhead, introduced by concurrency and recovery, so performance
measurement should be realistic now.



I'm ignoring the text suffix-tree part of this for now, because of the
issue with non-C locales that Alexander pointer out.


It seems to me that SP-GiST simply cannot work for full text comparisons
in non-C locales, because it's critically dependent on the assumption
that comparisons of strings are consistent with comparisons of prefixes
of those strings ... an assumption that's just plain false for most
non-C locales.

We can dodge that problem in the same way that we did in the btree
pattern_ops opclasses, namely implement the opclass only for the =
operator and the special operators ~~ etc.  I think I favor doing this
for the first round, because it's a simple matter of removing code
that's currently present in the patch.  Even with only = support
the opclass would be extremely useful.

Something we could consider later is a way to use the index for the
regular text comparison operators ( etc), but only when the operator
is using C collation.  This is not so much a matter for the index
implementation as it is about teaching the planner to optionally
consider collation when matching an operator call to the index.  It's
probably going to tie into the unfinished business of marking which
operators are collation sensitive and which are not.

In other news, I looked at the patch briefly, but I don't think I want
to review it fully without some documentation.  The absolute minimum
requirement IMO is documentation comparable to what we have for GIN,
ie a specification for the support methods and some indication of when
you'd use this index type in preference to others.  I'd be willing to
help copy-edit and SGML-ize such documentation, but I do not care to
reverse-engineer it from the code.

regards, tom lane



Regards,
Oleg
_
Oleg Bartunov, Research Scientist, Head of AstroNet (www.astronet.ru),
Sternberg Astronomical Institute, Moscow University, Russia
Internet: o...@sai.msu.su, http://www.sai.msu.su/~megera/
phone: +007(495)939-16-83, +007(495)939-23-83

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


[HACKERS] new GNU tar has warning options, useful for base backups

2011-10-06 Thread Peter Eisentraut
Until recently, when using GNU tar for creating base backups, you'd have
to ignore file changed as we read it and file removed before we read
it warnings, which would require a bit of craftiness if you wanted to
hide these messages while still seeing other warnings and errors from
tar.

As of GNU tar 1.23, there is a --warning option to selectively turn off
warnings.  So you now could/should do something like this:

tar -C $PGDATA -c --warning=no-file-changed --warning=no-file-removed -f 
$outname .

This might be a good addition to the documentation, but I notice that we
don't have any recipe for calling tar in the documentation to begin
with.



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


Re: [HACKERS] Double sorting split patch

2011-10-06 Thread Heikki Linnakangas

On 06.10.2011 11:22, Alexander Korotkov wrote:

Thanks. I'm going to continue work on application of this split method in
following areas:
1) range types
2) seg contrib module
3) cube contrib module (there situation is not so easy, probably some
heuristic of split method selection would be required)
Do you think that separation of some common parts of algorithm implemetation
is resonable in order to avoid code duplication? For example, different
application of algorithm could share function of splits enumeration along
axis which takes pointer to consider_split as an argument.


Dunno. I'd suggest that you just copy-paste the code for now, and we'll 
see how much duplication there is in the end.


--
  Heikki Linnakangas
  EnterpriseDB   http://www.enterprisedb.com

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


Re: [HACKERS] Inserting heap tuples in bulk in COPY

2011-10-06 Thread Heikki Linnakangas

On 25.09.2011 19:01, Robert Haas wrote:

On Wed, Sep 14, 2011 at 6:52 AM, Heikki Linnakangas
heikki.linnakan...@enterprisedb.com  wrote:

Why do you need new WAL replay routines?  Can't you just use the existing
XLOG_HEAP_NEWPAGE support?

By any large, I think we should be avoiding special-purpose WAL entries
as much as possible.


I tried that, but most of the reduction in WAL-size melts away with that.
And if the page you're copying to is not empty, logging the whole page is
even more expensive. You'd need to fall back to retail inserts in that case
which complicates the logic.


Where does it go?  I understand why it'd be a problem for partially
filled pages, but it seems like it ought to be efficient for pages
that are initially empty.


A regular heap_insert record leaves out a lot of information that can be 
deduced at replay time. It can leave out all the headers, including just 
the null bitmap + data. In addition to that, there's just the location 
of the tuple (RelFileNode+ItemPointer). At replay, xmin is taken from 
the WAL record header.


For a multi-insert record, you don't even need to store the RelFileNode 
and the block number for every tuple, just the offsets.


In comparison, a full-page image will include the full tuple header, and 
also the line pointers. If I'm doing my math right, a full-page image 
takes 25 bytes more data per tuple, than the special-purpose 
multi-insert record.


--
  Heikki Linnakangas
  EnterpriseDB   http://www.enterprisedb.com

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


Re: [HACKERS] Inserting heap tuples in bulk in COPY

2011-10-06 Thread Robert Haas
On Thu, Oct 6, 2011 at 7:33 AM, Heikki Linnakangas
heikki.linnakan...@enterprisedb.com wrote:
 A regular heap_insert record leaves out a lot of information that can be
 deduced at replay time. It can leave out all the headers, including just the
 null bitmap + data. In addition to that, there's just the location of the
 tuple (RelFileNode+ItemPointer). At replay, xmin is taken from the WAL
 record header.

 For a multi-insert record, you don't even need to store the RelFileNode and
 the block number for every tuple, just the offsets.

 In comparison, a full-page image will include the full tuple header, and
 also the line pointers. If I'm doing my math right, a full-page image takes
 25 bytes more data per tuple, than the special-purpose multi-insert record.

Interesting.  It's always seemed to me fairly inefficient in general
to store the whole RelFileNode.  For many people, the database and
tablespace OID will be constants, and even if they aren't, there
certainly aren't going to be 96 bits of entropy in the relfilenode.  I
thought about whether we could create some sort of mapping layer,
where say once per checkpoint we'd allocate a 4-byte integer to denote
a relfilenode, and WAL-log that mapping.  Then after that everyone
could just refer to the 4-byte integer instead of the whole
relfilenode.  But it seems like a lot of work for 8 bytes per record.
Then again, if you're getting that much benefit from shaving off 25
bytes per tuple, maybe it is, although I feel like FPW is the elephant
in the room.

But I digress...

-- 
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] patch - fix locale dependent regress test

2011-10-06 Thread Pavel Stehule
Hello

a order of on result in foreign server test depends on locales.

This fix renames a 'cs' identifier to 's0' identifier

Regards

Pavel Stehule

-- 
Sent 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 in walsender when calling out to do_pg_stop_backup (and others?)

2011-10-06 Thread Florian Pflug
On Oct5, 2011, at 15:30 , Magnus Hagander wrote:
 When walsender calls out to do_pg_stop_backup() (during base backups),
 it is not possible to terminate the process with a SIGTERM - it
 requires a SIGKILL. This can leave unkillable backends for example if
 archive_mode is on and archive_command is failing (or not set). A
 similar thing would happen in other cases if walsender calls out to
 something that would block (do_pg_start_backup() for example), but the
 stop one is easy to provoke.

Hm, this seems to be related to another buglet I noticed a while ago,
but then forgot about again. If one terminates pg_basebackup while it's
waiting for all required WAL to be archived, the backend process only
exits once that waiting phase is over. If, like in your failure case,
archive_command fails indefinity (or isn't set), the backend process
stays around forever.

Your patch would improve that only insofar as it'd at least allow an
immediate shutdown request to succeed - as it stands, that doesn't work
because, as you mentioned, the blocked walsender doesn't handle SIGTERM.

The question is, should we do more? To me, it'd make sense to terminate
a backend once it's connection is gone. We could, for example, make
pq_flush() set a global flag, and make CHECK_FOR_INTERRUPTS handle a
broken connection that same way as a SIGINT or SIGTERM.

Thoughts?

 ISTM one way to fix it is the attached, which is to have walsender set
 the global flags saying that we have received sigterm, which in turn
 causes the CHECK_FOR_INTERRUPTS() calls in the routines to properly
 exit the process. AFAICT it works fine. Any holes in this approach?

Seems sensible, but my knowledge about these code paths is quite limited..

 Second, I wonder if we should add a SIGINT handler as well, that would
 make it possible to send a cancel signal. Given that the end result
 would be the same (at least if we want to keep with the walsender is
 simple path), I'm not sure it's necessary - but it would at least
 help those doing pg_cancel_backend()... thoughts?

If all that's needed is a simple SIGINT handler that sets one flag, it'd
say let's add it. 

best regards,
Florian Pflug


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


Re: [HACKERS] alter table only ... drop constraint broken in HEAD

2011-10-06 Thread Robert Haas
On Wed, Oct 5, 2011 at 10:29 PM, Robert Haas robertmh...@gmail.com wrote:
 On Wed, Oct 5, 2011 at 5:53 PM, Alex Hunsaker bada...@gmail.com wrote:
 tldr:

 Seems to be broken by
 http://git.postgresql.org/gitweb/?p=postgresql.git;a=commit;h=4da99ea4231e3d8bbf28b666748c1028e7b7d665
 :
 commit 4da99ea4231e3d8bbf28b666748c1028e7b7d665
 Author: Robert Haas rh...@postgresql.org
 Date:   Mon Jun 27 10:27:17 2011 -0400

    Avoid having two copies of the HOT-chain search logic.

 Hmm... that's pretty terrible.  Yikes.  That commit wasn't intended to
 change any behavior, just to clean up the code, so I think the right
 thing to do here is figure out how I changed the behavior without
 meaning too, rather than to start patching up all the places that
 might have been affected by whatever the behavior change was.  I'm too
 tired to figure this out right now, but I'll spend some time staring
 at it tomorrow.

Oh, I see the problem, and I now agree that it's the DROP CONSTRAINT
code that is buggy.

Here's what happened. In the old code, when index_getnext() is walking
a HOT chain, before returning each tuple, it remembers the XMAX of
that tuple and the offset of the next tuple in the chain.  So in the
following scenario, we can fail to walk the entire HOT chain:

1. index_getnext() returns what is currently the last tuple in the
chain, remembering the xmax and next tid
2. the caller, or some other process, performs a HOT update of that tuple
3. now index_getnext() is called again, but it uses the remembered
xmax and tid, which are now out-of-date

The new code, on the other hand, doesn't remember the xmax and tid;
instead, it waits until the next call to index_getnext(), and then
fetches them from the previous-returned tuple at that time.  So it
always sees the most current values and, therefore, walks the entire
chain.

In this particular case, the DROP CONSTRAINT code is counting on the
fact that it won't see its own updates, even though it is bumping the
command counter after each one, which is clearly wrong.  I don't think
this was really safe even under the old coding, because there's no
guarantee that any particular update will be HOT, so we might have
found our own update anyway; it was just less likely.

I took a look around for other places that might have this same
problem and didn't find any, but I'm not too confident that that means
there are none there, since there are a fair number of things that can
call CommandCounterIncrement() indirectly.  shdepReassignOwned() does
a direct CCI from within a scan, but ISTM that any update we do there
would produce a new tuple that doesn't match the scan, so that should
be OK.

-- 
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] Bug with pg_ctl -w/wait and config-only directories

2011-10-06 Thread Bruce Momjian
Bruce Momjian wrote:
 Bruce Momjian wrote:
  Bruce Momjian wrote:
   Robert Haas wrote:
On Mon, Oct 3, 2011 at 11:04 PM, Bruce Momjian br...@momjian.us wrote:
 OK, here is a patch that adds a -C option to the postmaster so any
 config variable can be dumped, even while the server is running (there
 is no security check because we don't have a user name at this point),
  
  OK, here is an updated patch, with documentation, that I consider ready
  for commit to git head.
 
 And with a --help documentation patch.

Patch applied.  This competes this TODO item:

Allow pg_ctl to work properly with configuration files located outside
the PGDATA directory 

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

  + It's impossible for everything to be true. +

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


[HACKERS] [REVIEW] Patch for cursor calling with named parameters

2011-10-06 Thread Royce Ausburn
Initial Review for patch:

http://archives.postgresql.org/pgsql-hackers/2011-09/msg00744.php


Submission review

The patch is in context diff format and applies cleanly to the git master.  The 
patch includes an update to regression tests.  The regression tests pass.  The 
patch does not include updates to the documentation reflecting the new 
functionality.

Usability review

The patch adds a means of specifying named  cursor parameter arguments in 
pg/plsql.  

The syntax is straight forward:

cur1 CURSOR (param1 int, param2 int, param3 int) for select …;
…
open cur1(param3 := 4, param2 := 1, param1 := 5);


The old syntax continues to work:

cur1 CURSOR (param1 int, param2 int, param3 int) for select …;
…
open cur1(5, 1, 3);


• Does the patch actually implement that?

Yes, the feature works as advertised.

• Do we want that?

I very rarely use pg/plsql, so I won't speak to its utility.  However there has 
been some discussion about the idea:
http://archives.postgresql.org/pgsql-hackers/2010-09/msg01440.php

• Do we already have it?

Not AFAIK

• Does it follow SQL spec, or the community-agreed behavior?

There's some discussion about the syntax ( := or = ), I'm not sure there's a 
consensus yet.


• Does it include pg_dump support (if applicable)?

Yes -- pgplsql functions using named parameters are correctly dumped.

• Are there dangers?

Potentially.  See below.

• Have all the bases been covered?

I don't think so.  The new feature accepts opening a cursor with some parameter 
names not specified:

  open cur1(param3 := 4, 1, param1 := 5);

It seems that if a parameter is not named, its position is used to bind to a 
variable.  For example, the following fail:

psql:plsqltest.sql:26: ERROR:  cursor cur1 argument 2 param2 provided 
multiple times
LINE 10:   open cur1(param3 := 4, 1, param2 := 5);

and

psql:plsqltest.sql:26: ERROR:  cursor cur1 argument 2 param2 provided 
multiple times
LINE 10:   open cur1(param2 := 4, 2, param1 := 5);


I think that postgres ought to enforce some consistency here.  Use one way or 
the other, never both.





I can also produce some unhelpful errors when I give bad syntax.  For example:

psql:plsqltest.sql:28: ERROR:  cursor cur1 argument 1 param1 provided 
multiple times
LINE 11:   open cur1( param3 : = 4, 2, param1 := 5); 
(notice the space between the : and =)

--

psql:plsqltest.sql:28: ERROR:  cursor cur1 argument 1 param1 provided 
multiple times
LINE 11:   open cur1( param3 = 4, 2, param1 := 5); 
(Wrong assignment operator)

--

psql:plsqltest.sql:27: ERROR:  cursor cur1 argument 3 param3 provided 
multiple times
LINE 10:   open cur1( 1 , param3 := 2, param2 = 3 );
(Wrong assignment operator)

--

psql:plsqltest.sql:27: ERROR:  cursor cur1 argument 3 param3 provided 
multiple times
LINE 10: ... open cur1( param3 = param3 , param3 := 2, param2 = 3 );


--

  open cur1( param3 := param3 , param2 = 3, param1 := 1 );

psql:plsqltest.sql:29: ERROR:  column param2 does not exist
LINE 2: ,param2 = 3
 ^
QUERY:  SELECT 1
,param2 = 3
,param3;
CONTEXT:  PL/pgSQL function named_para_test line 7 at OPEN


I haven't been able to make something syntactically incorrect that doesn't 
produce an error, even if the error isn't spot on.  I haven't been able to make 
it crash, and when the syntax is correct, it has always produced correct 
results.

Performance review

I've done some limited performance testing and I can't really see a difference 
between the unpatched and patched master.

• Does it follow the project coding guidelines?

I believe so, but someone more familiar with them will probably spot violations 
better than me =)

• Are there portability issues?

I wouldn't know -- I don't have any experience with C portability.

• Will it work on Windows/BSD etc?

Tested under OS X, so BSD is presumably okay.  No idea about other unixes nor 
windows.

• Are the comments sufficient and accurate?

I'm happy enough with them.

• Does it do what it says, correctly?

Yes, excepting my comments above.

• Does it produce compiler warnings?

Yes:

In file included from gram.y:12962:
scan.c: In function ‘yy_try_NUL_trans’:
scan.c:16243: warning: unused variable ‘yyg’


But this was not added by this patch -- it's also in the unpatched master.

• Can you make it crash?

No.

--Royce







Re: [HACKERS] checkpoints are duplicated even while the system is idle

2011-10-06 Thread Robert Haas
On Wed, Oct 5, 2011 at 1:19 AM, Fujii Masao masao.fu...@gmail.com wrote:
 While the system is idle, we skip duplicate checkpoints for some
 reasons. But when wal_level is set to hot_standby, I found that
 checkpoints are wrongly duplicated even while the system is idle.
 The cause is that XLOG_RUNNING_XACTS WAL record always
 follows CHECKPOINT one when wal_level is set to hot_standby.
 So the subsequent checkpoint wrongly thinks that there is inserted
 record (i.e., XLOG_RUNNING_XACTS record) since the start of the
 last checkpoint, the system is not idle, and this checkpoint cannot
 be skipped. Is this intentional behavior? Or a bug?

If we can eliminate it in some cases where it isn't necessary, that
seems like a good thing, but I'm not sure I have a good handle on when
it is or isn't necessary.

-- 
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_upgrade - add config directory setting

2011-10-06 Thread Bruce Momjian
Bruce Momjian wrote:
 Bruce Momjian wrote:
  Tom Lane wrote:
   Bruce Momjian br...@momjian.us writes:
Tom Lane wrote:
Yeah.  I think the only sensible way to do this would be to provide an
operating mode for the postgres executable that would just parse the
config file and spit out requested values.
   
That would certainly solve the problem, though it would have to be
backpatched all the way back to 8.4, and it would require pg_upgrade
users to be on newer minor versions of Postgres.
   
   I would just say no to people who expect this to work against older
   versions of Postgres.  I think it's sufficient if we get this into HEAD
   so that it will work in the future.
  
  Well, it is going to work in the future only when the _old_ version is
  9.2+.  Specifically, pg_upgrade using the flag could be patched to just
  9.2, but the flag has to be supported on old and new backends for that
  to work.
 
 OK, I started working on #3, which was to start the servers to find the
 data_directory setting, and developed the attached patch which mostly
 does this.  However, I have found serious problems with pg_ctl -w/wait
 mode and config-only directories (which pg_upgrade uses), and will start
 a new thread to address this issue and then continue with this once that
 is resolved.

OK, I have modified the postmaster in PG 9.2 to allow output of the data
directory, and modified pg_ctl to use that, so starting in PG 9.2 pg_ctl
will work cleanly for config-only directories.

I will now work on pg_upgrade to also use the new flag to find the data
directory from a config-only install.  However, this is only available
in PG 9.2, and it will only be in PG 9.3 that you can hope to use this
feature (if old is PG 9.2 or later).  I am afraid the symlink hack will
have to be used for several more years, and if you are supporting
upgrades from pre-9.2, perhaps forever.

I did find that it is possible to use pg_ctl -w start on a config-only
install using this trick:

  su -l postgres \
-c env PGPORT=\5432\ /usr/lib/postgresql-9.1/bin/pg_ctl start -w \
-t 60 -s -D /var/lib/postgresql/9.1/data/ \
 -o '-D /etc/postgresql-9.1/ \
--data-directory=/var/lib/postgresql/9.1/data/ \
--silent-mode=true'

Unfortunately pg_upgrade doesn't support the -o option which would make
this possible for pg_upgrade.

One idea would be to add -o/-O options to pg_upgrade 9.2 to allow this
to work even with old installs, but frankly, this is so confusing I am
not sure we want to encourage people to do things like this.  Of course,
the symlink hack is even worse, so maybe there is some merit to this.

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

  + It's impossible for everything to be true. +

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


Re: [HACKERS] Inserting heap tuples in bulk in COPY

2011-10-06 Thread Heikki Linnakangas

On 06.10.2011 15:11, Robert Haas wrote:

On Thu, Oct 6, 2011 at 7:33 AM, Heikki Linnakangas
heikki.linnakan...@enterprisedb.com  wrote:

A regular heap_insert record leaves out a lot of information that can be
deduced at replay time. It can leave out all the headers, including just the
null bitmap + data. In addition to that, there's just the location of the
tuple (RelFileNode+ItemPointer). At replay, xmin is taken from the WAL
record header.

For a multi-insert record, you don't even need to store the RelFileNode and
the block number for every tuple, just the offsets.

In comparison, a full-page image will include the full tuple header, and
also the line pointers. If I'm doing my math right, a full-page image takes
25 bytes more data per tuple, than the special-purpose multi-insert record.


Interesting.  It's always seemed to me fairly inefficient in general
to store the whole RelFileNode.  For many people, the database and
tablespace OID will be constants, and even if they aren't, there
certainly aren't going to be 96 bits of entropy in the relfilenode.  I
thought about whether we could create some sort of mapping layer,
where say once per checkpoint we'd allocate a 4-byte integer to denote
a relfilenode, and WAL-log that mapping.  Then after that everyone
could just refer to the 4-byte integer instead of the whole
relfilenode.  But it seems like a lot of work for 8 bytes per record.
Then again, if you're getting that much benefit from shaving off 25
bytes per tuple, maybe it is, although I feel like FPW is the elephant
in the room.


A very simple optimization would be to leave out tablespace OID 
altogether if it's DEFAULTTABLESPACE_OID, and just set a flag somewhere. 
Then again, we could also just compress the WAL wholesale.


--
  Heikki Linnakangas
  EnterpriseDB   http://www.enterprisedb.com

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


Re: [HACKERS] checkpoints are duplicated even while the system is idle

2011-10-06 Thread Simon Riggs
On Wed, Oct 5, 2011 at 6:19 AM, Fujii Masao masao.fu...@gmail.com wrote:

 While the system is idle, we skip duplicate checkpoints for some
 reasons. But when wal_level is set to hot_standby, I found that
 checkpoints are wrongly duplicated even while the system is idle.
 The cause is that XLOG_RUNNING_XACTS WAL record always
 follows CHECKPOINT one when wal_level is set to hot_standby.
 So the subsequent checkpoint wrongly thinks that there is inserted
 record (i.e., XLOG_RUNNING_XACTS record) since the start of the
 last checkpoint, the system is not idle, and this checkpoint cannot
 be skipped. Is this intentional behavior? Or a bug?

I think it is avoidable behaviour, but not a bug.

Thinking some more about this, IMHO it is possible to improve the
situation greatly by returning to look at the true purpose of
checkpoints. Checkpoints exist to minimise the time taken during crash
recovery, and as starting points for backups/archive recoveries.

The current idea is that if there has been no activity then we skip
checkpoint. But all it takes is a single WAL record and off we go with
another checkpoint. If there hasn't been much WAL activity, there is
not much point in having another checkpoint record since there is
little if any time to be saved in recovery.

So why not avoid checkpoints until we have written at least 1 WAL file
worth of data? That way checkpoint records are always in different
files, so we are safer with regard to primary and secondary checkpoint
records. That would mean in some cases that dirty data would stay in
shared buffers for days or weeks? No, because the bgwriter would clean
it - but even if it did, so what? Recovery will still be incredibly
quick, which is the whole point.

Testing whether we're in a different segment is easy and much simpler
than trying to wriggle around trying to directly fix the problem you
mention. Patch attached.

-- 
 Simon Riggs   http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training  Services


spaced_checkpoints.v1.patch
Description: Binary data

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


Re: [HACKERS] checkpoints are duplicated even while the system is idle

2011-10-06 Thread Tom Lane
Simon Riggs si...@2ndquadrant.com writes:
 The current idea is that if there has been no activity then we skip
 checkpoint. But all it takes is a single WAL record and off we go with
 another checkpoint. If there hasn't been much WAL activity, there is
 not much point in having another checkpoint record since there is
 little if any time to be saved in recovery.

 So why not avoid checkpoints until we have written at least 1 WAL file
 worth of data?

+1, but I think you need to compare to the last checkpoint's REDO
pointer, not to the position of the checkpoint record itself.
Otherwise, the argument falls down if there was a lot of activity
during the last checkpoint (which is not unlikely in these days of
spread checkpoints).

Also I think the comment needs more extensive revision than you gave 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


[HACKERS] Re: [COMMITTERS] pgsql: Ensure that contrib/pgstattuple functions respond to cancel

2011-10-06 Thread Robert Haas
On Wed, Aug 31, 2011 at 4:24 PM, Robert Haas robertmh...@gmail.com wrote:
 On Fri, Apr 2, 2010 at 12:17 PM, Tom Lane t...@postgresql.org wrote:
 Log Message:
 ---
 Ensure that contrib/pgstattuple functions respond to cancel interrupts
 reasonably promptly, by adding CHECK_FOR_INTERRUPTS in the per-page loops.

 Tatsuhito Kasahara

 This patch seems to have overlooked pgstatindex().

Fixed.

-- 
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] Action requested - Application Softblock implemented | Issue report ID341057

2011-10-06 Thread Seiko Ishida (MP Tech Consulting LLC)
Hello all,

Thank you for all your responses to my inquiry.
We are aware that this application version 8.2.x is pretty old and PostgreSQL 
will stop releasing updates for the 8.2.X in December 2011.
http://www.postgresql.org/docs/8.2/interactive/release-8-2-22.html

We also confirmed this installation issue was addressed in the latest version 
of the installer. (8.3.x and 8.4.x) However, we would like to warn endusers who 
might be affected by this issue trying to install 8.2.x versions on Windows8, 
and to direct them to install the latest version. You provided the following 
URL, but better yet is there a download site for the newer version we can 
direct endusers to?

http://wiki.postgresql.org/wiki/PostgreSQL_Release_Support_Policy


The dialog box presented to users will look something like this.



Would this download site be a good URL for that?
http://www.enterprisedb.com/products-services-training/pgdownload

Thanks again for your help

Regards,

Seiko Ishida
Microsoft ISV Readiness, EcoSystem Engineering Team
Program Manager
v-sei...@microsoft.commailto:v-sei...@microsoft.com
Ref : 341057

-Original Message-
From: Dave Page [mailto:dp...@pgadmin.org]
Sent: Wednesday, October 05, 2011 7:14 AM
To: Robert Haas
Cc: Heikki Linnakangas; Seiko Ishida (MP Tech Consulting LLC); 
pgsql-hackers@postgresql.org; ISV Readiness W8 bugs; Manmeet Bawa
Subject: Re: [HACKERS] Action requested - Application Softblock implemented | 
Issue report ID341057

On Wed, Oct 5, 2011 at 3:04 PM, Robert Haas 
robertmh...@gmail.commailto:robertmh...@gmail.com wrote:

 That seems strange, since the MSFT guy is reporting that whatever he
 tested did work on Windows 7, but now that you've pointed out that
 Windows 8 won't be released until after we stop supporting 8.2, I am
 filled with uncaring.

It might have appeared to successfully install, but that doesn't mean it's 
setup the way we want it. For example, none of the administrative menu 
shortcuts in that version will do privilege escalation which is more or less 
required in Windows 7+.

--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com The Enterprise PostgreSQL Company


inline: Picture (Device Independent Bitmap) 1.jpg

Re: [HACKERS] Action requested - Application Softblock implemented | Issue report ID341057

2011-10-06 Thread Seiko Ishida (MP Tech Consulting LLC)
Hi Robert,

Great.  I will submit that for the response.
Thanks again for your quick assistance.

Regards,

Seiko Ishida
Microsoft ISV Readiness, EcoSystem Engineering Team
Program Manager
v-sei...@microsoft.commailto:v-sei...@microsoft.com

From: Robert Haas [mailto:robertmh...@gmail.com]
Sent: Wednesday, October 05, 2011 12:00 PM
To: Seiko Ishida (MP Tech Consulting LLC)
Cc: Dave Page; Heikki Linnakangas; pgsql-hackers@postgresql.org; ISV Readiness 
W8 bugs; Manmeet Bawa
Subject: Re: [HACKERS] Action requested - Application Softblock implemented | 
Issue report ID341057

On Wed, Oct 5, 2011 at 2:58 PM, Seiko Ishida (MP Tech Consulting LLC) 
v-sei...@microsoft.commailto:v-sei...@microsoft.com wrote:
Hello all,

Thank you for all your responses to my inquiry.
We are aware that this application version 8.2.x is pretty old and PostgreSQL 
will stop releasing updates for the 8.2.X in December 2011.
http://www.postgresql.org/docs/8.2/interactive/release-8-2-22.html

We also confirmed this installation issue was addressed in the latest version 
of the installer. (8.3.x and 8.4.x) However, we would like to warn endusers who 
might be affected by this issue trying to install 8.2.x versions on Windows8, 
and to direct them to install the latest version. You provided the following 
URL, but better yet is there a download site for the newer version we can 
direct endusers to?

http://wiki.postgresql.org/wiki/PostgreSQL_Release_Support_Policy


The dialog box presented to users will look something like this.



Would this download site be a good URL for that?
http://www.enterprisedb.com/products-services-training/pgdownload

That's not a PostgreSQL community web site, so it wouldn't be appropriate to 
send people there, I think.  However, you could direct them to the PostgreSQL 
download page:

http://www.postgresql.org/download/

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

Re: [HACKERS] [COMMITTERS] pgsql: Force strings passed to and from plperl to be in UTF8 encoding.

2011-10-06 Thread David E. Wheeler
On Oct 5, 2011, at 7:36 PM, Robert Haas wrote:

 The open commitfest? Even if its an important bug fix that should be
 backpatched?
 
 Considering that the issue appears to have been ignored from
 mid-February until early October, I don't see why it should now get to
 jump to the head of the queue.  Other people may have different
 opinions, of course.

Proper encoding of text data to and from Perl is more important every day, as 
it's used for more and more uses beyond ASCII. I think it's important to 
backport such issues modulo behavioral changes.

Best,

David


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


Re: [HACKERS] [REVIEW] Patch for cursor calling with named parameters

2011-10-06 Thread Tom Lane
Royce Ausburn royce...@inomial.com writes:
 Initial Review for patch:
 http://archives.postgresql.org/pgsql-hackers/2011-09/msg00744.php
 The patch adds a means of specifying named  cursor parameter arguments in 
 pg/plsql.  

   • Do we want that?

 I very rarely use pg/plsql, so I won't speak to its utility.  However there 
 has been some discussion about the idea:
 http://archives.postgresql.org/pgsql-hackers/2010-09/msg01440.php

I still think what I said in that message, which is that it's premature
to add this syntax to plpgsql cursors when we have thoughts of changing
it.  There is not any groundswell of demand from the field for named
parameters to cursors, so I think we can just leave this in abeyance
until the function case has settled.

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] checkpoints are duplicated even while the system is idle

2011-10-06 Thread Robert Haas
On Thu, Oct 6, 2011 at 12:06 PM, Tom Lane t...@sss.pgh.pa.us wrote:
 Simon Riggs si...@2ndquadrant.com writes:
 The current idea is that if there has been no activity then we skip
 checkpoint. But all it takes is a single WAL record and off we go with
 another checkpoint. If there hasn't been much WAL activity, there is
 not much point in having another checkpoint record since there is
 little if any time to be saved in recovery.

 So why not avoid checkpoints until we have written at least 1 WAL file
 worth of data?

 +1, but I think you need to compare to the last checkpoint's REDO
 pointer, not to the position of the checkpoint record itself.
 Otherwise, the argument falls down if there was a lot of activity
 during the last checkpoint (which is not unlikely in these days of
 spread checkpoints).

 Also I think the comment needs more extensive revision than you gave it.

If we go with this approach, we presumably also need to update the
documentation, especially for checkpoint_timeout (which will no longer
be a hard limit on the time between checkpoints).

I'm not entirely sure I understand the rationale, though.  I mean, if
very little has happened since the last checkpoint, then the
checkpoint will be very cheap.  In the totally degenerate case Fujii
Masao is reporting, where absolutely nothing has happened, it should
be basically free.  We'll loop through a whole bunch of things, decide
there's nothing to fsync, and call it a day.

-- 
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] [REVIEW] Patch for cursor calling with named parameters

2011-10-06 Thread Robert Haas
On Thu, Oct 6, 2011 at 12:23 PM, Tom Lane t...@sss.pgh.pa.us wrote:
 Royce Ausburn royce...@inomial.com writes:
 Initial Review for patch:
 http://archives.postgresql.org/pgsql-hackers/2011-09/msg00744.php
 The patch adds a means of specifying named  cursor parameter arguments in 
 pg/plsql.

       • Do we want that?

 I very rarely use pg/plsql, so I won't speak to its utility.  However there 
 has been some discussion about the idea:
 http://archives.postgresql.org/pgsql-hackers/2010-09/msg01440.php

 I still think what I said in that message, which is that it's premature
 to add this syntax to plpgsql cursors when we have thoughts of changing
 it.  There is not any groundswell of demand from the field for named
 parameters to cursors, so I think we can just leave this in abeyance
 until the function case has settled.

+1.  However, if that's the route we're traveling down, I think we had
better go ahead and remove the one remaining = operator from hstore
in 9.2:

CREATE OPERATOR = (
LEFTARG = text,
RIGHTARG = text,
PROCEDURE = hstore
);

We've been warning that this operator name was deprecated since 9.0,
so it's probably about time to take the next step, if we want to have
a chance of getting this sorted out in finite time.

-- 
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] checkpoints are duplicated even while the system is idle

2011-10-06 Thread Tom Lane
Robert Haas robertmh...@gmail.com writes:
 I'm not entirely sure I understand the rationale, though.  I mean, if
 very little has happened since the last checkpoint, then the
 checkpoint will be very cheap.  In the totally degenerate case Fujii
 Masao is reporting, where absolutely nothing has happened, it should
 be basically free.  We'll loop through a whole bunch of things, decide
 there's nothing to fsync, and call it a day.

I think the point is that a totally idle database should not continue to
emit WAL, not even at a slow rate.  There are also power-consumption
objections to allowing the checkpoint process to fire up to no purpose.

The larger issue is that we should not only be concerned with optimizing
for high load.  Doing nothing when there's nothing to be done is an
important part of good data-center citizenship.  See also the ongoing
work to avoid unnecessary process wakeups.

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] Patch for cursor calling with named parameters

2011-10-06 Thread Tom Lane
Robert Haas robertmh...@gmail.com writes:
 +1.  However, if that's the route we're traveling down, I think we had
 better go ahead and remove the one remaining = operator from hstore
 in 9.2:

Fair enough.

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] checkpoints are duplicated even while the system is idle

2011-10-06 Thread Robert Haas
On Thu, Oct 6, 2011 at 12:44 PM, Tom Lane t...@sss.pgh.pa.us wrote:
 Robert Haas robertmh...@gmail.com writes:
 I'm not entirely sure I understand the rationale, though.  I mean, if
 very little has happened since the last checkpoint, then the
 checkpoint will be very cheap.  In the totally degenerate case Fujii
 Masao is reporting, where absolutely nothing has happened, it should
 be basically free.  We'll loop through a whole bunch of things, decide
 there's nothing to fsync, and call it a day.

 I think the point is that a totally idle database should not continue to
 emit WAL, not even at a slow rate.  There are also power-consumption
 objections to allowing the checkpoint process to fire up to no purpose.

Hmm, OK.  I still think it's a little funny to say that
checkpoint_timeout will force a checkpoint every N minutes except when
it doesn't, but maybe there's no real harm in that as long as we
document it properly.

-- 
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] Review: Non-inheritable check constraints

2011-10-06 Thread Alex Hunsaker
On Thu, Oct 6, 2011 at 02:42, Nikhil Sontakke nikkh...@gmail.com wrote:
 Hi Alex,

 I didn't care for the changes to gram.y so I reworked it a bit so we
 now pass is_only to AddRelationNewConstraint() (like we do with
 is_local). Seemed simpler but maybe I missed something. Comments?


 Hmmm, your patch checks for a constraint being only via:

   !recurse  !recursing

 I hope that is good enough to conclusively conclude that the constraint is
 'only'. This check was not too readable in the existing code for me anyways
 ;). If we check at the grammar level, we can be sure. But I remember not
 being too comfortable about the right position to ascertain this
 characteristic.

Well I traced through it here was my thinking (maybe should be a comment?):

1: AlterTable() calls ATController() with recurse =
interpretInhOption(stmt-relation-inhOpt
2: ATController() calls ATPrepCmd() with recurse and recursing = false
3: ATPrepCmd() saves the recurse flag with the subtup
AT_AddConstraintRecurse, otherwise the subtype is AT_AddConstraint
4: ATExecCmd() calls ATExecAddConstraint() with recurse == true when
subtype == AT_AddConstraintRecurse, recurse = false otherwise
5: ATExecAddConstraint() calls ATAddCheckConstraint() with recuse and
recursing = false
6: now we are in ATAddCheckConstraint() where recurse ==
interpretInhOption(rv-inhOpt) and recursing == false. Recursing is
only true when ATAddCheckConstaint() loops through children and
recursively calls ATAddCheckConstraint()

So with it all spelled out now I see the constraint must be added to
child tables too check is dead code.

Ill take out that check and then mark it as ready for commiter (and
Ill add any comments about the logic of the recurse flag above if I
can think of a concise way to put it). Sound good?

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


Re: [HACKERS] checkpoints are duplicated even while the system is idle

2011-10-06 Thread Kevin Grittner
Robert Haas robertmh...@gmail.com wrote:
 Tom Lane t...@sss.pgh.pa.us wrote:
 
 I think the point is that a totally idle database should not
 continue to emit WAL, not even at a slow rate.  There are also
 power-consumption objections to allowing the checkpoint process
 to fire up to no purpose.
 
 Hmm, OK.  I still think it's a little funny to say that
 checkpoint_timeout will force a checkpoint every N minutes except
 when it doesn't, but maybe there's no real harm in that as long as
 we document it properly.
 
What will be the best way to determine, from looking only at a
standby, whether replication is healthy and progressing?  Going back
to the 8.1 warm standby days we have run pg_controldata and compared
the Time of latest checkpoint to current time; I'm hoping there's
a better way now, so that we can drop that kludge.  If not, I would
like a way to kick out a checkpoint from the master at some finite
and configurable interval for monitoring purposes.  I'm all for
having a nice, sharp fillet knife, but if that's not available,
please don't take away this rock I chipped at until it had an
edge.
 
Most likely there's something there which I've missed, but it's
really nice to be able to tell the difference between a quiet master
and broken replication.
 
-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] checkpoints are duplicated even while the system is idle

2011-10-06 Thread Simon Riggs
On Thu, Oct 6, 2011 at 5:06 PM, Tom Lane t...@sss.pgh.pa.us wrote:
 Simon Riggs si...@2ndquadrant.com writes:
 The current idea is that if there has been no activity then we skip
 checkpoint. But all it takes is a single WAL record and off we go with
 another checkpoint. If there hasn't been much WAL activity, there is
 not much point in having another checkpoint record since there is
 little if any time to be saved in recovery.

 So why not avoid checkpoints until we have written at least 1 WAL file
 worth of data?

 +1, but I think you need to compare to the last checkpoint's REDO
 pointer, not to the position of the checkpoint record itself.
 Otherwise, the argument falls down if there was a lot of activity
 during the last checkpoint (which is not unlikely in these days of
 spread checkpoints).

Agreed, that's better.

 Also I think the comment needs more extensive revision than you gave it.

OK, will do.

New version attached. Docs later.

Do we want this backpatched? If so, suggest just 9.1 and 9.0?

-- 
 Simon Riggs   http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training  Services


spaced_checkpoints.v2.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] [REVIEW] Patch for cursor calling with named parameters

2011-10-06 Thread David E . Wheeler
On Oct 6, 2011, at 9:46 AM, Tom Lane wrote:

 +1.  However, if that's the route we're traveling down, I think we had
 better go ahead and remove the one remaining = operator from hstore
 in 9.2:
 
 Fair enough.

Would it then be added as an alias for := for named function parameters? Or 
would that come still later?

Best,

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


Re: [HACKERS] [REVIEW] Patch for cursor calling with named parameters

2011-10-06 Thread Tom Lane
David E. Wheeler da...@kineticode.com writes:
 On Oct 6, 2011, at 9:46 AM, Tom Lane wrote:
 +1.  However, if that's the route we're traveling down, I think we had
 better go ahead and remove the one remaining = operator from hstore
 in 9.2:

 Fair enough.

 Would it then be added as an alias for := for named function parameters? Or 
 would that come still later?

Once we do that, it will be impossible not merely deprecated to use =
as an operator name.  I think that has to wait at least another release
cycle or two past where we're using it ourselves.

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] Patch for cursor calling with named parameters

2011-10-06 Thread David E. Wheeler
On Oct 6, 2011, at 10:37 AM, Tom Lane wrote:

 Would it then be added as an alias for := for named function parameters? Or 
 would that come still later?
 
 Once we do that, it will be impossible not merely deprecated to use =
 as an operator name.  I think that has to wait at least another release
 cycle or two past where we're using it ourselves.

Okay. I kind of like := so there's no rush AFAIC. :-)

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


Re: [HACKERS] [REVIEW] Patch for cursor calling with named parameters

2011-10-06 Thread Tom Lane
David E. Wheeler da...@kineticode.com writes:
 Would it then be added as an alias for := for named function parameters? Or 
 would that come still later?

 Once we do that, it will be impossible not merely deprecated to use =
 as an operator name.  I think that has to wait at least another release
 cycle or two past where we're using it ourselves.

 Okay. I kind of like := so there's no rush AFAIC. :-)

Hmm ... actually, that raises another issue that I'm not sure whether
there's consensus for or not.  Are we intending to keep name := value
syntax forever, as an alternative to the standard name = value syntax?
I can't immediately see a reason not to, other than the it's not
standard argument.

Because if we *are* going to keep it forever, there's no very good
reason why we shouldn't accept this plpgsql cursor patch now.  We'd
just have to remember to extend plpgsql to take = at the same time
we do that for core function calls.

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] Patch for cursor calling with named parameters

2011-10-06 Thread David E. Wheeler
On Oct 6, 2011, at 10:46 AM, Tom Lane wrote:

 Okay. I kind of like := so there's no rush AFAIC. :-)
 
 Hmm ... actually, that raises another issue that I'm not sure whether
 there's consensus for or not.  Are we intending to keep name := value
 syntax forever, as an alternative to the standard name = value syntax?
 I can't immediately see a reason not to, other than the it's not
 standard argument.

The only reason it would be required, I think, is if the SQL standard developed 
some other use for that operator.

 Because if we *are* going to keep it forever, there's no very good
 reason why we shouldn't accept this plpgsql cursor patch now.  We'd
 just have to remember to extend plpgsql to take = at the same time
 we do that for core function calls.

Makes sense.

Best,

David


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


Re: [HACKERS] [REVIEW] Patch for cursor calling with named parameters

2011-10-06 Thread Robert Haas
On Thu, Oct 6, 2011 at 1:46 PM, Tom Lane t...@sss.pgh.pa.us wrote:
 David E. Wheeler da...@kineticode.com writes:
 Would it then be added as an alias for := for named function parameters? 
 Or would that come still later?

 Once we do that, it will be impossible not merely deprecated to use =
 as an operator name.  I think that has to wait at least another release
 cycle or two past where we're using it ourselves.

 Okay. I kind of like := so there's no rush AFAIC. :-)

 Hmm ... actually, that raises another issue that I'm not sure whether
 there's consensus for or not.  Are we intending to keep name := value
 syntax forever, as an alternative to the standard name = value syntax?
 I can't immediately see a reason not to, other than the it's not
 standard argument.

 Because if we *are* going to keep it forever, there's no very good
 reason why we shouldn't accept this plpgsql cursor patch now.  We'd
 just have to remember to extend plpgsql to take = at the same time
 we do that for core function calls.

It's hard to see adding support for = and dropping support for := in
the same release.  That would be a compatibility nightmare.

If := is used by the standard for some other, incompatible purpose,
then I suppose we would want to add support for =, wait a few
releases, deprecate :=, wait a couple of releases, remove :=
altogether.  But IIRC we picked := precisely because the standard
didn't use it at all, or at least not for anything related... in which
case we may as well keep it around more or less indefinitely.

-- 
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] checkpoints are duplicated even while the system is idle

2011-10-06 Thread Tom Lane
Simon Riggs si...@2ndquadrant.com writes:
 Do we want this backpatched? If so, suggest just 9.1 and 9.0?

-1 for backpatching; it's more an improvement than a bug fix.

In any case, I think we still need to respond to the point Kevin made
about how to tell an idle master from broken replication.  Right now,
you will get at least a few bytes of data every checkpoint_timeout
seconds.  If we change this, you won't.

I'm inclined to think that the way to deal with that is not to force out
useless WAL data, but to add some sort of explicit I'm alive heartbeat
signal to the walsender/walreceiver protocol.  The hard part of that is
to figure out how to expose it where you can see it on the slave side
--- or do we have a status view that could handle 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] checkpoints are duplicated even while the system is idle

2011-10-06 Thread Tom Lane
Robert Haas robertmh...@gmail.com writes:
 On Thu, Oct 6, 2011 at 12:44 PM, Tom Lane t...@sss.pgh.pa.us wrote:
 I think the point is that a totally idle database should not continue to
 emit WAL, not even at a slow rate.  There are also power-consumption
 objections to allowing the checkpoint process to fire up to no purpose.

 Hmm, OK.  I still think it's a little funny to say that
 checkpoint_timeout will force a checkpoint every N minutes except when
 it doesn't, but maybe there's no real harm in that as long as we
 document it properly.

Well ... if we think that it's sane to only checkpoint once per WAL
segment, maybe we should just take out checkpoint_timeout.

We'd need some other mechanism to address replication use-cases, but see
my comments to Simon's followup patch just now.

regards, tom lane

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


[HACKERS] libpq, PQdescribePrepared - PQftype, PQfmod, no PQnullable

2011-10-06 Thread Alex Goncharov
My understanding is that libpq does not allow one to find if a result
set column is nullable.

Is this right?

(I know how to get a table column nullability information from
pg_attribute.attnotnull, but when coding around the libpq API:

  * Is, OMG, ugly.

  * Doesn't cover the arbitrary SELECT statements.
)

Thanks,

-- Alex -- alex-goncha...@comcast.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] checkpoints are duplicated even while the system is idle

2011-10-06 Thread Heikki Linnakangas

On 06.10.2011 20:58, Tom Lane wrote:

Robert Haasrobertmh...@gmail.com  writes:

On Thu, Oct 6, 2011 at 12:44 PM, Tom Lanet...@sss.pgh.pa.us  wrote:

I think the point is that a totally idle database should not continue to
emit WAL, not even at a slow rate.  There are also power-consumption
objections to allowing the checkpoint process to fire up to no purpose.



Hmm, OK.  I still think it's a little funny to say that
checkpoint_timeout will force a checkpoint every N minutes except when
it doesn't, but maybe there's no real harm in that as long as we
document it properly.


Well ... if we think that it's sane to only checkpoint once per WAL
segment, maybe we should just take out checkpoint_timeout.


Huh? Surely not, in my mind checkpoint_timeout is the primary way of 
controlling checkpoints, and checkpoint_segments you just set high 
enough so that you never reach it.


--
  Heikki Linnakangas
  EnterpriseDB   http://www.enterprisedb.com

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


Re: [HACKERS] checkpoints are duplicated even while the system is idle

2011-10-06 Thread Simon Riggs
On Thu, Oct 6, 2011 at 6:56 PM, Tom Lane t...@sss.pgh.pa.us wrote:
 Simon Riggs si...@2ndquadrant.com writes:
 Do we want this backpatched? If so, suggest just 9.1 and 9.0?

 -1 for backpatching; it's more an improvement than a bug fix.

OK, works for me.

 In any case, I think we still need to respond to the point Kevin made
 about how to tell an idle master from broken replication.  Right now,
 you will get at least a few bytes of data every checkpoint_timeout
 seconds.  If we change this, you won't.

 I'm inclined to think that the way to deal with that is not to force out
 useless WAL data, but to add some sort of explicit I'm alive heartbeat
 signal to the walsender/walreceiver protocol.  The hard part of that is
 to figure out how to expose it where you can see it on the slave side
 --- or do we have a status view that could handle that?

Different but related issue and yes, am on it, and yes, the way you just said.

I foresee a function that tells you the delay based on a protocol
message of 'k' for keepalive.

-- 
 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] checkpoints are duplicated even while the system is idle

2011-10-06 Thread Robert Haas
On Thu, Oct 6, 2011 at 1:56 PM, Tom Lane t...@sss.pgh.pa.us wrote:
 Simon Riggs si...@2ndquadrant.com writes:
 Do we want this backpatched? If so, suggest just 9.1 and 9.0?

 -1 for backpatching; it's more an improvement than a bug fix.

 In any case, I think we still need to respond to the point Kevin made
 about how to tell an idle master from broken replication.  Right now,
 you will get at least a few bytes of data every checkpoint_timeout
 seconds.  If we change this, you won't.

 I'm inclined to think that the way to deal with that is not to force out
 useless WAL data, but to add some sort of explicit I'm alive heartbeat
 signal to the walsender/walreceiver protocol.  The hard part of that is
 to figure out how to expose it where you can see it on the slave side
 --- or do we have a status view that could handle that?

As of 9.1, we already have something very much like this, in the
opposite direction.  See wal_receiver_status_interval and
replication_timeout.  I bet we could adapt that slightly to work in
the other direction, too.  But that'll only work with streaming
replication - do we care about the WAL shipping case?

-- 
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] checkpoints are duplicated even while the system is idle

2011-10-06 Thread Tom Lane
Robert Haas robertmh...@gmail.com writes:
 On Thu, Oct 6, 2011 at 1:56 PM, Tom Lane t...@sss.pgh.pa.us wrote:
 I'm inclined to think that the way to deal with that is not to force out
 useless WAL data, but to add some sort of explicit I'm alive heartbeat
 signal to the walsender/walreceiver protocol.  The hard part of that is
 to figure out how to expose it where you can see it on the slave side
 --- or do we have a status view that could handle that?

 As of 9.1, we already have something very much like this, in the
 opposite direction.  See wal_receiver_status_interval and
 replication_timeout.  I bet we could adapt that slightly to work in
 the other direction, too.  But that'll only work with streaming
 replication - do we care about the WAL shipping case?

I can't get excited about the WAL-shipping case. The artifact that we'll
generate a checkpoint record every few minutes does not create enough
WAL volume for WAL-shipping to reliably generate a heartbeat signal.
It'd be way too long between filling up segments if that were the only
WAL data being generated.

regards, tom lane

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


[HACKERS] Extend extension file_fdw

2011-10-06 Thread pasman pasmański
Hi.

I plan to extend file_fdw wrapper. I will add options to foreign
server: encoding, format, header, delimiter, dir.

And i have some asks:
- it's better to change name of extension or not
- other suggestions


-- 

pasman

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


Re: [HACKERS] checkpoints are duplicated even while the system is idle

2011-10-06 Thread Robert Haas
On Thu, Oct 6, 2011 at 2:26 PM, Tom Lane t...@sss.pgh.pa.us wrote:
 Robert Haas robertmh...@gmail.com writes:
 On Thu, Oct 6, 2011 at 1:56 PM, Tom Lane t...@sss.pgh.pa.us wrote:
 I'm inclined to think that the way to deal with that is not to force out
 useless WAL data, but to add some sort of explicit I'm alive heartbeat
 signal to the walsender/walreceiver protocol.  The hard part of that is
 to figure out how to expose it where you can see it on the slave side
 --- or do we have a status view that could handle that?

 As of 9.1, we already have something very much like this, in the
 opposite direction.  See wal_receiver_status_interval and
 replication_timeout.  I bet we could adapt that slightly to work in
 the other direction, too.  But that'll only work with streaming
 replication - do we care about the WAL shipping case?

 I can't get excited about the WAL-shipping case. The artifact that we'll
 generate a checkpoint record every few minutes does not create enough
 WAL volume for WAL-shipping to reliably generate a heartbeat signal.
 It'd be way too long between filling up segments if that were the only
 WAL data being generated.

Depends how you set archive_timeout, I think.

Anyway, I'm not saying we *have* to care about that case.  I'm just
asking whether we do, before we go start changing things.

-- 
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] checkpoints are duplicated even while the system is idle

2011-10-06 Thread Kevin Grittner
Simon Riggs si...@2ndquadrant.com wrote:
 
 I foresee a function that tells you the delay based on a protocol
 message of 'k' for keepalive.
 
If the delay you mention is basically a ping time or something
similar, that would answer the need I've been on about.  We need to
know, based on access to the replica, that the replication system is
alive and well even with an idle master; as it can otherwise be hard
to distinguish an idle master from a failed replication system
(broken connection, misconfigured replication, etc.).
 
Right now there is a periodic checkpoint which flows through the WAL
and affects the pg_controldata report on the replica -- we've been
using that for monitoring.  Any sort of heartbeat or ping which
provides sign-of-life on the connection, accessible on the replica,
should do for our purposes. If it only works over streaming
replication on a hot standby, that's OK -- we plan to be running
everything that way before 9.2 comes out, as long as we can
materialize traditional WAL files on the receiving end from the SR
stream.
 
-1 on changing the checkpoint behavior before 9.2, though.
 
-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] index-only scans

2011-10-06 Thread Tom Lane
Robert Haas robertmh...@gmail.com writes:
 Please find attached a patch implementing a basic version of
 index-only scans.  This patch is the work of my colleague Ibrar Ahmed
 and myself, and also incorporates some code from previous patches
 posted by Heikki Linnakanagas.

I'm starting to review this patch now.  Has any further work been done
since the first version was posted?  Also, was any documentation
written?  I'm a tad annoyed by having to reverse-engineer the changes
in the AM API specification from the code.

regards, tom lane

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


Re: [HACKERS] Range Types - typo + NULL string constructor

2011-10-06 Thread Alexander Korotkov
Hi, Jeff!

Heikki has recently commited my patch about picksplit for GiST on points and
boxes:
http://git.postgresql.org/gitweb/?p=postgresql.git;a=commitdiff;h=7f3bd86843e5aad84585a57d3f6b80db3c609916
I would like to try this picksplit method on ranges. I believe that it might
be much more efficient on highly overlapping ranges than current picksplit.
Range types patch isn't commited, yet. So, what way of work publishing is
prefered in this case? Add-on patch, separate branch in git or something
else?

--
With best regards,
Alexander Korotkov.


Re: [HACKERS] checkpoints are duplicated even while the system is idle

2011-10-06 Thread Simon Riggs
On Thu, Oct 6, 2011 at 7:18 PM, Robert Haas robertmh...@gmail.com wrote:

 As of 9.1, we already have something very much like this, in the
 opposite direction.

Yes Robert, I wrote it.

-- 
 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] index-only scans

2011-10-06 Thread Robert Haas
On Thu, Oct 6, 2011 at 3:15 PM, Tom Lane t...@sss.pgh.pa.us wrote:
 Robert Haas robertmh...@gmail.com writes:
 Please find attached a patch implementing a basic version of
 index-only scans.  This patch is the work of my colleague Ibrar Ahmed
 and myself, and also incorporates some code from previous patches
 posted by Heikki Linnakanagas.

 I'm starting to review this patch now.

Thanks!

 Has any further work been done
 since the first version was posted?  Also, was any documentation
 written?  I'm a tad annoyed by having to reverse-engineer the changes
 in the AM API specification from the code.

Not really.  We have detected a small performance regression when both
heap and index fit in shared_buffers and an index-only scan is used.
I have a couple of ideas for improving this.  One is to store a
virtual tuple into the slot instead of building a regular tuple, but
what do we do about tuples with OIDs?  Another is to avoid locking the
index buffer multiple times - right now it locks the index buffer to
get the TID, and then relocks it to extract the index tuple (after
checking that nothing disturbing has happened meanwhile).  It seems
likely that with some refactoring we could get this down to a single
lock/unlock cycle, but I haven't figured out exactly where the TID
gets copied out.

With regard to the AM API, the basic idea is we're just adding a
Boolean to say whether the AM is capable of returning index tuples.
If it's not, then we don't ever try an index-only scan.  If it is,
then we'll set the want_index_tuple flag if an index-only scan is
possible.  This requests that the AM attempt to return the tuple; but
the AM is also allowed to fail and not return the tuple whenever it
wants.  This is more or less the interface that Heikki suggested a
couple years back, but it might well be vulnerable to improvement.

Incidentally, if you happen to feel the urge to beat this around and
send it back rather than posting a list of requested changes, feel
free.

-- 
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] Bug in walsender when calling out to do_pg_stop_backup (and others?)

2011-10-06 Thread Magnus Hagander
On Thu, Oct 6, 2011 at 14:34, Florian Pflug f...@phlo.org wrote:
 On Oct5, 2011, at 15:30 , Magnus Hagander wrote:
 When walsender calls out to do_pg_stop_backup() (during base backups),
 it is not possible to terminate the process with a SIGTERM - it
 requires a SIGKILL. This can leave unkillable backends for example if
 archive_mode is on and archive_command is failing (or not set). A
 similar thing would happen in other cases if walsender calls out to
 something that would block (do_pg_start_backup() for example), but the
 stop one is easy to provoke.

 Hm, this seems to be related to another buglet I noticed a while ago,
 but then forgot about again. If one terminates pg_basebackup while it's
 waiting for all required WAL to be archived, the backend process only
 exits once that waiting phase is over. If, like in your failure case,
 archive_command fails indefinity (or isn't set), the backend process
 stays around forever.

Yes.


 Your patch would improve that only insofar as it'd at least allow an
 immediate shutdown request to succeed - as it stands, that doesn't work
 because, as you mentioned, the blocked walsender doesn't handle SIGTERM.

Exactly.


 The question is, should we do more? To me, it'd make sense to terminate
 a backend once it's connection is gone. We could, for example, make
 pq_flush() set a global flag, and make CHECK_FOR_INTERRUPTS handle a
 broken connection that same way as a SIGINT or SIGTERM.

The problem here is that we're hanging at a place where we don't touch
the socket. So we won't notice the socket is gone. We'd have to do a
select() or something like that at regular intervals to make sure it's
there, no?


-- 
 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] Bug in walsender when calling out to do_pg_stop_backup (and others?)

2011-10-06 Thread Magnus Hagander
On Thu, Oct 6, 2011 at 04:22, Fujii Masao masao.fu...@gmail.com wrote:
 On Wed, Oct 5, 2011 at 10:30 PM, Magnus Hagander mag...@hagander.net wrote:
 When walsender calls out to do_pg_stop_backup() (during base backups),
 it is not possible to terminate the process with a SIGTERM - it
 requires a SIGKILL. This can leave unkillable backends for example if
 archive_mode is on and archive_command is failing (or not set). A
 similar thing would happen in other cases if walsender calls out to
 something that would block (do_pg_start_backup() for example), but the
 stop one is easy to provoke.

 Good catch!

 ISTM one way to fix it is the attached, which is to have walsender set
 the global flags saying that we have received sigterm, which in turn
 causes the CHECK_FOR_INTERRUPTS() calls in the routines to properly
 exit the process. AFAICT it works fine. Any holes in this approach?

 Very simple patch. Looks fine.

Ok, thanks. Applied.


 Second, I wonder if we should add a SIGINT handler as well, that would
 make it possible to send a cancel signal. Given that the end result
 would be the same (at least if we want to keep with the walsender is
 simple path), I'm not sure it's necessary - but it would at least
 help those doing pg_cancel_backend()... thoughts?

 I don't think that's necessary because, as you suggested, there is no
 use case for *now*. We can add that handler when someone proposes
 the feature which requires that.

Yeah. It's certainly not something backpatch-worthy.

-- 
 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] index-only scans

2011-10-06 Thread Tom Lane
Robert Haas robertmh...@gmail.com writes:
 Not really.  We have detected a small performance regression when both
 heap and index fit in shared_buffers and an index-only scan is used.
 I have a couple of ideas for improving this.  One is to store a
 virtual tuple into the slot instead of building a regular tuple, but
 what do we do about tuples with OIDs?

Yeah, I was just looking at that.  I think it's going to be a clear win
to use a virtual tuple slot instead of constructing and deconstructing
a HeapTuple.  The obvious solution is to decree that you can't use an
index-only scan if the query requires fetching OID (or any other system
column).  This would be slightly annoying for catalog fetches but I'm
not sure I believe that catalog queries are that important a use-case.

I was also toying with the notion of pushing the slot fill-in into the
AM, so that the AM API is to return a filled TupleSlot not an
IndexTuple.  This would not save any cycles AFAICT --- at least in
btree, we still have to make a palloc'd copy of the IndexTuple so that
we can release lock on the index page.  The point of it would be to
avoid the assumption that the index's internal storage has exactly the
format of IndexTuple.  Don't know whether we'd ever have any actual use
for that flexibility, but it seems like it wouldn't cost much to
preserve the option.

 Another is to avoid locking the
 index buffer multiple times - right now it locks the index buffer to
 get the TID, and then relocks it to extract the index tuple (after
 checking that nothing disturbing has happened meanwhile).  It seems
 likely that with some refactoring we could get this down to a single
 lock/unlock cycle, but I haven't figured out exactly where the TID
 gets copied out.

Yeah, maybe, but let's get the patch committed before we start looking
for second-order optimizations.

 With regard to the AM API, the basic idea is we're just adding a
 Boolean to say whether the AM is capable of returning index tuples.
 If it's not, then we don't ever try an index-only scan.  If it is,
 then we'll set the want_index_tuple flag if an index-only scan is
 possible.  This requests that the AM attempt to return the tuple; but
 the AM is also allowed to fail and not return the tuple whenever it
 wants.

It was the allowed to fail bit that I was annoyed to discover only by
reading some well-buried code.

 Incidentally, if you happen to feel the urge to beat this around and
 send it back rather than posting a list of requested changes, feel
 free.

You can count on that ;-).  I've already found a lot of things I didn't
care for.

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] Patch for cursor calling with named parameters

2011-10-06 Thread Pavel Stehule
2011/10/6 Robert Haas robertmh...@gmail.com:
 On Thu, Oct 6, 2011 at 1:46 PM, Tom Lane t...@sss.pgh.pa.us wrote:
 David E. Wheeler da...@kineticode.com writes:
 Would it then be added as an alias for := for named function parameters? 
 Or would that come still later?

 Once we do that, it will be impossible not merely deprecated to use =
 as an operator name.  I think that has to wait at least another release
 cycle or two past where we're using it ourselves.

 Okay. I kind of like := so there's no rush AFAIC. :-)

 Hmm ... actually, that raises another issue that I'm not sure whether
 there's consensus for or not.  Are we intending to keep name := value
 syntax forever, as an alternative to the standard name = value syntax?
 I can't immediately see a reason not to, other than the it's not
 standard argument.

 Because if we *are* going to keep it forever, there's no very good
 reason why we shouldn't accept this plpgsql cursor patch now.  We'd
 just have to remember to extend plpgsql to take = at the same time
 we do that for core function calls.

 It's hard to see adding support for = and dropping support for := in
 the same release.  That would be a compatibility nightmare.

 If := is used by the standard for some other, incompatible purpose,
 then I suppose we would want to add support for =, wait a few
 releases, deprecate :=, wait a couple of releases, remove :=
 altogether.  But IIRC we picked := precisely because the standard
 didn't use it at all, or at least not for anything related... in which
 case we may as well keep it around more or less indefinitely.

+1

Pavel


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


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


Re: [HACKERS] libpq, PQdescribePrepared - PQftype, PQfmod, no PQnullable

2011-10-06 Thread Merlin Moncure
On Thu, Oct 6, 2011 at 1:02 PM, Alex Goncharov
alex-goncha...@comcast.net wrote:
 My understanding is that libpq does not allow one to find if a result
 set column is nullable.

 Is this right?

 (I know how to get a table column nullability information from
 pg_attribute.attnotnull, but when coding around the libpq API:

  * Is, OMG, ugly.

  * Doesn't cover the arbitrary SELECT statements.

why aren't you using PQgetisnull()?

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] index-only scans

2011-10-06 Thread Thom Brown
On 6 October 2011 21:11, Tom Lane t...@sss.pgh.pa.us wrote:
 Robert Haas robertmh...@gmail.com writes:
 Not really.  We have detected a small performance regression when both
 heap and index fit in shared_buffers and an index-only scan is used.
 I have a couple of ideas for improving this.  One is to store a
 virtual tuple into the slot instead of building a regular tuple, but
 what do we do about tuples with OIDs?

 Yeah, I was just looking at that.  I think it's going to be a clear win
 to use a virtual tuple slot instead of constructing and deconstructing
 a HeapTuple.  The obvious solution is to decree that you can't use an
 index-only scan if the query requires fetching OID (or any other system
 column).  This would be slightly annoying for catalog fetches but I'm
 not sure I believe that catalog queries are that important a use-case.

 I was also toying with the notion of pushing the slot fill-in into the
 AM, so that the AM API is to return a filled TupleSlot not an
 IndexTuple.  This would not save any cycles AFAICT --- at least in
 btree, we still have to make a palloc'd copy of the IndexTuple so that
 we can release lock on the index page.  The point of it would be to
 avoid the assumption that the index's internal storage has exactly the
 format of IndexTuple.  Don't know whether we'd ever have any actual use
 for that flexibility, but it seems like it wouldn't cost much to
 preserve the option.

 Another is to avoid locking the
 index buffer multiple times - right now it locks the index buffer to
 get the TID, and then relocks it to extract the index tuple (after
 checking that nothing disturbing has happened meanwhile).  It seems
 likely that with some refactoring we could get this down to a single
 lock/unlock cycle, but I haven't figured out exactly where the TID
 gets copied out.

 Yeah, maybe, but let's get the patch committed before we start looking
 for second-order optimizations.

+1

Been testing this today with a few regression tests and haven't seen
anything noticeably broken.  Does what it says on the tin.

-- 
Thom Brown
Twitter: @darkixion
IRC (freenode): dark_ixion
Registered Linux user: #516935

EnterpriseDB UK: 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] libpq, PQdescribePrepared - PQftype, PQfmod, no PQnullable

2011-10-06 Thread Alex Goncharov
,--- I/Alex (Thu, 06 Oct 2011 14:02:14 -0400) *
| My understanding is that libpq does not allow one to find if a result
| set column is nullable.
,--- You/Merlin (Thu, 6 Oct 2011 15:16:18 -0500) *
| why aren't you using PQgetisnull()?

This function is not about the nullability of a column but rather
about the value in a result set cell:

  PQgetisnull: Tests a field for a null value. 
  
 int PQgetisnull(const PGresult *res, int row_number, int column_number);

Notice the 'row_number'. 

-- Alex -- alex-goncha...@comcast.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] libpq, PQdescribePrepared - PQftype, PQfmod, no PQnullable

2011-10-06 Thread Merlin Moncure
On Thu, Oct 6, 2011 at 3:22 PM, Alex Goncharov
alex-goncha...@comcast.net wrote:
 ,--- I/Alex (Thu, 06 Oct 2011 14:02:14 -0400) *
 | My understanding is that libpq does not allow one to find if a result
 | set column is nullable.
 ,--- You/Merlin (Thu, 6 Oct 2011 15:16:18 -0500) *
 | why aren't you using PQgetisnull()?

 This function is not about the nullability of a column but rather
 about the value in a result set cell:

  PQgetisnull: Tests a field for a null value.

     int PQgetisnull(const PGresult *res, int row_number, int column_number);

 Notice the 'row_number'.


right -- get it.  well, your question is doesn't make sense then --
any column can be transformed in ad hoc query, so it only makes sense
to test individual values post query..btw, if you don't like
querying system catalogs, check out information_schema.columns.

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] libpq, PQdescribePrepared - PQftype, PQfmod, no PQnullable

2011-10-06 Thread Florian Pflug
On Oct6, 2011, at 22:38 , Merlin Moncure wrote:
 On Thu, Oct 6, 2011 at 3:22 PM, Alex Goncharov
 alex-goncha...@comcast.net wrote:
 ,--- I/Alex (Thu, 06 Oct 2011 14:02:14 -0400) *
 | My understanding is that libpq does not allow one to find if a result
 | set column is nullable.

 ,--- You/Merlin (Thu, 6 Oct 2011 15:16:18 -0500) *
 | why aren't you using PQgetisnull()?
 
 This function is not about the nullability of a column but rather
 about the value in a result set cell:
 
  PQgetisnull: Tests a field for a null value.
 
 int PQgetisnull(const PGresult *res, int row_number, int column_number);
 
 Notice the 'row_number'.
 
 right -- get it.  well, your question is doesn't make sense then --
 any column can be transformed in ad hoc query, so it only makes sense
 to test individual values post query..btw, if you don't like
 querying system catalogs, check out information_schema.columns.

Sure, but there are still a lot of cases where the database could deduce
(quite easily) that a result column cannot be null. Other databases do
that - for example, I believe to remember that Microsoft SQL Server preserves
NOT NULL constraints if you do

  CREATE TABLE bar AS SELECT * from foo;

So the question makes perfect sense, and the answer is: No, postgres currently
doesn't support that, i.e. doesn't deduce the nullability of result columns,
not even in the simplest cases.

best regards,
Florian Pflug


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


Re: [HACKERS] libpq, PQdescribePrepared - PQftype, PQfmod, no PQnullable

2011-10-06 Thread Merlin Moncure
On Thu, Oct 6, 2011 at 4:16 PM, Florian Pflug f...@phlo.org wrote:
 Sure, but there are still a lot of cases where the database could deduce
 (quite easily) that a result column cannot be null. Other databases do
 that - for example, I believe to remember that Microsoft SQL Server preserves
 NOT NULL constraints if you do

  CREATE TABLE bar AS SELECT * from foo;

 So the question makes perfect sense, and the answer is: No, postgres currently
 doesn't support that, i.e. doesn't deduce the nullability of result columns,
 not even in the simplest cases.

hm, good point.  not sure how it's useful though.  I suppose an
application could leverage that for validation purposes, but that's a
stretch I think.

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] Bug in walsender when calling out to do_pg_stop_backup (and others?)

2011-10-06 Thread Florian Pflug
On Oct6, 2011, at 21:48 , Magnus Hagander wrote:
 The question is, should we do more? To me, it'd make sense to terminate
 a backend once it's connection is gone. We could, for example, make
 pq_flush() set a global flag, and make CHECK_FOR_INTERRUPTS handle a
 broken connection that same way as a SIGINT or SIGTERM.
 
 The problem here is that we're hanging at a place where we don't touch
 the socket. So we won't notice the socket is gone. We'd have to do a
 select() or something like that at regular intervals to make sure it's
 there, no?

We do emit NOTICEs saying pg_stop_backup still waiting ...  repeatedly,
so we should notice a dead connection sooner or later. When I tried, I even
got a message in the log complaining about the broken pipe.

As it stands, the interval between two NOTICEs grows exponentially - we
send the first after waiting 5 second, the next after waiting 60 seconds,
and then after waiting 120, 240, 480, ... seconds. This means that that the
backend would in the worst case linger the same amount of time *after*
pg_basebackup was cancelled that pg_basebackup waited for *before* it was
cancelled.

It'd be nice to generally terminate a backend if the client vanishes, but so
far I haven't had any bright ideas. Using FASYNC and F_SETOWN unfortunately
sends a signal *everytime* the fd becomes readable or writeable, not only on
EOF. Doing select() in CHECK_FOR_INTERRUPTS seems far too expensive. We could
make the postmaster keep the fd's of around even after forking a backend, and
make it watch for broken connections using select(). But with a large 
max_backends
settings, we'd risk running out of fds in the postmaster...

best regards,
Florian Pflug


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


Re: [HACKERS] libpq, PQdescribePrepared - PQftype, PQfmod, no PQnullable

2011-10-06 Thread Alex Goncharov
,--- I/Alex (Thu, 06 Oct 2011 14:02:14 -0400) *
| My understanding is that libpq does not allow one to find if a result
| set column is nullable.
,--- You/Merlin (Thu, 6 Oct 2011 15:16:18 -0500) *
| why aren't you using PQgetisnull()?
,--- I/Alex (Thu, 06 Oct 2011 16:22:28 -0400) *
| This function is not about the nullability of a column but rather
| about the value in a result set cell:
|  int PQgetisnull(const PGresult *res, int row_number, int column_number);
| Notice the 'row_number'. 
,--- Merlin Moncure (Thu, 6 Oct 2011 15:38:59 -0500) *
| right -- get it.  well, your question is doesn't make sense then --

What?..

* It makes complete logical sense to ask a question if a result set
  column may ever have a NULL cell.

* It can be done for a table using pg_attribute.attnotnull.

* It can be done, at the C API level, in a wide variety of other
  databases, including the two most often mentioned in this audience:
  Oracle (through and OCI call) and MySQL (at least through ODBC.)

| any column can be transformed in ad hoc query, so it only makes sense
| to test individual values post query..

What query?

Look at the subject line: it mentioned PQdescribePrepared.

I execute PQprepare, and then PQdescribePrepared -- I never fetch the
data.  When the statement is described, plenty information can be
obtained about the columns -- but not its nullability (what I wanted
to be confirmed or denied -- for libpq API.)

| btw, if you don't like querying system catalogs, check out
| information_schema.columns.

Than was not my question, right?  (What difference is there between
using pg_X tables of information_schema?)

,--- Florian Pflug (Thu, 6 Oct 2011 23:16:53 +0200) *
| Sure, but there are still a lot of cases where the database could deduce
| (quite easily) that a result column cannot be null.

Right. Of course.  I can do it in 'psql'.

| Other databases do that - for example, I believe to remember that
| Microsoft SQL Server preserves NOT NULL constraints if you do
| 
|   CREATE TABLE bar AS SELECT * from foo;

I don't know a database where this would not be true.

| So the question makes perfect sense, and the answer is: No, postgres currently
| doesn't support that, i.e. doesn't deduce the nullability of result columns,
| not even in the simplest cases.

You are wrong: as in my original mail, use pg_attribute.attnotnull to
see why I say this.

,--- Merlin Moncure (Thu, 6 Oct 2011 16:28:56 -0500) *
| hm, good point.  not sure how it's useful though.  I suppose an
| application could leverage that for validation purposes, but that's a
| stretch I think.
`*

Thanks for sharing your knowledge of applications.

(Look, I appreciate anybody's reply and readiness to help, but if you
have a limited expertise in the subject area, why bother replying?)

-- Alex -- alex-goncha...@comcast.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] libpq, PQdescribePrepared - PQftype, PQfmod, no PQnullable

2011-10-06 Thread Andrew Dunstan



On 10/06/2011 06:02 PM, Alex Goncharov wrote:


(Look, I appreciate anybody's reply and readiness to help, but if you
have a limited expertise in the subject area, why bother replying?)




People are trying to help you. Please be a little less sensitive. 
Sneering at Merlin is not likely to win you friends. He's well known 
around here as being quite knowledgeable.


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] libpq, PQdescribePrepared - PQftype, PQfmod, no PQnullable

2011-10-06 Thread Florian Pflug
On Oct7, 2011, at 00:02 , Alex Goncharov wrote:
 ,--- Florian Pflug (Thu, 6 Oct 2011 23:16:53 +0200) *
 | Sure, but there are still a lot of cases where the database could deduce
 | (quite easily) that a result column cannot be null.
 
 Right. Of course.  I can do it in 'psql'.

For the result of an *arbitrary* query?

I think what you are missing is that there is *huge* difference between
tables (as created by CREATE TABLE) and result sets produced by SELECT
statements.

The former can carry all sorts of constraints like NOT NULL, CHECK,
REFERENCES, ..., and their structure as well as the constraints they carry
are stored in the catalog tables in the schema pg_catalog.

The latter cannot carry any constraints, and their meta-data thus consist
simply of a list of column names and types. Their meta-data is also
transient in nature, since it differs for every SELECT you issue.

Views are a kind of mixture between the two - their meta-data isn't any
richer than that of a SELECT statement, but since VIEWs aren't transient
objects like statements, their meta-data *is* reflected in the catalog.

 | Other databases do that - for example, I believe to remember that
 | Microsoft SQL Server preserves NOT NULL constraints if you do
 | 
 |   CREATE TABLE bar AS SELECT * from foo;
 
 I don't know a database where this would not be true.

Ähm... postgres would be one where the resulting table doesn't have any
NOT NULL columns. Ever.

 | So the question makes perfect sense, and the answer is: No, postgres 
 currently
 | doesn't support that, i.e. doesn't deduce the nullability of result columns,
 | not even in the simplest cases.
 
 You are wrong: as in my original mail, use pg_attribute.attnotnull to
 see why I say this.

Nope, you miss-understood what I said. I said result columns, meaning the
columns resulting from a SELECT statement. Postgres doesn't deduce the 
nullability
of these columns. The fact that postgres supports NOT NULL constraints on tables
(which is what pg_attribute.attnotnull is for) really has nothing to do with 
that.

best regards,
Florian Pflug


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


Re: [HACKERS] libpq, PQdescribePrepared - PQftype, PQfmod, no PQnullable

2011-10-06 Thread Bruce Momjian
Alex Goncharov wrote:
 ,--- Merlin Moncure (Thu, 6 Oct 2011 16:28:56 -0500) *
 | hm, good point.  not sure how it's useful though.  I suppose an
 | application could leverage that for validation purposes, but that's a
 | stretch I think.
 `*
 
 Thanks for sharing your knowledge of applications.
 
 (Look, I appreciate anybody's reply and readiness to help, but if you
 have a limited expertise in the subject area, why bother replying?)

FYI, I see 867 Postgres posts mentioning Merlin Moncure in the past
year:


http://search.postgresql.org/search?q=Merlin+Moncurem=1l=NULLd=365s=rp=44

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

  + It's impossible for everything to be true. +

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


Re: [HACKERS] libpq, PQdescribePrepared - PQftype, PQfmod, no PQnullable

2011-10-06 Thread Alex Goncharov
,--- You/Florian (Fri, 7 Oct 2011 01:00:40 +0200) *
| On Oct7, 2011, at 00:02 , Alex Goncharov wrote:
|  ,--- Florian Pflug (Thu, 6 Oct 2011 23:16:53 +0200) *
|  | Sure, but there are still a lot of cases where the database could deduce
|  | (quite easily) that a result column cannot be null.
|  
|  Right. Of course.  I can do it in 'psql'.
| 
| For the result of an *arbitrary* query?

In 'psql', no: I was commenting specifically, and confirming what you
said, on your

 a lot of cases where the database could deduce (quite easily) that a
 result column cannot be null

| I think what you are missing is that there is *huge* difference between
| tables (as created by CREATE TABLE) and result sets produced by SELECT
| statements.

Actually, no, I am not missing the huge difference -- again, I was
just agreeing with you.  Agreeing that there is a lot of cases where
the nullability can be trivially deduced, even in 'psql'. (That also
meant disagreeing with the message posted before yours.)
 
| The former can carry all sorts of constraints like NOT NULL, CHECK,
| REFERENCES, ..., and their structure as well as the constraints they carry
| are stored in the catalog tables in the schema pg_catalog.

Yes.

| The latter cannot carry any constraints, and their meta-data thus consist
| simply of a list of column names and types. Their meta-data is also
| transient in nature, since it differs for every SELECT you issue.

Right: but for (most?) every SELECT, one can logically deduce whether
it can be guaranteed that a given column will never have a NULL value.
Since in a given SELECT, the result column are a combination of either
other columns, or expressions, including literals.

Now, I am not even wondering about a 100% percent reliable
determination by a hypothetical 'PQfisnullable(PQresult *r, int idx)'.

But if libpq can tell me about column names, types and sizes (PQfname,
PQftype, PQfmod), why would it be impossible to have 'PQfisnullable'?

Today I tested that it is done in: Oracle, DB2, MySQL, Teradata,
Informix, Netezza and Vertica (in many of these via ODBC.)

This is conceptually feasible.

And in PostgreSQL, this could be done by combining

  (1)   Oid PQftable(const PGresult *res, int column_number);
  (2)   int PQftablecol(const PGresult *res, int column_number);
  (3)   a SQL query of pg_attribute,attnotnull

I have not tried this yet, hesitating to walk into a monstrosity and
hoping that there is some hidden way to get the information through
one of

  int PQfmod(const PGresult *res, int column_number);
  int PQgetisnull(const PGresult *res, int row_number, int column_number);

(the latter with an odd 'row_number'; I actually tried row_number= 0
and -1, after preparing a statement. No luck.)  

| Views are a kind of mixture between the two - their meta-data isn't any
| richer than that of a SELECT statement, but since VIEWs aren't transient
| objects like statements, their meta-data *is* reflected in the
| catalog.

Again, combining (1), (2) and (3) above should give a good answer here.

|  | Other databases do that - for example, I believe to remember that
|  | Microsoft SQL Server preserves NOT NULL constraints if you do
|  | 
|  |   CREATE TABLE bar AS SELECT * from foo;
|  
|  I don't know a database where this would not be true.
| 
| Ähm... postgres would be one where the resulting table doesn't have any
| NOT NULL columns. Ever.

Not sure what you mean here:

--
http://www.postgresql.org/docs/8.4/interactive/ddl-constraints.html#AEN2290:

A not-null constraint simply specifies that a column must not assume
the null value. 

CREATE TABLE products (
product_no integer NOT NULL,
name text NOT NULL,
price numeric
);

The NOT NULL constraint has an inverse: the NULL constraint.

CREATE TABLE products (
product_no integer NULL,
name text NULL,
price numeric NULL
);
--

| 
|  | So the question makes perfect sense, and the answer is: No, postgres 
currently
|  | doesn't support that, i.e. doesn't deduce the nullability of result 
columns,
|  | not even in the simplest cases.
|  
|  You are wrong: as in my original mail, use pg_attribute.attnotnull to
|  see why I say this.
| 
| Nope, you miss-understood what I said.

You said, not even in the simplest cases -- and this is what caused
my statement.

| I said result columns, meaning the columns resulting from a SELECT
| statement.

Then I misunderstood you, indeed -- I thought you included an inquiry
about a table.  Sorry for the misunderstanding then.

| Postgres doesn't deduce the nullability of these columns. The fact
| that postgres supports NOT NULL constraints on tables (which is what
| pg_attribute.attnotnull is for) really has nothing to do with that.

  create table t1(nn1 char(1) not null, yn1 char(1) null);
  create table t2(nn2 char(1) not null, yn2 char(1) null);

  (may use pg_attribute.attnotnull on t1, t2, is I 

Re: [HACKERS] libpq, PQdescribePrepared - PQftype, PQfmod, no PQnullable

2011-10-06 Thread Alex Goncharov
,--- You/Bruce (Thu, 6 Oct 2011 19:09:16 -0400 (EDT)) *
|  (Look, I appreciate anybody's reply and readiness to help, but if you
|  have a limited expertise in the subject area, why bother replying?)
| 
| FYI, I see 867 Postgres posts mentioning Merlin Moncure in the past
| year:
| 
|   
http://search.postgresql.org/search?q=Merlin+Moncurem=1l=NULLd=365s=rp=44

I watch most of the PostgreSQL technical lists all the time and know
who is who.

I didn't mean to be disparaging (and said, Look, I appreciate
anybody's reply and readiness to help).

But really, before replying, one should think about the posted
question, and resist opinionating on the topics little thought about
and worked with.

To this:

,--- Merlin Moncure (Thu, 6 Oct 2011 15:16:18 -0500) *
| why aren't you using PQgetisnull()?
`*

I replied politely:

,--- I/Alex (Thu, 06 Oct 2011 16:22:28 -0400) *
| This function is not about the nullability of a column but rather
| about the value in a result set cell:
| 
|   PQgetisnull: Tests a field for a null value. 
|   
|  int PQgetisnull(const PGresult *res, int row_number, int column_number);
| 
| Notice the 'row_number'. 
`-*

To this:

,--- Merlin Moncure (Thu, 6 Oct 2011 15:38:59 -0500) *
| right -- get it.  well, your question is doesn't make sense then --
|
| btw, if you don't like querying system catalogs, check out
| information_schema.columns.
|
`*

it was harder; still, I stayed in the technical area:

,--- I/Alex (Thu, 06 Oct 2011 18:02:41 -0400) *
|
| What?..
| 
| * It makes complete logical sense to ask a question if a result set
|   column may ever have a NULL cell.
| 
| * It can be done for a table using pg_attribute.attnotnull.
| 
| * It can be done, at the C API level, in a wide variety of other
|   databases, including the two most often mentioned in this audience:
|   Oracle (through and OCI call) and MySQL (at least through ODBC.)
|
`-*

To this:

,--- Merlin Moncure (Thu, 6 Oct 2011 16:28:56 -0500) *
| hm, good point.  not sure how it's useful though.  I suppose an
| application could leverage that for validation purposes, but that's a
| stretch I think.
`*

it was plain hard -- the expressed opinion didn't relate to the
original question, and was, besides, quite unfounded.

,--- Andrew Dunstan (Thu, 06 Oct 2011 18:30:44 -0400) *
| People are trying to help you. Please be a little less sensitive. 
| Sneering at Merlin is not likely to win you friends. 
`-*

I know.

I wouldn't have been sensitive about an opinion on a side topic (not
sure how it's useful though) (did anybody asked about that?), had
Merlin also offered sound and relevant technical points.  He hadn't.

On the technical point now:

It's clear enough for me at this point, that I had not overlooked
anything in libpq and it doesn't support finding a result set column
nullability (no hypothetical PQfisnullable function or a hidden way to
use other PQf* functions for this purpose.)

I will resort to the ugly method I outlined in my previous message,
combining:

,--- I/Alex (Thu, 06 Oct 2011 19:42:13 -0400) *
|
|   (1)   Oid PQftable(const PGresult *res, int column_number);
|   (2)   int PQftablecol(const PGresult *res, int column_number);
|   (3)   a SQL query of pg_attribute,attnotnull
|
`-*

Thanks everybody who replied!

P.S. And on the odd chance that somebody thinks that this
 functionality would be possible and helpful to add to libpq, and
 the problem is in the lack of human resources: I would be more
 then happy to dig into some PostgreSQL (the product) development
 under somebody's coaching, to start with.  This topic or other.
 I just wouldn't know where to start myself.

-- Alex -- alex-goncha...@comcast.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] libpq, PQdescribePrepared - PQftype, PQfmod, no PQnullable

2011-10-06 Thread Andres Freund
On Friday, October 07, 2011 01:42:13 AM Alex Goncharov wrote:
 ,--- You/Florian (Fri, 7 Oct 2011 01:00:40 +0200) *
 
 | On Oct7, 2011, at 00:02 , Alex Goncharov wrote:
 |  ,--- Florian Pflug (Thu, 6 Oct 2011 23:16:53 +0200) *
 |  
 |  | Sure, but there are still a lot of cases where the database could
 |  | deduce (quite easily) that a result column cannot be null.
 | 
 |  
 | 
 |  Right. Of course.  I can do it in 'psql'.
 |
 | 
 |
 | For the result of an arbitrary query?
 
 In 'psql', no: I was commenting specifically, and confirming what you
 said, on your
 
  a lot of cases where the database could deduce (quite easily) that a
  result column cannot be null
Could you quickly explain what exactly you want that information for? Just 
because it has been done before doesn't necessarily mean its a good idea...


Thanks,

Andres

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


Re: [HACKERS] libpq, PQdescribePrepared - PQftype, PQfmod, no PQnullable

2011-10-06 Thread Alex Goncharov
The obvious typos (sorry if this is a duplicate message, I sent the
first one from a wrong address):

,--- I/Alex (Thu, 06 Oct 2011 19:42:13 -0400) *
|   (may use pg_attribute.attnotnull on t1, t2, is I didn't see the 'create's.
(may use pg_attribute.attnotnull on t1, t2, if I didn't see the 'create's.
 
|   Now, for this statement, I can easily identify non-nullable columns.
Now, for this statement, I can easily identify the non-nullable columns:

-- Alex -- alex-goncha...@comcast.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] libpq, PQdescribePrepared - PQftype, PQfmod, no PQnullable

2011-10-06 Thread Alex Goncharov
,--- You/Andres (Fri, 7 Oct 2011 02:28:30 +0200) *
|   a lot of cases where the database could deduce (quite easily) that a
|   result column cannot be null
| Could you quickly explain what exactly you want that information for? Just 
| because it has been done before doesn't necessarily mean its a good idea...

I am not writing a database application here (i.e. I am not storing
the data).  I am responding to a client requirement, basically:

  Given a SELECT (or possibly, simpler, a table name), tell me which
  columns are non-nullable?

I can give the answer about the tables trivially in 'psql' (using
pg_attribute.attnotnull).  But it has to be done inside the C code I
wrote a couple of years ago, already using libpq, preparing and
describing arbitrary statements...  If I could get the required
information through some use of PQ* functions...

But, oh well, I'll PQexec(a-fancy-select-from-pg_attribute).

Ugly :(

-- Alex -- alex-goncha...@comcast.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] libpq, PQdescribePrepared - PQftype, PQfmod, no PQnullable

2011-10-06 Thread anara...@anarazel.de


Alex Goncharov alex-goncha...@comcast.net schrieb:

,--- You/Andres (Fri, 7 Oct 2011 02:28:30 +0200) *
|   a lot of cases where the database could deduce (quite easily) that
a
|   result column cannot be null
| Could you quickly explain what exactly you want that information for?
Just 
| because it has been done before doesn't necessarily mean its a good
idea...

I am not writing a database application here (i.e. I am not storing
the data).  I am responding to a client requirement, basically:

  Given a SELECT (or possibly, simpler, a table name), tell me which
  columns are non-nullable?
That doesnt explain why it's  needed. To get community buyin into a feature the 
community - or at least parts of it - need to understand why its needed.

Greetings, Andres



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


Re: [HACKERS] libpq, PQdescribePrepared - PQftype, PQfmod, no PQnullable

2011-10-06 Thread Alex Goncharov
,--- You/anara...@anarazel.de (Fri, 07 Oct 2011 02:54:39 +0200) *
|
|   Given a SELECT (or possibly, simpler, a table name), tell me which
|   columns are non-nullable?
| That doesnt explain why it's  needed.

It's  needed for some meta analysis. That's as much as I can say.

| To get community buyin into a feature the community - or at least
| parts of it - need to understand why its needed.

Take a look at these APIs:

  
http://download.oracle.com/javase/6/docs/api/java/sql/ResultSetMetaData.html#isNullable(int)
  
int isNullable(int column) throws SQLException
Indicates the nullability of values in the designated column.

  http://msdn.microsoft.com/en-us/library/ms716289(v=VS.85).aspx
  
NullablePtr [Output] Pointer to a buffer in which to return a
value that indicates whether the column allows NULL values.

A common and natural question to be answered about result sets.

-- Alex -- alex-goncha...@comcast.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] libpq, PQdescribePrepared - PQftype, PQfmod, no PQnullable

2011-10-06 Thread Florian Pflug
On Oct7, 2011, at 01:42 , Alex Goncharov wrote:
 Right: but for (most?) every SELECT, one can logically deduce whether
 it can be guaranteed that a given column will never have a NULL value.
 Since in a given SELECT, the result column are a combination of either
 other columns, or expressions, including literals.

Sure. Deducing nullability isn't a hard problem, at least not if it's
OK to simply say nullable if things get too complex.

 And in PostgreSQL, this could be done by combining
 
  (1)   Oid PQftable(const PGresult *res, int column_number);
  (2)   int PQftablecol(const PGresult *res, int column_number);
  (3)   a SQL query of pg_attribute,attnotnull

That won't work. I'm pretty sure that you'll get the wrong answer
for queries involving OUTER joins, e.g.

  SELECT * FROM foo LEFT JOIN bar ON bar.foo_id = foo.foo_id

 I have not tried this yet, hesitating to walk into a monstrosity and
 hoping that there is some hidden way to get the information through
 one of
 
  int PQfmod(const PGresult *res, int column_number);
  int PQgetisnull(const PGresult *res, int row_number, int column_number);

Let me assure you that there's no hidden way. The feature is simply
unsupported.

 Now, for this statement, I can easily identify non-nullable columns.
 
  select
   t1.nn1, -- guaranteed: not null
   t1.ny1, -- nullable
   t2.nn2, -- guaranteed: not null
   t2.ny2  -- nullable
  from t1, t1; 

Sure. So can I. But postgres can't, since nobody's implemented the necessary
algorithm so far. You're very welcome to produce a patch, though. Should you
decide to do that, I recommend that you discuss the design of this *before*
starting work (in a separate thread). Otherwise, you might discover objections
to the general approach, or even to the whole feature, only after you put
considerable effort into this.

best regards,
Florian Pflug



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


Re: [HACKERS] Extend extension file_fdw

2011-10-06 Thread Shigeru Hanada

(2011/10/07 3:39), pasman pasmański wrote:

Hi.

I plan to extend file_fdw wrapper. I will add options to foreign
server: encoding, format, header, delimiter, dir.


+1 for existing options.  Using foreign server options as default would 
make definition of file_fdw foreign tables very simple.  OTOH, 
specification of new dir option needs to be clarified.  Is it always 
prefixed to filename option?



And i have some asks:
- it's better to change name of extension or not


IMO, no.  Do you have any candidate?


- other suggestions


What about other format options such as escape and null?

Regards,
--
Shigeru Hanada

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


Re: [HACKERS] libpq, PQdescribePrepared - PQftype, PQfmod, no PQnullable

2011-10-06 Thread Alex Goncharov
,--- You/Florian (Fri, 7 Oct 2011 03:21:23 +0200) *
| Sure. Deducing nullability isn't a hard problem, at least not if it's
| OK to simply say nullable if things get too complex.

Yes.

|  And in PostgreSQL, this could be done by combining
|  
|   (1)   Oid PQftable(const PGresult *res, int column_number);
|   (2)   int PQftablecol(const PGresult *res, int column_number);
|   (3)   a SQL query of pg_attribute,attnotnull
| 
| That won't work. I'm pretty sure that you'll get the wrong answer
| for queries involving OUTER joins, e.g.
| 
|   SELECT * FROM foo LEFT JOIN bar ON bar.foo_id = foo.foo_id

That's a good point.  But I'll do with what I manage to get.  I am
pretty sure that in my client's use, this is not going to be an issue.

And OTOH, I am not sure that other databases will give me a good
answer.  I'll play with them soon, out of technical curiosity.

|  I have not tried this yet, hesitating to walk into a monstrosity and
|  hoping that there is some hidden way to get the information through
|  one of
|  
|   int PQfmod(const PGresult *res, int column_number);
|   int PQgetisnull(const PGresult *res, int row_number, int column_number);
| 
| Let me assure you that there's no hidden way. The feature is simply
| unsupported.

Oh, great -- that's the second best answer I hoped for: just didn't
want to go down the expensive and not fool-proof way by mistake.  Had
to ask this list.

|  Now, for this statement, I can easily identify non-nullable columns.
|  
|   select
|  t1.nn1, -- guaranteed: not null
|  t1.ny1, -- nullable
|  t2.nn2, -- guaranteed: not null
|  t2.ny2  -- nullable
|   from t1, t1;   
| 
| Sure. So can I. But postgres can't, since nobody's implemented the necessary
| algorithm so far. You're very welcome to produce a patch, though.

I've looked into the 'src/interfaces/libpq' and other parts of 'src'
more than once and suspect that I won't be able to find where to plug
this in correctly, even if I figure out a meaningful algorithm.

| Should you decide to do that,

Unlikely: in a couple of days I hope to have my implementation as I
described before, then there will be no need for our application to
wait for the desired PQfnullable function.  Besides, our application
has to work with any libpq.so.5, so no new PQ* function can be called.

I'd only venture to do it for the personal goal of contributing to
PostgreSQL.  Who knows, but unlikely -- a too high barrier to entry.

| I recommend that you discuss the design of this *before* starting
| work (in a separate thread). Otherwise, you might discover
| objections to the general approach, or even to the whole feature,
| only after you put considerable effort into this.
| 
| best regards,
| Florian Pflug

Thank you: this is all very valuable,

-- Alex -- alex-goncha...@comcast.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] pg_upgrade - add config directory setting

2011-10-06 Thread Bruce Momjian
Bruce Momjian wrote:
 I will now work on pg_upgrade to also use the new flag to find the data
 directory from a config-only install.  However, this is only available
 in PG 9.2, and it will only be in PG 9.3 that you can hope to use this
 feature (if old is PG 9.2 or later).  I am afraid the symlink hack will
 have to be used for several more years, and if you are supporting
 upgrades from pre-9.2, perhaps forever.

The attached patch uses postmaster -C data_directory to allow
config-only upgrades.  It will allow a normal 9.1 cluster to be upgraded
to a 9.2 config-only cluster.

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

  + It's impossible for everything to be true. +
diff --git a/contrib/pg_upgrade/option.c b/contrib/pg_upgrade/option.c
new file mode 100644
index bdb7ddb..3ab1b5c
*** a/contrib/pg_upgrade/option.c
--- b/contrib/pg_upgrade/option.c
*** parseCommandLine(int argc, char *argv[])
*** 112,121 
--- 112,123 
  
  			case 'd':
  old_cluster.pgdata = pg_strdup(optarg);
+ old_cluster.pgconfig = pg_strdup(optarg);
  break;
  
  			case 'D':
  new_cluster.pgdata = pg_strdup(optarg);
+ new_cluster.pgconfig = pg_strdup(optarg);
  break;
  
  			case 'g':
*** check_required_directory(char **dirpath,
*** 319,321 
--- 321,381 
  #endif
  		(*dirpath)[strlen(*dirpath) - 1] = 0;
  }
+ 
+ /*
+  * adjust_data_dir
+  *
+  * If a configuration-only directory was specified, find the real data dir
+  * by quering the running server.  This has limited checking because we
+  * can't check for a running server because we can't find postmaster.pid.
+  */
+ void
+ adjust_data_dir(ClusterInfo *cluster)
+ {
+ 	char		filename[MAXPGPATH];
+ 	char		cmd[MAXPGPATH], cmd_output[MAX_STRING];
+ 	FILE	   *fd, *output;
+ 
+ 	/* If there is no postgresql.conf, it can't be a config-only dir */
+ 	snprintf(filename, sizeof(filename), %s/postgresql.conf, cluster-pgconfig);
+ 	if ((fd = fopen(filename, r)) == NULL)
+ 		return;
+ 	fclose(fd);
+ 
+ 	/* If PG_VERSION exists, it can't be a config-only dir */
+ 	snprintf(filename, sizeof(filename), %s/PG_VERSION, cluster-pgconfig);
+ 	if ((fd = fopen(filename, r)) != NULL)
+ 	{
+ 		fclose(fd);
+ 		return;
+ 	}
+ 
+ 	/* Must be a configuration directory, so find the real data directory. */
+ 
+ 	prep_status(Finding the real data directory for the %s cluster,
+ CLUSTER_NAME(cluster));
+ 
+ 	/*
+ 	 * We don't have a data directory yet, so we can't check the PG
+ 	 * version, so this might fail --- only works for PG 9.2+.   If this
+ 	 * fails, pg_upgrade will fail anyway because the data files will not
+ 	 * be found.
+ 	 */
+ 	snprintf(cmd, sizeof(cmd), \%s/postmaster\ -D \%s\ -C data_directory,
+ 			 cluster-bindir, cluster-pgconfig);
+ 
+ 	if ((output = popen(cmd, r)) == NULL ||
+ 		fgets(cmd_output, sizeof(cmd_output), output) == NULL)
+ 		pg_log(PG_FATAL, Could not get data directory using %s: %s\n,
+ 		cmd, getErrorText(errno));
+ 
+ 	pclose(output);
+ 
+ 	/* Remove trailing newline */
+ 	if (strchr(cmd_output, '\n') != NULL)
+ 		*strchr(cmd_output, '\n') = '\0';
+ 
+ 	cluster-pgdata = pg_strdup(cmd_output);
+ 
+ 	check_ok();
+ }
diff --git a/contrib/pg_upgrade/pg_upgrade.c b/contrib/pg_upgrade/pg_upgrade.c
new file mode 100644
index 0568aca..273561e
*** a/contrib/pg_upgrade/pg_upgrade.c
--- b/contrib/pg_upgrade/pg_upgrade.c
*** main(int argc, char **argv)
*** 68,73 
--- 68,76 
  
  	parseCommandLine(argc, argv);
  
+ 	adjust_data_dir(old_cluster);
+ 	adjust_data_dir(new_cluster);
+ 
  	output_check_banner(live_check);
  
  	setup(argv[0], live_check);
diff --git a/contrib/pg_upgrade/pg_upgrade.h b/contrib/pg_upgrade/pg_upgrade.h
new file mode 100644
index 46aed74..0fb16ed
*** a/contrib/pg_upgrade/pg_upgrade.h
--- b/contrib/pg_upgrade/pg_upgrade.h
*** typedef struct
*** 187,192 
--- 187,193 
  	ControlData controldata;	/* pg_control information */
  	DbInfoArr	dbarr;			/* dbinfos array */
  	char	   *pgdata;			/* pathname for cluster's $PGDATA directory */
+ 	char	   *pgconfig;		/* pathname for cluster's config file directory */
  	char	   *bindir;			/* pathname for cluster's executable directory */
  	unsigned short port;		/* port number where postmaster is waiting */
  	uint32		major_version;	/* PG_VERSION of cluster */
*** void print_maps(FileNameMap *maps, int n
*** 361,366 
--- 362,368 
  /* option.c */
  
  void		parseCommandLine(int argc, char *argv[]);
+ void		adjust_data_dir(ClusterInfo *cluster);
  
  /* relfilenode.c */
  
diff --git a/contrib/pg_upgrade/server.c b/contrib/pg_upgrade/server.c
new file mode 100644
index 8c4aec9..d512ef3
*** a/contrib/pg_upgrade/server.c
--- b/contrib/pg_upgrade/server.c
*** start_postmaster(ClusterInfo *cluster)
*** 169,175 
  	snprintf(cmd, sizeof(cmd),
  			 SYSTEMQUOTE \%s/pg_ctl\ -w -l \%s\ -D \%s\ 
  			 -o \-p 

Re: [HACKERS] [PATCH] Log crashed backend's query v3

2011-10-06 Thread gabrielle
On Wed, Oct 5, 2011 at 5:14 PM, Marti Raudsepp ma...@juffo.org wrote:
 I think you intended to use the Waiting on Author status -- that
 leaves the commitfest entry open. I will re-open the commitfest entry
 myself, I hope that's OK.

No worries, and yeah, I picked the wrong checkbox. :)

 Here is version 3 of the patch.

Looks good, and performs as advertised.  Thanks!

gabrielle

-- 
Sent 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] Patch for cursor calling with named parameters

2011-10-06 Thread Royce Ausburn
Forgive my ignorance -- do I need to be doing anything else now seeing as I 
started the review?  


On 07/10/2011, at 7:15 AM, Pavel Stehule wrote:

 2011/10/6 Robert Haas robertmh...@gmail.com:
 On Thu, Oct 6, 2011 at 1:46 PM, Tom Lane t...@sss.pgh.pa.us wrote:
 David E. Wheeler da...@kineticode.com writes:
 Would it then be added as an alias for := for named function parameters? 
 Or would that come still later?
 
 Once we do that, it will be impossible not merely deprecated to use =
 as an operator name.  I think that has to wait at least another release
 cycle or two past where we're using it ourselves.
 
 Okay. I kind of like := so there's no rush AFAIC. :-)
 
 Hmm ... actually, that raises another issue that I'm not sure whether
 there's consensus for or not.  Are we intending to keep name := value
 syntax forever, as an alternative to the standard name = value syntax?
 I can't immediately see a reason not to, other than the it's not
 standard argument.
 
 Because if we *are* going to keep it forever, there's no very good
 reason why we shouldn't accept this plpgsql cursor patch now.  We'd
 just have to remember to extend plpgsql to take = at the same time
 we do that for core function calls.
 
 It's hard to see adding support for = and dropping support for := in
 the same release.  That would be a compatibility nightmare.
 
 If := is used by the standard for some other, incompatible purpose,
 then I suppose we would want to add support for =, wait a few
 releases, deprecate :=, wait a couple of releases, remove :=
 altogether.  But IIRC we picked := precisely because the standard
 didn't use it at all, or at least not for anything related... in which
 case we may as well keep it around more or less indefinitely.
 
 +1
 
 Pavel
 
 
 --
 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
 
 
 -- 
 Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
 To make changes to your subscription:
 http://www.postgresql.org/mailpref/pgsql-hackers


-- 
Sent 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 only ... drop constraint broken in HEAD

2011-10-06 Thread Alex Hunsaker
On Thu, Oct 6, 2011 at 07:24, Robert Haas robertmh...@gmail.com wrote:
 On Wed, Oct 5, 2011 at 10:29 PM, Robert Haas robertmh...@gmail.com wrote:
 On Wed, Oct 5, 2011 at 5:53 PM, Alex Hunsaker bada...@gmail.com wrote:
 tldr:

 Seems to be broken by
 http://git.postgresql.org/gitweb/?p=postgresql.git;a=commit;h=4da99ea4231e3d8bbf28b666748c1028e7b7d665
 :
 commit 4da99ea4231e3d8bbf28b666748c1028e7b7d665
 Author: Robert Haas rh...@postgresql.org
 Date:   Mon Jun 27 10:27:17 2011 -0400

    Avoid having two copies of the HOT-chain search logic.


 Oh, I see the problem, and I now agree that it's the DROP CONSTRAINT
 code that is buggy.

Want me to roll this fix in as part of the alter table only constraint
patch? Or keep it split out? We might want to backpatch to at least
8.3 where HOT was introduced (yes looks like the bug existed back
then). I suppose its a fairly narrow chance to hit this bug so I could
see the argument for not back patching...

 I took a look around for other places that might have this same
 problem and didn't find any, but I'm not too confident that that means
 there are none there, since there are a fair number of things that can
 call CommandCounterIncrement() indirectly.

I skimmed for the direct easy to find ones as well. Didn't see any
other than the one you note below.

  shdepReassignOwned() does
 a direct CCI from within a scan, but ISTM that any update we do there
 would produce a new tuple that doesn't match the scan, so that should
 be OK.

Well its on purpose so I hope its ok :-)

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


Re: [HACKERS] Range Types - typo + NULL string constructor

2011-10-06 Thread Jeff Davis
On Thu, 2011-10-06 at 23:26 +0400, Alexander Korotkov wrote:
 Hi, Jeff!
 
 
 Heikki has recently commited my patch about picksplit for GiST on
 points and boxes:
 http://git.postgresql.org/gitweb/?p=postgresql.git;a=commitdiff;h=7f3bd86843e5aad84585a57d3f6b80db3c609916
 I would like to try this picksplit method on ranges. I believe that it
 might be much more efficient on highly overlapping ranges than current
 picksplit. Range types patch isn't commited, yet. So, what way of work
 publishing is prefered in this case? Add-on patch, separate branch in
 git or something else?

Sounds great! Thank you.

The repo is available here:

http://git.postgresql.org/gitweb/?p=users/jdavis/postgres.git;a=log;h=refs/heads/rangetypes

I'd prefer to include it in the initial patch. If the current GiST code
is going to be replaced, then there's not much sense reviewing/testing
it.

You may need to consider unbounded and empty ranges specially. I made an
attempt to do so in the current GiST code, and you might want to take a
look at that first. I'm not particularly attached to my approach, but we
should do something reasonable with unbounded and empty ranges.

Regards,
Jeff Davis


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


Re: [HACKERS] [v9.2] make_greater_string() does not return a string in some cases

2011-10-06 Thread Kyotaro HORIGUCHI
Thank you for reviewing. 

The new version of this patch is attached to this message.


  But it seems to me that if the datatype is BYTEAOID then
  there's no need to restore anything at all, because we're not
  going to call pg_mbcliplen() in that case anyway.  So I think
  the logic here can be simplified.

 I agree with you. The original code does not restore the changed
byte. I removed the lastchar preservation from
make_greater_string().

  Also, you haven't completely fixed the style issues.  Function
  definitions should look like this:
..
  Opening curly braces should be on a line by themselves, not at the end
  of the preceding if, while, etc. line.
 
  finnaly is spelled incorrectly.

 I'm very sorry for left mixed defferent style a lot. I think
I've done the correction of the styles for function definition
and braces. The misspelled word is removed with whole sentense
because re-cheking just before return had been removed.

 Oh, and there's this:
 
 wchar.c: In function ‘pg_utf8_increment’:
 wchar.c:1376: warning: unused variable ‘success’
 wchar.c: In function ‘pg_eucjp_increment’:
 wchar.c:1433: warning: unused variable ‘success’

Oops... I have rebased the patch and removed all warnings.
make check has been passed all-ok.

I confirmed that the planner decides to use index with proper
boundaries for like expression with the certain characters on
problematic code point, on the database encodings both UTF-8 and
EUC-JP with the database locale is C, and database locale is
ja_JP.UTF8. And also for bytea ends with 0xff and 0x00, 0x01.

This is the third version of the patch.

Regards,

-- 
Kyotaro Horiguchi
NTT Open Source Software Center
diff --git a/src/backend/utils/adt/selfuncs.c b/src/backend/utils/adt/selfuncs.c
index 8ceea82..59f8c37 100644
--- a/src/backend/utils/adt/selfuncs.c
+++ b/src/backend/utils/adt/selfuncs.c
@@ -5664,6 +5664,19 @@ pattern_selectivity(Const *patt, Pattern_Type ptype)
 
 
 /*
+ * This function is character increment function for bytea used in
+ * make_greater_string() that has same interface with pg_wchar_tbl.charinc.
+ */
+static bool
+byte_increment(unsigned char *ptr, int len)
+{
+	if (*ptr = 255) return false;
+
+	(*ptr)++;
+	return true;
+}
+
+/*
  * Try to generate a string greater than the given string or any
  * string it is a prefix of.  If successful, return a palloc'd string
  * in the form of a Const node; else return NULL.
@@ -5702,6 +5715,7 @@ make_greater_string(const Const *str_const, FmgrInfo *ltproc, Oid collation)
 	int			len;
 	Datum		cmpstr;
 	text	   *cmptxt = NULL;
+	character_incrementer charincfunc;
 
 	/*
 	 * Get a modifiable copy of the prefix string in C-string format, and set
@@ -5763,29 +5777,33 @@ make_greater_string(const Const *str_const, FmgrInfo *ltproc, Oid collation)
 		}
 	}
 
+	if (datatype == BYTEAOID)
+		charincfunc = byte_increment;
+	else
+		charincfunc = pg_database_encoding_character_incrementer();
+
 	while (len  0)
 	{
-		unsigned char *lastchar = (unsigned char *) (workstr + len - 1);
-		unsigned char savelastchar = *lastchar;
+		int charlen = 1;
+		unsigned char *lastchar;
+		Const	   *workstr_const;
+		
+		if (datatype != BYTEAOID)
+			charlen = len - pg_mbcliplen(workstr, len, len - 1);
+		
+		lastchar = (unsigned char *) (workstr + len - charlen);
 
 		/*
-		 * Try to generate a larger string by incrementing the last byte.
+		 * Try to generate a larger string by incrementing the last byte or
+		 * character.
 		 */
-		while (*lastchar  (unsigned char) 255)
-		{
-			Const	   *workstr_const;
-
-			(*lastchar)++;
 
-			if (datatype != BYTEAOID)
-			{
-/* do not generate invalid encoding sequences */
-if (!pg_verifymbstr(workstr, len, true))
-	continue;
-workstr_const = string_to_const(workstr, datatype);
-			}
-			else
+		if (charincfunc(lastchar, charlen))
+		{
+			if (datatype == BYTEAOID)
 workstr_const = string_to_bytea_const(workstr, len);
+			else
+workstr_const = string_to_const(workstr, datatype);
 
 			if (DatumGetBool(FunctionCall2Coll(ltproc,
 			   collation,
@@ -5804,20 +5822,10 @@ make_greater_string(const Const *str_const, FmgrInfo *ltproc, Oid collation)
 			pfree(workstr_const);
 		}
 
-		/* restore last byte so we don't confuse pg_mbcliplen */
-		*lastchar = savelastchar;
-
 		/*
-		 * Truncate off the last character, which might be more than 1 byte,
-		 * depending on the character encoding.
+		 * Truncate off the last character or byte.
 		 */
-		if (datatype != BYTEAOID  pg_database_encoding_max_length()  1)
-			len = pg_mbcliplen(workstr, len, len - 1);
-		else
-			len -= 1;
-
-		if (datatype != BYTEAOID)
-			workstr[len] = '\0';
+		len -= charlen;
 	}
 
 	/* Failed... */
diff --git a/src/backend/utils/mb/wchar.c b/src/backend/utils/mb/wchar.c
index f23732f..e8a1bc8 100644
--- a/src/backend/utils/mb/wchar.c
+++ b/src/backend/utils/mb/wchar.c
@@ -1336,6 +1336,194 @@ pg_utf8_islegal(const unsigned char *source, int length)
 
 /*