[HACKERS] set the cost of an aggregate function

[Jaime Casanova]

Hi,

why we can't do $subject? it could have any benefit on the planner?

-- 
Atentamente,
Jaime Casanova
Soporte y capacitación de PostgreSQL
Asesoría y desarrollo de sistemas
Guayaquil - Ecuador
Cel. +59387171157

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

Re: [HACKERS] Patch: Remove gcc dependency in definition of inline functions

[Marko Kreen]

On 11/29/09, Tom Lane  wrote:
> Kurt Harriman  writes:
>  > (Does anybody still use a C compiler that doesn't support
>  > inline functions?)

+1 for modern C.

> The question isn't so much that, it's whether the compiler supports
>  inline functions with the same behavior as gcc.  At minimum that
>  would require
> * not generating extra copies of the function
> * not whining about unreferenced static functions
>  How many compilers have you tested this patch against?  Which ones
>  does it actually offer any benefit for?

Those are not correctness problems.  Compilers with non-optimal or
missing 'inline' do belong together with compilers without working int64.
We may spend some effort to be able to compile on them, but they
should not affect our coding style.

'static inline' is superior to macros because of type-safety and
side-effects avoidance.  I'd suggest event removing any HAVE_INLINE
ifdefs and let autoconf undef the 'inline' if needed.  Less duplicated
code to maintain.  The existence of compilers in active use without
working 'inline' seems quite hypothetical these days.

-- 
marko

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

[HACKERS] Fwd: psql+krb5

[rahimeh khodadadi]

-- Forwarded message --
From: rahimeh khodadadi 
Date: 2009/11/29
Subject: Re: psql+krb5
To: Denis Feklushkin 


These items have added after my sending.

I repeat again my configurations:


*
1) The configuration of  krb5.conf is:
 [realms]
   EXAMPLE.COM  ={

 kdc=star :88
 admin_server=star:749
 default_domain= example.com
}
.*

2) Then, I created principal as* "  postgres/s...@example.com "* and its
password is saved in* '/usr/local/pgsql/data/postgresql.keytab' .*


(star is localhost IP, but in hosts.conf I configure like: 213.233.169.93
star)

3) I setup *postgresql.conf *as below:

krb_server_keyfile = '/usr/local/pgsql/data/
postgresql.keytab'
krb_srvname = 'postgres/s...@example.com'

krb_server_hostname = 'star' # empty string matches any keytab entry
krb_caseins_users = off

4) I *create user "frank"*  in Psql .

5) Then I set up* hba.conf :*

hostall all 0.0.0.0/0  krb5
hostall all 127.0.0.1/32   krb5


When I want to connect to Postgresql, it gives error.

# *kinit frank*

[r...@star bin]# *./psql -h star  -U frank  -d test*

psql: *krb5_sendauth: Bad application version was sent (via sendauth)*

I should mention that * both postgresql server and krb-server are in same
system* and* my IP is acquring from dhcp server  of university*.  Where is
wrong.

2009/11/29 Denis Feklushkin 

> On Sun, 29 Nov 2009 14:23:52 +0330
> rahimeh khodadadi  wrote:
>
> > Thanks for your replying. My detail of configuration is:
> >
> > I try to setup kerberos authentication in Postgresql 8.1.18 on centos.
> >
> > But I have some problem.
> >
> > 1) The configuration of  krb5.conf is:
> >  [realms]
> >   EXAMPLE.COM  > > ={
> >
> > kdc=star :88
> > admin_server=star:749
> > default_domain= example.com > >
> > > >
> > > }
> > > .
> > >
> > > 2) Then, I created principal as "  postgres/s...@example.com > > s...@example.com> " and its password is saved in
> > > '/usr/local/pgsql/data/postgresql.keytab' .
> > >
> > >
> > > (star is localhost IP, but in hosts.conf I configure like:
> > > 213.233.169.93 star)
> > >
> > > 3) I setup postgresql.conf as below:
> > >
> > > krb_server_keyfile = '/usr/local/pgsql/data/
> > > postgresql.keytab'
> > > krb_srvname = 'postgres/s...@example.com'
> > >
> > > krb_server_hostname = 'star' # empty string matches any
> > > keytab entry
> > > krb_caseins_users = off
> > >
> > > 4) I create user "frank"  in Psql .
> > >
> > > 5) Then I set up hba.conf :
> > >
> > > hostall all 0.0.0.0/0
> > >  krb5
> > > hostall all 127.0.0.1/32
> > >   krb5
> > >
> > >
> > > When I want to connect to Postgresql, it gives error.
> > >
> > > # kinit frank
> > >
> > > [r...@star bin]# ./psql -h star  -U frank  -d test
> > >
> > > psql: krb5_sendauth: Bad application version was sent (via sendauth)
> > >
> >
> > some changes in users gives below error :
> > "[r...@www bin]# ./psql -h 213.233.168.249  -U postgres
> >   psql: Kerberos 5 authentication rejected:  Wrong principal in
> > request"
> >
> >
> > > I should mention that  both postgresql server and krb-server are in
> > > same system and my IP is acquring from dhcp server  of university.
> > > Where is wrong.
> > >
> >
> >
> >
> > 2009/11/29 Denis Feklushkin 
> >
> > > On Sun, 29 Nov 2009 10:48:30 +0330
> > > rahimeh khodadadi  wrote:
> > >
> > > > Hi,
> > > >
> > > > When I want to connect to psql via krb5 in Linux, it gives me
> > > > error like: "[r...@www bin]# ./psql -h 213.233.168.249  -U
> > > > postgres psql: Kerberos 5 authentication rejected:  Wrong
> > > > principal in request"
> > >
> > > Что в логах KDC?
>  !!!
>
> И ещё, в тексте который Вы дали встречаются пробелы в именах
> принципалов и странные записи ""
>
> При настройке важно чтобы ничего этого небыло
>



-- 
With Best Regards
Miss.KHodadadi



-- 
With Best Regards
Miss.KHodadadi

Re: [HACKERS] ProcessUtility_hook

[Itagaki Takahiro]

Euler Taveira de Oliveira  wrote:

> The functionality is divided in two parts. The first part is a hook in the
> utility module. The idea is capture the commands that doesn't pass through
> executor. I'm afraid that that hook will be used only for capturing non-DML
> queries. If so, why don't we hack the tcop/postgres.c and grab those queries
> from the same point we log statements?

DDLs can be used from user defined functions. It has the same reason
why we have executor hooks instead of tcop hooks.


> The second part is to use that hook to capture non-DML commands for
> pg_stat_statements module.

- I fixed a bug that it should handle only isTopLevel command.
- A new parameter pg_stat_statements.track_ddl (boolean) is
  added to enable or disable the feature.

> Do we need to have rows = 0 for non-DML commands?
> Maybe NULL could be an appropriate value.

Yes, but it requires additional management to handle 0 and NULL
separately. I don't think it is worth doing.

> The PREPARE command stopped to count
>  the number of rows. Should we count the rows in EXECUTE command or in the
> PREPARE command?

It requires major rewrites of EXECUTE command to pass the number of
affected rows to caller. I doubt it is worth fixing because almost all
drivers use protocol-level prepared statements instead of PREPARE+EXECUTE.

> The other command that doesn't count properly is COPY. Could
> you fix that?

I added codes for it.

> I'm concerned about storing some commands that expose passwords
> like CREATE ROLE foo PASSWORD 'secret'; I know that the queries are only
> showed to superusers but maybe we should add this information to documentation
> or provide a mechanism to exclude those commands.

I think there is no additonal problem there because we can see the 'secret'
command using pg_stat_activity even now.


> I don't know if it is worth the trouble adding some code to capture VACUUM and
> ANALYZE commands called inside autovacuum.

I'd like to exclude VACUUM and ANALYZE by autovacuum because
pg_stat_statements should not filled with those commands.


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



ProcessUtility_hook_20091130.patch
Description: Binary data

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

Re: [HACKERS] Application name patch - v4

[Andres Freund]

Hi,

On Monday 30 November 2009 01:16:43 Florian G. Pflug wrote:
> Tom Lane wrote:
> > : One possibility would be to make it possible to issue SETs that
> >
> > behave : as if set in a startup packet - imho its an implementation
> > detail that : SET currently is used.
> >
> > I think there's a good deal of merit in this, and it would't be hard
> > at all to implement, seeing that we already have SET LOCAL and SET
> > SESSION. We could add a third keyword, say SET DEFAULT, that would
> > have the behavior of setting the value in a fashion that would
> > persist across resets.  I'm not sure that DEFAULT is exactly le mot
> > juste here, but agreeing on a keyword would probably be the hardest
> > part of making it happen.
> Hm, but without a way to prevent the users of a connection pool from
> issuing "SET DEFAULT", that leaves a connection pool with no way to
> revert a connection to a known state.
Perhaps we should only allow a few parameters to be SET as a connection 
default - then the pooler would have to issue those just as it has to do for 
actual connection defaults.

> How about "SET CONNECTION", with an additional GUC called
> connection_setup which can only be set to true, never back to false.
> Once connection_setup is set to true, further SET CONNECTION attempts
> would fail.
How would that help the pooler case? The next connection to it might be from a 
different application.

Andres


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

Re: [HACKERS] draft RFC: concept for partial, wal-based replication

[Craig Ringer]

Boszormenyi Zoltan wrote:

> c. splitting wal into different replication sets

Just a side note: in addition to its use for partial replication, this
might have potential for performance-prioritizing databases or tablespaces.

Being able to separate WAL logging so that different DBs, tablespaces,
etc went to different sets of WAL logs would allow a DBA to give some
databases or tablespaces dedicated WAL logging space on faster storage.
If partial recovery is implemented, it might also permit less important
databases to be logged to fast-but-unsafe storage such as a non-BBU disk
controller with write cache enabled, without putting more important
databases in the same cluster in danger.

More importantly, if the WAL writing was done in different wal writer
backends, the admin could also use nice and ionice to encourage the OS
to favour WAL logging for some DBs over others.

Currently all these things require splitting the install into multiple
clusters, incurring config management and backup overhead and most
importantly partitioning shared memory.

OTOH, even with split WAL logging, you still have the shared bgwriter to
contend with, and the effects of an unimportant query pushing data
related to more performance-critical DBs out of shm or OS cache. So
perhaps splitting the cluster is actually the best answer, and a
complete implementation of DB prioritization would land up looking a lot
like multiple Pg clusters multiplexed on one port anyway...

In any case, I thought it worth mentioning as something that may be
worth keeping in mind - or considering and disregarding - while looking
at the WAL changes involved in partial replication.

--
Craig Ringer

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

Re: [HACKERS] SE-PgSQL patch review

[KaiGai Kohei]

Itagaki Takahiro wrote:
> KaiGai Kohei  wrote:
>>   -- keep it smaller, and step-by-step enhancement
> 
> I'd prefer "smaller concept" rather than "smaller patch".

For the last a few days, I've talked with Itagaki-san off list to make clear
where is the point of his suggestion.

In summary, it was similar approach with what I already proposed in the CF#2,
but rejected.

During the first commit-fest of v8.5 development cycle, Stephen Frost suggested
to rework the default PG's access controls to host other optional security
features, not only the default one.
Then, I submitted a large patch titled as "Reworks for Access Controls",
but it contained 3.5KL of changeset on the core routines, and 4KL of new codes
into "src/backend/security/*" except for documentations and testcases.
Then, this approach was rejected (not "returned with feedback") due to the scale
and complexity.

After the fest, we discussed the direction to implement SE-PgSQL.
Basically, it needs to keep the changeset small, and the rest of features (such
as row-level granurality, access control decision cache, ...) shoule be added
step-by-step consistently, according to the suggestion in the v8.4 development
cycle. Heikki Linnakangas also suggested we need developer documentation which
introduces SE-PgSQL compliant permission checks and specification of security
hooks, after the reworks are rejected.

So, I boldly removed most of the features from SE-PgSQL except for its core
functionalities, and added developer documentation (README) and widespread
source code comments to introduce the implementations instead.
In the result, the current proposal is near to naked one.
 - No access controls except for database, schema, table and column.
 - No row-level granularity in access controls.
 - No access control decision chache.
 - No security OID within HeapTupleHeader.

I believe the current patch is designed according to the past suggestions.
However, his suggestion seems to me backing to the rejected approach again.

I've been torn between the past suggestions and his suggestion.
So, I asked him to get off reviewing the patch, because of the deadlock in
the development process. At the current moment, I'd like to respect
suggestions in the past discussions more.

Thanks for paying your efforts in spite of differences in opinions.
-- 
OSS Platform Development Division, NEC
KaiGai Kohei 

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

Re: [HACKERS] New VACUUM FULL

[Itagaki Takahiro]

Thanks for review!

Jeff Davis  wrote:

>  * "VACUUM (FULL REPLACE) pg_class" should be rejected, not silently
> turned into "VACUUM (FULL INPLACE) pg_class".

Hmmm, it requires to remember whether REPLACE is specified or not
for the non-INPLACE vacuum, but I don't want to add VACOPT_REPLACE
only for the purpose.

I just removed "FULL REPLACE" syntax; Only "FULL" and "FULL INPLACE" are
available. VACUUM FULL without INPLACE behaves as cluster-like rewrites
for non-system tables. FULL INPLACE is a traditional vacuum full.
System catalogs are always vacuumed with INPLACE version.
  - VACUUM FULL / VACUUM (FULL) => rewritten version
  - VACUUM (FULL INPLACE)   => traditional version

>  * In vacuumdb, the code for handling INPLACE is a little ugly. Perhaps
> it should be changed to always use your new options syntax? That might
> be more code, but it would be a little more readable.

It might make the code cleaner, but I want vacuumdb in 8.5 to work on older
versions of servers unless we use the new feature. Older servers can only
accept older syntax, so I avoided using the new syntax if not needed.
(The new patch still uses two versions of syntax.)

>  * The patch should be merged with CVS HEAD. The changes required are
> minor; but notice that there is a minor style difference in the assert
> in vacuum().
>  * vacuumdb should reject -i without -f
>  * The replace or inplace option is a "magical" default, because "VACUUM
> FULL" defaults to "replace" for user tables and "inplace" for system
> tables. I tried to make that more clear in my documentation suggestions.
>  * There are some windows line endings in the patch, which should be
> removed.

Done.

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



vacuum-full_20091130.patch
Description: Binary data

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

Re: [HACKERS] Listen / Notify - what to do when the queue is full

[Jeff Davis]

On Fri, 2009-11-20 at 10:35 +0100, Joachim Wieland wrote:
> On Thu, Nov 19, 2009 at 11:04 PM, Joachim Wieland  wrote:
> > Given your example, what I am proposing now is to stop reading from
> > the queue once we see a not-yet-committed notification but once the
> > queue is full, read the uncommitted notifications, effectively copying
> > them over into the backend's own memory... Once the transaction
> > commits and sends a signal, we can process, send and discard the
> > previously copied notifications. In the above example, at some point
> > one, two or all three backends would see that the queue is full and
> > everybody would read the uncommitted notifications of the other one,
> > copy them into the own memory and space will be freed in the queue.
> 
> Attached is the patch that implements the described modifications.

This is a first-pass review.

Comments:

 * Why don't we read all notifications into backend-local memory at
every opportunity? It looks like sometimes it's only reading the
committed ones, and I don't see the advantage of leaving it in the SLRU.

Code comments:

 * I see a compiler warning:
ipci.c: In function ‘CreateSharedMemoryAndSemaphores’:
ipci.c:222: warning: implicit declaration of function ‘AsyncShmemInit’
 * sanity_check test fails, needs updating
 * guc test fails, needs updating
 * no docs

The overall design looks pretty good to me. From my very brief pass over
the code, it looks like:
 * When the queue is full, the transaction waits until there is room
before it's committed. This may have an effect on 2PC, but the consensus
seemed to be that we could restrict the combination of 2PC + NOTIFY.
This also introduces the possibility of starvation, I suppose, but that
seems remote.
 * When the queue is full, the inserter tries to signal the listening
backends, and tries to make room in the queue.
 * Backends read the notifications when signaled, or when inserting (in
case the inserting backend is also the one preventing the queue from
shrinking).

I haven't looked at everything yet, but this seems like it's in
reasonable shape from a high level. Joachim, can you clean the patch up,
include docs, and fix the tests? If so, I'll do a full review.

Regards,
Jeff Davis


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

Re: [HACKERS] Add YAML option to explain

[Itagaki Takahiro]

Itagaki Takahiro  wrote:

> My rewrite is relatively large. Please reversely-review the patch.

I rethink the code cleanup should be done with another patch even if needed.
Here is a lite version of yaml YAML explan patch.

All of the logic for indent is done in ExplainYAMLLineStarting().
It can remove undesirable linebreaks from the item list and end of output.

Please check whether the v3.1 patch works as your expectation.
If ok, we can move it to "Ready for Committer".

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



yaml.explain.v3.1.patch
Description: Binary data

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

Re: [HACKERS] Application name patch - v4

[Fujii Masao]

On Mon, Nov 30, 2009 at 11:21 AM, Fujii Masao  wrote:
> On Mon, Nov 30, 2009 at 10:20 AM, Tom Lane  wrote:
>> Fujii Masao  writes:
>>> Why doesn't application_name appear in postgresql.conf.sample?
>>> That is expected to be set from only libpq?
>>
>> It would seem pretty silly to set it in the conf file.  You *can*,
>> if you want, but I see no reason to list it there.
>
> Yeah, I see your point. But, is it a policy not to put such parameter
> (other than that for debug) on postgresql.conf.sample?

Ooops! I missed GUC_NOT_IN_SAMPLE parameters. Sorry for noise.

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] draft RFC: concept for partial, wal-based replication

[Fujii Masao]

On Mon, Nov 30, 2009 at 4:56 AM, Boszormenyi Zoltan  wrote:
> I don't think its reasonable trying to discuss and implement this all
> in one huge patch so I propose implementing at least 1) as a seperate
> patch.

I agree with this development plan.

> Now there unforatunately are two different schools of thought how to 
> implement transfering base data.
> 1. archive_command like transfer command taking a source path/host and target 
> path/host
>   - very flexible (think e.g. not transferring the data for multiple slaves 
> over the whole country)
>   - harder to setup
>   - more in style of classical wal archiving
> 2. add the capability to the WAL Streaming patch's libpq based protocol
>   - no additional configuration needed
>   - inflexible
>   - makes usage from non streaming replication is impossible
>
> I favor 1. but only lightly so.

I favor 2 ;) Because I think that it's too hard for users to set up
a transfer command. One of streaming replication's merits is that
users no longer need to specify a transfer command for log-shipping.
So users can configure and use replication without complex settings.
But, #1 would spoil this merit.

> Detail Questions:
> - How to deal with multiple transfer requests at the same time?
>  There would be a need for multiple full backup requests for
>  individual tables by several clients at once.
>  Currently pg_start_backup() isn't allowed from
>  two clients in parallel, the second one gets an error.
>  We thought that pg_start_backup() and pg_stop_backup()
>  can turn into simple reference counts. IIRC, WALs
>  are still generated and _shipped to slaves_ during
>  a full backup, they are simply not yet applied to
>  base table files. So, in this case a pg_stop_backup()
>  issued from a slave decreases refcount of the base backups
>  and the slave can simply resume applying its newly
>  received WALs to base files.

I'm not sure how. But at first multiple online-backup feature
rather than backup-shipping itself might have to be addressed.

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] [PATCH 4/4] Add tests to dblink covering use of COPY TO FUNCTION

[Jeff Davis]

On Sun, 2009-11-29 at 18:53 -0800, Daniel Farina wrote:
> On Sun, Nov 29, 2009 at 6:35 PM, Jeff Davis  wrote:
> > What if the network buffer is flushed in the middle of a line? Is that
> > possible, or is there a guard against that somewhere?
> 
> What do you mean?  They both catenate onto one stream of bytes, it
> shouldn't matter where the flush boundaries are...

Nevermind, for some reason I thought you were talking about interleaving
the data rather than concatenating.

Regards,
Jeff Davis


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

Re: [HACKERS] draft RFC: concept for partial, wal-based replication

[Itagaki Takahiro]

Boszormenyi Zoltan  wrote:

> we tried to discuss on a lower level what should be needed
> for a partial replication based on streaming replication.

We need to discuss a "partial recovery" before the partial replication.

There are some related items in out ToDo list and previous discussions:

http://wiki.postgresql.org/wiki/Todo
- Allow WAL logging to be turned off for a table,
  but the table might be dropped or truncated during crash recovery
- Allow WAL logging to be turned off for a table,
  but the table would avoid being truncated/dropped

- rmgr_hook
http://archives.postgresql.org/pgsql-hackers/2008-12/msg01361.php

Could you try them first as a preparation for the partial replication?

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] [PATCH 4/4] Add tests to dblink covering use of COPY TO FUNCTION

[Daniel Farina]

On Sun, Nov 29, 2009 at 6:35 PM, Jeff Davis  wrote:
> What if the network buffer is flushed in the middle of a line? Is that
> possible, or is there a guard against that somewhere?

What do you mean?  They both catenate onto one stream of bytes, it
shouldn't matter where the flush boundaries are...

It so happens as a convenient property of the textual modes is that
adding more payload is purely concatenative (not true for binary,
where there's a header that would cause confusion to the receiving
side)

fdr

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

Re: [HACKERS] [PATCH 4/4] Add tests to dblink covering use of COPY TO FUNCTION

[Jeff Davis]

On Thu, 2009-11-26 at 18:30 -0800, Daniel Farina wrote:
> Okay, so this thread sort of wandered into how we could refactor other
> elements of COPY.  Do we have a good sense on what we should do to the
> current patch (or at least the idea represented by it) to get it into
> a committable state within finite time?

We're in the middle of a commitfest, so a lot of hackers are
concentrating on other patches. In a week or two, when it winds down,
people will be more willing to make decisions on new proposals and
designs. I still think this thread has been productive.

> I think adding a bytea and/or text mode is once such improvement...I
> am still reluctant to give up on INTERNAL because the string buffer
> passed in the INTERNAL scenario is ideal for C programmers -- the
> interface is even simpler than dealing with varlena types.  But I
> agree that auxiliary modes should exist to enable easier hacking.

I like the idea of an internal mode as well. We may need some kind of
performance numbers to justify avoiding the extra memcpy, though.

> The thorniest issue in my mind is how state can be initialized
> retained and/or modified between calls to the bytestream-acceptance
> function.
> 
> Arguably it is already in a state where it is no worse than dblink,
> which itself has a global hash table to manage state.

The idea of using a separate type of object (e.g. "CREATE COPYSTREAM")
to bundle the init/read/write/end functions together might work. That
also allows room to specify what the functions should accept
(internal/bytea/text).

I think that's the most elegant solution (at least it sounds good to
me), but others might not like the idea of a new object type just for
this feature. Perhaps if it fits nicely within an overall SQL/MED-like
infrastructure, it will be easier to justify.

> Also, if you look carefully at the dblink test suite I submitted,
> you'll see an interesting trick: one can COPY from multiple sources
> consecutively to a single COPY on a remote node when in text mode
> (binary mode has a header that cannot be so neatly catenated).  This
> is something that's pretty hard to enable with any automatic
> startup-work-cleanup approach.

What if the network buffer is flushed in the middle of a line? Is that
possible, or is there a guard against that somewhere?

Regards,
Jeff Davis


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

Re: [HACKERS] [PATCH 4/4] Add tests to dblink covering use of COPY TO FUNCTION

[Jeff Davis]

On Fri, 2009-11-27 at 20:28 -0500, Greg Smith wrote:
> In the context of the read case, I'm not as sure it's so black and 
> white.  While the current situation does map better to a function that 
> produces a stream of bytes, that's not necessarily the optimal approach 
> for all situations.  It's easy to imagine a function intended for 
> accelerating bulk loading that is internally going to produce a stream 
> of already processed records. 

The binary COPY mode is one of the closest things I can think of to
"already-processed records". Is binary COPY slow? If so, the only thing
faster would have to be machine-specific, I think.

> I think there's a very valid use-case for both approaches.
...
> COPY target FROM FUNCTION foo() WITH RECORDS;

In what format would the records be?

Also, this still doesn't really answer why INSERT ... SELECT isn't good
enough. If the records really are in their internal format, then
INSERT ... SELECT seems like the way to go.

Regards,
Jeff Davis


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

Re: [HACKERS] Application name patch - v4

[Fujii Masao]

On Mon, Nov 30, 2009 at 10:20 AM, Tom Lane  wrote:
> Fujii Masao  writes:
>> Why doesn't application_name appear in postgresql.conf.sample?
>> That is expected to be set from only libpq?
>
> It would seem pretty silly to set it in the conf file.  You *can*,
> if you want, but I see no reason to list it there.

Yeah, I see your point. But, is it a policy not to put such parameter
(other than that for debug) on postgresql.conf.sample?

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] Application name patch - v4

[Tom Lane]

Fujii Masao  writes:
> Why doesn't application_name appear in postgresql.conf.sample?
> That is expected to be set from only libpq?

It would seem pretty silly to set it in the conf file.  You *can*,
if you want, but I see no reason to list it 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] cvs chapters in our docs

[Ron Mayer]

Brendan Jurd wrote:
> 2009/11/29 Bruce Momjian :
>> Wow, we mention 28k modems --- we are legacy software:  ;-)
>>
>> This initial checkout is a little slower than simply downloading
>> a tar.gz file; expect it to take 40 minutes
>> or so if you have a 28.8K modem.
> 
> Yes, and what about all the people using carrier pidgeon to download
> Postgres?  I think our documentation is neglecting this substantial
> and vital portion of our user community.

Never underestimate the bandwidth of a carrier pigeon with a flash
card tied to his leg. [1]

   "11-month-old bird armed with a 4GB memory stick... the carrier pigeon
   delivered 4GB of data 60 miles in a little over an hour"


[1] 
http://www.dslreports.com/shownews/Carrier-Pigeon-Officially-Beats-DSL-104393


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

Re: [HACKERS] Application name patch - v4

[Fujii Masao]

On Sun, Nov 29, 2009 at 8:47 AM, Tom Lane  wrote:
> Dave Page  writes:
>> Updated application name patch, including a GUC assign hook to clean
>> the application name of any unsafe characters, per discussion.
>
> Applied with assorted editorialization.  There were a couple of
> definitional issues that I don't recall if we had consensus on:

Why doesn't application_name appear in postgresql.conf.sample?
That is expected to be set from only libpq?

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] Application name patch - v4

[Florian G. Pflug]

Tom Lane wrote:

: One possibility would be to make it possible to issue SETs that
behave : as if set in a startup packet - imho its an implementation
detail that : SET currently is used.

I think there's a good deal of merit in this, and it would't be hard
at all to implement, seeing that we already have SET LOCAL and SET
SESSION. We could add a third keyword, say SET DEFAULT, that would
have the behavior of setting the value in a fashion that would
persist across resets.  I'm not sure that DEFAULT is exactly le mot
juste here, but agreeing on a keyword would probably be the hardest
part of making it happen.


Hm, but without a way to prevent the users of a connection pool from
issuing "SET DEFAULT", that leaves a connection pool with no way to
revert a connection to a known state.

How about "SET CONNECTION", with an additional GUC called
connection_setup which can only be set to true, never back to false.
Once connection_setup is set to true, further SET CONNECTION attempts
would fail.

In a way, this mimics startup-packet SETs without actually doing things
in the startup packet.

best regards,
Florian Pflug



smime.p7s
Description: S/MIME Cryptographic Signature

Re: [HACKERS] Application name patch - v4

[Tom Lane]

Dimitri Fontaine  writes:
> Le 29 nov. 2009 à 18:22, Tom Lane a écrit :
>>> I think we should use GUC_NO_RESET_ALL.
>> 
>> I agree with you, but it seems we have at least as many votes to not do
>> that.  Any other votes out there?

> Driven by the pooler use case (pgbouncer, even), I'd say RESET ALL should 
> reset also the application name. And the connection value is not tied any 
> more to something sensible as soon as you have pooling in there...

The thing is that the libpq API treats application_name as a *property
of the connection*.  You shouldn't expect it to go away on RESET ALL,
any more than you'd expect RESET ALL to cause you to be reconnected to
some other database.

If a pooler wants application_name to be cleared when it issues RESET
ALL, I think it ought to be setting the name via SET, not via the libpq
connection option.

But it's certainly true that using GUC_NO_RESET_ALL would be a quick
kluge rather than a proper solution.  Andres Freund suggested upthread
that we should fix this by extending SET:

: One possibility would be to make it possible to issue SETs that behave
: as if set in a startup packet - imho its an implementation detail that
: SET currently is used.

I think there's a good deal of merit in this, and it would't be hard at
all to implement, seeing that we already have SET LOCAL and SET SESSION.
We could add a third keyword, say SET DEFAULT, that would have the
behavior of setting the value in a fashion that would persist across
resets.  I'm not sure that DEFAULT is exactly le mot juste here, but
agreeing on a keyword would probably be the hardest part of making it
happen.

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] Writeable CTE patch

[Alex Hunsaker]

On Sat, Nov 28, 2009 at 11:59, Tom Lane  wrote:
> 1. I thought we'd agreed at
> http://archives.postgresql.org/pgsql-hackers/2009-10/msg00558.php
> that the patch should support WITH on DML statements, eg
>        with (some-query) insert into foo ...
> This might not take much more than grammar additions, but it's
> definitely lacking at the moment.

Hrm ? A few messages down you say SELECT should be a good start

http://archives.postgresql.org/pgsql-hackers/2009-10/msg01081.php

> 2. The handling of rules on DML WITH queries is far short of sufficient.

Ick.

> Perhaps it would be acceptable to just throw ERROR_FEATURE_NOT_SUPPORTED
> when there are DO ALSO or conditional DO INSTEAD rules applying to the
> target of a DML WITH query.

+1

> 3. I'm pretty unimpressed with the code added to ExecutePlan.
> I wonder whether it would be practical to fix both #2 and #3 by having the
> representation of DML WITH queries look more like the representation of
> rule rewrite output

Interesting...  This seems like the best solution ( assuming its
workable ).  It also looks like it might make #1 easier as well.

However, I think the current approach does have some virtue in that I
was surprised how little the patch was.  Granted that is partly due to
ExecutePlan knowing to much.

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

Re: [HACKERS] plperl and inline functions -- first draft

[Joshua Tolley]

On Sat, Nov 28, 2009 at 10:15:40PM -0500, Tom Lane wrote:
> Joshua Tolley  writes:
> > Makes sense on both counts. Thanks for the help. How does the attached look?
> 
> Applied with minor corrections, mainly around the state save/restore
> logic.  I also put in some code to fix the memory leak noted by Tim Bunce,
> but am waiting for some confirmation that it's right before
> back-patching the pre-existing bug of the same ilk.
> 
>   regards, tom lane

Yay, and thanks. For the record, I'm can't claim to know whether your fix is
the Right Thing or not, so I'm witholding comment.

--
Joshua Tolley / eggyknap
End Point Corporation
http://www.endpoint.com


signature.asc
Description: Digital signature

Re: [HACKERS] Patch: Remove gcc dependency in definition of inline functions

[Tom Lane]

Kurt Harriman  writes:
> (Does anybody still use a C compiler that doesn't support
> inline functions?)

The question isn't so much that, it's whether the compiler supports
inline functions with the same behavior as gcc.  At minimum that
would require
* not generating extra copies of the function
* not whining about unreferenced static functions
How many compilers have you tested this patch against?  Which ones
does it actually offer any benefit for?

regards, tom lane

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

Re: [HACKERS] cvs chapters in our docs

[Brendan Jurd]

2009/11/29 Bruce Momjian :
> Wow, we mention 28k modems --- we are legacy software:  ;-)
>
>     This initial checkout is a little slower than simply downloading
>     a tar.gz file; expect it to take 40 minutes
>     or so if you have a 28.8K modem.

Yes, and what about all the people using carrier pidgeon to download
Postgres?  I think our documentation is neglecting this substantial
and vital portion of our user community.

Cheers,
BJ

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

Re: [HACKERS] Application name patch - v4

[Dimitri Fontaine]

Hi,

Le 29 nov. 2009 à 18:22, Tom Lane a écrit :
>> I think we should use GUC_NO_RESET_ALL.
> 
> I agree with you, but it seems we have at least as many votes to not do
> that.  Any other votes out there?

Driven by the pooler use case (pgbouncer, even), I'd say RESET ALL should reset 
also the application name. And the connection value is not tied any more to 
something sensible as soon as you have pooling in there...

Regards,
-- 
dim

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

Re: [HACKERS] draft RFC: concept for partial, wal-based replication

[Boszormenyi Zoltan]

Hi,

we tried to discuss on a lower level what should be needed
for a partial replication based on streaming replication.

a. transferring base data after a slave got added a relation/index/...
(and initial setup)
b. communicating the the slaves which relations they currently should
have available
c. splitting wal into different replication sets
d. some configuration frontend. Possibly directly via sql or via functions

I don't think its reasonable trying to discuss and implement this all
in one huge patch so I propose implementing at least 1) as a seperate
patch.

a) is very useful outside the context of this specific feature and
kind of a requisite so i suggest tackling that first.

Plan:
M: checkpoint, full page writes, access share lock on the relation
S: stop receiving wal
SM: Using $method transfer required base data for every required segment/fork
M: normal writes
S: restart wal replay

Do you see any fundamental problem with this?

Now there unforatunately are two different schools of thought how to implement 
transfering base data.
1. archive_command like transfer command taking a source path/host and target 
path/host
   - very flexible (think e.g. not transferring the data for multiple slaves 
over the whole country)
   - harder to setup
   - more in style of classical wal archiving
2. add the capability to the WAL Streaming patch's libpq based protocol
   - no additional configuration needed
   - inflexible
   - makes usage from non streaming replication is impossible

I favor 1. but only lightly so.

Detail Questions:
- How to deal with multiple transfer requests at the same time?
  There would be a need for multiple full backup requests for
  individual tables by several clients at once.
  Currently pg_start_backup() isn't allowed from
  two clients in parallel, the second one gets an error.
  We thought that pg_start_backup() and pg_stop_backup()
  can turn into simple reference counts. IIRC, WALs
  are still generated and _shipped to slaves_ during
  a full backup, they are simply not yet applied to
  base table files. So, in this case a pg_stop_backup()
  issued from a slave decreases refcount of the base backups
  and the slave can simply resume applying its newly
  received WALs to base files.
- Keep track of current number of transfers
  We would also need a way to query the refcount of the
  base backup, so if a slave dies, the master can be
  recovered manually, so it can also resume applying leftover WALs.

b) The slave needs to know whether a relation got added to itself in
order to request a base backup of the relevant files.

I would suggest adding a new wal record type for this:
  - DROP_RELATION_SLAVE(node_id, relation)
  - ADD_RELATION_SLAVE(node_id, relation)

We thought about two ways of administering the replication set:
- slaves with full replication, optionally and explicitely excluding
relations
- slaves with minimal replication, explicitely included relations
The above WAL record types will be needed both cases,
but we also need two new catalog tables:
- the slave nodes, indicating the type of the slave above
- explicit table indications (treated depending on the type of the slave)

Any out of band communication has severe problems with
crashes/unavailability of the slave or not allowing classic, non
streaming, wal replication.

Questions:
- How to deal with the fact that the slave may be unavailable during adding 
something to
  its replication set?
  - Possibly forbid all DDL to the table until the slave got the update

c)
What to filter:
Every slave gets a node_id which is assigned in a system catalog on the master
pg_node(nodeoid) (per cluster)
And a catalog contains all replicated relations.
pg_replication_set(nodeoid, classoid, acknowledged_on_slave) (per database)

How to filter:
Heap2, Heap, Btree, Gin, Gist, Storage, Sequence need to be filtered by 
database/relation.
I am by far not yet familiar enough with the relevant code to see if it is 
feasible and worthwile at all to filter clog, transaction and multixact per 
database.

Where to filter:
I propose doing so in the walsender. While this would prohibit using
classical wal based standby I do not see a big problem in that.

If done via wal streaming it would be a simple addition of a node_id in 
PQstartXLogStreaming. This id obviously should not be resettable to something 
else...

Questions:
- How to deal with access to the different database-wide catalogs?
  - Storing that data cluster wide seems really ugly.
  - read the code...

d) I do not have any strong or even moderate opinions about this. I think its 
sensible to get something prototypish, function based done before deciding 
about the real interface and getting into syntax wars.


Steps:

1:
- Transfer of Relations during runtime
  - needed to use wal-splitting
  - internally:
- Possibility 1:
  - transfer_command = ... %filename%
  - should not error out if data changes beneath it.
  - called for every file (i.e. eve

[HACKERS] Patch: Remove gcc dependency in definition of inline functions

[Kurt Harriman]

Hi,

The attached patch is offered for the 2010-01 commitfest.
It's also available in my git repository in the "submitted" branch:
  
http://git.postgresql.org/gitweb?p=users/harriman/share.git;a=shortlog;h=refs/heads/submitted

In palloc.h and pg_list.h, some inline functions are defined if
allowed by the compiler.  Previously the test was gcc-specific.
This patch enables inlining on more platforms, relying on the
Autoconf-generated "configure" script to determine whether
the compiler supports inline functions.

Depending on the compiler, the keyword for defining an inline
function might be spelled as inline, __inline, or __inline__,
or none of these.  "configure" finds out, and except in the
first case, puts a suitable "#define inline" into pg_config.h.
This hasn't changed.

What's new is that "configure" will add "#define HAVE_INLINE 1"
into pg_config.h if it detects that the compiler accepts inline
function definitions.  palloc.h and pg_list.h will condition
their inline function definitions on HAVE_INLINE instead of
the gcc-specific __GNUC__.

After applying the patch, run these commands to update
the configure script and pg_config.h.in:
autoconf; autoheader

(Does anybody still use a C compiler that doesn't support
inline functions?)

Regards,
... kurt


Remove gcc dependency in definition of "inline" functions.

In palloc.h and pg_list.h, some inline functions are defined if
allowed by the compiler.  Previously the test was gcc-specific.
Now the test is platform independent, relying on the Autoconf-
generated "configure" script to determine whether the compiler
supports inline functions.

Depending on the compiler, the keyword for defining an inline
function might be spelled as inline, __inline, or __inline__,
or none of these.  "configure" finds out, and except in the
first case, puts a suitable "#define inline" into pg_config.h.
This hasn't changed.

What's new is that "configure" now adds "#define HAVE_INLINE 1"
into pg_config.h upon finding that the compiler accepts inline
function definitions.  palloc.h and pg_list.h now condition
their inline function definitions on HAVE_INLINE instead of
the gcc-specific __GNUC__.

---
 configure.in  |4 
 src/backend/nodes/list.c  |   10 --
 src/backend/utils/mmgr/mcxt.c |9 -
 src/include/nodes/pg_list.h   |4 ++--
 src/include/utils/palloc.h|8 
 5 files changed, 18 insertions(+), 17 deletions(-)
--- Kurt Harriman  2009-11-29 08:22:05 -0800

diff --git a/configure.in b/configure.in
index 612e843..467f40d 100644
--- a/configure.in
+++ b/configure.in
@@ -1105,6 +1105,10 @@ PGAC_STRUCT_SOCKADDR_STORAGE
 PGAC_STRUCT_SOCKADDR_STORAGE_MEMBERS
 PGAC_STRUCT_ADDRINFO
 
+if test "$ac_cv_c_inline" != no; then
+  AC_DEFINE(HAVE_INLINE, 1, [Define to 1 if inline functions are allowed in C])
+fi
+
 AC_CHECK_TYPES([struct cmsgcred, struct fcred, struct sockcred], [], [],
 [#include 
 #include 
diff --git a/src/backend/nodes/list.c b/src/backend/nodes/list.c
index 1ba85f4..66fa3d6 100644
--- a/src/backend/nodes/list.c
+++ b/src/backend/nodes/list.c
@@ -1224,12 +1224,10 @@ list_copy_tail(List *oldlist, int nskip)
 }
 
 /*
- * When using non-GCC compilers, we can't define these as inline
- * functions in pg_list.h, so they are defined here.
- *
- * TODO: investigate supporting inlining for some non-GCC compilers.
+ * pg_list.h defines inline versions of this function if allowed by the 
+ * compiler; in which case the definitions below are skipped.
  */
-#ifndef __GNUC__
+#ifndef HAVE_INLINE
 
 ListCell *
 list_head(List *l)
@@ -1248,7 +1246,7 @@ list_length(List *l)
 {
return l ? l->length : 0;
 }
-#endif   /* ! __GNUC__ */
+#endif   /* ! HAVE_INLINE */
 
 /*
  * Temporary compatibility functions
diff --git a/src/backend/utils/mmgr/mcxt.c b/src/backend/utils/mmgr/mcxt.c
index 4939046..2c5ddbb 100644
--- a/src/backend/utils/mmgr/mcxt.c
+++ b/src/backend/utils/mmgr/mcxt.c
@@ -628,11 +628,10 @@ repalloc(void *pointer, Size size)
  * MemoryContextSwitchTo
  * Returns the current context; installs the given context.
  *
- * This is inlined when using GCC.
- *
- * TODO: investigate supporting inlining for some non-GCC compilers.
+ * palloc.h defines an inline version of this function if allowed by the 
+ * compiler; in which case the definition below is skipped.
  */
-#ifndef __GNUC__
+#ifndef HAVE_INLINE
 
 MemoryContext
 MemoryContextSwitchTo(MemoryContext context)
@@ -645,7 +644,7 @@ MemoryContextSwitchTo(MemoryContext context)
CurrentMemoryContext = context;
return old;
 }
-#endif   /* ! __GNUC__ */
+#endif   /* ! HAVE_INLINE */
 
 /*
  * MemoryContextStrdup
diff --git a/src/include/nodes/pg_list.h b/src/include/nodes/pg_list.h
index 04da862..4e3e08d 100644
--- a/src/include/nodes/pg_list.h
+++ b/src/include/nodes/pg_list.h
@@ -74,7 +74,7 @@ struct ListCell
  * arguments. Therefore, we implement them using GCC inline functions,
  * and as regular functions with non

Re: [HACKERS] Timezones (in 8.5?)

[Pavel Stehule]

2009/11/29 David E. Wheeler :
> On Nov 28, 2009, at 5:40 PM, Bruce Momjian wrote:
>
>> I think there is general agreement that we should have a timezone data
>> type which validates against pg_timezone_names().name.  It might be
>> enough to just document how users can create such a domain data type,
>> but I don't know of a way to do that.  Is this a TODO?
>
> From 
> http://justatheory.com/computers/databases/postgresql/citext-patch-submitted.html
>
> CREATE OR REPLACE FUNCTION is_timezone( tz TEXT ) RETURNS BOOLEAN as $$
> BEGIN
>  PERFORM now() AT TIME ZONE tz;
>  RETURN TRUE;
> EXCEPTION WHEN invalid_parameter_value THEN
>  RETURN FALSE;
> END;
> $$ language plpgsql STABLE;
>
> CREATE DOMAIN timezone AS CITEXT
> CHECK ( is_timezone( value ) );
>
> It could also be TEXT I suppose, but "America/Los_Angeles" and 
> "america/los_angeles" should be considered the same.

nice :)

Pavel

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

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

Re: [HACKERS] Timezones (in 8.5?)

[David E. Wheeler]

On Nov 28, 2009, at 5:40 PM, Bruce Momjian wrote:

> I think there is general agreement that we should have a timezone data
> type which validates against pg_timezone_names().name.  It might be
> enough to just document how users can create such a domain data type,
> but I don't know of a way to do that.  Is this a TODO?

>From 
>http://justatheory.com/computers/databases/postgresql/citext-patch-submitted.html

CREATE OR REPLACE FUNCTION is_timezone( tz TEXT ) RETURNS BOOLEAN as $$
BEGIN
  PERFORM now() AT TIME ZONE tz;
  RETURN TRUE;
EXCEPTION WHEN invalid_parameter_value THEN
  RETURN FALSE;
END;
$$ language plpgsql STABLE;

CREATE DOMAIN timezone AS CITEXT
CHECK ( is_timezone( value ) );

It could also be TEXT I suppose, but "America/Los_Angeles" and 
"america/los_angeles" should be considered the same.

Best,

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

Re: [HACKERS] cvs chapters in our docs

[Bruce Momjian]

Tom Lane wrote:
> Bruce Momjian  writes:
> > Robert Haas wrote:
> >> I have to say I'm not really impressed by the idea of removing things
> >> from our documentation and replacing them with pages on the wiki.  The
> >> documentation is better-written and easier to navigate.  Yeah, the
> >> part about 28K modems is pretty silly, but we can fix that without
> >> throwing the baby out with the bathwater...
> 
> > Wow, we mention 28k modems --- we are legacy software:  ;-)
> 
> The depressing thing is we can't even blame that on Berkeley ...
> if memory serves, I wrote it :-(

There is no mention of paper tape or punch cards in our docs.  :-)

-- 
  Bruce Momjian  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] Application name patch - v4

[Tom Lane]

Dave Page  writes:
> On Sat, Nov 28, 2009 at 11:47 PM, Tom Lane  wrote:
>> 1. The patch prevents non-superusers from seeing other users'
>> application names in pg_stat_activity.  This seems at best pretty
>> debatable to me.  Yes, it supports usages in which you want to put
>> security-sensitive information into the appname, but at the cost of
>> disabling (perfectly reasonable) usages where you don't.  If we made
>> the app name universally visible, people simply wouldn't put security
>> sensitive info in it, the same as they don't put it on the command line.
>> Should we change this?

> Uh, yeah, I guess. That wasn't a concious decision, more a copy n
> paste inherited 'feature'.

OK.  Everybody seems to agree it should not be hidden, so I'll go change
that.

>> 2. I am wondering if we should mark application_name as
>> GUC_NO_RESET_ALL.

> I think we should use GUC_NO_RESET_ALL.

I agree with you, but it seems we have at least as many votes to not do
that.  Any other votes out there?

regards, tom lane

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

Re: [HACKERS] LDAP where DN does not include UID attribute

[Magnus Hagander]

On Sun, Nov 29, 2009 at 13:05, Magnus Hagander  wrote:
> On Fri, Sep 18, 2009 at 02:24, Robert Fleming  wrote:
>> On Thu, Sep 17, 2009 at 11:15 AM, Magnus Hagander 
>> wrote:
>>>
>>> On Thu, Sep 17, 2009 at 18:02, Robert Fleming  wrote:
>>> > Following a discussion on the pgsql-admin list
>>> > , I
>>> > have
>>> > created a patch to (optionally) allow PostgreSQL to do a LDAP search to
>>> > determine the user's DN (as is done in Apache, MediaWiki, Bugzilla, et
>>> > al.)
>>> > instead of building the DN from a prefix and suffix.
>>> > This is necessary for schemas where the login attribute is not in the
>>> > DN,
>>> > such as is described here
>>> >  (look
>>> > for
>>> > "name-based").  This patch is against PostgreSQL 8.4.0 from Debian
>>> > Lenny-backports.  If this would be a welcome addition, I can port it
>>> > forward
>>> > to the latest from postgresql.org.
>>> > Thanks in advance for your feedback.
>>>
>>> This sounds like a very useful feature, and one that I can then remove
>>> from my personal TODO list without having to do much work :-)
>>>
>>> A couple of comments:
>>>
>>> First of all, please read up on the PostgreSQL coding style, or at
>>> least look at the code around yours. This doesn't look anything like
>>> our standards.
>>
>> Sorry, the 8.1 manual said all contributions go through pgindent so I didn't
>> spend too much time on that.  I see that the 8.4 manual clarifies that
>> pgindent won't get run till release time.  In any case, I've re-formatted
>> the patch using the Emacs settings from the 8.1 manual (why are they gone
>> from the 8.4 manual)?  It seems like most of the rest of the Coding
>> Conventions have to do with error reporting.  Do please let me know if
>> there's something else I can do.
>>>
>>> Second, this appears to require an anonymous bind to the directory,
>>> which is something we should not encourage people to enable on their
>>> LDAP servers. I think we need to also take parameters with a DN and a
>>> password to bind with in order to do the search, and then re-bind as
>>> the user when found.
>>
>> The new patch implements the initial bind using new configuration parameters
>> "ldapbinddn" and "ldapbindpasswd".  I've also added a "ldapsearchattribute"
>> just to be complete.
>
> I've finally had the time to look at this patch some more, for the
> current commitfest - sorry about the delay.
>
> Other than minor style changes (make the indentation be the same as
> the code around it), I noticed one fairly large issue.
>
> The way that LDAP is re-initialized both breaks Windows (in the error
> reporting part) and not as obvious but even more importantly, it
> breaks TLS. If you enable TLS for LDAP it will only be used for the
> search, not for the actual password check - which really removes the
> point of it :-)
>
> I assume we need the second ldap_init() because we want to do a new
> ldap_simple_bind()? In that case, we realliy need to re-initialize the
> whole connection, including TLS.
>
> I also notice that it's hardcoded to retrieve the "uid" attribute,
> while the one being searched for can be configured. ISTM it's better
> to set both of these to the hba->ldapsearchattribute value - so we
> won't fail if "uid" does not exist.
>
> Also, as I'm sure you're aware there are no documentation updates included.
>
> I'll be happy to work on this to get it ready for commit, or do you
> want to do the updates?

Here's a patch with my work so far. Major points missing is the
sanitizing of the username and the docs.

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


ldap_search.patch
Description: Binary data

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

Re: [HACKERS] Application name patch - v4

[Dave Page]

On Sat, Nov 28, 2009 at 11:47 PM, Tom Lane  wrote:
> Dave Page  writes:
>> Updated application name patch, including a GUC assign hook to clean
>> the application name of any unsafe characters, per discussion.
>
> Applied with assorted editorialization.  There were a couple of
> definitional issues that I don't recall if we had consensus on:
>
> 1. The patch prevents non-superusers from seeing other users'
> application names in pg_stat_activity.  This seems at best pretty
> debatable to me.  Yes, it supports usages in which you want to put
> security-sensitive information into the appname, but at the cost of
> disabling (perfectly reasonable) usages where you don't.  If we made
> the app name universally visible, people simply wouldn't put security
> sensitive info in it, the same as they don't put it on the command line.
> Should we change this?

Uh, yeah, I guess. That wasn't a concious decision, more a copy n
paste inherited 'feature'.

> (While I'm looking at it, I wonder why client_addr and client_port
> are similarly hidden.)
>
> 2. I am wondering if we should mark application_name as
> GUC_NO_RESET_ALL.  As-is, the value sent at libpq initialization
> will be lost during RESET ALL, which would probably surprise people.
> On the other hand, not resetting it might surprise other people.
> If we were able to send it in the startup packet then this wouldn't
> be a problem, but we are far from being able to do that.

In the use cases I envisage for this, the appname is more a property
of the connection than the session, thus I wouldn't expect it to
change following a RESET ALL. That said, one could then argue that it
should RESET to the connection-time value...

I think we should use GUC_NO_RESET_ALL.

-- 
Dave Page
EnterpriseDB UK: 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] cvs chapters in our docs

[Tom Lane]

Bruce Momjian  writes:
> Robert Haas wrote:
>> I have to say I'm not really impressed by the idea of removing things
>> from our documentation and replacing them with pages on the wiki.  The
>> documentation is better-written and easier to navigate.  Yeah, the
>> part about 28K modems is pretty silly, but we can fix that without
>> throwing the baby out with the bathwater...

> Wow, we mention 28k modems --- we are legacy software:  ;-)

The depressing thing is we can't even blame that on Berkeley ...
if memory serves, I wrote it :-(

regards, tom lane

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

Re: [HACKERS] compile error with -DOPTIMIZER_DEBUG

[Tom Lane]

Bruce Momjian  writes:
> Tom Lane wrote:
>> You know, the last couple of times I've touched that code, I've been
>> wondering why we bother to maintain it.  Personally I always use pprint()
>> when I'm interested in a printout of a plan tree.  Is anyone actually
>> using the printout code in allpaths.c?

> I thought OPTIMIZER_DEBUG showed us all the possible paths, not just the
> final plan.

Yeah, but we could repoint that code at pprint.

regards, tom lane

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

[HACKERS] A small issue in CommentCast function

[Gokulakannan Somasundaram]

Hi,
   I think this is more of code uniformity issue. Nevertheless, i just
thought of putting it here.
   In the CommentCast function, i think both sourcetype and targettype
should be in the arguments, whereas in the code, source type is made as the
name and the target type is passed in as argument. So both of them should
get stored in the objargs member of the CommentStmt struct.

Thanks,
Gokul.

Re: [HACKERS] Timezones (in 8.5?)

[Bruce Momjian]

Andrew Gierth wrote:
> > "Bruce" == Bruce Momjian  writes:
> 
>  Bruce> I think there is general agreement that we should have a
>  Bruce> timezone data type which validates against
>  Bruce> pg_timezone_names().name.
> 
> What happens when pg_timezone_names output changes? (which it can do,
> especially if the install is using the OS tzdata; even if not using OS
> tzdata, it's not expected to be stable even between point releases)

Uh, wow, yea, that would invalidate stored data --- yuck.

-- 
  Bruce Momjian  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] cvs chapters in our docs

[Bruce Momjian]

Robert Haas wrote:
> I have to say I'm not really impressed by the idea of removing things
> from our documentation and replacing them with pages on the wiki.  The
> documentation is better-written and easier to navigate.  Yeah, the
> part about 28K modems is pretty silly, but we can fix that without
> throwing the baby out with the bathwater...

Wow, we mention 28k modems --- we are legacy software:  ;-)

 This initial checkout is a little slower than simply downloading
 a tar.gz file; expect it to take 40 minutes
 or so if you have a 28.8K modem.

-- 
  Bruce Momjian  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] LDAP where DN does not include UID attribute

[Magnus Hagander]

On Fri, Sep 18, 2009 at 02:24, Robert Fleming  wrote:
> On Thu, Sep 17, 2009 at 11:15 AM, Magnus Hagander 
> wrote:
>>
>> On Thu, Sep 17, 2009 at 18:02, Robert Fleming  wrote:
>> > Following a discussion on the pgsql-admin list
>> > , I
>> > have
>> > created a patch to (optionally) allow PostgreSQL to do a LDAP search to
>> > determine the user's DN (as is done in Apache, MediaWiki, Bugzilla, et
>> > al.)
>> > instead of building the DN from a prefix and suffix.
>> > This is necessary for schemas where the login attribute is not in the
>> > DN,
>> > such as is described here
>> >  (look
>> > for
>> > "name-based").  This patch is against PostgreSQL 8.4.0 from Debian
>> > Lenny-backports.  If this would be a welcome addition, I can port it
>> > forward
>> > to the latest from postgresql.org.
>> > Thanks in advance for your feedback.
>>
>> This sounds like a very useful feature, and one that I can then remove
>> from my personal TODO list without having to do much work :-)
>>
>> A couple of comments:
>>
>> First of all, please read up on the PostgreSQL coding style, or at
>> least look at the code around yours. This doesn't look anything like
>> our standards.
>
> Sorry, the 8.1 manual said all contributions go through pgindent so I didn't
> spend too much time on that.  I see that the 8.4 manual clarifies that
> pgindent won't get run till release time.  In any case, I've re-formatted
> the patch using the Emacs settings from the 8.1 manual (why are they gone
> from the 8.4 manual)?  It seems like most of the rest of the Coding
> Conventions have to do with error reporting.  Do please let me know if
> there's something else I can do.
>>
>> Second, this appears to require an anonymous bind to the directory,
>> which is something we should not encourage people to enable on their
>> LDAP servers. I think we need to also take parameters with a DN and a
>> password to bind with in order to do the search, and then re-bind as
>> the user when found.
>
> The new patch implements the initial bind using new configuration parameters
> "ldapbinddn" and "ldapbindpasswd".  I've also added a "ldapsearchattribute"
> just to be complete.

I've finally had the time to look at this patch some more, for the
current commitfest - sorry about the delay.

Other than minor style changes (make the indentation be the same as
the code around it), I noticed one fairly large issue.

The way that LDAP is re-initialized both breaks Windows (in the error
reporting part) and not as obvious but even more importantly, it
breaks TLS. If you enable TLS for LDAP it will only be used for the
search, not for the actual password check - which really removes the
point of it :-)

I assume we need the second ldap_init() because we want to do a new
ldap_simple_bind()? In that case, we realliy need to re-initialize the
whole connection, including TLS.

I also notice that it's hardcoded to retrieve the "uid" attribute,
while the one being searched for can be configured. ISTM it's better
to set both of these to the hba->ldapsearchattribute value - so we
won't fail if "uid" does not exist.

Also, as I'm sure you're aware there are no documentation updates included.

I'll be happy to work on this to get it ready for commit, or do you
want to do the updates?

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

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

Re: [HACKERS] compile error with -DOPTIMIZER_DEBUG

[Bruce Momjian]

Tom Lane wrote:
> Heikki Linnakangas  writes:
> > Jan Urba??ski wrote:
> >> ISTM that there's a superfluous curly brace in print_path (which only
> >> gets compiled with -DOPTIMIZER_DEBUG.
> 
> > Thanks, committed.
> 
> You know, the last couple of times I've touched that code, I've been
> wondering why we bother to maintain it.  Personally I always use pprint()
> when I'm interested in a printout of a plan tree.  Is anyone actually
> using the printout code in allpaths.c?

I thought OPTIMIZER_DEBUG showed us all the possible paths, not just the
final plan.

-- 
  Bruce Momjian  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] Timezones (in 8.5?)

[Andrew Gierth]

> "Bruce" == Bruce Momjian  writes:

 Bruce> I think there is general agreement that we should have a
 Bruce> timezone data type which validates against
 Bruce> pg_timezone_names().name.

What happens when pg_timezone_names output changes? (which it can do,
especially if the install is using the OS tzdata; even if not using OS
tzdata, it's not expected to be stable even between point releases)

-- 
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] plperl and inline functions -- first draft

[Alexey Klyukin]

On Nov 29, 2009, at 4:40 AM, Tom Lane wrote:

> Alexey Klyukin  writes:
> 
>> Isn't it also the case with the existing plperl code ? I've noticed that 
>> free(prodesc) is called when it's no longer used (i.e. in 
>> plperl_compile_callback:1636), but refcount of desc->reference is never 
>> decremented.
> 
> I've been experimenting with this and confirmed that there is a leak;
> not only in the DO patch but in the pre-existing code, if a plperl
> function is redefined repeatedly.
> 
> Is this the correct way to release the SV* reference?
> 
>   if (reference)
>   SvREFCNT_dec(reference);


Yes. In fact this only decreases the reference count, making the interpreter 
free the memory referred to when it becomes 0, but since prodesc->reference has 
refcount of 1 this would do the right thing.

--
Alexey Klyukin  http://www.CommandPrompt.com/
The PostgreSQL Company - Command Prompt, Inc


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