[HACKERS] Review: Hot standby

[Pavan Deolasee]

I think we should avoid the #define's like below which uses a local
variable. I guess the same #define is used elsewhere in the code as well. If
the code is rearranged or the variable name is changed, the code would
break.

The use of malloc should also be avoided (unless the memory subsystem is not
up yet; I haven't checked).


***
*** 706,713 
(errcode(ERRCODE_OUT_OF_MEMORY),
 errmsg("out of memory")));
Assert(snapshot->subxip == NULL);
snapshot->subxip = (TransactionId *)
!   malloc(arrayP->maxProcs * PGPROC_MAX_CACHED_SUBXIDS *
sizeof(TransactionId));
if (snapshot->subxip == NULL)
ereport(ERROR,
(errcode(ERRCODE_OUT_OF_MEMORY),
--- 1003,1011 
(errcode(ERRCODE_OUT_OF_MEMORY),
 errmsg("out of memory")));
Assert(snapshot->subxip == NULL);
+ #define maxNumSubXids (arrayP->maxProcs * PGPROC_MAX_CACHED_SUBXIDS)
snapshot->subxip = (TransactionId *)
!   malloc(maxNumSubXids * sizeof(TransactionId));
if (snapshot->subxip == NULL)
ereport(ERROR,
(errcode(ERRCODE_OUT_OF_MEMORY),


Thanks,
Pavan

-- 
Pavan Deolasee
EnterpriseDB http://www.enterprisedb.com

Re: [HACKERS] Opening a recovering DB in for read-only access?

[Philip Warner]

Alex Hunsaker wrote
>
> Uhh sounds like you are describing hot standby (currently in the works
> for 8.4) see:
>   

Yep. That's exactly what I'm talking about. Thanks for the links!



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

Re: [HACKERS] Opening a recovering DB in for read-only access?

[Alex Hunsaker]

On Thu, Nov 20, 2008 at 21:45, Philip Warner <[EMAIL PROTECTED]> wrote:
>
> Sounds somewhat evil, I know, but I was wondering if it was even
> remotely possible with the current design?
>
> The reason: we are contemplating using pg_standy to create a
> warm-standby. It would be a bonus if we would run read-only queries
> against this DB to take some of the load off or production servers.
>
> We currently use slony to provide warm-standby *and* read-only access,
> but pg_standby is a great deal more appealing...especially if there was
> some way to do read-only access at the same time.
>
> FWIW, the data would not even need to be completely consistent ... the
> kinds of things we are looking at offloading are large summary-type
> sequential scans of big tables.

Uhh sounds like you are describing hot standby (currently in the works
for 8.4) see:
http://wiki.postgresql.org/wiki/Hot_Standby
http://archives.postgresql.org/pgsql-hackers/2008-11/msg5.php

Synchronous replication might also be of interest
http://archives.postgresql.org/pgsql-hackers/2008-11/msg00987.php

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

Re: [HACKERS] autovacuum and reloptions

[Euler Taveira de Oliveira]

Euler Taveira de Oliveira escreveu:

> [Sorry for the delay. I'm preparing the final patch and in a day or so
> I'll post it.]
> 
Here is the patch that replace pg_autovaccum catalog with reloptions. I
refactored the reloptions.c to support multiple parameters and made the
action of adding a new option an easy task. I'm testing it yet, so don't
expect it to work properly. I'll prepare docs as soon as I finish the
tests. Do i have to prepare some regression tests?

I don't provide a pg_autovacuum view as suggested by Itagari-san [1] but
if others agree that we need it, I will work on it. I don't if we need a
function (wrapper around getRelOption()) to get an option from
reloptions array.

I add an ugly-hack to \d+ foo. IMHO, it'll be good to know what options
are used by table/index foo (we already do it for oids) but I'm not
happy with my suggestion.

I move RelationGet*() functions from rel.h. That's because we need some
knowledge that's only in reloptions.c (getRelOptions). But I want to
avoid including reloptions.h at some files.

Comments?

PS> don't forget to remove include/catalog/pg_autovacuum.h


[1] http://archives.postgresql.org/pgsql-hackers/2008-11/msg00830.php


-- 
  Euler Taveira de Oliveira
  http://www.timbira.com/


relopt27.diff.gz
Description: GNU Zip compressed data

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

Re: [HACKERS] SSL configure patch: review

[Alex Hunsaker]

On Fri, Aug 1, 2008 at 13:31, Alvaro Herrera <[EMAIL PROTECTED]> wrote:
> Something that's bothering me is that PGSSLKEY is inconsistent with the
> sslkey conninfo parameter.  PGSSLKEY specifies an engine (basically a
> driver for specialized hardware AFAICT) from which the key is to be
> loaded, but sslkey is a simple filename.  This means that there's no way
> to load a key from hardware if you want to specify it per connection.
> Not that I have any such hardware, but it looks bogus.
>
> Obviously one still wants to be able to specify a different file name
> from the default; I tried to see if there's any way to load an engine
> that would load the key from a file, but could not extract any sense
> from the man page:
> http://www.openssl.org/docs/crypto/engine.html
>
> Maybe this means that we should provide separate parameters, say
> "sslkey" and "sslkeyfile", and a new env var PGSSLKEYFILE.
>
> Thoughts?  Am I overengineering this stuff?

I think the patch as it stands is ok for now (mainly because I don't
have any hardware either so I can't test or attest to how it should be
done i.e. if those params would be enough)

Should PGROOTCERT be PGSSLROOTCERT to be more congruent with all the
other ssl params?

Find attached an updated patch against HEAD (no other changes).   I
also gave it a look over and tested it to make sure it worked as
described.
*** a/doc/src/sgml/libpq.sgml
--- b/doc/src/sgml/libpq.sgml
***
*** 309,314 
--- 309,358 
  
  
  
+  sslcert
+  
+   
+This parameter specifies the file name of the client SSL
+certificate.  This option is only available if
+PostgreSQL is compiled with SSL support.
+   
+  
+ 
+ 
+ 
+  sslkey
+  
+   
+This parameter specifies the file name of the client SSL key.
+This option is only available if PostgreSQL is
+compiled with SSL support.
+   
+  
+ 
+ 
+ 
+  sslrootcert
+  
+   
+This parameter specifies the file name of the root SSL certificate.
+This option is only available if PostgreSQL is
+compiled with SSL support.
+   
+  
+ 
+ 
+ 
+  sslcrl
+  
+   
+This parameter specifies the file name of the SSL certificate
+revocation list (CRL).  This option is only available if
+PostgreSQL is compiled with SSL support.
+   
+  
+ 
+ 
+ 
   krbsrvname
   

***
*** 5767,5772  myEventProc(PGEventId evtId, void *evtInfo, void *passThrough)
--- 5811,5838 
  
   

+PGROOTCERT
+   
+   PGROOTCERT specifies the file name where the SSL
+   root certificate is stored.  This can be overridden by the
+   sslrootcert connection parameter.
+  
+ 
+ 
+ 
+  
+   
+PGSSLCRL
+   
+   PGSSLCRL specifies the file name where the SSL certificate
+   revocation list is stored.  This can be overridden by the
+   sslcrl connection parameter.
+  
+ 
+ 
+ 
+  
+   
 PGKRBSRVNAME

PGKRBSRVNAME sets the Kerberos service name to use
*** a/src/interfaces/libpq/fe-connect.c
--- b/src/interfaces/libpq/fe-connect.c
***
*** 186,191  static const PQconninfoOption PQconninfoOptions[] = {
--- 186,206 
  	{"sslverify", "PGSSLVERIFY", DefaultSSLVerify, NULL,
  	"SSL-Verify", "", 5},		/* sizeof("chain") == 5 */
  
+ 	/* These parameters are only present in an SSL-enabled build. */
+ #ifdef USE_SSL
+ 	{"sslcert", "PGSSLCERT", NULL, NULL,
+ 	"SSL-Client-Cert", "", 64},
+ 
+ 	{"sslkey", "PGSSLKEY", NULL, NULL,
+ 	"SSL-Client-Key", "", 64},
+ 
+ 	{"sslrootcert", "PGROOTCERT", NULL, NULL,
+ 	"SSL-Root-Certificate", "", 64},
+ 
+ 	{"sslcrl", "PGSSLCRL", NULL, NULL,
+ 	"SSL-Revocation-List", "", 64},
+ #endif
+ 
  #if defined(KRB5) || defined(ENABLE_GSS) || defined(ENABLE_SSPI)
  	/* Kerberos and GSSAPI authentication support specifying the service name */
  	{"krbsrvname", "PGKRBSRVNAME", PG_KRB_SRVNAM, NULL,
***
*** 423,428  connectOptions1(PGconn *conn, const char *conninfo)
--- 438,451 
  	tmp = conninfo_getval(connOptions, "sslverify");
  	conn->sslverify = tmp ? strdup(tmp) : NULL;
  #ifdef USE_SSL
+ 	tmp = conninfo_getval(connOptions, "sslkey");
+ 	conn->sslkey = tmp ? strdup(tmp) : NULL;
+ 	tmp = conninfo_getval(connOptions, "sslcert");
+ 	conn->sslcert = tmp ? strdup(tmp) : NULL;
+ 	tmp = conninfo_getval(connOptions, "sslrootcert");
+ 	conn->sslrootcert = tmp ? strdup(tmp) : NULL;
+ 	tmp = conninfo_getval(connOptions, "sslcrl");
+ 	conn->sslcrl = tmp ? strdup(tmp) : NULL;
  	tmp = conninfo_getval(connOptions, "requiressl");
  	if (tmp && tmp[0] == '1')
  	{
***
*** 2035,2040 ***

Re: [HACKERS] Updates of SE-PostgreSQL 8.4devel patches (r1197)

[Bruce Momjian]

KaiGai Kohei wrote:
> >> Please consider SELinux/SE-PostgreSQL requires various kind of objects
> >> (including database objects) to be labeled properly at the initial state.
> >> If it allows clients to turn on row-level security feature, it means many
> >> "unlabeled" tuples appear suddenly. In generally, these have to be labeled
> >> before the system get being available.
> > 
> > I was thinking more about people are using the SQL-level row
> > permissions.  Are they going to want it for all tables for all
> > databases, or not at all?
> 
> We don't have a reason why the SQL-level row permissions should be toggled
> in global only. It it designed based on DAC policy, so it is quite natural
> to be controled by owner of resources.
> (However, it is not implemented yet.)

Yes, that was my question --- how will SQL-level row permissions be
controlled by the user.

> > Actually, you called it "sepostgresql_mode", SE*, but how are we going
> > to control the SQL-level row permissions?  Are we using this name?  I
> > didn't think so because it seems so associated withe SE-Linux, and if
> > not, how would one say whether a table has SQL-level row permissions?
> 
> A new GUC variable of "row_acl_is_enabled" allows us to toggle the SQL-level
> row permission feature, when the binary is built with "--enable-row-acl".

I assumed we would always enable SQL-level row permissions.  Why would
it be a compile-time option?

> In this case, the "sepostgresql_*" are enclosed by #ifdef HAVE_SELINUX, so
> these are not available.

Right.

> >>> On a related note, KaiGai, you are now starting the long road of getting
> >>> feedback with the ultimate goal of getting your patch into CVS.  I will
> >>> warn you that there is often much work during this stage, and it might
> >>> stretch into January as we request adjustments, but ultimately your
> >>> feature and Postgres will be better for it.  Thanks for sticking with
> >>> it.
> >> Don't worry, I'm be available for the works, and give a lot for inclusion
> >> of the feature at v8.4.
> > 
> > Great.  Sometimes these bold additions can require perhaps thirty
> > changes before they are added to CVS, I am afraid to say.  It can be a
> > painful part of open source development, even though in the end it is
> > worth it. (While it is happening, it doesn't seem very useful.)
> 
> s/painful/dynamic/g :-)

That's looking on the bright side of things.  :-)

> If you can ready to post some of comments, earlier is happier for me.

My guess is we will continue to send you ideas.

> It is unclear for me when the CommtiFest:Nov is finished, but it is sure
> we don't have enough days.

This is the last commit fest for 8.4 so it will probably go well into
December, perhaps January.

> In my guess, I'll be able to complete to put a flag within TupleDesc to
> control security field of HeapTupleHeader by tomorrow. And I have a plan
> to check its behavior in this weekend before merge them into my tree.

Nice.  I will look over your newest version as soon as I can.

-- 
  Bruce Momjian  <[EMAIL PROTECTED]>http://momjian.us
  EnterpriseDB http://enterprisedb.com

  + If your life is a hard drive, Christ can be your backup. +

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

[HACKERS] Opening a recovering DB in for read-only access?

[Philip Warner]

Sounds somewhat evil, I know, but I was wondering if it was even
remotely possible with the current design?

The reason: we are contemplating using pg_standy to create a
warm-standby. It would be a bonus if we would run read-only queries
against this DB to take some of the load off or production servers.

We currently use slony to provide warm-standby *and* read-only access,
but pg_standby is a great deal more appealing...especially if there was
some way to do read-only access at the same time.

FWIW, the data would not even need to be completely consistent ... the
kinds of things we are looking at offloading are large summary-type
sequential scans of big tables.


-- 

Philip Warner| __---_
Albatross Consulting Pty. Ltd.   |/   -  \
(A.B.N. 75 008 659 498)  |  /(@)   __---_
Tel: (+61) 03 5330 3171  | _  \
Fax: (+61) 03 5330 3172  | ___ |
http://www.rhyme.com.au  
|/   \|
 |----
GPG key available upon request.  |  /
 |/

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

Re: [HACKERS] Replay attack of query cancel

[Bruce Momjian]

This bug has not been fixed, but it is on the TODO list:

o Prevent query cancel packets from being replayed by an attacker,
especially when using SSL 

I am going to consider this item closed meaning I am not going to track
that it is fixed for 8.4;  it is just documented on our TODO as a known
limitation.

---

Magnus Hagander wrote:
> Tom Lane wrote:
> > Alvaro Herrera <[EMAIL PROTECTED]> writes:
> >> Andrew Gierth wrote:
> >>> 2. The server accepts either the old-style or the secure cancel
> >>> request from the client, but doesn't allow old-style requests
> >>> once a valid secure request has been seen.
> > 
> >> Hmm, I think there should be a way to turn off acceptance of old-style
> >> without necessarily requiring a new-style request.  Otherwise, how are
> >> you protected from DoS if you have never sent a cancel request at all?
> > 
> > Assuming you were using SSL, it's hard to see how an attacker is going
> > to get your cancel key without having seen a cancel request.
> 
> Not only that, but he'll have to see an *old-style* cancel request,
> since the new style doesn't contain the key.
> 
> And if you're *not* using SSL, the attacker can just sniff they key off
> the initial packet instead.
> 
> 
> > However, I dislike Andrew's proposal above even without that issue,
> > because it means *still more* changeable state that has to be magically
> > shared between postmaster and backends.  If we want to have a way for
> > people to disable insecure cancels, we should just have a postmaster
> > configuration parameter that does it.
> 
> Agreed. Your security policy also should not depend on what your client
> happens to do, it should be enforceable.
> 
> 
> //Magnus
> 
> 
> -- 
> Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-hackers

-- 
  Bruce Momjian  <[EMAIL PROTECTED]>http://momjian.us
  EnterpriseDB http://enterprisedb.com

  + If your life is a hard drive, Christ can be your backup. +

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

Re: [HACKERS] Updates of SE-PostgreSQL 8.4devel patches (r1197)

[Tom Lane]

KaiGai Kohei <[EMAIL PROTECTED]> writes:
> It is unclear for me when the CommtiFest:Nov is finished, but it is sure
> we don't have enough days.

This commitfest goes until it's done.  I knew at the beginning that we'd
not be done at the end of November.  The original schedule allowed two
months for this fest; we'll see whether that was optimistic or not.

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] Updates of SE-PostgreSQL 8.4devel patches (r1197)

[KaiGai Kohei]

Please consider SELinux/SE-PostgreSQL requires various kind of objects
(including database objects) to be labeled properly at the initial state.
If it allows clients to turn on row-level security feature, it means many
"unlabeled" tuples appear suddenly. In generally, these have to be labeled
before the system get being available.


I was thinking more about people are using the SQL-level row
permissions.  Are they going to want it for all tables for all
databases, or not at all?


We don't have a reason why the SQL-level row permissions should be toggled
in global only. It it designed based on DAC policy, so it is quite natural
to be controled by owner of resources.
(However, it is not implemented yet.)


Actually, you called it "sepostgresql_mode", SE*, but how are we going
to control the SQL-level row permissions?  Are we using this name?  I
didn't think so because it seems so associated withe SE-Linux, and if
not, how would one say whether a table has SQL-level row permissions?


A new GUC variable of "row_acl_is_enabled" allows us to toggle the SQL-level
row permission feature, when the binary is built with "--enable-row-acl".
In this case, the "sepostgresql_*" are enclosed by #ifdef HAVE_SELINUX, so
these are not available.


On a related note, KaiGai, you are now starting the long road of getting
feedback with the ultimate goal of getting your patch into CVS.  I will
warn you that there is often much work during this stage, and it might
stretch into January as we request adjustments, but ultimately your
feature and Postgres will be better for it.  Thanks for sticking with
it.

Don't worry, I'm be available for the works, and give a lot for inclusion
of the feature at v8.4.


Great.  Sometimes these bold additions can require perhaps thirty
changes before they are added to CVS, I am afraid to say.  It can be a
painful part of open source development, even though in the end it is
worth it. (While it is happening, it doesn't seem very useful.)


s/painful/dynamic/g :-)

If you can ready to post some of comments, earlier is happier for me.
It is unclear for me when the CommtiFest:Nov is finished, but it is sure
we don't have enough days.

In my guess, I'll be able to complete to put a flag within TupleDesc to
control security field of HeapTupleHeader by tomorrow. And I have a plan
to check its behavior in this weekend before merge them into my tree.

Thanks,
--
OSS Platform Development Division, NEC
KaiGai Kohei <[EMAIL PROTECTED]>

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

Re: [HACKERS] How should pg_standby get over the gap of timeline?

[Fujii Masao]

On Fri, Nov 21, 2008 at 12:15 AM, Pavan Deolasee
<[EMAIL PROTECTED]> wrote:
>
>
> On Thu, Nov 20, 2008 at 8:36 PM, Heikki Linnakangas
> <[EMAIL PROTECTED]> wrote:
>>
>> That seems like a dangerous assumption. What if the standby had fallen
>> behind before the failover? It's not safe to failover back to the original
>> primary in that case. We'd need some kind of safeguards against that.
>>
>
> For synchronous replication, what if we ensure that the standby has received
> the WAL (atleast in its buffers) before writing it to disk on the primary ?
> If we do that, I think the old standby can never fall behind the primary and
> it would be easy for the old primary to join back the replication without a
> fresh backup.

In the current patch, since the WAL are written and sent concurrently for
the performance gain, we cannot guarantee whether the old standby fall
behind or not. I think that the setup procedure which can resolve both
cases is required.

> Of course, this doesn't work for async replication.

Yeah, in asynch replication, some committed transaction may disappear
regardless of whether the fresh backup is used or not. But, since the
current patch guarantee "Replicate Ahead Log" rule even if asynch case,
we can recover the old primary by using the WAL on the old standby
consistently.

Regards,

-- 
Fujii Masao
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center

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

Re: [HACKERS] How should pg_standby get over the gap of timeline?

[Fujii Masao]

On Fri, Nov 21, 2008 at 12:06 AM, Heikki Linnakangas
<[EMAIL PROTECTED]> wrote:
> Fujii Masao wrote:
>>
>> Hi, Heikki. Thanks for the comment!
>>
>> On Thu, Nov 20, 2008 at 11:24 PM, Heikki Linnakangas
>> <[EMAIL PROTECTED]> wrote:
>>>
>>> Fujii Masao wrote:

 In the current Synch Rep patch, the standby cannot catch up with the
 primary which has a bigger timeline.
>>>
>>> That would only happen if you've performed an archive recovery in the
>>> primary. If you've done PITR in the primary, I don't think there's any
>>> guarantee that it's even possible to catch up the standby. The standby
>>> might
>>> already have replayed a WAL file from an earlier timeline, that isn't
>>> part
>>> of the history of the bigger timeline.
>>
>> I assume the situation of making the standby (the original primary) catch
>> up
>> with the primary (the original standby) after failover. Since a timeline
>> is
>> incremented when a failover finishes archive recovery on a standby, the
>> timelines differ between two servers.
>
> That seems like a dangerous assumption. What if the standby had fallen
> behind before the failover? It's not safe to failover back to the original
> primary in that case. We'd need some kind of safeguards against that.

Yeah, it's a legitimate concern. As the safeguard, I'm going to delete the
XLOG files which may be inconsistent from the standby before making it
catch up. The XLOG file including the recovery starting point and the
subsequent ones may be inconsistent. Then, they need to be copied from
the primary. I'm writing down the draft of this procedure at wiki.
http://wiki.postgresql.org/wiki/NTT%27s_Development_Projects#Procedure

But, it's overkill to overwrite all the XLOG files which may be inconsistent.
In the future, I'm going to provide the tool to compare the content of XLOG
between two servers and tell the user which files should be overwritten.

-- 
Fujii Masao
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center

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

Re: [HACKERS] Updates of SE-PostgreSQL 8.4devel patches (r1197)

[Bruce Momjian]

KaiGai Kohei wrote:
> Bruce Momjian wrote:
> > Bruce Momjian wrote:
> >>> However, the toggle of row-level security feature should be controled
> >>> via a GUC option, not a discretionary option.
> >>> I'll add a "sepostgresql_row_level" option defined as bool to control
> >>> it on start up time.
> >> This sounds similar to BSD capability were certain security settings can
> >> only be changed in single-user mode.
> > 
> > Actually, an interesting idea would be to allow "sepostgresql_row_level"
> > to be turned on, but not off.  That means if it was turned on in
> > postgresql.conf, it could not be turned off, but if it is off in
> > postgresql.conf, it could be turned on via SET or via ALTER
> > USER/DATABASE;  I think that would be a nice capability.
> 
> I think the "sepostgresql_mode" and "sepostgresql_row_level" should not
> be toggled frequently.
> 
> Please consider SELinux/SE-PostgreSQL requires various kind of objects
> (including database objects) to be labeled properly at the initial state.
> If it allows clients to turn on row-level security feature, it means many
> "unlabeled" tuples appear suddenly. In generally, these have to be labeled
> before the system get being available.

I was thinking more about people are using the SQL-level row
permissions.  Are they going to want it for all tables for all
databases, or not at all?  I assumed they would want to be more
selective.  I agree the SE-Linux users would probably not toggle it for
different objects and databases frequently.

Actually, you called it "sepostgresql_mode", SE*, but how are we going
to control the SQL-level row permissions?  Are we using this name?  I
didn't think so because it seems so associated withe SE-Linux, and if
not, how would one say whether a table has SQL-level row permissions?

> > On a related note, KaiGai, you are now starting the long road of getting
> > feedback with the ultimate goal of getting your patch into CVS.  I will
> > warn you that there is often much work during this stage, and it might
> > stretch into January as we request adjustments, but ultimately your
> > feature and Postgres will be better for it.  Thanks for sticking with
> > it.
> 
> Don't worry, I'm be available for the works, and give a lot for inclusion
> of the feature at v8.4.

Great.  Sometimes these bold additions can require perhaps thirty
changes before they are added to CVS, I am afraid to say.  It can be a
painful part of open source development, even though in the end it is
worth it. (While it is happening, it doesn't seem very useful.)

-- 
  Bruce Momjian  <[EMAIL PROTECTED]>http://momjian.us
  EnterpriseDB http://enterprisedb.com

  + If your life is a hard drive, Christ can be your backup. +

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

Re: [HACKERS] Updates of SE-PostgreSQL 8.4devel patches (r1197)

[KaiGai Kohei]

Bruce Momjian wrote:

Bruce Momjian wrote:

However, the toggle of row-level security feature should be controled
via a GUC option, not a discretionary option.
I'll add a "sepostgresql_row_level" option defined as bool to control
it on start up time.

This sounds similar to BSD capability were certain security settings can
only be changed in single-user mode.


Actually, an interesting idea would be to allow "sepostgresql_row_level"
to be turned on, but not off.  That means if it was turned on in
postgresql.conf, it could not be turned off, but if it is off in
postgresql.conf, it could be turned on via SET or via ALTER
USER/DATABASE;  I think that would be a nice capability.


I think the "sepostgresql_mode" and "sepostgresql_row_level" should not
be toggled frequently.

Please consider SELinux/SE-PostgreSQL requires various kind of objects
(including database objects) to be labeled properly at the initial state.
If it allows clients to turn on row-level security feature, it means many
"unlabeled" tuples appear suddenly. In generally, these have to be labeled
before the system get being available.


On a related note, KaiGai, you are now starting the long road of getting
feedback with the ultimate goal of getting your patch into CVS.  I will
warn you that there is often much work during this stage, and it might
stretch into January as we request adjustments, but ultimately your
feature and Postgres will be better for it.  Thanks for sticking with
it.


Don't worry, I'm be available for the works, and give a lot for inclusion
of the feature at v8.4.

Thanks,
--
OSS Platform Development Division, NEC
KaiGai Kohei <[EMAIL PROTECTED]>

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

Re: [HACKERS] Re: Updated interval patches - ECPG [was, intervalstyle....]

[Ron Mayer]

Michael Meskes wrote:

On Wed, Nov 12, 2008 at 02:28:56PM -0800, Ron Mayer wrote:

Merging of the interval style into ecpg attached.

Thanks for caring about the ecpg changes too.


Thanks for the comments.  Updated the patch.


I know little enough about ecpg that I can't really tell if these changes
are for the better or worse.
The closer pgtypeslib is to the backend the better. 


One thing in the patch that's probably a bug is that the
constants in src/include/utils/dt.h and src/include/utils/datetime.h
under the section "Fields for time decoding" seem not to match, so

Assuming you mean src/interfaces/ecpg/pgtypeslib/dt.h. The numbers should match 
IMO.


Ok.  I copy&pasted them from datetime.h to dt.h.
This changes a number of values that were like
#define DOY 13
#define DOW 14
to
#define DOY 15
#define DOW 16
and I'm not quite sure what the consequences of that might be,
but the regression tests still pass.


Also one files seems to be missing, there are no changes to
test/expected/pgtypeslib-dt_test.c in the patch, but when changing dt_test.pgc
this file should be changed too.

Could you add this to your work too?


Got it.
Patch attached.


*** a/src/interfaces/ecpg/pgtypeslib/dt.h
--- b/src/interfaces/ecpg/pgtypeslib/dt.h
***
*** 25,30  typedef double fsec_t;
--- 25,46 
  #define USE_SQL_DATES 2
  #define USE_GERMAN_DATES  3
  
+ #define INTSTYLE_POSTGRES 0
+ #define INTSTYLE_POSTGRES_VERBOSE 1
+ #define INTSTYLE_SQL_STANDARD 2
+ #define INTSTYLE_ISO_8601 3
+ 
+ #define INTERVAL_FULL_RANGE (0x7FFF)
+ #define INTERVAL_MASK(b) (1 << (b))
+ #define MAX_INTERVAL_PRECISION 6
+ 
+ #define DTERR_BAD_FORMAT  (-1)
+ #define DTERR_FIELD_OVERFLOW  (-2)
+ #define DTERR_MD_FIELD_OVERFLOW (-3)  /* triggers hint about DateStyle */
+ #define DTERR_INTERVAL_OVERFLOW (-4)
+ #define DTERR_TZDISP_OVERFLOW (-5)
+ 
+ 
  #define DAGO  "ago"
  #define EPOCH "epoch"
  #define INVALID   "invalid"
***
*** 77,82  typedef double fsec_t;
--- 93,101 
   * Furthermore, the values for YEAR, MONTH, DAY, HOUR, MINUTE, SECOND
   * must be in the range 0..14 so that the associated bitmasks can fit
   * into the left half of an INTERVAL's typmod value.
+  *
+  * Copy&pasted these values from src/include/utils/datetime.h
+  * 2008-11-20, changing a number of their values.
   */
  
  #define RESERV0
***
*** 92,111  typedef double fsec_t;
  #define HOUR  10
  #define MINUTE11
  #define SECOND12
! #define DOY   13
! #define DOW   14
! #define UNITS 15
! #define ADBC  16
  /* these are only for relative dates */
! #define AGO   17
! #define ABS_BEFORE18
! #define ABS_AFTER 19
  /* generic fields to help with parsing */
! #define ISODATE 20
! #define ISOTIME 21
  /* reserved for unrecognized string values */
  #define UNKNOWN_FIELD 31
  
  /*
   * Token field definitions for time parsing and decoding.
   * These need to fit into the datetkn table type.
--- 111,133 
  #define HOUR  10
  #define MINUTE11
  #define SECOND12
! #define MILLISECOND 13
! #define MICROSECOND 14
! #define DOY   15
! #define DOW   16
! #define UNITS 17
! #define ADBC  18
  /* these are only for relative dates */
! #define AGO   19
! #define ABS_BEFORE20
! #define ABS_AFTER 21
  /* generic fields to help with parsing */
! #define ISODATE 22
! #define ISOTIME 23
  /* reserved for unrecognized string values */
  #define UNKNOWN_FIELD 31
  
+ 
  /*
   * Token field definitions for time parsing and decoding.
   * These need to fit into the datetkn table type.
***
*** 164,176  typedef double fsec_t;
  /*
   * Bit mask definitions for time parsing.
   */
! 
  #define DTK_M(t)  (0x01 << (t))
! 
  #define DTK_DATE_M(DTK_M(YEAR) | DTK_M(MONTH) | DTK_M(DAY))
  #define DTK_TIME_M(DTK_M(HOUR) | DTK_M(MINUTE) | DTK_M(SECOND))
  
! #define MAXDATELEN51  /* maximum possible length of 
an input date
 * string (not 
counting tr. null) */
  #define MAXDATEFIELDS 25  /* maximum possible number of fields in 
a date
 * string */
--- 186,198 
  /*
   * Bit mask definitions for time parsing.
   */
! /* Copy&pasted these values from src/include/utils/datetime.h */
  #define DTK_M(t)  (0x01 << (t))
! #define DTK_ALL_SECS_M (DTK_M(SECOND) | DTK_M(MILLISECOND) | 
DTK_M(MICROSECOND))
  #define DTK_DATE_M(DTK_M(YEAR) | DTK_M(MONTH) | DTK_M(DAY))
  #define DTK_TIME_M(DTK_M(HOUR) | DTK_M(MINUTE) | DTK_M(SECOND))
  
! #define MAXDATELEN  

Re: [HACKERS] Proposed Patch to Improve Performance of Multi-Batch Hash Join for Skewed Data Sets

[Tom Lane]

"Lawrence, Ramon" <[EMAIL PROTECTED]> writes:
> We propose a patch that improves hybrid hash join's performance for
> large multi-batch joins where the probe relation has skew.
> ...
> The basic idea
> is to keep build relation tuples in a small in-memory hash table that
> have join values that are frequently occurring in the probe relation.

I looked at this patch a little.

I'm a tad worried about what happens when the values that are frequently
occurring in the outer relation are also frequently occurring in the
inner (which hardly seems an improbable case).  Don't you stand a severe
risk of blowing out the in-memory hash table?  It doesn't appear to me
that the code has any way to back off once it's decided that a certain
set of join key values are to be treated in-memory.  Splitting the main
join into more batches certainly doesn't help with that.

Also, AFAICS the benefit of this patch comes entirely from avoiding dump
and reload of tuples bearing the most common values, which means it's a
significant waste of cycles when there's only one batch.  It'd be better
to avoid doing any of the extra work in the single-batch case.

One thought that might address that point as well as the difficulty of
getting stats in nontrivial cases is to wait until we've overrun memory
and are forced to start batching, and at that point determine on-the-fly
which are the most common hash values from inspection of the hash table
as we dump it out.  This would amount to optimizing on the basis of
frequency in the *inner* relation not the outer, but offhand I don't see
any strong theoretical basis why that wouldn't be just as good.  It
could lose if the first work_mem worth of inner tuples isn't
representative of what follows; but this hardly seems more dangerous
than depending on MCV stats that are for the whole outer relation rather
than the portion of it being selected.

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] Proposal: new border setting in psql

[Bruce Momjian]

D'Arcy J.M. Cain wrote:
> So what have we decided about this suggestion.  Should I submit the
> patch or just forget about it?  So far some people like it and some
> people think that it is unneccessary.  No one so far has suggested that
> it would harm the system or people's use of it.

Has there been any consensus on this?

-- 
  Bruce Momjian  <[EMAIL PROTECTED]>http://momjian.us
  EnterpriseDB http://enterprisedb.com

  + If your life is a hard drive, Christ can be your backup. +

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

Re: [HACKERS] Re: [BUGS] libpq does not manage SSL callbacks properly when other libraries are involved.

[Bruce Momjian]

Bruce Momjian wrote:
> Thanks for the review, Magnus.  I have adjusted the patch to use the
> same mutex every time the counter is accessed, and adjusted the
> pqsecure_destroy() call to properly decrement in the right place.
> 
> Also, I renamed the libpq global destroy function to be clearer
> (the function is not exported).

Here is an updated version of the patch to match CVS HEAD.

-- 
  Bruce Momjian  <[EMAIL PROTECTED]>http://momjian.us
  EnterpriseDB http://enterprisedb.com

  + If your life is a hard drive, Christ can be your backup. +
Index: src/backend/libpq/be-secure.c
===
RCS file: /cvsroot/pgsql/src/backend/libpq/be-secure.c,v
retrieving revision 1.86
diff -c -c -r1.86 be-secure.c
*** src/backend/libpq/be-secure.c	20 Nov 2008 09:29:36 -	1.86
--- src/backend/libpq/be-secure.c	20 Nov 2008 21:42:24 -
***
*** 88,94 
  static int	verify_cb(int, X509_STORE_CTX *);
  static void info_cb(const SSL *ssl, int type, int args);
  static void initialize_SSL(void);
- static void destroy_SSL(void);
  static int	open_server_SSL(Port *);
  static void close_SSL(Port *);
  static const char *SSLerrmessage(void);
--- 88,93 
***
*** 193,209 
  }
  
  /*
-  *	Destroy global context
-  */
- void
- secure_destroy(void)
- {
- #ifdef USE_SSL
- 	destroy_SSL();
- #endif
- }
- 
- /*
   * Indicate if we have loaded the root CA store to verify certificates
   */
  bool
--- 192,197 
***
*** 843,853 
  	}
  }
  
  /*
!  *	Destroy global SSL context.
   */
  static void
! destroy_SSL(void)
  {
  	if (SSL_context)
  	{
--- 831,842 
  	}
  }
  
+ #ifdef NOT_USED
  /*
!  *	Destroy global SSL context
   */
  static void
! destroy_global_SSL(void)
  {
  	if (SSL_context)
  	{
***
*** 855,860 
--- 844,850 
  		SSL_context = NULL;
  	}
  }
+ #endif
  
  /*
   *	Attempt to negotiate SSL connection.
Index: src/interfaces/libpq/fe-secure.c
===
RCS file: /cvsroot/pgsql/src/interfaces/libpq/fe-secure.c,v
retrieving revision 1.107
diff -c -c -r1.107 fe-secure.c
*** src/interfaces/libpq/fe-secure.c	13 Nov 2008 09:45:25 -	1.107
--- src/interfaces/libpq/fe-secure.c	20 Nov 2008 21:42:25 -
***
*** 44,49 
--- 44,50 
  #endif
  #include 
  #endif
+ 
  #include 
  
  #ifdef ENABLE_THREAD_SAFETY
***
*** 57,72 
  #ifdef USE_SSL
  #include 
  #include 
  #if (SSLEAY_VERSION_NUMBER >= 0x00907000L)
  #include 
  #endif
  #if (SSLEAY_VERSION_NUMBER >= 0x00907000L) && !defined(OPENSSL_NO_ENGINE)
  #include 
  #endif
- #endif   /* USE_SSL */
- 
- 
- #ifdef USE_SSL
  
  #ifndef WIN32
  #define USER_CERT_FILE		".postgresql/postgresql.crt"
--- 58,70 
  #ifdef USE_SSL
  #include 
  #include 
+ 
  #if (SSLEAY_VERSION_NUMBER >= 0x00907000L)
  #include 
  #endif
  #if (SSLEAY_VERSION_NUMBER >= 0x00907000L) && !defined(OPENSSL_NO_ENGINE)
  #include 
  #endif
  
  #ifndef WIN32
  #define USER_CERT_FILE		".postgresql/postgresql.crt"
***
*** 91,110 
  static int	verify_cb(int ok, X509_STORE_CTX *ctx);
  static int	client_cert_cb(SSL *, X509 **, EVP_PKEY **);
  static int	init_ssl_system(PGconn *conn);
  static int	initialize_SSL(PGconn *);
  static void destroy_SSL(void);
  static PostgresPollingStatusType open_client_SSL(PGconn *);
  static void close_SSL(PGconn *);
  static char *SSLerrmessage(void);
  static void SSLerrfree(char *buf);
- #endif
  
- #ifdef USE_SSL
  static bool pq_initssllib = true;
- 
  static SSL_CTX *SSL_context = NULL;
  #endif
  
  /*
   * Macros to handle disabling and then restoring the state of SIGPIPE handling.
   * Note that DISABLE_SIGPIPE() must appear at the start of a block.
--- 89,120 
  static int	verify_cb(int ok, X509_STORE_CTX *ctx);
  static int	client_cert_cb(SSL *, X509 **, EVP_PKEY **);
  static int	init_ssl_system(PGconn *conn);
+ static void destroy_ssl_system(void);
  static int	initialize_SSL(PGconn *);
  static void destroy_SSL(void);
  static PostgresPollingStatusType open_client_SSL(PGconn *);
  static void close_SSL(PGconn *);
  static char *SSLerrmessage(void);
  static void SSLerrfree(char *buf);
  
  static bool pq_initssllib = true;
  static SSL_CTX *SSL_context = NULL;
+ 
+ #ifdef ENABLE_THREAD_SAFETY
+ static int ssl_open_connections = 0;
+ 
+ #ifndef WIN32
+ static pthread_mutex_t ssl_config_mutex = PTHREAD_MUTEX_INITIALIZER;
+ #else
+ static pthread_mutex_t ssl_config_mutex = NULL;
+ static long win32_ssl_create_mutex = 0;
  #endif
  
+ #endif	/* ENABLE_THREAD_SAFETY */
+ 
+ #endif /* SSL */
+ 
+ 
  /*
   * Macros to handle disabling and then restoring the state of SIGPIPE handling.
   * Note that DISABLE_SIGPIPE() must appear at the start of a block.
***
*** 725,770 
  init_ssl_system(PGconn *conn)
  {
  #ifdef ENABLE_THREAD_SAFETY
! #ifndef WIN32
! 	static pthread_mutex_t in

Re: [HACKERS] Cool hack with recursive queries

[Alvaro Herrera]

David Fetter escribió:
> On Wed, Nov 19, 2008 at 10:21:06PM -, David Rowley wrote:
> > Gregory Stark wrote:
> > > So based on Graeme Job's T-SQL hack over at thedailywtf.com I adapted the
> > > T-SQL code to Postgres and got this. Thought others might find it amusing.
> > > 
> > That's pretty amazing.
> > 
> > I think we should add a regression test with that. :)
> 
> +1 for adding a regression test :)

It's too slow for that :-(

-- 
Alvaro Herrerahttp://www.CommandPrompt.com/
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

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

Re: [HACKERS] WIP parallel restore patch

[Kenneth Marshall]

On Thu, Nov 20, 2008 at 02:26:14PM -0500, Andrew Dunstan wrote:
>
>
> Kenneth Marshall wrote:
>> Okay, I have had a chance to run some timing benchmarks.
>> Here are my results for the parallel pg_restore patch:
>>
>> Ken
>> --
>> Server settings:
>>
>>max_connections = 100   # (change requires restart)
>>shared_buffers = 256MB# min 128kB
>>work_mem = 128MB# min 64kB
>>maintenance_work_mem = 256MB# min 1MB
>>
>>fsync = on # turns forced synchronization on or off
>>
>>synchronous_commit = off# immediate fsync at commit
>>
>>full_page_writes = on # recover from partial page writes
>>checkpoint_segments = 10 # in logfile segments, min 1, 16MB each
>>autovacuum = on # Enable autovacuum subprocess? 'on'
>>
>> The total final database size is 6.5GB. Here are the timings for
>> the different run parallelism from 1 to 8 on a 4-core AMD box:
>>
>> -bash-3.00$ time pg_restore -U postgres -p 5435 -d rttest /tmp/rtout.pz
>> ...
>>
>> real19m3.175s
>> user1m2.968s
>> sys 0m8.202s
>>
>> improvement - 0%
>>
>> -bash-3.00$ time pg_restore -U postgres -p 5435 -m 2 -d rttest 
>> /tmp/rtout.pz
>> ...
>> real12m55.680s
>> user1m12.440s
>> sys 0m8.343s
>>
>> improvement - 32%
>>
>> -bash-3.00$ time pg_restore -U postgres -p 5435 -m 4 -d rttest 
>> /tmp/rtout.pz
>> ...
>> real9m45.056s
>> user1m1.892s
>> sys 0m8.980s
>>
>> improvement - 49%
>>
>> The system only has 4 cores, but here are the results with "-m 8":
>>
>> -bash-3.00$ time pg_restore -U postgres -p 5435 -m 8 -d rttest 
>> /tmp/rtout.pz
>> ...
>> real8m15.320s
>> user0m55.206s
>> sys 0m8.678s
>>
>> improvement - 53%
>>
>>
>>   
>
> Interesting.
>
> Can you try with two changes? Turn fsync off, and use the 
> --truncate-before-load switch.
>
> In general, though, this is fairly much in line with other experience, i.e. 
> we can get up to about n/2 times speedup with n cores.
>
> thanks
>
> andrew
>
Okay, here is the same test run with:

Cheers,
Ken


fsync = off
--truncate-before-load

-bash-3.00$ time pg_restore -U postgres -p 5435 --truncate-before-load -d rttest
 /tmp/rtout.pz
...
real16m25.031s
user1m3.707s
sys 0m8.776s
improvement - 0%

-bash-3.00$ time pg_restore -U postgres -p 5435 -m 2 --truncate-before-load -d r
ttest /tmp/rtout.pz
...
real10m26.730s
user0m48.782s
sys 0m7.214s
improvement - 36%

-bash-3.00$ time pg_restore -U postgres -p 5435 -m 4 --truncate-before-load -d r
ttest /tmp/rtout.pz
...
real8m5.061s
user0m48.657s
sys 0m7.602s
improvement - 51%

-bash-3.00$ time pg_restore -U postgres -p 5435 -m 8 --truncate-before-load -d r
ttest /tmp/rtout.pz
...
real6m18.787s
user0m45.361s
sys 0m7.811s
improvement - 62%


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

Re: [HACKERS] Cool hack with recursive queries

[David Fetter]

On Wed, Nov 19, 2008 at 10:21:06PM -, David Rowley wrote:
> Gregory Stark wrote:
> > So based on Graeme Job's T-SQL hack over at thedailywtf.com I adapted the
> > T-SQL code to Postgres and got this. Thought others might find it amusing.
> > 
> That's pretty amazing.
> 
> I think we should add a regression test with that. :)

+1 for adding a regression test :)

Cheers,
David.
-- 
David Fetter <[EMAIL PROTECTED]> http://fetter.org/
Phone: +1 415 235 3778  AIM: dfetter666  Yahoo!: dfetter
Skype: davidfetter  XMPP: [EMAIL PROTECTED]

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

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

Re: [HACKERS] array_agg and array_accum (patch)

[Merlin Moncure]

On Thu, Nov 20, 2008 at 4:11 PM, Tom Lane <[EMAIL PROTECTED]> wrote:
> "Robert Haas" <[EMAIL PROTECTED]> writes:
>> It looks to me like section 34.10 of the docs might benefit from some
>> sort of update in light of this patch, since the builtin array_agg now
>> does the same thing as the proposed user-defined array_accum, only
>> better.  Presumably we should either pick a different example, or add
>> a note that a builtin is available that does the same thing more
>> efficiently.
>
> I did the latter.  If you can think of an equally plausible and short
> example of a polymorphic aggregate, we could certainly replace the
> example instead ...

maybe show how to stack arrays?
see: 
http://www.nabble.com/text-array-accumulate-to-multidimensional-text-array-td20098591.html

IMO a good example of how you can write aggregates in a language other
than C, which is IMO an underutilized technique.

CREATE OR REPLACE FUNCTION array_cat1(p1 anyarray, p2 anyarray)
RETURNS anyarray AS
$$
  SELECT CASE WHEN $1 =  '{}'::text[] THEN ARRAY[p2] ELSE ARRAY_CAT(p1, p2) END;
$$ LANGUAGE sql;

CREATE AGGREGATE array_stack(anyarray)
(
   sfunc = array_cat1,
   stype = anyarray,
   initcond = '{}'
);

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] Autoconf, libpq and replacement function

[Tom Lane]

Magnus Hagander <[EMAIL PROTECTED]> writes:
> AFAICS, we're not doing this for any other functions though - or am I
> too tired and just looking in the wrong place? Or is that because
> they're just function definitions and not #defines?
> (I want to be sure to stick whatever new file there is in the same place..)

src/include/rusagestub.h is doing things more or less this way.

Also, right now we have got

src/include/getaddrinfo.h
src/include/getopt_long.h

which really make me itch now that I am contemplating the possibility
that we try to use libc-supplied code for these functions along with
our own header definitions.

I think the reason we've avoided putting such stuff into include/port/
is that right now that directory consists exclusively of OS-specific
files.  Maybe we need another include subdirectory?

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] Re: [COMMITTERS] pgsql: Silence compiler warning about ignored return value.

[Magnus Hagander]

Magnus Hagander wrote:
> Tom Lane wrote:
>> Magnus Hagander <[EMAIL PROTECTED]> writes:
>>> Tom Lane wrote:
 We're definitely *not* fortify-clean, although maybe trying to become so
 would be a good idea.  From memory, what I have seen in past builds on
 Red Hat systems is a lot of warnings about ignoring the return value
 from fwrite() and related functions.
>>> So. Should I revert it?
>> I dunno.  Was that the only such warning you got on your build?  That
>> would be a bit odd given that I see many more ...
> 
> Yeah, it happens in more places. I wasn't doing a make clean first, so
> that was the only one of the files that were built *that time* that gave
> the warning. I see it in more places now.
> 
> I think I'll go figure out how to turn fortify off :-)

Actually, better not. That's what caught that string format bug after all...

//Magnus


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

Re: [HACKERS] Re: [COMMITTERS] pgsql: Silence compiler warning about ignored return value.

[Magnus Hagander]

Tom Lane wrote:
> Magnus Hagander <[EMAIL PROTECTED]> writes:
>> Tom Lane wrote:
>>> We're definitely *not* fortify-clean, although maybe trying to become so
>>> would be a good idea.  From memory, what I have seen in past builds on
>>> Red Hat systems is a lot of warnings about ignoring the return value
>>> from fwrite() and related functions.
> 
>> So. Should I revert it?
> 
> I dunno.  Was that the only such warning you got on your build?  That
> would be a bit odd given that I see many more ...

Yeah, it happens in more places. I wasn't doing a make clean first, so
that was the only one of the files that were built *that time* that gave
the warning. I see it in more places now.

I think I'll go figure out how to turn fortify off :-)

//Magnus

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

Re: [HACKERS] Autoconf, libpq and replacement function

[Magnus Hagander]

Tom Lane wrote:
> Magnus Hagander <[EMAIL PROTECTED]> writes:
>> Tom Lane wrote:
>>> Not really.  I'd suggest making the callers do something like
>>>
>>> #ifdef HAVE_FNMATCH
>>> #include 
>>> #else
>>> #include "port/pg_fnmatch.h"
>>> #endif
> 
>> How's that actually different from the
>> #ifdef HAVE_FNMATCH
>> #include <-- happens in fe-secure.c
>> #else
>> #define <-- happens in port.h
>> #endif
> 
> What's bothering me is that port.h gets included *everywhere*, and
> might perhaps conflict with some indirect or accidental inclusion
> of .
> 
> It would also allow someone to forget the
>   #ifdef HAVE_FNMATCH
>   #include 
>   #endif
> part and have it still work, if they were testing on a broken platform.
> It's better that both inclusions appear together instead of having the
> alternative code paths effectively appear in two unrelated files.

Ok, I see your argument now.

AFAICS, we're not doing this for any other functions though - or am I
too tired and just looking in the wrong place? Or is that because
they're just function definitions and not #defines?

(I want to be sure to stick whatever new file there is in the same place..)

//Magnus

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

Re: [HACKERS] Autoconf, libpq and replacement function

[Tom Lane]

Magnus Hagander <[EMAIL PROTECTED]> writes:
> Tom Lane wrote:
>> Not really.  I'd suggest making the callers do something like
>> 
>> #ifdef HAVE_FNMATCH
>> #include 
>> #else
>> #include "port/pg_fnmatch.h"
>> #endif

> How's that actually different from the
> #ifdef HAVE_FNMATCH
> #include <-- happens in fe-secure.c
> #else
> #define <-- happens in port.h
> #endif

What's bothering me is that port.h gets included *everywhere*, and
might perhaps conflict with some indirect or accidental inclusion
of .

It would also allow someone to forget the
#ifdef HAVE_FNMATCH
#include 
#endif
part and have it still work, if they were testing on a broken platform.
It's better that both inclusions appear together instead of having the
alternative code paths effectively appear in two unrelated files.

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] array_agg and array_accum (patch)

[Tom Lane]

"Robert Haas" <[EMAIL PROTECTED]> writes:
> It looks to me like section 34.10 of the docs might benefit from some
> sort of update in light of this patch, since the builtin array_agg now
> does the same thing as the proposed user-defined array_accum, only
> better.  Presumably we should either pick a different example, or add
> a note that a builtin is available that does the same thing more
> efficiently.

I did the latter.  If you can think of an equally plausible and short
example of a polymorphic aggregate, we could certainly replace the
example instead ...

regards, tom lane

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

Re: [HACKERS] Autoconf, libpq and replacement function

[Magnus Hagander]

Tom Lane wrote:
> Magnus Hagander <[EMAIL PROTECTED]> writes:
>> Tom Lane wrote:
>>> Since fnmatch and the #define's are supposed to be provided by
>>> , I think you should probably put the substitute definitions
>>> in a substitute fnmatch.h, not port.h, to avoid that risk.
> 
>> Do we have an example where we do that before? I assume there is some
>> autoconfy way to make that include file only "appear" in the include
>> path if the system one doesn't exist or is broken?
> 
> Not really.  I'd suggest making the callers do something like
> 
>   #ifdef HAVE_FNMATCH
>   #include 
>   #else
>   #include "port/pg_fnmatch.h"
>   #endif

How's that actually different from the
#ifdef HAVE_FNMATCH
#include <-- happens in fe-secure.c
#else
#define <-- happens in port.h
#endif

If HAVE_FNMATCH isn't set, we still have the same problem, no?

//Magnus

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

Re: [HACKERS] [GENERAL] db_user_namespace, md5 and changing passwords

[Bruce Momjian]

Bruce Momjian wrote:
> Magnus Hagander wrote:
> > >> Not sure I care enough to dive into what it would actually mean. My
> > >> guess is that it's very uncommon to use db_user_namespace in any of
> > >> these scenarios (in fact I think it's very uncommon to use it at all,
> > >> but even more uncommon in these cases)
> > > 
> > > The documentation changes highlight that we are going to validate for
> > > most external authentications using the server username, so the external
> > > authentication has to be set up to use that server username.  Were the
> > > docs not clear on that?  Do I need a mention of db_user_namespace in the
> > > authentication docs?
> > 
> > AFAICS, the changes only say MD5 doesn't work. I think it should be made
> > more clear.
> > 
> > And yes, it probably makes sense to put it around the authentication
> > docs as well as a warning to people - that's where they'll go looking if
> > something doesn't work.
> 
> OK, documentation updated stating that all authentication has to user
> the server username, and added a mention in the client-auth docs too.

Applied to CVS HEAD.  Not sure if it should be backpatched so I didn't. 
We do have two bug reports for 8.3 but none for earlier releases where
it was also broken.

---


> 
> -- 
>   Bruce Momjian  <[EMAIL PROTECTED]>http://momjian.us
>   EnterpriseDB http://enterprisedb.com
> 
>   + If your life is a hard drive, Christ can be your backup. +

> Index: doc/src/sgml/client-auth.sgml
> ===
> RCS file: /cvsroot/pgsql/doc/src/sgml/client-auth.sgml,v
> retrieving revision 1.111
> diff -c -c -r1.111 client-auth.sgml
> *** doc/src/sgml/client-auth.sgml 18 Nov 2008 13:10:20 -  1.111
> --- doc/src/sgml/client-auth.sgml 20 Nov 2008 03:56:43 -
> ***
> *** 702,707 
> --- 702,709 
>   If you are at all concerned about password
>   sniffing attacks then md5 is preferred.
>   Plain password should always be avoided if possible.
> + md5 cannot be used with  + linkend="guc-db-user-namespace">.
>  
>   
>  
> Index: doc/src/sgml/config.sgml
> ===
> RCS file: /cvsroot/pgsql/doc/src/sgml/config.sgml,v
> retrieving revision 1.195
> diff -c -c -r1.195 config.sgml
> *** doc/src/sgml/config.sgml  11 Nov 2008 02:42:31 -  1.195
> --- doc/src/sgml/config.sgml  20 Nov 2008 03:56:44 -
> ***
> *** 706,711 
> --- 706,722 
>   before the user name is looked up by the server.
>  
>   
> +
> + db_user_namespace causes the client's and
> + server's user name representation to differ.
> + Authentication checks are always done with the server's user name
> + so authentication methods must be configured for the
> + server's user name, not the client's.  Because
> + md5 uses the user name as salt on both the
> + client and server, md5 cannot be used with
> + db_user_namespace.
> +
> + 
>  
>   
>This feature is intended as a temporary measure until a
> Index: src/backend/libpq/auth.c
> ===
> RCS file: /cvsroot/pgsql/src/backend/libpq/auth.c,v
> retrieving revision 1.171
> diff -c -c -r1.171 auth.c
> *** src/backend/libpq/auth.c  18 Nov 2008 13:10:20 -  1.171
> --- src/backend/libpq/auth.c  20 Nov 2008 03:56:44 -
> ***
> *** 371,376 
> --- 371,380 
>   break;
>   
>   case uaMD5:
> + if (Db_user_namespace)
> + ereport(FATAL,
> + 
> (errcode(ERRCODE_INVALID_AUTHORIZATION_SPECIFICATION),
> +  errmsg("MD5 authentication is 
> not supported when \"db_user_namespace\" is enabled")));
>   sendAuthRequest(port, AUTH_REQ_MD5);
>   status = recv_and_check_password_packet(port);
>   break;
> Index: src/backend/libpq/hba.c
> ===
> RCS file: /cvsroot/pgsql/src/backend/libpq/hba.c,v
> retrieving revision 1.172
> diff -c -c -r1.172 hba.c
> *** src/backend/libpq/hba.c   28 Oct 2008 12:10:43 -  1.172
> --- src/backend/libpq/hba.c   20 Nov 2008 03:56:47 -
> ***
> *** 846,852 
> --- 846,861 
>   else if (strcmp(token, "reject") == 0)
>   parsedline->auth_method = uaReject;
>   else if (strcmp(token, "md5") == 0)
> + {
> + if (Db_user_namespace)
> + {
> + ereport(LOG,
> + (errcode(ERRCODE_CONFIG_FILE_ERROR),
> +

Re: [HACKERS] Autoconf, libpq and replacement function

[Tom Lane]

Magnus Hagander <[EMAIL PROTECTED]> writes:
> Tom Lane wrote:
>> Since fnmatch and the #define's are supposed to be provided by
>> , I think you should probably put the substitute definitions
>> in a substitute fnmatch.h, not port.h, to avoid that risk.

> Do we have an example where we do that before? I assume there is some
> autoconfy way to make that include file only "appear" in the include
> path if the system one doesn't exist or is broken?

Not really.  I'd suggest making the callers do something like

#ifdef HAVE_FNMATCH
#include 
#else
#include "port/pg_fnmatch.h"
#endif

regards, tom lane

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

Re: [HACKERS] Autoconf, libpq and replacement function

[Magnus Hagander]

Tom Lane wrote:
> Magnus Hagander <[EMAIL PROTECTED]> writes:
>>> Also, judging from the comments in the autoconf manual, you'd better
>>> use AC_FUNC_FNMATCH not just test whether the function exists.
> 
>> Ok, will look at switching to that.
> 
> Hmm ... actually there's still possibly an issue there: what if the
> system provides a broken version of fnmatch?  AC_FUNC_FNMATCH will not
> set HAVE_FNMATCH, and then we might end up with #define conflicts
> anyway.
> 
> Since fnmatch and the #define's are supposed to be provided by
> , I think you should probably put the substitute definitions
> in a substitute fnmatch.h, not port.h, to avoid that risk.

Do we have an example where we do that before? I assume there is some
autoconfy way to make that include file only "appear" in the include
path if the system one doesn't exist or is broken?

//Magnus

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

Re: [HACKERS] Autoconf, libpq and replacement function

[Tom Lane]

Magnus Hagander <[EMAIL PROTECTED]> writes:
> Tom Lane wrote:
>> Surely we must *not* be providing our own definitions of these symbols
>> when using a system version of fnmatch.

> That's the define that I reversed in the second patch I sent. It's
> supposed to be ifndef.

Okay.

>> Also, judging from the comments in the autoconf manual, you'd better
>> use AC_FUNC_FNMATCH not just test whether the function exists.

> Ok, will look at switching to that.

Hmm ... actually there's still possibly an issue there: what if the
system provides a broken version of fnmatch?  AC_FUNC_FNMATCH will not
set HAVE_FNMATCH, and then we might end up with #define conflicts
anyway.

Since fnmatch and the #define's are supposed to be provided by
, I think you should probably put the substitute definitions
in a substitute fnmatch.h, not port.h, to avoid that risk.

regards, tom lane

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

Re: [HACKERS] Autoconf, libpq and replacement function

[Magnus Hagander]

Tom Lane wrote:
> Magnus Hagander <[EMAIL PROTECTED]> writes:
>> *** a/src/include/port.h
>> --- b/src/include/port.h
>> ***
>> *** 393,398  extern void unsetenv(const char *name);
>> --- 393,409 
>>   extern void srandom(unsigned int seed);
>>   #endif
>   
>> + #ifdef HAVE_FNMATCH
>> + extern int fnmatch(const char *, const char *, int);
>> + #define FNM_NOMATCH1   /* Match failed. */
>> + #define FNM_NOSYS  2   /* Function not implemented. */
>> + #define FNM_NOESCAPE   0x01/* Disable backslash escaping. */
>> + #define FNM_PATHNAME   0x02/* Slash must be matched by slash. */
>> + #define FNM_PERIOD 0x04/* Period must be matched by period. */
>> + #define FNM_CASEFOLD   0x08/* Pattern is matched case-insensitive 
>> */
>> + #define FNM_LEADING_DIR0x10/* Ignore / after Imatch. */
>> + #endif
>> + 
> 
> Surely we must *not* be providing our own definitions of these symbols
> when using a system version of fnmatch.

That's the define that I reversed in the second patch I sent. It's
supposed to be ifndef.


> Also, judging from the comments in the autoconf manual, you'd better
> use AC_FUNC_FNMATCH not just test whether the function exists.

Ok, will look at switching to that.

//Magnus

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

Re: [HACKERS] Autoconf, libpq and replacement function

[Tom Lane]

Magnus Hagander <[EMAIL PROTECTED]> writes:
> *** a/src/include/port.h
> --- b/src/include/port.h
> ***
> *** 393,398  extern void unsetenv(const char *name);
> --- 393,409 
>   extern void srandom(unsigned int seed);
>   #endif
  
> + #ifdef HAVE_FNMATCH
> + extern int fnmatch(const char *, const char *, int);
> + #define FNM_NOMATCH 1   /* Match failed. */
> + #define FNM_NOSYS   2   /* Function not implemented. */
> + #define FNM_NOESCAPE0x01/* Disable backslash escaping. */
> + #define FNM_PATHNAME0x02/* Slash must be matched by slash. */
> + #define FNM_PERIOD  0x04/* Period must be matched by period. */
> + #define FNM_CASEFOLD0x08/* Pattern is matched case-insensitive 
> */
> + #define FNM_LEADING_DIR 0x10/* Ignore / after Imatch. */
> + #endif
> + 

Surely we must *not* be providing our own definitions of these symbols
when using a system version of fnmatch.

Also, judging from the comments in the autoconf manual, you'd better
use AC_FUNC_FNMATCH not just test whether the function exists.

regards, tom lane

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

Re: [HACKERS] pg_upgrade: How to deal with toast

[Tom Lane]

Zdenek Kotala <[EMAIL PROTECTED]> writes:
> Tom Lane napsal(a):
>> No, it's a really horrid idea.  Nullable attributes complicate the C
>> code, and what in the world are we buying with it anyway?  Just decide
>> what the field should contain and put it in there.

> The problem what we try to solve is to perform this change during upgrade 
> from 
> 8.3->8.4. Extra value is a problem because it requires extra space and there 
> is 
> not free space. It is temporal solution(hack) for 8.3->8.4.

Solution of what?  The original concern you had was about
TOAST_MAX_CHUNK_SIZE changing from 8.3 to 8.4.  If that's a problem then
it has to be solved anyway, and the solution has to involve being able
to push some chunks off-page to make room.  If it's not a problem then
let's just leave the toast representation as-is till later.

regards, tom lane

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

Re: [HACKERS] Re: [COMMITTERS] pgsql: Silence compiler warning about ignored return value.

[Tom Lane]

Magnus Hagander <[EMAIL PROTECTED]> writes:
> Tom Lane wrote:
>> We're definitely *not* fortify-clean, although maybe trying to become so
>> would be a good idea.  From memory, what I have seen in past builds on
>> Red Hat systems is a lot of warnings about ignoring the return value
>> from fwrite() and related functions.

> So. Should I revert it?

I dunno.  Was that the only such warning you got on your build?  That
would be a bit odd given that I see many more ...

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] Problem with Bitmap Heap Scan

[Tom Lane]

I wrote:
> Heikki Linnakangas <[EMAIL PROTECTED]> writes:
>> Attached is a patch that changes create_bitmap_subplan so that the 
>> condition put into Recheck condition is never stronger than the 
>> condition automatically handled by the index. Does that look right to you?

> I think this is still too simplistic, but will look closer.  One point
> is that it's not accounting for the bitmap AND/OR structure that might
> be above the individual indexscans.  The original coding avoided that
> problem by making all the decisions at the top level, and I'm inclined
> to stick with that approach.

I've fixed this by reverting create_bitmap_subplan to its previous
behavior with two output lists.  The scheme you suggested is a bit
logically cleaner, but aside from the issue of AND/OR conditions it
has one unpleasant feature: the RECHECK condition would get copies of
derived clauses.  For instance "col LIKE 'foo%'" would end up with
a plan like

Recheck: col >= 'foo' AND col < 'fop'
Filter: col ~~ 'foo%'
Index Cond: col >= 'foo' AND col < 'fop'

The tests in create_bitmap_scan_plan aren't smart enough to recognize
that the recheck conditions are redundant given the filter condition.

The former and now-restored behavior avoids this problem, though it has
the assumption that every indexclause condition came from scan_clauses
(or has been put into bitmapqualorig in the join case), else it might
fail to enforce special operators.  That's certainly true at the moment
though it seems a bit ugly to assume it here.  It might be a good idea
to try to refactor the representation of special/derived quals to make
this stuff a bit more straightforward.  I don't care to tackle that now
though.

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] Updated posix fadvise patch v19

[Grzegorz Jaskiewicz]

On 2008-11-20, at 11:08, Grzegorz Jaskiewicz wrote:


this doesn't apply cleanly to head anymore, can you please post v21 ?
I would love to test it here :)

bollocks, it's already in cvs head - isn't it ? ... :D

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

Re: [HACKERS] WIP parallel restore patch

[Andrew Dunstan]

Kenneth Marshall wrote:

Okay, I have had a chance to run some timing benchmarks.
Here are my results for the parallel pg_restore patch:

Ken
--
Server settings:

   max_connections = 100   # (change requires restart)
   shared_buffers = 256MB# min 128kB
   work_mem = 128MB# min 64kB
   maintenance_work_mem = 256MB# min 1MB

   fsync = on # turns forced synchronization on or off

   synchronous_commit = off# immediate fsync at commit

   full_page_writes = on # recover from partial page writes
   checkpoint_segments = 10 # in logfile segments, min 1, 16MB each
   autovacuum = on # Enable autovacuum subprocess? 'on'

The total final database size is 6.5GB. Here are the timings for
the different run parallelism from 1 to 8 on a 4-core AMD box:

-bash-3.00$ time pg_restore -U postgres -p 5435 -d rttest /tmp/rtout.pz
...

real19m3.175s
user1m2.968s
sys 0m8.202s

improvement - 0%

-bash-3.00$ time pg_restore -U postgres -p 5435 -m 2 -d rttest /tmp/rtout.pz
...
real12m55.680s
user1m12.440s
sys 0m8.343s

improvement - 32%

-bash-3.00$ time pg_restore -U postgres -p 5435 -m 4 -d rttest /tmp/rtout.pz
...
real9m45.056s
user1m1.892s
sys 0m8.980s

improvement - 49%

The system only has 4 cores, but here are the results with "-m 8":

-bash-3.00$ time pg_restore -U postgres -p 5435 -m 8 -d rttest /tmp/rtout.pz
...
real8m15.320s
user0m55.206s
sys 0m8.678s

improvement - 53%


  


Interesting.

Can you try with two changes? Turn fsync off, and use the 
--truncate-before-load switch.


In general, though, this is fairly much in line with other experience, 
i.e. we can get up to about n/2 times speedup with n cores.


thanks

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] WIP parallel restore patch

[Kenneth Marshall]

Okay, I have had a chance to run some timing benchmarks.
Here are my results for the parallel pg_restore patch:

Ken
--
Server settings:

   max_connections = 100   # (change requires restart)
   shared_buffers = 256MB# min 128kB
   work_mem = 128MB# min 64kB
   maintenance_work_mem = 256MB# min 1MB

   fsync = on # turns forced synchronization on or off

   synchronous_commit = off# immediate fsync at commit

   full_page_writes = on # recover from partial page writes
   checkpoint_segments = 10 # in logfile segments, min 1, 16MB each
   autovacuum = on # Enable autovacuum subprocess? 'on'

The total final database size is 6.5GB. Here are the timings for
the different run parallelism from 1 to 8 on a 4-core AMD box:

-bash-3.00$ time pg_restore -U postgres -p 5435 -d rttest /tmp/rtout.pz
...

real19m3.175s
user1m2.968s
sys 0m8.202s

improvement - 0%

-bash-3.00$ time pg_restore -U postgres -p 5435 -m 2 -d rttest /tmp/rtout.pz
...
real12m55.680s
user1m12.440s
sys 0m8.343s

improvement - 32%

-bash-3.00$ time pg_restore -U postgres -p 5435 -m 4 -d rttest /tmp/rtout.pz
...
real9m45.056s
user1m1.892s
sys 0m8.980s

improvement - 49%

The system only has 4 cores, but here are the results with "-m 8":

-bash-3.00$ time pg_restore -U postgres -p 5435 -m 8 -d rttest /tmp/rtout.pz
...
real8m15.320s
user0m55.206s
sys 0m8.678s

improvement - 53%


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

Re: [HACKERS] TODO list request: FK to unique expression indexes

[Robert Haas]

> Allowing foreign keys to point to expression indexes seems to open a can
> of worms and I am not sure there is enough demand to warrant it.

It does open a can of worms.  I've often wanting something related,
which is the ability to make a foreign key references a PARTIAL index.

...Robert

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

Re: [HACKERS] Re: [COMMITTERS] pgsql: Silence compiler warning about ignored return value.

[Magnus Hagander]

Tom Lane wrote:
> Peter Eisentraut <[EMAIL PROTECTED]> writes:
>> It looks like you are building in fortify mode?  I tried that a while 
>> ago and got a few more warnings.  Are we trying to be fortify clean, and 
>> if so, what is our approach?
> 
> We're definitely *not* fortify-clean, although maybe trying to become so
> would be a good idea.  From memory, what I have seen in past builds on
> Red Hat systems is a lot of warnings about ignoring the return value
> from fwrite() and related functions.  Which might not be an unreasonable
> thing to try to get rid of, though I think many of the cases are in
> places where there's no useful recovery action to be taken anyway.
> (Whatcha gonna do if you fail to write the postmaster log ... try to
> log a complaint message?)
> 
> In any case I agree that anything we try to do about this should be a
> system-wide effort not a one-line hack.
> 
>> Also, considering my recent complaint about various brittleness in the 
>> regression test driver, more well hidden ignorings of errors are not 
>> exactly my favorite solution.
> 
> +1

So. Should I revert it?

//Magnus


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

Re: [HACKERS] Autoconf, libpq and replacement function

[Magnus Hagander]

Magnus Hagander wrote:
> Peter Eisentraut wrote:
>> Magnus Hagander wrote:
>>> How do I make this work with the autoconf magic? I see there is an
>>> AC_CHECK_FNMATCH and AC_REPLACE_FNMATCH and so, but I have a feeling I
>>> need to do something different since it's libpq?
>> AC_*_FNMATCH will figure out whether you need fnmatch(), so something
>> involving those is necessary.
>>
>> For libpq, check libpq's Makefile for, say, snprintf, to get an idea
>> about the "something different".
>>
>> Altogether, this might not be a trivial case.
> 
> Hmm. If I did it the right way, it actually didn't seem too hard once I
> found a good example. Attached - does that seem reasonable?
> 
> (will add msvc code as well)

That was easy. I also reversed the accidentally-reversed #ifdef in port.h.

//Mangus

*** a/configure
--- b/configure
***
*** 16930,16936  LIBS=`echo "$LIBS" | sed -e 's/-ledit//g' -e 's/-lreadline//g'`
  
  
  
! for ac_func in crypt getopt getrusage inet_aton random rint srandom strdup strerror strlcat strlcpy strtol strtoul unsetenv
  do
  as_ac_var=`echo "ac_cv_func_$ac_func" | $as_tr_sh`
  { echo "$as_me:$LINENO: checking for $ac_func" >&5
--- 16930,16937 
  
  
  
! 
! for ac_func in crypt fnmatch getopt getrusage inet_aton random rint srandom strdup strerror strlcat strlcpy strtol strtoul unsetenv
  do
  as_ac_var=`echo "ac_cv_func_$ac_func" | $as_tr_sh`
  { echo "$as_me:$LINENO: checking for $ac_func" >&5
*** a/configure.in
--- b/configure.in
***
*** 1235,1241  fi
  pgac_save_LIBS="$LIBS"
  LIBS=`echo "$LIBS" | sed -e 's/-ledit//g' -e 's/-lreadline//g'`
  
! AC_REPLACE_FUNCS([crypt getopt getrusage inet_aton random rint srandom strdup strerror strlcat strlcpy strtol strtoul unsetenv])
  
  LIBS="$pgac_save_LIBS"
  
--- 1235,1241 
  pgac_save_LIBS="$LIBS"
  LIBS=`echo "$LIBS" | sed -e 's/-ledit//g' -e 's/-lreadline//g'`
  
! AC_REPLACE_FUNCS([crypt fnmatch getopt getrusage inet_aton random rint srandom strdup strerror strlcat strlcpy strtol strtoul unsetenv])
  
  LIBS="$pgac_save_LIBS"
  
*** a/src/include/pg_config.h.in
--- b/src/include/pg_config.h.in
***
*** 143,148 
--- 143,151 
  /* Define to 1 if you have the `fdatasync' function. */
  #undef HAVE_FDATASYNC
  
+ /* Define to 1 if you have the `fnmatch' function. */
+ #undef HAVE_FNMATCH
+ 
  /* Define to 1 if you have the `fpclass' function. */
  #undef HAVE_FPCLASS
  
*** a/src/include/port.h
--- b/src/include/port.h
***
*** 393,398  extern void unsetenv(const char *name);
--- 393,409 
  extern void srandom(unsigned int seed);
  #endif
  
+ #ifndef HAVE_FNMATCH
+ extern int fnmatch(const char *, const char *, int);
+ #define FNM_NOMATCH		1		/* Match failed. */
+ #define FNM_NOSYS		2		/* Function not implemented. */
+ #define FNM_NOESCAPE	0x01	/* Disable backslash escaping. */
+ #define FNM_PATHNAME	0x02	/* Slash must be matched by slash. */
+ #define FNM_PERIOD		0x04	/* Period must be matched by period. */
+ #define FNM_CASEFOLD	0x08	/* Pattern is matched case-insensitive */
+ #define FNM_LEADING_DIR	0x10	/* Ignore / after Imatch. */
+ #endif
+ 
  /* thread.h */
  extern char *pqStrerror(int errnum, char *strerrbuf, size_t buflen);
  
*** a/src/interfaces/libpq/Makefile
--- b/src/interfaces/libpq/Makefile
***
*** 34,40  OBJS=	fe-auth.o fe-connect.o fe-exec.o fe-misc.o fe-print.o fe-lobj.o \
  	fe-protocol2.o fe-protocol3.o pqexpbuffer.o pqsignal.o fe-secure.o \
  	libpq-events.o \
  	md5.o ip.o wchar.o encnames.o noblock.o pgstrcasecmp.o thread.o \
! 	$(filter crypt.o getaddrinfo.o inet_aton.o open.o snprintf.o strerror.o strlcpy.o win32error.o, $(LIBOBJS))
  
  ifeq ($(PORTNAME), cygwin)
  override shlib = cyg$(NAME)$(DLSUFFIX)
--- 34,40 
  	fe-protocol2.o fe-protocol3.o pqexpbuffer.o pqsignal.o fe-secure.o \
  	libpq-events.o \
  	md5.o ip.o wchar.o encnames.o noblock.o pgstrcasecmp.o thread.o \
! 	$(filter crypt.o fnmatch.o getaddrinfo.o inet_aton.o open.o snprintf.o strerror.o strlcpy.o win32error.o, $(LIBOBJS))
  
  ifeq ($(PORTNAME), cygwin)
  override shlib = cyg$(NAME)$(DLSUFFIX)
***
*** 80,86  backend_src = $(top_srcdir)/src/backend
  # For port modules, this only happens if configure decides the module
  # is needed (see filter hack in OBJS, above).
  
! crypt.c getaddrinfo.c inet_aton.c noblock.c open.c pgstrcasecmp.c snprintf.c strerror.c strlcpy.c thread.c win32error.c pgsleep.c: % : $(top_srcdir)/src/port/%
  	rm -f $@ && $(LN_S) $< .
  
  md5.c ip.c: % : $(backend_src)/libpq/%
--- 80,86 
  # For port modules, this only happens if configure decides the module
  # is needed (see filter hack in OBJS, above).
  
! crypt.c fnmatch.c getaddrinfo.c inet_aton.c noblock.c open.c pgstrcasecmp.c snprintf.c strerror.c strlcpy.c thread.c win32error.c pgsleep.c: % : $(top_srcdir)/src/port/%
  	rm -f $@ && $(LN_S) $< .
  
  md5.c ip.c: % : $(backend_src)/libpq/%
***
*** 123,129  uninstall: uninstall-lib
 

Re: [HACKERS] Autoconf, libpq and replacement function

[Magnus Hagander]

Peter Eisentraut wrote:
> Magnus Hagander wrote:
>> How do I make this work with the autoconf magic? I see there is an
>> AC_CHECK_FNMATCH and AC_REPLACE_FNMATCH and so, but I have a feeling I
>> need to do something different since it's libpq?
> 
> AC_*_FNMATCH will figure out whether you need fnmatch(), so something
> involving those is necessary.
> 
> For libpq, check libpq's Makefile for, say, snprintf, to get an idea
> about the "something different".
> 
> Altogether, this might not be a trivial case.

Hmm. If I did it the right way, it actually didn't seem too hard once I
found a good example. Attached - does that seem reasonable?

(will add msvc code as well)

//Magnus
*** a/configure
--- b/configure
***
*** 16930,16936  LIBS=`echo "$LIBS" | sed -e 's/-ledit//g' -e 's/-lreadline//g'`
  
  
  
! for ac_func in crypt getopt getrusage inet_aton random rint srandom strdup strerror strlcat strlcpy strtol strtoul unsetenv
  do
  as_ac_var=`echo "ac_cv_func_$ac_func" | $as_tr_sh`
  { echo "$as_me:$LINENO: checking for $ac_func" >&5
--- 16930,16937 
  
  
  
! 
! for ac_func in crypt fnmatch getopt getrusage inet_aton random rint srandom strdup strerror strlcat strlcpy strtol strtoul unsetenv
  do
  as_ac_var=`echo "ac_cv_func_$ac_func" | $as_tr_sh`
  { echo "$as_me:$LINENO: checking for $ac_func" >&5
*** a/configure.in
--- b/configure.in
***
*** 1235,1241  fi
  pgac_save_LIBS="$LIBS"
  LIBS=`echo "$LIBS" | sed -e 's/-ledit//g' -e 's/-lreadline//g'`
  
! AC_REPLACE_FUNCS([crypt getopt getrusage inet_aton random rint srandom strdup strerror strlcat strlcpy strtol strtoul unsetenv])
  
  LIBS="$pgac_save_LIBS"
  
--- 1235,1241 
  pgac_save_LIBS="$LIBS"
  LIBS=`echo "$LIBS" | sed -e 's/-ledit//g' -e 's/-lreadline//g'`
  
! AC_REPLACE_FUNCS([crypt fnmatch getopt getrusage inet_aton random rint srandom strdup strerror strlcat strlcpy strtol strtoul unsetenv])
  
  LIBS="$pgac_save_LIBS"
  
*** a/src/include/pg_config.h.in
--- b/src/include/pg_config.h.in
***
*** 143,148 
--- 143,151 
  /* Define to 1 if you have the `fdatasync' function. */
  #undef HAVE_FDATASYNC
  
+ /* Define to 1 if you have the `fnmatch' function. */
+ #undef HAVE_FNMATCH
+ 
  /* Define to 1 if you have the `fpclass' function. */
  #undef HAVE_FPCLASS
  
*** a/src/include/port.h
--- b/src/include/port.h
***
*** 393,398  extern void unsetenv(const char *name);
--- 393,409 
  extern void srandom(unsigned int seed);
  #endif
  
+ #ifdef HAVE_FNMATCH
+ extern int fnmatch(const char *, const char *, int);
+ #define FNM_NOMATCH		1		/* Match failed. */
+ #define FNM_NOSYS		2		/* Function not implemented. */
+ #define FNM_NOESCAPE	0x01	/* Disable backslash escaping. */
+ #define FNM_PATHNAME	0x02	/* Slash must be matched by slash. */
+ #define FNM_PERIOD		0x04	/* Period must be matched by period. */
+ #define FNM_CASEFOLD	0x08	/* Pattern is matched case-insensitive */
+ #define FNM_LEADING_DIR	0x10	/* Ignore / after Imatch. */
+ #endif
+ 
  /* thread.h */
  extern char *pqStrerror(int errnum, char *strerrbuf, size_t buflen);
  
*** a/src/interfaces/libpq/Makefile
--- b/src/interfaces/libpq/Makefile
***
*** 34,40  OBJS=	fe-auth.o fe-connect.o fe-exec.o fe-misc.o fe-print.o fe-lobj.o \
  	fe-protocol2.o fe-protocol3.o pqexpbuffer.o pqsignal.o fe-secure.o \
  	libpq-events.o \
  	md5.o ip.o wchar.o encnames.o noblock.o pgstrcasecmp.o thread.o \
! 	$(filter crypt.o getaddrinfo.o inet_aton.o open.o snprintf.o strerror.o strlcpy.o win32error.o, $(LIBOBJS))
  
  ifeq ($(PORTNAME), cygwin)
  override shlib = cyg$(NAME)$(DLSUFFIX)
--- 34,40 
  	fe-protocol2.o fe-protocol3.o pqexpbuffer.o pqsignal.o fe-secure.o \
  	libpq-events.o \
  	md5.o ip.o wchar.o encnames.o noblock.o pgstrcasecmp.o thread.o \
! 	$(filter crypt.o fnmatch.o getaddrinfo.o inet_aton.o open.o snprintf.o strerror.o strlcpy.o win32error.o, $(LIBOBJS))
  
  ifeq ($(PORTNAME), cygwin)
  override shlib = cyg$(NAME)$(DLSUFFIX)
***
*** 80,86  backend_src = $(top_srcdir)/src/backend
  # For port modules, this only happens if configure decides the module
  # is needed (see filter hack in OBJS, above).
  
! crypt.c getaddrinfo.c inet_aton.c noblock.c open.c pgstrcasecmp.c snprintf.c strerror.c strlcpy.c thread.c win32error.c pgsleep.c: % : $(top_srcdir)/src/port/%
  	rm -f $@ && $(LN_S) $< .
  
  md5.c ip.c: % : $(backend_src)/libpq/%
--- 80,86 
  # For port modules, this only happens if configure decides the module
  # is needed (see filter hack in OBJS, above).
  
! crypt.c fnmatch.c getaddrinfo.c inet_aton.c noblock.c open.c pgstrcasecmp.c snprintf.c strerror.c strlcpy.c thread.c win32error.c pgsleep.c: % : $(top_srcdir)/src/port/%
  	rm -f $@ && $(LN_S) $< .
  
  md5.c ip.c: % : $(backend_src)/libpq/%
***
*** 123,129  uninstall: uninstall-lib
  	rm -f '$(DESTDIR)$(datadir)/pg_service.conf.sample'
  
  clean distclean: clean-lib
! 	rm -f $(OBJS) pg_config_paths.

Re: [HACKERS] pg_upgrade: How to deal with toast

[Zdenek Kotala]

Bruce Momjian napsal(a):

Zdenek Kotala wrote:

Tom Lane napsal(a):

Zdenek Kotala <[EMAIL PROTECTED]> writes:

Heikki Linnakangas napsal(a):
Perhaps we should just add the new attid attribute to the toast table, 
but mark it as nullable?

Hmm, It seems to me as a good idea.

No, it's a really horrid idea.  Nullable attributes complicate the C
code, and what in the world are we buying with it anyway?  Just decide
what the field should contain and put it in there.
The problem what we try to solve is to perform this change during upgrade from 
8.3->8.4. Extra value is a problem because it requires extra space and there is 
not free space. It is temporal solution(hack) for 8.3->8.4.


Once we have the 'require free space' capability in a major Postgres
release, can't we use that to make space for the new TOAST field we will
need?



The problem is between 8.3 and 8.4. Unfortunately 8.3 does not have this 
capability. And if it will be backported then space reservation on toast table 
will be too expensive - you need to move one tuple which usually has BLCKSZ/4).


Zdenek

--
Sent 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: How to deal with toast

[Bruce Momjian]

Zdenek Kotala wrote:
> Tom Lane napsal(a):
> > Zdenek Kotala <[EMAIL PROTECTED]> writes:
> >> Heikki Linnakangas napsal(a):
> >>> Perhaps we should just add the new attid attribute to the toast table, 
> >>> but mark it as nullable?
> > 
> >> Hmm, It seems to me as a good idea.
> > 
> > No, it's a really horrid idea.  Nullable attributes complicate the C
> > code, and what in the world are we buying with it anyway?  Just decide
> > what the field should contain and put it in there.
> 
> The problem what we try to solve is to perform this change during upgrade 
> from 
> 8.3->8.4. Extra value is a problem because it requires extra space and there 
> is 
> not free space. It is temporal solution(hack) for 8.3->8.4.

Once we have the 'require free space' capability in a major Postgres
release, can't we use that to make space for the new TOAST field we will
need?

-- 
  Bruce Momjian  <[EMAIL PROTECTED]>http://momjian.us
  EnterpriseDB http://enterprisedb.com

  + If your life is a hard drive, Christ can be your backup. +

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

Re: [HACKERS] TODO list request: FK to unique expression indexes

[Bruce Momjian]

David E. Wheeler wrote:
> On Nov 19, 2008, at 9:12 AM, Josh Berkus wrote:
> 
> > Folks,
> >
> > Since it's too late to look at this for 8.4, can the following go on  
> > the TODO list?
> >
> > Referential Integrity
> >
> > [] Allow creation of FKs targeting unique expression indexes on the  
> > referenced table.  Syntax: REFERENCES  ( ( column  
> > expression ) )
> >
> > Reason: current FK rules do not allow creating FKs to columns which  
> > are defined as, for example, unique(lower(column)).  This forces  
> > users to either abandon RI for that table, to store duplicate data,  
> > or create superfluous indexes.
> >
> > Hmmm ... I suppose the above would require enabling expression  
> > indexes for PKs as well, no?
> 
> In 8.4 you should be able to get around this particular example using  
> citext.

Yes, good idea on citext.  

Allowing foreign keys to point to expression indexes seems to open a can
of worms and I am not sure there is enough demand to warrant it.

-- 
  Bruce Momjian  <[EMAIL PROTECTED]>http://momjian.us
  EnterpriseDB http://enterprisedb.com

  + If your life is a hard drive, Christ can be your backup. +

-- 
Sent 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: How to deal with toast

[Zdenek Kotala]

Tom Lane napsal(a):

Zdenek Kotala <[EMAIL PROTECTED]> writes:

Heikki Linnakangas napsal(a):
Perhaps we should just add the new attid attribute to the toast table, 
but mark it as nullable?



Hmm, It seems to me as a good idea.


No, it's a really horrid idea.  Nullable attributes complicate the C
code, and what in the world are we buying with it anyway?  Just decide
what the field should contain and put it in there.


The problem what we try to solve is to perform this change during upgrade from 
8.3->8.4. Extra value is a problem because it requires extra space and there is 
not free space. It is temporal solution(hack) for 8.3->8.4.


Another thing what we can do is to perform "fake" page conversion of heap which 
will retoast a toasted value which are present on heap tuples. The toasted table 
will contains two kind of tuples, but in normal situation only converted tuples 
should be accessed.


Zdenek

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

Re: [HACKERS] Re: [COMMITTERS] pgsql: Silence compiler warning about ignored return value.

[Tom Lane]

Peter Eisentraut <[EMAIL PROTECTED]> writes:
> It looks like you are building in fortify mode?  I tried that a while 
> ago and got a few more warnings.  Are we trying to be fortify clean, and 
> if so, what is our approach?

We're definitely *not* fortify-clean, although maybe trying to become so
would be a good idea.  From memory, what I have seen in past builds on
Red Hat systems is a lot of warnings about ignoring the return value
from fwrite() and related functions.  Which might not be an unreasonable
thing to try to get rid of, though I think many of the cases are in
places where there's no useful recovery action to be taken anyway.
(Whatcha gonna do if you fail to write the postmaster log ... try to
log a complaint message?)

In any case I agree that anything we try to do about this should be a
system-wide effort not a one-line hack.

> Also, considering my recent complaint about various brittleness in the 
> regression test driver, more well hidden ignorings of errors are not 
> exactly my favorite solution.

+1

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] Problem with Bitmap Heap Scan

[Tom Lane]

Heikki Linnakangas <[EMAIL PROTECTED]> writes:
> Tom Lane wrote:
>> Hmm, the ~~ condition should get treated as a "filter" not a "recheck".
>> I suppose I broke this somewhere ...

> I started to look at this last night. The culprit seems to be this patch:

Yeah, it appears that I oversimplified matters in that patch.  I think
that I mistakenly decided that create_bitmap_subplan() only needed one
output parameter because there was no longer a need to distinguish
between lossy and nonlossy operators.  What I forgot was that the
"nonlossy" output was also coming from the "indexquals" rather than
the "indexclauses", and that's different precisely in the case where
we've got a special index operator such as LIKE.

I think what probably has to happen is revert most of that
simplification and have create_bitmap_subplan return suitable
representations of both the indexquals and indexclauses.

> Attached is a patch that changes create_bitmap_subplan so that the 
> condition put into Recheck condition is never stronger than the 
> condition automatically handled by the index. Does that look right to you?

I think this is still too simplistic, but will look closer.  One point
is that it's not accounting for the bitmap AND/OR structure that might
be above the individual indexscans.  The original coding avoided that
problem by making all the decisions at the top level, and I'm inclined
to stick with that approach.

regards, tom lane

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

Re: [HACKERS] WIP parallel restore patch

[Andrew Dunstan]

Magnus Hagander wrote:

Andrew Dunstan wrote:
  

Attached is my latest parallel restore patch. I think it's functionally
complete for Unix.

Many bugs have been fixed since the last patch, and the hardcoded
limitation to two table dependencies is removed. It seems fairly robust
in my recent testing.

Remaining to be done:

. code cleanup
. better error checking in a few places
. final decision re command line option names/defaults
. documentation
. Windows support.



I've looked around this a bit, and it's fairly clear where the issue
comes in with Windows - we get heap corruption. Most likely because we
have multiple threads working on the same data somewhere.

I notice for example that we're doing a shallow copy of the
ArchiveHandle with a simple memcpy() for each thread. But that struct
contains a number of things like file descriptors and pointers. Have you
verified for each and every one of those that it actually doesn't get
modified anywhere? If not, a deep copy may be needed to make sure of that.

Other than that, are there any global variables that may be addressed
from more than one worker? If so they need to be marked as TLS.

And yes, I got hit by the lack of error checking a couple of times
during my testing - it would probably be a good idea to add that as soon
as possible, it helps a lot with the debugging.

If I run it with just a single thread, it also crashes in PQfinish()
called from die_horribly(), when trying to free conn->pgpass, which has
a value (non-NULL) but is not a valid pointer. This crash happens in the
worker thread, after it has logged that "fseek is required" - that's an
indicator something being passed down to the thread is either wrong or
being scribbled upon after the fact.

I didn't dig into these questions specifically - since you have already
been reading up on this code to do the patch you can probably reach the
answer to them much quicker :-) So I'll stick to the questions.


  


OK, Thanks, this will help. I thought I had caught the ArchiveHandle 
things, but there might be one or two I missed.


I'll try to have a new version in a few days.

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] Hot Standby (commit fest version - v5)

[Pavan Deolasee]

On Thu, Nov 20, 2008 at 9:03 PM, Tom Lane <[EMAIL PROTECTED]> wrote:

>
> I don't think you can do that.  Couldn't someone else have run
> heap_page_prune between vacuum's first and second visit to the page?
>
>
>

You mean the second visit in the first pass where we again check for
HeapTupleSatisfiesVacuum ? We hold exclusive lock continuously in the first
pass. So its not possible for someone else to call heap_page_prune.  If its
the second visit in the second heap scan, then it removes only the dead
tuples recorded in the first pass. So we should be good there too.

Thanks,
Pavan

-- 
Pavan Deolasee
EnterpriseDB http://www.enterprisedb.com

Re: [HACKERS] Hot Standby (commit fest version - v5)

[Simon Riggs]

On Thu, 2008-11-20 at 10:33 -0500, Tom Lane wrote:
> "Pavan Deolasee" <[EMAIL PROTECTED]> writes:
> > I wonder if we should refactor lazy_scan_heap() so that *all* the real work
> > of collecting information about dead tuples happens only in
> > heap_page_prune(). Frankly, there is only a rare chance that a tuple may
> > become DEAD after the pruning happened on the page. We can ignore such
> > tuples; they will be vacuumed/pruned in the next cycle.
> 
> > This would save us a second check of HeapTupleSatisfiesVacuum on the tuples
> > which are just now checked in heap_page_prune(). In addition, the following
> > additional WAL records are then not necessary because heap_page_prune() must
> > have already logged the latestRemovedXid.
> 
> I don't think you can do that.  Couldn't someone else have run
> heap_page_prune between vacuum's first and second visit to the page?

I just looked at that in more detail and decided it was more difficult
than it first appeared. So I've left it for now.

-- 
 Simon Riggs   www.2ndQuadrant.com
 PostgreSQL Training, Services and Support


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

Re: [HACKERS] pg_upgrade: How to deal with toast

[Tom Lane]

Zdenek Kotala <[EMAIL PROTECTED]> writes:
> Heikki Linnakangas napsal(a):
>> Perhaps we should just add the new attid attribute to the toast table, 
>> but mark it as nullable?

> Hmm, It seems to me as a good idea.

No, it's a really horrid idea.  Nullable attributes complicate the C
code, and what in the world are we buying with it anyway?  Just decide
what the field should contain and put it in there.

regards, tom lane

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

Re: [HACKERS] Re: [COMMITTERS] pgsql: Silence compiler warning about ignored return value.

[Grzegorz Jaskiewicz]

On 2008-11-20, at 15:29, Peter Eisentraut wrote:


Well, the warning is explicitly put in there for this specific  
function because you are supposed to process the return value.  I'm  
sure a more smarter compiler would even warn "variable is assigned a  
value that is never used". ;-)  (Note that gcc in general doesn't  
work about unused return values, only for those functions that glibc  
explicitly marks as candidates.)


afaik you need to use -Wall and -O3 to get that type of warning with 4.3


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

Re: [HACKERS] Re: [COMMITTERS] pgsql: Silence compiler warning about ignored return value.

[Magnus Hagander]

Peter Eisentraut wrote:
> Magnus Hagander wrote:
>> Heikki Linnakangas wrote:
>>> Magnus Hagander wrote:
 Log Message:
 ---
 Silence compiler warning about ignored return value. Our comment
 already
 clearly stated that we are aware that we're ignoring it.
>>> I think the usual way is to call the function like:
>>>
>>>  (void) function_with_return_value()
>>
>> I tried that first, of course. gcc is too smart about that - it still
>> throws the warning in this case.
> 
> Well, the warning is explicitly put in there for this specific function
> because you are supposed to process the return value.  I'm sure a more
> smarter compiler would even warn "variable is assigned a value that is
> never used". ;-)  (Note that gcc in general doesn't work about unused
> return values, only for those functions that glibc explicitly marks as
> candidates.)
> 
> It looks like you are building in fortify mode?  I tried that a while
> ago and got a few more warnings.  Are we trying to be fortify clean, and
> if so, what is our approach?

I'm building in whatever is the default on Ubuntu 8.10. It may be that
they have switched to fortify mode. How do I check that?


> Also, considering my recent complaint about various brittleness in the
> regression test driver, more well hidden ignorings of errors are not
> exactly my favorite solution.

Right, the other thing to do would be to check the error value :-) I
just assumed that since the comment explicitly said we wanted to ignore
it, we'd want to get rid of the warning. If the comment hadn't been
there, I'd have looked at a different way to do it.

//Magnus

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

Re: [HACKERS] Error arguments in pl_exec.c

[Magnus Hagander]

Tom Lane wrote:
> Magnus Hagander <[EMAIL PROTECTED]> writes:
>> Attached patch seems right to me - objections?
> 
> Good catch.  What gave you these warnings exactly?

Same here - the new version of gcc.

Will go ahead and apply.

//Magnus


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

Re: [HACKERS] Re: [COMMITTERS] pgsql: Silence compiler warning about ignored return value.

[Magnus Hagander]

Tom Lane wrote:
> Magnus Hagander <[EMAIL PROTECTED]> writes:
>> Heikki Linnakangas wrote:
>>> I think the usual way is to call the function like:
>>> (void) function_with_return_value()
> 
>> I tried that first, of course. gcc is too smart about that - it still
>> throws the warning in this case.
> 
> I think you must have a broken version of gcc.  I don't like this
> patch either.  The (void) is the standard way and should work;
> futhermore, if you're getting a warning here, why aren't you getting
> a whole lot of others?  It's not like we are careful to use (void)
> everywhere.

It's:
gcc (Ubuntu 4.3.2-1ubuntu11) 4.3.2

Error was:
pg_regress.c:282: warning: ignoring return value of ‘system’, declared
with attribute warn_unused_result

It's because system() is flagged with __attribute(warn_unused_result)__.
That's why we're not seeing it for other functions. There's a paragraph
about the difference in the GCC docs.

If that's in the new versions of gcc, I expect it to show up on other
platforms as well as time passes.

//Magnus


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

Re: [HACKERS] Hot Standby (commit fest version - v5)

[Tom Lane]

"Pavan Deolasee" <[EMAIL PROTECTED]> writes:
> I wonder if we should refactor lazy_scan_heap() so that *all* the real work
> of collecting information about dead tuples happens only in
> heap_page_prune(). Frankly, there is only a rare chance that a tuple may
> become DEAD after the pruning happened on the page. We can ignore such
> tuples; they will be vacuumed/pruned in the next cycle.

> This would save us a second check of HeapTupleSatisfiesVacuum on the tuples
> which are just now checked in heap_page_prune(). In addition, the following
> additional WAL records are then not necessary because heap_page_prune() must
> have already logged the latestRemovedXid.

I don't think you can do that.  Couldn't someone else have run
heap_page_prune between vacuum's first and second visit to the page?

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] Updates of SE-PostgreSQL 8.4devel patches (r1197)

[Bruce Momjian]

Bruce Momjian wrote:
> > However, the toggle of row-level security feature should be controled
> > via a GUC option, not a discretionary option.
> > I'll add a "sepostgresql_row_level" option defined as bool to control
> > it on start up time.
> 
> This sounds similar to BSD capability were certain security settings can
> only be changed in single-user mode.

Actually, an interesting idea would be to allow "sepostgresql_row_level"
to be turned on, but not off.  That means if it was turned on in
postgresql.conf, it could not be turned off, but if it is off in
postgresql.conf, it could be turned on via SET or via ALTER
USER/DATABASE;  I think that would be a nice capability.

On a related note, KaiGai, you are now starting the long road of getting
feedback with the ultimate goal of getting your patch into CVS.  I will
warn you that there is often much work during this stage, and it might
stretch into January as we request adjustments, but ultimately your
feature and Postgres will be better for it.  Thanks for sticking with
it.

-- 
  Bruce Momjian  <[EMAIL PROTECTED]>http://momjian.us
  EnterpriseDB http://enterprisedb.com

  + If your life is a hard drive, Christ can be your backup. +

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

Re: [HACKERS] Re: [COMMITTERS] pgsql: Silence compiler warning about ignored return value.

[Peter Eisentraut]

Magnus Hagander wrote:

Heikki Linnakangas wrote:

Magnus Hagander wrote:

Log Message:
---
Silence compiler warning about ignored return value. Our comment already
clearly stated that we are aware that we're ignoring it.

I think the usual way is to call the function like:

 (void) function_with_return_value()


I tried that first, of course. gcc is too smart about that - it still
throws the warning in this case.


Well, the warning is explicitly put in there for this specific function 
because you are supposed to process the return value.  I'm sure a more 
smarter compiler would even warn "variable is assigned a value that is 
never used". ;-)  (Note that gcc in general doesn't work about unused 
return values, only for those functions that glibc explicitly marks as 
candidates.)


It looks like you are building in fortify mode?  I tried that a while 
ago and got a few more warnings.  Are we trying to be fortify clean, and 
if so, what is our approach?


Also, considering my recent complaint about various brittleness in the 
regression test driver, more well hidden ignorings of errors are not 
exactly my favorite solution.


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

Re: [HACKERS] Error arguments in pl_exec.c

[Tom Lane]

Magnus Hagander <[EMAIL PROTECTED]> writes:
> Attached patch seems right to me - objections?

Good catch.  What gave you these warnings exactly?

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] Re: [COMMITTERS] pgsql: Silence compiler warning about ignored return value.

[Tom Lane]

Magnus Hagander <[EMAIL PROTECTED]> writes:
> Heikki Linnakangas wrote:
>> I think the usual way is to call the function like:
>> (void) function_with_return_value()

> I tried that first, of course. gcc is too smart about that - it still
> throws the warning in this case.

I think you must have a broken version of gcc.  I don't like this
patch either.  The (void) is the standard way and should work;
futhermore, if you're getting a warning here, why aren't you getting
a whole lot of others?  It's not like we are careful to use (void)
everywhere.

regards, tom lane

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

Re: [HACKERS] Autoconf, libpq and replacement function

[Peter Eisentraut]

Magnus Hagander wrote:

How do I make this work with the autoconf magic? I see there is an
AC_CHECK_FNMATCH and AC_REPLACE_FNMATCH and so, but I have a feeling I
need to do something different since it's libpq?


AC_*_FNMATCH will figure out whether you need fnmatch(), so something 
involving those is necessary.


For libpq, check libpq's Makefile for, say, snprintf, to get an idea 
about the "something different".


Altogether, this might not be a trivial case.


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

Re: [HACKERS] Error arguments in pl_exec.c

[Gregory Stark]

Magnus Hagander <[EMAIL PROTECTED]> writes:

> I get the following warnings in HEAD
>
> pl_exec.c: In function ‘exec_stmt_raise’:
> pl_exec.c:2538: warning: format not a string literal and no format arguments
> pl_exec.c:2538: warning: format not a string literal and no format arguments
>
>
> Attached patch seems right to me - objections?

ick. that's actually a security hole. Thankfully it's new code in cvs head.

-- 
  Gregory Stark
  EnterpriseDB  http://www.enterprisedb.com
  Ask me about EnterpriseDB's Slony Replication support!

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

Re: [HACKERS] How should pg_standby get over the gap of timeline?

[Pavan Deolasee]

On Thu, Nov 20, 2008 at 8:36 PM, Heikki Linnakangas <
[EMAIL PROTECTED]> wrote:

>
> That seems like a dangerous assumption. What if the standby had fallen
> behind before the failover? It's not safe to failover back to the original
> primary in that case. We'd need some kind of safeguards against that.
>
>
For synchronous replication, what if we ensure that the standby has received
the WAL (atleast in its buffers) before writing it to disk on the primary ?
If we do that, I think the old standby can never fall behind the primary and
it would be easy for the old primary to join back the replication without a
fresh backup.

Of course, this doesn't work for async replication.

Thanks,
Pavan

-- 
Pavan Deolasee
EnterpriseDB http://www.enterprisedb.com

[HACKERS] Re: [COMMITTERS] pgsql: Silence compiler warning about ignored return value.

[Magnus Hagander]

Heikki Linnakangas wrote:
> Magnus Hagander wrote:
>> Log Message:
>> ---
>> Silence compiler warning about ignored return value. Our comment already
>> clearly stated that we are aware that we're ignoring it.
> 
> I think the usual way is to call the function like:
> 
>  (void) function_with_return_value()

I tried that first, of course. gcc is too smart about that - it still
throws the warning in this case.

//Magnus


-- 
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: Silence compiler warning about ignored return value.

[Heikki Linnakangas]

Magnus Hagander wrote:

Log Message:
---
Silence compiler warning about ignored return value. Our comment already
clearly stated that we are aware that we're ignoring it.


I think the usual way is to call the function like:

 (void) function_with_return_value()

--
  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] How should pg_standby get over the gap of timeline?

[Heikki Linnakangas]

Fujii Masao wrote:

Hi, Heikki. Thanks for the comment!

On Thu, Nov 20, 2008 at 11:24 PM, Heikki Linnakangas
<[EMAIL PROTECTED]> wrote:

Fujii Masao wrote:

In the current Synch Rep patch, the standby cannot catch up with the
primary which has a bigger timeline.

That would only happen if you've performed an archive recovery in the
primary. If you've done PITR in the primary, I don't think there's any
guarantee that it's even possible to catch up the standby. The standby might
already have replayed a WAL file from an earlier timeline, that isn't part
of the history of the bigger timeline.


I assume the situation of making the standby (the original primary) catch up
with the primary (the original standby) after failover. Since a timeline is
incremented when a failover finishes archive recovery on a standby, the
timelines differ between two servers.


That seems like a dangerous assumption. What if the standby had fallen 
behind before the failover? It's not safe to failover back to the 
original primary in that case. We'd need some kind of safeguards against 
that.


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

[HACKERS] Error arguments in pl_exec.c

[Magnus Hagander]

I get the following warnings in HEAD

pl_exec.c: In function ‘exec_stmt_raise’:
pl_exec.c:2538: warning: format not a string literal and no format arguments
pl_exec.c:2538: warning: format not a string literal and no format arguments


Attached patch seems right to me - objections?

//Magnus
Index: pl_exec.c
===
RCS file: /cvsroot/pgsql/src/pl/plpgsql/src/pl_exec.c,v
retrieving revision 1.224
diff -c -r1.224 pl_exec.c
*** pl_exec.c	5 Nov 2008 00:07:54 -	1.224
--- pl_exec.c	20 Nov 2008 14:58:11 -
***
*** 2538,2545 
  	ereport(stmt->elog_level,
  			(err_code ? errcode(err_code) : 0,
  			 errmsg_internal("%s", err_message),
! 			 (err_detail != NULL) ? errdetail(err_detail) : 0,
! 			 (err_hint != NULL) ? errhint(err_hint) : 0));
  
  	estate->err_text = NULL;	/* un-suppress... */
  
--- 2538,2545 
  	ereport(stmt->elog_level,
  			(err_code ? errcode(err_code) : 0,
  			 errmsg_internal("%s", err_message),
! 			 (err_detail != NULL) ? errdetail("%s", err_detail) : 0,
! 			 (err_hint != NULL) ? errhint("%s", err_hint) : 0));
  
  	estate->err_text = NULL;	/* un-suppress... */
  

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

Re: [HACKERS] How should pg_standby get over the gap of timeline?

[Fujii Masao]

Hi, Heikki. Thanks for the comment!

On Thu, Nov 20, 2008 at 11:24 PM, Heikki Linnakangas
<[EMAIL PROTECTED]> wrote:
> Fujii Masao wrote:
>>
>> In the current Synch Rep patch, the standby cannot catch up with the
>> primary which has a bigger timeline.
>
> That would only happen if you've performed an archive recovery in the
> primary. If you've done PITR in the primary, I don't think there's any
> guarantee that it's even possible to catch up the standby. The standby might
> already have replayed a WAL file from an earlier timeline, that isn't part
> of the history of the bigger timeline.

I assume the situation of making the standby (the original primary) catch up
with the primary (the original standby) after failover. Since a timeline is
incremented when a failover finishes archive recovery on a standby, the
timelines differ between two servers.

Regards,

-- 
Fujii Masao
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center

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

Re: [HACKERS] Hot Standby (commit fest version - v5)

[Pavan Deolasee]

On Thu, Nov 20, 2008 at 7:50 PM, Simon Riggs <[EMAIL PROTECTED]> wrote:



>
> (I assume you mean bgwriter, not archiver process).
>
>
Yeah, its the bgwriter, IIRC hung while taking checkpoint.

Thanks,
Pavan

-- 
Pavan Deolasee
EnterpriseDB http://www.enterprisedb.com

Re: [HACKERS] How should pg_standby get over the gap of timeline?

[Heikki Linnakangas]

Fujii Masao wrote:

In the current Synch Rep patch, the standby cannot catch up with the
primary which has a bigger timeline.


That would only happen if you've performed an archive recovery in the 
primary. If you've done PITR in the primary, I don't think there's any 
guarantee that it's even possible to catch up the standby. The standby 
might already have replayed a WAL file from an earlier timeline, that 
isn't part of the history of the bigger timeline.


--
  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] WIP parallel restore patch

[Magnus Hagander]

Andrew Dunstan wrote:
> 
> Attached is my latest parallel restore patch. I think it's functionally
> complete for Unix.
> 
> Many bugs have been fixed since the last patch, and the hardcoded
> limitation to two table dependencies is removed. It seems fairly robust
> in my recent testing.
> 
> Remaining to be done:
> 
> . code cleanup
> . better error checking in a few places
> . final decision re command line option names/defaults
> . documentation
> . Windows support.

I've looked around this a bit, and it's fairly clear where the issue
comes in with Windows - we get heap corruption. Most likely because we
have multiple threads working on the same data somewhere.

I notice for example that we're doing a shallow copy of the
ArchiveHandle with a simple memcpy() for each thread. But that struct
contains a number of things like file descriptors and pointers. Have you
verified for each and every one of those that it actually doesn't get
modified anywhere? If not, a deep copy may be needed to make sure of that.

Other than that, are there any global variables that may be addressed
from more than one worker? If so they need to be marked as TLS.

And yes, I got hit by the lack of error checking a couple of times
during my testing - it would probably be a good idea to add that as soon
as possible, it helps a lot with the debugging.

If I run it with just a single thread, it also crashes in PQfinish()
called from die_horribly(), when trying to free conn->pgpass, which has
a value (non-NULL) but is not a valid pointer. This crash happens in the
worker thread, after it has logged that "fseek is required" - that's an
indicator something being passed down to the thread is either wrong or
being scribbled upon after the fact.

I didn't dig into these questions specifically - since you have already
been reading up on this code to do the patch you can probably reach the
answer to them much quicker :-) So I'll stick to the questions.

//Magnus

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

Re: [HACKERS] Hot Standby (commit fest version - v5)

[Simon Riggs]

On Thu, 2008-11-20 at 12:03 +0530, Pavan Deolasee wrote:

> On Sat, Nov 1, 2008 at 10:02 PM, Simon Riggs <[EMAIL PROTECTED]>
> wrote:
> Hot Standby patch, including all major planned features.
> 
> 
> While experimenting with the patch, I noticed that sometimes the
> archiver process indefinitely waits for WALInsertLock. I haven't spent
> much time debugging that, but the following chunk clearly seems to be
> buggy. The WALInsertLock is not released if (leavingArchiveRecovery ==
> true).

Mmmm, it seems this is correct. I had to reconstruct this section of
code after recent bitrot, so it looks I introduced a bug doing that.
What I'm surprised about is that I got a similar hang myself in testing
and in my notes I have this ticked as resolved.

The fix is trivial, though I'm sorry it was there for you to find at
all.

(I assume you mean bgwriter, not archiver process).

-- 
 Simon Riggs   www.2ndQuadrant.com
 PostgreSQL Training, Services and Support


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

[HACKERS] How should pg_standby get over the gap of timeline?

[Fujii Masao]

Hi,

In the current Synch Rep patch, the standby cannot catch up with the
primary which has a bigger timeline. So, whenever making the standby
catch up, a fresh base backup is required. This is obviously undesirable,
and I'd like to get rid of this restriction.

Postgres itself can recover up to a bigger timeline without a base
backup. The remaining problem is that pg_standby cannot get over the
gap of timeline. It continues waiting for the XLOG file with out-of-date
timeline, and redo doesn't progress.

My idea is that introducing a new option into pg_standby, which makes
the restoring fail if there is the XLOG file with the same logid and segid
even if the target file doesn't exist. Once failing to restore, the startup
process can switch the timeline and try to restore the XLOG file with
new timeline.

Is this idea reasonable? Any comments welcome!

Regards,

-- 
Fujii Masao
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center

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

Re: [HACKERS] Visibility map, partial vacuums

[Heikki Linnakangas]

I committed the changes to FSM truncation yesterday, that helps with the 
truncation of the visibility map as well. Attached is an updated 
visibility map patch.


There's two open issues:

1. The bits in the visibility map are set in the 1st phase of lazy 
vacuum. That works, but it means that after a delete or update, it takes 
two vacuums until the bit in the visibility map is set. The first vacuum 
removes the dead tuple, and only the second sees that there's no dead 
tuples and sets the bit.


2. Should modify the output of VACUUM VERBOSE to say how many pages were 
actually scanned. What other information is relevant, or is no longer 
relevant, with partial vacuums.


--
  Heikki Linnakangas
  EnterpriseDB   http://www.enterprisedb.com
*** src/backend/access/heap/Makefile
--- src/backend/access/heap/Makefile
***
*** 12,17  subdir = src/backend/access/heap
  top_builddir = ../../../..
  include $(top_builddir)/src/Makefile.global
  
! OBJS = heapam.o hio.o pruneheap.o rewriteheap.o syncscan.o tuptoaster.o
  
  include $(top_srcdir)/src/backend/common.mk
--- 12,17 
  top_builddir = ../../../..
  include $(top_builddir)/src/Makefile.global
  
! OBJS = heapam.o hio.o pruneheap.o rewriteheap.o syncscan.o tuptoaster.o visibilitymap.o
  
  include $(top_srcdir)/src/backend/common.mk
*** src/backend/access/heap/heapam.c
--- src/backend/access/heap/heapam.c
***
*** 47,52 
--- 47,53 
  #include "access/transam.h"
  #include "access/tuptoaster.h"
  #include "access/valid.h"
+ #include "access/visibilitymap.h"
  #include "access/xact.h"
  #include "access/xlogutils.h"
  #include "catalog/catalog.h"
***
*** 195,200  heapgetpage(HeapScanDesc scan, BlockNumber page)
--- 196,202 
  	int			ntup;
  	OffsetNumber lineoff;
  	ItemId		lpp;
+ 	bool		all_visible;
  
  	Assert(page < scan->rs_nblocks);
  
***
*** 233,252  heapgetpage(HeapScanDesc scan, BlockNumber page)
  	lines = PageGetMaxOffsetNumber(dp);
  	ntup = 0;
  
  	for (lineoff = FirstOffsetNumber, lpp = PageGetItemId(dp, lineoff);
  		 lineoff <= lines;
  		 lineoff++, lpp++)
  	{
  		if (ItemIdIsNormal(lpp))
  		{
- 			HeapTupleData loctup;
  			bool		valid;
  
! 			loctup.t_data = (HeapTupleHeader) PageGetItem((Page) dp, lpp);
! 			loctup.t_len = ItemIdGetLength(lpp);
! 			ItemPointerSet(&(loctup.t_self), page, lineoff);
  
! 			valid = HeapTupleSatisfiesVisibility(&loctup, snapshot, buffer);
  			if (valid)
  scan->rs_vistuples[ntup++] = lineoff;
  		}
--- 235,266 
  	lines = PageGetMaxOffsetNumber(dp);
  	ntup = 0;
  
+ 	/*
+ 	 * If the all-visible flag indicates that all tuples on the page are
+ 	 * visible to everyone, we can skip the per-tuple visibility tests.
+ 	 */
+ 	all_visible = PageIsAllVisible(dp);
+ 
  	for (lineoff = FirstOffsetNumber, lpp = PageGetItemId(dp, lineoff);
  		 lineoff <= lines;
  		 lineoff++, lpp++)
  	{
  		if (ItemIdIsNormal(lpp))
  		{
  			bool		valid;
  
! 			if (all_visible)
! valid = true;
! 			else
! 			{
! HeapTupleData loctup;
! 
! loctup.t_data = (HeapTupleHeader) PageGetItem((Page) dp, lpp);
! loctup.t_len = ItemIdGetLength(lpp);
! ItemPointerSet(&(loctup.t_self), page, lineoff);
  
! valid = HeapTupleSatisfiesVisibility(&loctup, snapshot, buffer);
! 			}
  			if (valid)
  scan->rs_vistuples[ntup++] = lineoff;
  		}
***
*** 1860,1865  heap_insert(Relation relation, HeapTuple tup, CommandId cid,
--- 1874,1880 
  	TransactionId xid = GetCurrentTransactionId();
  	HeapTuple	heaptup;
  	Buffer		buffer;
+ 	bool		all_visible_cleared;
  
  	if (relation->rd_rel->relhasoids)
  	{
***
*** 1920,1925  heap_insert(Relation relation, HeapTuple tup, CommandId cid,
--- 1935,1946 
  
  	RelationPutHeapTuple(relation, buffer, heaptup);
  
+ 	if (PageIsAllVisible(BufferGetPage(buffer)))
+ 	{
+ 		all_visible_cleared = true;
+ 		PageClearAllVisible(BufferGetPage(buffer));
+ 	}
+ 
  	/*
  	 * XXX Should we set PageSetPrunable on this page ?
  	 *
***
*** 1943,1948  heap_insert(Relation relation, HeapTuple tup, CommandId cid,
--- 1964,1970 
  		Page		page = BufferGetPage(buffer);
  		uint8		info = XLOG_HEAP_INSERT;
  
+ 		xlrec.all_visible_cleared = all_visible_cleared;
  		xlrec.target.node = relation->rd_node;
  		xlrec.target.tid = heaptup->t_self;
  		rdata[0].data = (char *) &xlrec;
***
*** 1994,1999  heap_insert(Relation relation, HeapTuple tup, CommandId cid,
--- 2016,2026 
  
  	UnlockReleaseBuffer(buffer);
  
+ 	/* Clear the bit in the visibility map if necessary */
+ 	if (all_visible_cleared)
+ 		visibilitymap_clear(relation, 
+ 			ItemPointerGetBlockNumber(&(heaptup->t_self)));
+ 
  	/*
  	 * If tuple is cachable, mark it for invalidation from the caches in case
  	 * we abort.  Note it is OK to do this after releasing the buffer, because
***
*** 2070,2075  heap_delete(Relation relation, ItemPointer tid,
--- 2097,2103 --

[HACKERS] Autoconf, libpq and replacement function

[Magnus Hagander]

Hi!

I want to use the fnmatch() function in libpq, to support wildcard
certificate matching. This function is part of the standard
(http://www.opengroup.org/onlinepubs/95399/functions/fnmatch.html),
but obviously not implemented on all platforms (naturally, it's missing
on win32, because that makes life harder..)

Anyway. NetBSD has an implementation of this at
http://cvsweb.netbsd.org/bsdweb.cgi/src/lib/libc/gen/fnmatch.c?rev=1.21&content-type=text/x-cvsweb-markup&only_with_tag=MAIN
that we can "steal" if we need.

How do I make this work with the autoconf magic? I see there is an
AC_CHECK_FNMATCH and AC_REPLACE_FNMATCH and so, but I have a feeling I
need to do something different since it's libpq?

(this might be very obvious how to do, but if it is, please excuse my
extreme autoconf-newbiesm)

//Magnus

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

Re: [HACKERS] Cool hack with recursive queries

[Harald Armin Massa]

1st)   it turns out PostgreSQL allows code that is more compact than
MSQL: 19 lines instead of 46 lines
2nd)  now there will be a really compelling reason for DBAs worldwide
to upgrade to 8.4; after release everyone without Mandelbrot in SQL is
just a lame noob
3rd)  maybe THAT could be the final straw to argue against MySQL: "But
it cannot do Mandelbrot, so it is not l33t" It's easier then to argue
ACID and stuff.


-- 
GHUM Harald Massa
persuadere et programmare
Harald Armin Massa
Spielberger Straße 49
70435 Stuttgart
0173/9409607
no fx, no carrier pigeon
-
EuroPython 2009 will take place in Birmingham - Stay tuned!

-- 
Sent 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: How to deal with toast

[Zdenek Kotala]

Heikki Linnakangas napsal(a):

It the new attribute is added to the end, the old tuples will be 
compatible as is. If there's no null bitmap (or it's shorter than 
expectd), attributes missing from a tuple are implicitly NULL. That's 
how we support ALTER TABLE ADD COLUMN without rewriting all the data.




Yeah, but better is to put int column before chunk data. It save space 
(alignment). I prefer to put attno as a third column.


Zdenek

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

Re: [HACKERS] Updated posix fadvise patch v19

[Grzegorz Jaskiewicz]

this doesn't apply cleanly to head anymore, can you please post v21 ?
I would love to test it here :)


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

Re: [HACKERS] Hot Standby (commit fest version - v5)

[Pavan Deolasee]

On Thu, Nov 20, 2008 at 3:38 PM, Simon Riggs <[EMAIL PROTECTED]> wrote:

>
> I would suggest that we just remove the switch statement:
>switch (HeapTupleSatisfiesVacuum(tuple.t_data, OldestXmin, buf))
> and alter the following if test since tupgone is also removed.
> That will cause HEAPTUPLE_DEAD rows to be fed to heap_freeze_tuple().
> Comments on that function claim that is a bad thing, but we know that
> any row that was *not* removed by heap_page_prune() and is now dead must
> have died very recently and so will never be frozen.
>
> Let me know if you're happy with that change and I'll make it so.
>
>

Yeah, I think we should be safe. We continuously hold EX lock on the buffer
since the prune operation is carried out. So the only new DEAD tuples may
arrive because some transaction aborted in between, changing
INSERT_IN_PROGRESS tuple to DEAD. But these tuples won't pass the cutoff_xid
test and should never be frozen.

Thanks,
Pavan

-- 
Pavan Deolasee
EnterpriseDB http://www.enterprisedb.com

Re: [HACKERS] Transactions and temp tables

[Heikki Linnakangas]

Emmanuel Cecchet wrote:
I still quite did not get what the big deal was if an ON COMMIT DELETE 
ROWS temp table was created inside a transaction.


In case the transaction that created a temp table rolls back, the table 
needs to be removed. Removing a temporary table belonging to another 
backend is problematic; the local buffers in the original backend need 
to be dropped, as well as the entry in the on commit actions list.


Why the new checks you are doing in lock.c would not work with dropped 
temp tables? Could it be possible to drop the lock as soon as the temp 
table is dropped inside a transaction?


If you release the lock early on a table that you drop, another 
transactions would be free to access the table, even though it's about 
to be dropped.



I will try to find more time to review the patch tonight.


Thanks!

Thinking about this whole thing yet more, I wonder if we could have a 
more holistic approach and make temporary tables work just like regular 
ones. The problems we've identified this far are:


1. If the prepared transaction keeps the temp table locked, the backend 
can't exit, because the shutdown hook tries to drop all temp tables.


2. When a prepared transaction that has deleted a temporary table 
commits (or one that created one aborts), we need to drop all the local 
buffers from the original backend's private buffer cache.


3. When a prepared transaction that has deleted a temporary table 
commits (or one that created one aborts), we need to remove the 
on-commit entry from the original backend's private list.


Is there more? I think we can solve all the above problems:

1. Modify RemoveTempRelations so that it doesn't block if it can't 
immediately acquire lock on the to-be-removed object. That way the 
original backend can exit even if a prepared transaction is holding a 
lock on a temporary object.


To avoid conflict with a new backend that's assigned the same backendid, 
divorce the temporary namespace naming from backendid so that the 
temporary namespace name stays reserved for the prepared transaction.


2. Flush and drop all local buffers on temporary tables that have been 
created or dropped in the transaction at PREPARE TRANSACTION already.


3. Add on-commit field to pg_class, and only keep a list of temporary 
tables that have been accessed in the current transaction in 
backend-private memory.


--
  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] pg_upgrade: How to deal with toast

[Heikki Linnakangas]

Zdenek Kotala wrote:

Heikki Linnakangas napsal(a):
Perhaps we should just add the new attid attribute to the toast table, 
but mark it as nullable? We wouldn't need to fill it in in the 
8.3->8.4 conversion but new tuples would include it.

 >
In the future release that we actually need it, we'll make it 
non-nullable, and write a pre-upgrade script to retoast tuples that 
don't have it yet.


Hmm, It seems to me as a good idea. It will complicated preupgrade 
script for 8.4->8.5 script but we will have one year to developed it :-).


What we need to do is during conversion to add nullbit array for each 
tuple in toasttable. I think there is enough space on all platform to 
reuse gap between tuple header and data.


It the new attribute is added to the end, the old tuples will be 
compatible as is. If there's no null bitmap (or it's shorter than 
expectd), attributes missing from a tuple are implicitly NULL. That's 
how we support ALTER TABLE ADD COLUMN without rewriting all the data.


--
  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] pg_upgrade: How to deal with toast

[Zdenek Kotala]

Heikki Linnakangas napsal(a):

Zdenek Kotala wrote:
The question is if we should do toast modification now to avoid 
potential future problems or if we will solve it when any on-disk 
format change requires it?


Perhaps we should just add the new attid attribute to the toast table, 
but mark it as nullable? We wouldn't need to fill it in in the 8.3->8.4 
conversion but new tuples would include it.

>
In the future release that we actually need it, we'll make it 
non-nullable, and write a pre-upgrade script to retoast tuples that 
don't have it yet.


Hmm, It seems to me as a good idea. It will complicated preupgrade script for 
8.4->8.5 script but we will have one year to developed it :-).


What we need to do is during conversion to add nullbit array for each tuple in 
toasttable. I think there is enough space on all platform to reuse gap between 
tuple header and data.



Hmm. That would change TOAST_MAX_CHUNK_SIZE, though.


Yes, it change it. :(

but I can convert easily residx to offset end (residx*TOAST_MAX_CHUNK_SIZE -+ 
something) during page conversion.


I prefer do it now, because there could be small risk that it will not 
possible to do it in the future. When in-place upgrade will be 
implemented nobody will want to perform backup/restore.


I feel we should avoid doing anything extra, risking that we get nothing 
finished.


Agree.

Zdenek



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

Re: [HACKERS] [PATCHES] Infrastructure changes for recovery (v8)

[Simon Riggs]

On Thu, 2008-11-20 at 15:19 +0530, Pavan Deolasee wrote:

> Do you intend to split the patch into smaller pieces ? The latest hot
> standby patch is almost 10K+ lines. Splitting that would definitely
> help the review process.

If it helps you, then I'll do it. Hang on an hour or so.

-- 
 Simon Riggs   www.2ndQuadrant.com
 PostgreSQL Training, Services and Support


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

Re: [HACKERS] Hot Standby (commit fest version - v5)

[Simon Riggs]

On Thu, 2008-11-20 at 11:51 +0530, Pavan Deolasee wrote:

> I wonder if we should refactor lazy_scan_heap() so that *all* the real
> work of collecting information about dead tuples happens only in
> heap_page_prune(). Frankly, there is only a rare chance that a tuple
> may become DEAD after the pruning happened on the page. We can ignore
> such tuples; they will be vacuumed/pruned in the next cycle.
> 
> This would save us a second check of HeapTupleSatisfiesVacuum on the
> tuples which are just now checked in heap_page_prune(). In addition,
> the following additional WAL records are then not necessary because
> heap_page_prune() must have already logged the latestRemovedXid.

I like this idea. I've attempted to plug every gap, but perhaps the best
way here is to remove the gap completely.

In my testing, I only saw this case happen a couple of times in many
tests. Rarely executed code gives sporadic bugs, so I would be happy to
remove it and the standby support stuff that goes with it.

I would suggest that we just remove the switch statement:
switch (HeapTupleSatisfiesVacuum(tuple.t_data, OldestXmin, buf))
and alter the following if test since tupgone is also removed.
That will cause HEAPTUPLE_DEAD rows to be fed to heap_freeze_tuple().
Comments on that function claim that is a bad thing, but we know that
any row that was *not* removed by heap_page_prune() and is now dead must
have died very recently and so will never be frozen.

Let me know if you're happy with that change and I'll make it so.

-- 
 Simon Riggs   www.2ndQuadrant.com
 PostgreSQL Training, Services and Support


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

Re: [HACKERS] [PATCHES] Infrastructure changes for recovery (v8)

[Pavan Deolasee]

On Thu, Nov 20, 2008 at 3:12 PM, Simon Riggs <[EMAIL PROTECTED]> wrote:

>
>
> The latest Hot Standby patch includes the latest version of
> "infrastructure changes" patch. Thanks for reviewing.
>
>
Do you intend to split the patch into smaller pieces ? The latest hot
standby patch is almost 10K+ lines. Splitting that would definitely help the
review process.

Thanks,
Pavan

-- 
Pavan Deolasee
EnterpriseDB http://www.enterprisedb.com

Re: [HACKERS] [PATCHES] Infrastructure changes for recovery (v8)

[Simon Riggs]

On Thu, 2008-11-20 at 11:06 +0530, Pavan Deolasee wrote:
> 
> 
> On Mon, Nov 17, 2008 at 9:01 PM, Simon Riggs <[EMAIL PROTECTED]>
> wrote:
> 
> 
> It is, in a later version. Apologies if you're reviewing the
> wrong one.
> 
> 
> 
> The most recent version I can find is v9, but I remember you mentioned
> v10 somewhere else. 
> Can you please confirm if v9 is the latest version and point to the
> latest version if not ? I've some free cycles and would like to help
> with the review process.

The latest Hot Standby patch includes the latest version of
"infrastructure changes" patch. Thanks for reviewing.

-- 
 Simon Riggs   www.2ndQuadrant.com
 PostgreSQL Training, Services and Support


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

Re: [HACKERS] HEAD build failure on win32 mingw

[ITAGAKI Takahiro]

Peter Eisentraut <[EMAIL PROTECTED]> wrote:

> This code would only be executed if you have a man.tar.gz in the doc 
> directory.  If you do, it is probably an old one that indeed does not 
> contain the man7 directory.  So delete the man.tar.gz (and build a new 
> one if you are so inclined).

I used a nightly snapshot(snapshot/postgresql-snapshot.tar.bz2)
and it includes *.gz files. It would be the cause.

Thanks. I'll delete the file before building.

Regards,
---
ITAGAKI Takahiro
NTT Open Source Software Center



-- 
Sent 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: How to deal with toast

[Heikki Linnakangas]

Zdenek Kotala wrote:
The question is if we should do toast modification now to avoid 
potential future problems or if we will solve it when any on-disk format 
change requires it?


Perhaps we should just add the new attid attribute to the toast table, 
but mark it as nullable? We wouldn't need to fill it in in the 8.3->8.4 
conversion but new tuples would include it.


In the future release that we actually need it, we'll make it 
non-nullable, and write a pre-upgrade script to retoast tuples that 
don't have it yet.


Hmm. That would change TOAST_MAX_CHUNK_SIZE, though.

I prefer do it now, because there could be small risk that it will not 
possible to do it in the future. When in-place upgrade will be 
implemented nobody will want to perform backup/restore.


I feel we should avoid doing anything extra, risking that we get nothing 
finished.


--
  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] Problem with Bitmap Heap Scan

[Heikki Linnakangas]

Tom Lane wrote:

"Rushabh Lathia" <[EMAIL PROTECTED]> writes:

Simple select give wrong result when it uses the Bitmap Heap Scan path.


It's generally appropriate to mention which PG version you're working
with when you report a bug.


postgres=# explain select proname from pg_proc where proname like 'my_pro1';
 QUERY
PLAN




-
 Bitmap Heap Scan on pg_proc  (cost=4.26..8.27 rows=1 width=64)
   Recheck Cond: (proname ~~ 'my_pro1'::text)
   ->  Bitmap Index Scan on pg_proc_proname_args_nsp_index  (cost=0.00..4.26
row
s=1 width=0)
 Index Cond: ((proname >= 'my'::name) AND (proname < 'mz'::name))
(4 rows)


Hmm, the ~~ condition should get treated as a "filter" not a "recheck".
I suppose I broke this somewhere ...


I started to look at this last night. The culprit seems to be this patch:


Author: Tom Lane <[EMAIL PROTECTED]>
Date:   Sun Apr 13 19:18:14 2008 +

Phase 2 of project to make index operator lossiness be determined at runtime
instead of plan time.  Extend the amgettuple API so that the index AM 
returns
a boolean indicating whether the indexquals need to be rechecked, and make
that rechecking happen in nodeIndexscan.c (currently the only place where
it's expected to be needed; other callers of index_getnext are just erroring
out for now).  For the moment, GIN and GIST have stub logic that just always
sets the recheck flag to TRUE --- I'm hoping to get Teodor to handle pushing
that control down to the opclass consistent() functions.  The planner no
longer pays any attention to amopreqcheck, and that catalog column will go
away in due course.


and the changes around create_bitmap_scan_plan in particular.

create_bitmap_subplan puts the original ~~ qual into the recheck 
condition, even though the indexqual is only ((proname >= 'my'::name) 
AND (proname < 'mz'::name)). So, the condition put into the recheck 
condition is stronger than the checked by the index.


create_bitmap_scan_plan puts all index clauses that are not in the the 
Recheck condition into the Filter. If the condition in the recheck 
condition is stronger than the condition normally checked by the index, 
that's wrong.


Attached is a patch that changes create_bitmap_subplan so that the 
condition put into Recheck condition is never stronger than the 
condition automatically handled by the index. Does that look right to you?


--
  Heikki Linnakangas
  EnterpriseDB   http://www.enterprisedb.com
*** src/backend/optimizer/plan/createplan.c
--- src/backend/optimizer/plan/createplan.c
***
*** 1197,1202  create_bitmap_subplan(PlannerInfo *root, Path *bitmapqual,
--- 1197,1203 
  		IndexPath  *ipath = (IndexPath *) bitmapqual;
  		IndexScan  *iscan;
  		ListCell   *l;
+ 		List	   *scan_clauses;
  
  		/* Use the regular indexscan plan build machinery... */
  		iscan = create_indexscan_plan(root, ipath, NIL, NIL);
***
*** 1210,1216  create_bitmap_subplan(PlannerInfo *root, Path *bitmapqual,
  		plan->plan_rows =
  			clamp_row_est(ipath->indexselectivity * ipath->path.parent->tuples);
  		plan->plan_width = 0;	/* meaningless */
! 		*qual = get_actual_clauses(ipath->indexclauses);
  		foreach(l, ipath->indexinfo->indpred)
  		{
  			Expr	   *pred = (Expr *) lfirst(l);
--- 1211,1233 
  		plan->plan_rows =
  			clamp_row_est(ipath->indexselectivity * ipath->path.parent->tuples);
  		plan->plan_width = 0;	/* meaningless */
! 
! 		/*
! 		 * Put those indexquals that are automatically handled by the index to
! 		 * the Recheck condition. Don't include clauses that are derived from,
! 		 * but not directly included in the original scan quals. The original
! 		 * clause they're derived from need to be checked anyway in the Filter,
! 		 * even for non-lossy bitmaps.
! 		 */
! 		scan_clauses = get_actual_clauses(ipath->indexclauses);
! 		*qual = NIL;
! 		foreach(l, iscan->indexqualorig)
! 		{
! 			Expr	   *q = (Expr *) lfirst(l);
! 			if (list_member(scan_clauses, q))
! *qual = lappend(*qual, q);
! 		}
! 
  		foreach(l, ipath->indexinfo->indpred)
  		{
  			Expr	   *pred = (Expr *) lfirst(l);

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

Re: [HACKERS] HEAD build failure on win32 mingw

[Peter Eisentraut]

ITAGAKI Takahiro wrote:

HEAD is failed to be built on win32 mingw.
It requires manual 'mkdir man7' or so.

Are there any changes in build process?
Or am I missing something required to build?


$ make
/bin/sh.exe: man7/.timestamp: No such file or directory

$ mkdir doc/man7<- HERE
$ make
(succeeded to build)

$ make install
../config/install-sh: man7/*.7 does not exist.

$ touch doc/man7/dummy.7<- HERE
$ make install
(succeeded to install)




This code would only be executed if you have a man.tar.gz in the doc 
directory.  If you do, it is probably an old one that indeed does not 
contain the man7 directory.  So delete the man.tar.gz (and build a new 
one if you are so inclined).


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

Re: [HACKERS] Problem with Bitmap Heap Scan

[rushabh]

Tom Lane wrote:

"Rushabh Lathia" <[EMAIL PROTECTED]> writes:
  

Simple select give wrong result when it uses the Bitmap Heap Scan path.



It's generally appropriate to mention which PG version you're working
with when you report a bug.
  

I was on 8.4 CVS Head, Next time will sure take care of this.
  

postgres=# explain select proname from pg_proc where proname like 'my_pro1';
 QUERY
PLAN



  


-
 Bitmap Heap Scan on pg_proc  (cost=4.26..8.27 rows=1 width=64)
   Recheck Cond: (proname ~~ 'my_pro1'::text)
   ->  Bitmap Index Scan on pg_proc_proname_args_nsp_index  (cost=0.00..4.26
row
s=1 width=0)
 Index Cond: ((proname >= 'my'::name) AND (proname < 'mz'::name))
(4 rows)



Hmm, the ~~ condition should get treated as a "filter" not a "recheck".
I suppose I broke this somewhere ...
  

Oh ok.

regards, tom lane