Re: [HACKERS] Issues with Quorum Commit

[Robert Haas]

On Wed, Oct 13, 2010 at 2:43 AM, Heikki Linnakangas
 wrote:
> On 13.10.2010 08:21, Fujii Masao wrote:
>>
>> On Sat, Oct 9, 2010 at 4:31 AM, Heikki Linnakangas
>>   wrote:
>>>
>>> It shouldn't be too hard to fix. Walsender needs to be able to read WAL
>>> from
>>> preceding timelines, like recovery does, and walreceiver needs to write
>>> the
>>> incoming WAL to the right file.
>>
>> And walsender seems to need to transfer the current timeline history to
>> the standby. Otherwise, the standby cannot recover the WAL file with new
>> timeline. And the standby might need to create the timeline history file
>> in order to recover the WAL file with new timeline even after it's
>> restarted.
>
> Yes, true, you need that too.
>
> It might be good to divide this work into two phases, teaching archive
> recovery to notice new timelines appearing in the archive first, and doing
> the walsender/walreceiver changes after that.

There's another problem here we should think about, too.  Suppose you
have a master and two standbys.  The master dies.  You promote one of
the standbys, which turns out to be behind the other.  You then
repoint the other standby at the one you promoted.  Congratulations,
your database is now very possible corrupt, and you may very well get
no warning of that fact.  It seems to me that we would be well-advised
to install some kind of bullet-proof safeguard against this kind of
problem, so that you will KNOW that the standby needs to be re-synced.
 I mention this because I have a vague feeling that timelines are
supposed to prevent you from getting different WAL histories confused
with each other, but they don't actually cover all the cases that can
happen.

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

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

Re: [HACKERS] Issues with Quorum Commit

[Heikki Linnakangas]

On 13.10.2010 08:21, Fujii Masao wrote:

On Sat, Oct 9, 2010 at 4:31 AM, Heikki Linnakangas
  wrote:

It shouldn't be too hard to fix. Walsender needs to be able to read WAL from
preceding timelines, like recovery does, and walreceiver needs to write the
incoming WAL to the right file.


And walsender seems to need to transfer the current timeline history to
the standby. Otherwise, the standby cannot recover the WAL file with new
timeline. And the standby might need to create the timeline history file
in order to recover the WAL file with new timeline even after it's restarted.


Yes, true, you need that too.

It might be good to divide this work into two phases, teaching archive 
recovery to notice new timelines appearing in the archive first, and 
doing the walsender/walreceiver changes after 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

Re: [HACKERS] leaky views, yet again

[Robert Haas]

2010/10/12 KaiGai Kohei :
> I noticed the previous patch has an obvious conflict to the latest
> git mater, and it does not have any documentation updates.
>
> So, I rebased the patch, and added descriptions about secure views.
> Rest of parts are unchanged.

It seems that we have rough agreement that the existing VIEW feature
is adequate for column filtering but not for row filtering.  The
attack vector is that the planner might reorder quals such that a
value not intended to be visible to the user is passed to a function
which has a side-effect that can expose the value passed to it: either
overtly (as by a user-defined function that writes it to the table or
prints it using RAISE NOTICE) or in some more subtle way (as in the
case where division by zero exposes throws an exception when passed
zero, but not some other value).  With the possible exception of Tom,
everyone seems to agree that it would be a good step forward to
provide a way of plugging these holes, even if it didn't cover subtler
information leaks such as by reading the EXPLAIN output or timing
query execution.

1. Does anyone wish to argue (or continue arguing) that plugging these
more overt information leaks is not worthwhile?

2. Supposing that the answer to question #1 is in the negative, does
anyone wish to argue that this patch as currently written is an
adequate solution to this problem?  It seems obvious to me that it
isn't.

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

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

Re: [HACKERS] security hook on authorization

[Robert Haas]

2010/8/24 KaiGai Kohei :
> I tried to revise the patch. It allows plugins to get control next to
> client authentication, but before returning the status to users.
>
> This change enables plugins which should be invoked on authentication
> failed to utilize this hook, not only assignment of session security
> label.
> At the same time, it disables to hook on SET SESSION AUTHORIZATION.
> But it is a bit unclear whether we should hook here, or not.

Stephen -

You've been listed as a reviewer for this in the CF app since 9/17 -
are you planning to review it?

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

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

Re: [HACKERS] WIP: extensible enums

[Robert Haas]

On Fri, Oct 1, 2010 at 7:12 AM, Andrew Dunstan  wrote:
> On 10/01/2010 04:35 AM, Dean Rasheed wrote:
>>
>> 2). In enum_ccmp(), when you cache the full list of elements, you're
>> not updating mycache->sort_list_length, so it will keep fetching the
>> full list each time. Also, I think that function could use a few more
>> comments.
>
> Good catch. Will fix.
>
>> 3). I think you need to update psql so that \dT+ returns the enum
>> elements in the right order.
>
> Yeah. Will do.
>
> I will post a revised patch soon.

Should we postpone this to the next CommitFest?

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

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

Re: [HACKERS] levenshtein_less_equal (was: multibyte charater set in levenshtein function)

[Robert Haas]

On Fri, Oct 8, 2010 at 12:51 PM, Alexander Korotkov
 wrote:
> Sorry, I'm pretty unconversant in git. Now, it should be ok.

I spent some time hacking on this.  It doesn't appear to be too easy
to get levenshtein_less_equal() working without slowing down plain old
levenshtein() by about 6%.  The slowdown was similar on the 0.2
version of the patch, the 0.3.1 version of the patch, and another
version I put together that's a bit different from either.  The test
query was:

SELECT * FROM words WHERE levenshtein(a, 'extensize') <= 2;

In the version I was experimenting with, I had a large chunk of code
that looked like this:

if (max_d >= 0)
{
  /* Do stuff */
}

Removing that block of code buys back most of the performance loss
that occurs in the case where max_d < 0.  I have to conclude that
sticking more stuff into the main loop defeats some optimization
opportunities that would otherwise be available, even in the case
where the branch isn't taken.

So I'm tempted to employ the previously-discussed sledgehammer of
compiling levenshtein_internal twice, with and without support for
max_d.  A patch taking this approach is attached.  I'm not totally
sold on this approach, if someone has a constructive suggestion.
Comments?

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


levenshtein_less_equal-0.4.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] Bug / shortcoming in has_*_privilege

[Itagaki Takahiro]

>>  wrote:
>> > Excerpts from Jim Nasby's message of jue jun 10 17:54:43 -0400 2010:
>> >> test...@workbook=# select has_table_privilege( 'public', 'test', 'SELECT' 
>> >> );
>> >> ERROR:  role "public" does not exist
>> >
>> > Here's a patch implementing this idea.

I applied it almost as-is, except an unused variable in
get_role_oid_or_public().

>> BTW, does the patch need to be back-patched to older versions?
> There's no intention to do so.

OK. Applied only to HEAD. The issue was reported as a bug,
but we will consider the change as an improvement.

-- 
Itagaki Takahiro

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

Re: [HACKERS] Issues with Quorum Commit

[Fujii Masao]

On Sat, Oct 9, 2010 at 4:31 AM, Heikki Linnakangas
 wrote:
>> Yes. But if there is no unsent WAL when the master goes down,
>> we can start new standby without new backup by copying the
>> timeline history file from new master to new standby and
>> setting recovery_target_timeline to 'latest'.
>
> .. and restart the standby.

Yes.

> It's a pretty severe shortcoming at the moment. For starters, it means that
> you need a shared archive, even if you set wal_keep_segments to a high
> number. Secondly, it's a lot of scripting to get it working, I don't like
> the thought of testing failovers in synchronous replication if I have to do
> all that. Frankly, this seems more important to me than synchronous
> replication.

There seems to be difference in outlook between us. I prefer sync rep.
But I'm OK to address that first if it's not hard.

> It shouldn't be too hard to fix. Walsender needs to be able to read WAL from
> preceding timelines, like recovery does, and walreceiver needs to write the
> incoming WAL to the right file.

And walsender seems to need to transfer the current timeline history to
the standby. Otherwise, the standby cannot recover the WAL file with new
timeline. And the standby might need to create the timeline history file
in order to recover the WAL file with new timeline even after it's restarted.

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] Issues with Quorum Commit

[Fujii Masao]

On Sat, Oct 9, 2010 at 1:41 AM, Josh Berkus  wrote:
>
>> And, I'd like to know whether the master waits forever because of the
>> standby failure in other solutions such as Oracle DataGuard, MySQL
>> semi-synchronous replication.
>
> MySQL used to be fond of simiply failing sliently.  Not sure what 5.4 does,
> or Oracle.  In any case MySQL's replication has always really been async
> (except Cluster, which is a very different database), so it's not really a
> comparison.

IIRC, MySQL *semi-synchronous* replication is not async, so it can be
comparison. Of course, though MySQL default replication is async.

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] Issues with Quorum Commit

[Fujii Masao]

On Sat, Oct 9, 2010 at 12:12 AM, Markus Wanner  wrote:
> On 10/08/2010 04:48 PM, Fujii Masao wrote:
>> I believe many systems require write-availability.
>
> Sure. Make sure you have enough standbies to fail over to.

Unfortunately even enough standbys don't increase write-availability
unless you choose wait-forever. Because, after promoting one of
standbys to new master, you must keep all the transactions waiting
until at least one standby has connected to and caught up with new
master. Currently this wait time is not short.

> (I think there are even more situations where read-availability is much
> more important, though).

Even so, we should not ignore the write-availability aspect.

>>> Start with 0 (i.e. replication off), then add standbies, then increase
>>> quorum_commit to your new requirements.
>>
>> No. This only makes the procedure of failover more complex.
>
> Huh? This doesn't affect fail-over at all. Quite the opposite, the
> guarantees and requirements remain the same even after a fail-over.

Hmm.. that increases the number of procedures which the users must
perform at the failover. At least, the users seem to have to wait
until the standby has caught up with new master, increase quorum_commit
and then reload the configuration file.

>> What is a full-cluster crash?
>
> The event that all of your cluster nodes are down (most probably due to
> power failure, but fires or other catastrophic events can be other
> causes). Chances for that to happen can certainly be reduced by
> distributing to distant locations, but that equally certainly increases
> latency, which isn't always an option.

Yep.

>> Why does it cause a split-brain?
>
> First master node A fails, a standby B takes over, but then fails as
> well. Let node C take over. Then the power aggregates catches fire, the
> infamous full-cluster crash (where "lights out management" gets a
> completely new meaning ;-) ).
>
> Split brain would be the situation that arises if all three nodes (A, B
> and C) start up again and think they have been the former master, so
> they can now continue to apply new transactions. Their data diverges,
> leading to what could be seen as a split-brain from the outside.
>
> Obviously, you must disallow A and B to take the role of the master
> after recovery.

Yep. Something like STONITH would be required.

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] security label support, revised

[Bruce Momjian]

Robert Haas wrote:
> On Tue, Sep 28, 2010 at 3:57 AM, Shigeru HANADA
>  wrote:
> > On Mon, 27 Sep 2010 21:07:33 -0400
> > Robert Haas  wrote:
> >> I found and fixed a few more issues and committed this. ?The pg_dump
> >> support had a few escaping bugs, and I added tab completion support
> >> for psql. ?Considering the size of the patch, it seems likely that
> >> there are some issues we both overlooked, but this is as solid as I
> >> can make it for right now.
> > Some OIDs used in SECURITY LABEL patch have already been used for
> > some functions such as pg_stat_get_xact_numscans().
> >
> > The src/include/catalog/duplicate_oids script reports that 3037 ~
> > 3040 are used two or more times.
> >
> > Though all regression tests finish successfully, should this be
> > fixed ?
> 
> Woops.  Thanks for the report, fixed.  I wish we had a regression test
> that would catch these mistakes.  It's easy to forget to run this
> script.

Attached it the script I use for checks that eventually calls
src/tools/pgtest.

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

  + It's impossible for everything to be true. +
#!/bin/bash

. traprm

[ "$1" = "-q" ] && QUIET="Y"

if [ ! -f COPYRIGHT ]
thencd /pgtop || exit 1
fi

chown -R postgres .

# skip for <= PG 8.1, SGML has tabs
if ! grep 'AC_INIT(\[PostgreSQL\]' configure.in | egrep -q '\[(6\.|7\.|8\.[01])'
then
echo "Checking SGML"
cd doc/src/sgml
gmake check > $TMP/0 2>&1  
if grep -q 'Error' < $TMP/0
thenecho "SGML error"
cat $TMP/0
exit 1
fi

gmake check-tabs

# Run only at night to check for HISTORY build problems
# in HISTORY.html.
if [ ! -t 0 ]
thengmake HISTORY.html > $TMP/0 2>&1
if grep -q 'Error' < $TMP/0
thenecho "SGML error"
cat $TMP/0
exit 1
fi
fi

# fails on /bin/sh
cd -
fi

echo "Checking duplicate oids"
cd src/include/catalog
duplicate_oids > $TMP/0
if [ -s $TMP/0 ]
thenecho "Duplicate system oids"
cat $TMP/0
exit 1
fi
cd -

# supress assembler warning
(aspg /pg/tools/pgtest "$@"; echo "$?" > $TMP/ret) |
# use only one grep so we don't buffer output
egrep -v ': Warning: using `%|^SPI.c:.*: warning: |^ppport.h:[0-9][0-9]*: 
warning: |^/usr/libdata/perl5/5.00503/i386-bsdos/CORE/patchlevel.h|plperl.c:.*: 
warning: (implicit|passing)|variable .fast. might be clobbered|warning: unused 
variable .yyg.'

rm -fr src/test/regress/tmp_check

[ ! "$QUIET" ] && bell

exit `cat $TMP/ret`


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

Re: [HACKERS] Bug / shortcoming in has_*_privilege

[Alvaro Herrera]

Excerpts from Itagaki Takahiro's message of mar oct 12 23:05:36 -0300 2010:
> Hi,
> 
> On Tue, Sep 7, 2010 at 6:16 AM, Alvaro Herrera
>  wrote:
> > Excerpts from Jim Nasby's message of jue jun 10 17:54:43 -0400 2010:
> >> test...@workbook=# select has_table_privilege( 'public', 'test', 'SELECT' 
> >> );
> >> ERROR:  role "public" does not exist
> >
> > Here's a patch implementing this idea.
> 
> It specially treats only "public" in all lower cases, right?
> The pseudo-role name is described as "PUBLIC" (upper) in docs,
> but we accept only "public" (lower) as the pseudo-name.

Yeah, only lowercase.  Identifiers other than "public" (all lowercase)
are allowed as role names, so we cannot use them for this purpose.  Keep
in mind that the docs say PUBLIC without the quotes, which is lowercased.

> BTW, does the patch need to be back-patched to older versions?

There's no intention to do so.

-- 
Álvaro Herrera 
The PostgreSQL Company - Command Prompt, Inc.
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] Re: [GENERAL] Text search parser's treatment of URLs and emails

[Sushant Sinha]

On Tue, 2010-10-12 at 19:31 -0400, Tom Lane wrote:
> This seems much of a piece with the existing proposal to allow
> individual "words" of a URL to be reported separately:
> https://commitfest.postgresql.org/action/patch_view?id=378
> 
> As I said in that thread, this could be done in a backwards-compatible
> way using the tsearch parser's existing ability to report multiple
> overlapping tokens out of the same piece of text.  But I'd like to see
> one unified proposal and patch for this and Sushant's patch, not
> independent hacks changing the behavior in the same area.
> 
>   regards, tom lane
What Tom has suggested will require me to look into a different piece of
code and so this will take some time before I can update the patch.

-Sushant.



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

Re: [HACKERS] Bug / shortcoming in has_*_privilege

[Robert Haas]

On Tue, Oct 12, 2010 at 10:05 PM, Itagaki Takahiro
 wrote:
> BTW, does the patch need to be back-patched to older versions?
> Since they use get_roleid_checked() instead of get_role_oid(), the fix
> cannot be applied cleanly to them, though it will be similar codes.

I would interpret this a a feature, not a bug fix, so no back-patch.

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

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

Re: [HACKERS] Bug / shortcoming in has_*_privilege

[Itagaki Takahiro]

Hi,

On Tue, Sep 7, 2010 at 6:16 AM, Alvaro Herrera
 wrote:
> Excerpts from Jim Nasby's message of jue jun 10 17:54:43 -0400 2010:
>> test...@workbook=# select has_table_privilege( 'public', 'test', 'SELECT' );
>> ERROR:  role "public" does not exist
>
> Here's a patch implementing this idea.

It specially treats only "public" in all lower cases, right?
The pseudo-role name is described as "PUBLIC" (upper) in docs,
but we accept only "public" (lower) as the pseudo-name.

BTW, does the patch need to be back-patched to older versions?
Since they use get_roleid_checked() instead of get_role_oid(), the fix
cannot be applied cleanly to them, though it will be similar codes.

-- 
Itagaki Takahiro

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

Re: [HACKERS] security hook on table creation

[KaiGai Kohei]

(2010/10/12 23:09), Robert Haas wrote:
> On Tue, Oct 12, 2010 at 9:20 AM, KaiGai Kohei  wrote:
>> As I introduced before, the idea of two hooks makes obvious where
>> we should put the security hooks; it is next to the existing DAC
>> checking. It is the best guideline, even if we will touch the code
>> around object creation in the future version.
>>
>> If the creation-hook would be put on the place far from existing
>> DAC checks, what provides us a guideline to deploy security hooks?
>> It seems to me the idea of only post-creation hook try to lose
>> this kind of benefit instead of half dozen of exceptions.
>>
>> I think MVCC visibility is just an actualization of the matters.
>> The point is that we can be released from the task to consider
>> where is the right place for security hooks, as long as it will
>> be placed next to the existing DAC checks.
>> It seems to me its simplicity of design is unignorable benefit.
> 
> In either design, you have to decide where to put the post-creation
> hook.  In your design, you ALSO need to decide where to put the
> pre-creation hook.  Deciding where to put the pre-creation hook may be
> simple, but it is not as simple as not having it at all.
> 
If the post-creation hook have two tasks (access control and fix-up
security labels) concurrently, things we need to consider and assess
is not equal to the case when we only fix-up security labels on the
post-creation hooks.
MVCC visibility is a typical example. Elsewhere, we need to check up
various things (such as completeness of the hook coverage, side-effects
of CommandCounterIncrement(), ...) without reliable guidelines.

I'm saying we can go through an easy way, as long as we design these
hooks according to a simple principle based on the existing features.

* pre-creation hooks (for access control) shall be put next to the
  existing DAC checks.
* post-creation hooks (for fix-up security label) shall be put next
  to the simple_heap_*(). Because OID and labels to be assigned are
  already given, we don't need to consider such a complex things.

Even if we need to decide the place of two hooks, it seems to me
still simpler than security hooks from the scratch.

> A possibly legitimate reason to have a pre-creation hook is to prevent
> users from consuming more excessive system resources by repeatedly
> performing operations that SE-Linux will end up denying, but only
> after they're basically complete.
> 
We had similar discussion before when I tried to work on something
related to table-inheritance.
MergeAttributes() checks ownership of the parent table appeared in
the INHERITS() clause, then it immediately raises an error even if
one of them was not owned by the current user, because it allows
users to prevent accesses unprivileged tables, if we check these
ownership at once later.

I learned existing privilege-checks are located with their reasons.
So, it seems to me a good strategy to follow on existing design.

Thanks,
-- 
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] SQL command to edit postgresql.conf, with comments

[Darren Duncan]

Richard Broersma wrote:

On Tue, Oct 12, 2010 at 3:54 PM, Josh Berkus  wrote:

Oh.  Now that's an interesting perspective ... you're suggesting that we
take the comments and apply them as COMMENTS on the specific pg_settings?


On a side note regarding comments, I'd like to make a request for a
more comprehensive commenting mechanism.  The first though that comes
to my mind would allow for comments to be stored and annotated using
XML or sgml.  It'd be nice to be able to generate user documentation
from selected comments taken from application derived database
objects.

I don't know, maybe this is already possible.


When you start going there, you have new issues to consider.  (For the record I 
also prefer plain text for comments.)


I should point out that the group making the Perl 6 language has already been 
looking into such matters extensively of essentially unifying code comments and 
code documentation into a common metadata both accessible in the source code and 
programmatically at runtime.


I think this approach goes beyond comments as we normally know them, which I 
just think of plain text strings associated with some code element.


But if you want to pursue bringing documentation into this, I suggest looking at 
what Perl 6, and other languages, have done.


While some of the results of the Perl 6 discussion may have just been in the 
forums, these urls at least are related to it:


 - http://perlcabal.org/syn/S02.html#Whitespace_and_Comments
 - http://github.com/perl6/specs/raw/master/S26-documentation.pod for

I'm not proposing adopting their syntax, but some features or design may be 
useful to learn from.


I also want to point out that the FoxPro language constitutes some prior art 
about including comments as data fields in their runtime-accessible objects.


-- Darren Duncan

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

Re: [HACKERS] SQL command to edit postgresql.conf, with comments

[Darren Duncan]

Josh Berkus wrote first:

Postgres has "COMMENT ON ..." SQL for various database objects and I
assumed that said comments would be stored in the system catalog.


Oh.  Now that's an interesting perspective ... you're suggesting that we
take the comments and apply them as COMMENTS on the specific pg_settings?


Yes, something along those lines.

I should point out that in general I consider a "COMMENT ON" to just be a less 
ambiguous form of a "-- " or "/* */" and that it would be a good idea for a code 
or config parser to preserve comments given in the latter formats in the same 
way it preserves the former when it can reasonably infer what to associate the 
latter comments with.



That wouldn't solve the issues of ordering, or of comments in the file
not associated with a setting, but might be a good 80% solution.


Yes, and for something like this an 80% solution is good.

As for ordering, that can be handled with more metadata, like a "visual order" 
number.


Tom Lane wrote second:

Well, if the settings were stored in a catalog ... which they are not
... that might be a useful idea.


The catalog is where they *should* be stored, at least from the user's 
perspective.


The reason that COMMENT ON isn't a terribly helpful analogy is that it
puts the burden on the user to associate specific comment texts with
specific database objects.  The problem with parsing and modifying
freestyle comments as seen in postgresql.conf is exactly (or mostly)
that there's no reasonable way of mechanically determining which
comments are associated with which setting.  We could invent some
arbitrary rule or other, but it'd have little to do with the way
people perceive what's in such a file.


There's no reason that you can't have both kinds of comments.  You could have 
comments that are specially formatted such that the parser will then recognize 
they are associated with something specific and so put them in the system 
catalog.  And then you can have other comments not formatted that way and the 
parser will then pass over them like whitespace.



I agree with Josh's proposal: keep mechanically-generated settings in a
separate file, and don't even pretend to allow comments to be kept there.


I agree with the separate files approach as being the most practical in at least 
the short term.  However, I think that this file could support comments too, but 
they would just be limited to end-of-line comments on the same line as the 
setting, and it would be only these comments appearing in the system catalog by 
default.  Comments in the file for user editing would only appear in the catalog 
if specially formatted, which for now could just mean taking end-of-line comments.


-- Darren Duncan

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

Re: [HACKERS] Review: Fix snapshot taking inconsistencies

[Tom Lane]

Marko Tiikkaja  writes:
> On 2010-10-13 2:10 AM +0300, Tom Lane wrote:
>> BTW, this patch seems to be also the time to remove the AtStart_Cache()
>> call in CommandCounterIncrement, as foreseen in the comment there.

> Frankly, I have no idea what to do about this.

Just delete the call.  The only reason I didn't remove it in 2007 is
I was afraid to risk changing things in late beta; but that's not the
situation now.

regards, tom lane

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

Re: [HACKERS] SQL command to edit postgresql.conf, with comments

[Tom Lane]

Josh Berkus  writes:
>> Postgres has "COMMENT ON ..." SQL for various database objects and I
>> assumed that said comments would be stored in the system catalog.

> Oh.  Now that's an interesting perspective ... you're suggesting that we
> take the comments and apply them as COMMENTS on the specific pg_settings?

Well, if the settings were stored in a catalog ... which they are not
... that might be a useful idea.

The reason that COMMENT ON isn't a terribly helpful analogy is that it
puts the burden on the user to associate specific comment texts with
specific database objects.  The problem with parsing and modifying
freestyle comments as seen in postgresql.conf is exactly (or mostly)
that there's no reasonable way of mechanically determining which
comments are associated with which setting.  We could invent some
arbitrary rule or other, but it'd have little to do with the way
people perceive what's in such a file.

I agree with Josh's proposal: keep mechanically-generated settings in a
separate file, and don't even pretend to allow comments to be kept 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] Review: Fix snapshot taking inconsistencies

[Marko Tiikkaja]

On 2010-10-13 2:10 AM +0300, Tom Lane wrote:

Marko Tiikkaja  writes:

That's actually just my ignorance I forgot to mention.  As I understand
it, our code currently first pushes one snapshot and then does multiple
PushActiveSnapshot (or PushUpdatedSnapshot)/PopActiveSnapshot rounds
before popping the oldest snapshot off the stack (and releasing it).  So
in the patch, I would've had to push the snapshot twice the first time
to avoid it being released.


It looks to me like you've added quite a lot of management overhead that
wasn't there before.  Wouldn't it be better to just not pop the snapshot
till you're done with it?


Yes, you're absolutely right.


It'd be better if the logic was something
along the lines of:


That's exactly what I had in mind, so +1.


The spi.c change also changes the logic; the SPI code currently takes a
new snapshot for every query if the caller doesn't provide a snapshot.


[ squint... ]  Oh.  I see now, but that is horribly ugly and
underdocumented.  The code was previously treating the snapshot argument
as a constant and relying on that constant value to tell it what to do
each time through the loop.  Now you've got it changing the flag and
then changing it back sometime later.  Ick.

I think what you need to do to make this understandable is to move the
snapshot push/pop logic outside the per-command loop, instead of hacking
things around to keep it exactly where it was before.  We may well need
to adjust the API of snapmgr.c to make that sane.


*blushes*

Yeah, that makes a lot more sense.


BTW, this patch seems to be also the time to remove the AtStart_Cache()
call in CommandCounterIncrement, as foreseen in the comment there.


Frankly, I have no idea what to do about this.


Regards,
Marko Tiikkaja

--
Sent 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: [GENERAL] Text search parser's treatment of URLs and emails

[Tom Lane]

Bruce Momjian  writes:
> [ sent to hackers where it belongs ]
> Thom Brown wrote:
>> It could be me being picky, but I don't regard parameters or page
>> fragments as part of the URL path.

> Wow, that is a tough one.  One the one hand, it seems nice to be able to
> split stuff out more, but on the other hand we would be making url_path
> less useful because people would need to piece things together to get
> the old behavior.  In fact to piece things together we would need to add
> '?' and '#' optionally, which seems kind of hard.  Perhaps we should
> keep url_path unchanged and add file_path that has your suggestion. 

This seems much of a piece with the existing proposal to allow
individual "words" of a URL to be reported separately:
https://commitfest.postgresql.org/action/patch_view?id=378

As I said in that thread, this could be done in a backwards-compatible
way using the tsearch parser's existing ability to report multiple
overlapping tokens out of the same piece of text.  But I'd like to see
one unified proposal and patch for this and Sushant's patch, not
independent hacks changing the behavior in the same area.

regards, tom lane

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

Re: [HACKERS] Review: Fix snapshot taking inconsistencies

[Tom Lane]

Marko Tiikkaja  writes:
> On 2010-10-13 1:21 AM +0300, Tom Lane wrote:
>> I started looking at this patch, and I'm wondering why you inserted all
>> the Register/UnregisterSnapshot calls that weren't there before

> That's actually just my ignorance I forgot to mention.  As I understand 
> it, our code currently first pushes one snapshot and then does multiple 
> PushActiveSnapshot (or PushUpdatedSnapshot)/PopActiveSnapshot rounds 
> before popping the oldest snapshot off the stack (and releasing it).  So 
> in the patch, I would've had to push the snapshot twice the first time 
> to avoid it being released.

It looks to me like you've added quite a lot of management overhead that
wasn't there before.  Wouldn't it be better to just not pop the snapshot
till you're done with it?

> Thinking about it now, that might be a better option.  Or maybe we 
> should change the snapshot API to make this more convenient?

Well, I'm not in love with the current snapshot API by any means,
particularly not PushUpdatedSnapshot which seems to be the only
API-sanctioned way to put a new CID into a snapshot without taking
a whole new snapshot.  It'd be better if the logic was something
along the lines of:

* at start of a query, PushActiveSnapshot(GetTransactionSnapshot()).

* between commands of a query, CommandCounterIncrement and
  then directly modify the curcid of the active snapshot;
  AFAICS there's no reason to make another copy of it at this
  point.  Especially not if we can see it has refcount 1.

* at end of query, PopActiveSnapshot().

where a "query" is whatever we think the unit of noticing commits by
other backends ought to be.

> The spi.c change also changes the logic; the SPI code currently takes a 
> new snapshot for every query if the caller doesn't provide a snapshot.

[ squint... ]  Oh.  I see now, but that is horribly ugly and
underdocumented.  The code was previously treating the snapshot argument
as a constant and relying on that constant value to tell it what to do
each time through the loop.  Now you've got it changing the flag and
then changing it back sometime later.  Ick.

I think what you need to do to make this understandable is to move the
snapshot push/pop logic outside the per-command loop, instead of hacking
things around to keep it exactly where it was before.  We may well need
to adjust the API of snapmgr.c to make that sane.

BTW, this patch seems to be also the time to remove the AtStart_Cache()
call in CommandCounterIncrement, as foreseen in the comment 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] SQL command to edit postgresql.conf, with comments

[Richard Broersma]

On Tue, Oct 12, 2010 at 3:54 PM, Josh Berkus  wrote:

> Oh.  Now that's an interesting perspective ... you're suggesting that we
> take the comments and apply them as COMMENTS on the specific pg_settings?

On a side note regarding comments, I'd like to make a request for a
more comprehensive commenting mechanism.  The first though that comes
to my mind would allow for comments to be stored and annotated using
XML or sgml.  It'd be nice to be able to generate user documentation
from selected comments taken from application derived database
objects.

I don't know, maybe this is already possible.

-- 
Regards,
Richard Broersma Jr.

Visit the Los Angeles PostgreSQL Users Group (LAPUG)
http://pugs.postgresql.org/lapug

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

Re: [HACKERS] SQL command to edit postgresql.conf, with comments

[Josh Berkus]

> Postgres has "COMMENT ON ..." SQL for various database objects and I
> assumed that said comments would be stored in the system catalog.

Oh.  Now that's an interesting perspective ... you're suggesting that we
take the comments and apply them as COMMENTS on the specific pg_settings?

That wouldn't solve the issues of ordering, or of comments in the file
not associated with a setting, but might be a good 80% solution.

-- 
  -- Josh Berkus
 PostgreSQL Experts Inc.
 http://www.pgexperts.com

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

Re: [HACKERS] Review: Fix snapshot taking inconsistencies

[Marko Tiikkaja]

On 2010-10-13 1:21 AM +0300, Tom Lane wrote:

Marko Tiikkaja  writes:

Here's a new version of the patch, deprecating pg_parse_and_rewrite.


I started looking at this patch, and I'm wondering why you inserted all
the Register/UnregisterSnapshot calls that weren't there before (eg, why
did spi.c have to change at all?).  ISTM either these are not necessary,
or there is a pre-existing snapshot management bug that's independent
of the question of just when to take new snapshots.  I couldn't find
anything in the thread claiming the latter though.


That's actually just my ignorance I forgot to mention.  As I understand 
it, our code currently first pushes one snapshot and then does multiple 
PushActiveSnapshot (or PushUpdatedSnapshot)/PopActiveSnapshot rounds 
before popping the oldest snapshot off the stack (and releasing it).  So 
in the patch, I would've had to push the snapshot twice the first time 
to avoid it being released.


Thinking about it now, that might be a better option.  Or maybe we 
should change the snapshot API to make this more convenient?


The spi.c change also changes the logic; the SPI code currently takes a 
new snapshot for every query if the caller doesn't provide a snapshot.



Regards,
Marko Tiikkaja

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

Re: [HACKERS] SQL command to edit postgresql.conf, with comments

[Darren Duncan]

Josh Berkus wrote:

 The fact that Postgres already
explicitly supports comment metadata in its system catalog means it must
already know something about this.


We support what?


Postgres has "COMMENT ON ..." SQL for various database objects and I assumed 
that said comments would be stored in the system catalog.


  http://www.postgresql.org/docs/9.0/interactive/sql-comment.html


The solution to this is simple, and was previously discussed on this list:

(a) have a postgresql.conf.auto
(b) add a default include for postgresql.conf.auto at the beginning of
PostgreSQL.conf
(c) SQL updates go to postgresql.conf.auto, which consists only of
"setting = value" in alphabetical order.
(d) We document that settings which are changed manually in
postgresql.conf will override postgresql.conf.auto.
(e) $$profit$$!!


I agree that this looks like an effective solution.

-- Darren Duncan

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

Re: [HACKERS] Review: Fix snapshot taking inconsistencies

[Tom Lane]

Marko Tiikkaja  writes:
> Here's a new version of the patch, deprecating pg_parse_and_rewrite.

I started looking at this patch, and I'm wondering why you inserted all
the Register/UnregisterSnapshot calls that weren't there before (eg, why
did spi.c have to change at all?).  ISTM either these are not necessary,
or there is a pre-existing snapshot management bug that's independent
of the question of just when to take new snapshots.  I couldn't find
anything in the thread claiming the latter 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

[HACKERS] Re: [GENERAL] Text search parser's treatment of URLs and emails

[Bruce Momjian]

[ sent to hackers where it belongs ]

Thom Brown wrote:
> Hi,
> 
> I noticed that if I run this:
> 
> SELECT alias, description, token FROM
> ts_debug('http://www.postgresql.org:2345/directory/page.html?version=9.1&build=alpha1#summary');
> 
> I get:
> 
>   alias   |  description  |  token
> --+---+-
>  protocol | Protocol head | http://
>  url  | URL   |
> www.postgresql.org:2345/directory/page.html?version=9.1&build=alpha1#summary
>  host | Host  | www.postgresql.org:2345
>  url_path | URL path  |
> /directory/page.html?version=9.1&build=alpha1#summary
> (4 rows)
> 
> 
> It could be me being picky, but I don't regard parameters or page
> fragments as part of the URL path.  Ideally, I'd sort of expect:
> 
> alias |  description  |  token
> --+---+-
>  protocol | Protocol head | http://
>  url  | URL   |
> www.postgresql.org:2345/directory/page.html?version=9.1&build=alpha1#summary
>  host | Host  | www.postgresql.org
>  port | Port  | 2345
>  url_path | URL path  | /directory/page.html
>  query_string | Query string  | version=9.1&build=alpha1
>  fragment | Page fragment | summary
> (7 rows)
> 
> ... of course that's if there was support for query strings and page
> fragments, which there isn't.  But if changes were made to support my
> definition of a URL path, they'd have to be considered breaking
> changes.
> 

Wow, that is a tough one.  One the one hand, it seems nice to be able to
split stuff out more, but on the other hand we would be making url_path
less useful because people would need to piece things together to get
the old behavior.  In fact to piece things together we would need to add
'?' and '#' optionally, which seems kind of hard.  Perhaps we should
keep url_path unchanged and add file_path that has your suggestion. 
That would allow more fine-grained control without breaking backward
compatibility.  We already duplicate some data with url and url_path, so
having file_path as another place we duplicate some seems OK.

> But my main gripe is with the name "url_path".
> 
> Also:
> 
> SELECT alias, description, token FROM ts_debug('myname+prior...@gmail.com');
> 
> Yields:
> 
>alias   |   description   |   token
> ---+-+
>  asciiword | Word, all ASCII | myname
>  blank | Space symbols   | +
>  email | Email address   | prior...@gmail.com
> (3 rows)
> 
> The entire string I entered is a valid email address, and isn't
> totally uncommon.  Shouldn't that take such email address styles be
> taken into account?  The example above incorrectly identifies the
> email address since the real destination address would most likely be
> myn...@gmail.com.

I had no idea '+' could be part of an email address, and in fact it is a
modifier that is stripped off when delivering the email:

http://my.brandeis.edu/bboard/q-and-a-fetch-msg?msg_id=Nu

I didn't even know that was possible.  It is used as an email delivery
flag.  I agree that needs to be corrected.  We fixed URLs in 9.0 with:

   Use more standards-compliant rules for parsing URL tokens
   (Tom Lane)

so I think it is reasonable to fix email addresses in 9.1.  Care to
submit a patch?   You can lookup Tom's change as a guide.

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

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

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

Re: [HACKERS] SQL command to edit postgresql.conf, with comments

[Josh Berkus]

Darren, All,

> All you would need is for the file parser to retain the
> comments as metadata, include them in the relations that the SQL
> commands see where the latter can also edit them as data, and then write
> out the updated file with comments.

"All you need to do" in order to trisect and angle is to divide it into
three equal parts using just a compass and straightedge.

Given the number of different ways people can comment up pg.conf files,
and the fact that they can have the same settings set multiple times in
the file, both commented and uncommented, and that both the settings and
the comments can be in any order, with user-created spacing, you've
added a *huge* burden of parsing code to any .conf updater.  One which
would, in terms of LOC, exceed the updating code by more than 300%.

>  The fact that Postgres already
> explicitly supports comment metadata in its system catalog means it must
> already know something about this.

We support what?

The solution to this is simple, and was previously discussed on this list:

(a) have a postgresql.conf.auto
(b) add a default include for postgresql.conf.auto at the beginning of
PostgreSQL.conf
(c) SQL updates go to postgresql.conf.auto, which consists only of
"setting = value" in alphabetical order.
(d) We document that settings which are changed manually in
postgresql.conf will override postgresql.conf.auto.
(e) $$profit$$!!

--Josh Berkus

-- 
  -- Josh Berkus
 PostgreSQL Experts Inc.
 http://www.pgexperts.com

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

Re: [HACKERS] SQL command to edit postgresql.conf, with comments

[Kevin Grittner]

Darren Duncan  wrote:
 
> why is it not possible to retain the single existing postgres.conf
> file format (human writable) *and* have it machine/SQL-editable
> *and* maintain the comments?
 
My recollection of the previous discussion wasn't that there was no
way to do it, but rather that each person could thing of a somewhat
different way to do it and no two people agreed on which was best,
so no consensus was reached.
 
-Kevin

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

Re: [HACKERS] SQL command to edit postgresql.conf, with comments

[Darren Duncan]

Dimitri Fontaine wrote:

Andrew Dunstan  writes:

On 10/12/2010 05:02 PM, Dimitri Fontaine wrote:

  So, what you do is have a file per GUC, file name is the GUC name, first
line contains *ONLY* current value, the rest of the file is comments.

You're joking, right?


No. I just want both comments and SQL commands. If you refuse this
simple file scheme, keep your postgresql.conf and don't remote edit it.

That's my proposal, I'm happy that it comes with laughter :)


Maybe I missed something important, but why is it not possible to retain the 
single existing postgres.conf file format (human writable) *and* have it 
machine/SQL-editable *and* maintain the comments?  I should think that it would 
be possible to do all of these without too much trouble.  All you would need is 
for the file parser to retain the comments as metadata, include them in the 
relations that the SQL commands see where the latter can also edit them as data, 
and then write out the updated file with comments.  The fact that Postgres 
already explicitly supports comment metadata in its system catalog means it must 
already know something about this.  If something is missing, then expand the 
catalog so it represents all the details you want to preserve. -- Darren Duncan


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

Re: [HACKERS] SQL command to edit postgresql.conf, with comments

[Dimitri Fontaine]

Andrew Dunstan  writes:
> On 10/12/2010 05:02 PM, Dimitri Fontaine wrote:
>>   So, what you do is have a file per GUC, file name is the GUC name, first
>> line contains *ONLY* current value, the rest of the file is comments.
>
> You're joking, right?

No. I just want both comments and SQL commands. If you refuse this
simple file scheme, keep your postgresql.conf and don't remote edit it.

That's my proposal, I'm happy that it comes with laughter :)
-- 
Dimitri Fontaine
http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support

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

Re: [HACKERS] SQL command to edit postgresql.conf, with comments

[Andrew Dunstan]

On 10/12/2010 05:02 PM, Dimitri Fontaine wrote:

  So, what you do is have a file per GUC, file name is the GUC name, first
line contains *ONLY* current value, the rest of the file is comments.


You're joking, right?

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] Git cvsserver serious issue

[Andrew Dunstan]

On 10/12/2010 03:57 PM, Magnus Hagander wrote:

There's a simpler solution which I have just tested. Instead of patching,
use the Pg driver instead of SQLite. Set the dbname to %m. If the database
doesn't exist the cvs checkout will fail. So we just set up databases for
the modules we want to export (master and RELn_m_STABLE for the live
branches).


A database per branch seems like a horrible idea in general, but if it
works us around the bug, it seems like a doable idea.. As long as
we'll never have a branch called "postgres" or "git" (already in use
on that box).

I'll look into it.

Should be up and working now. master branch is ready, working on
prepping rel9_0_stable.

CVSROOT is :pserver:anonym...@git.postgresql.org:/postgresql.git

module name is master or REL9_0_STABLE. master available now,
rel_9_0_stable sohuld be available in about half an hour.



Thanks for this.

I have tested it (see 
) 
and there is a new Release of the buildfarm client to support its use: 
see 


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] host name support in pg_hba.conf

[Tom Lane]

Peter Eisentraut  writes:
>> Hopefully final patch, which addresses the above issues, adds some
>> documentation enhancements, and the possibility to quote host names (in
>> case someone wants to have a host named "samehost").

Oh, I had an idea for a small improvement to this.  It doesn't seem
unlikely that pg_hba.conf could contain multiple entries with the same
host name (but, presumably, different user and/or database names).  As
this is coded, you'll do a forward DNS lookup for each one until finding
the complete match.  You could easily prevent that by adding an
additional cache field to struct Port, along the lines of
+1 = remote_hostname is known to resolve to client's IP address
-1 = remote_hostname is known NOT to resolve to client's IP address
0 = we have not done the forward DNS lookup yet.
With this additional field we could guarantee to do not more than two
DNS lookups per connection attempt.

It also seems worth taking a second look at the order of tests in
check_hba().  I suspect that on average check_db() and check_role()
will now be much cheaper than the client IP test; should they be
done first?  Of course, if you assume that "all" is the typical
entry in those columns, this doesn't win.

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

SQL command to edit postgresql.conf, with comments (was: [HACKERS] Issues with two-server Synch Rep)

[Dimitri Fontaine]

Robert Haas  writes:
> That's probably not going to happen until we have a way to update
> postgresql.conf via SQL.  Which, I maintain, as I have maintained
> before, is not going to happen until we get rid of the comments,
> because otherwise absolutely any implementation anyone proposes will
> get criticized for failing to handle them sensibly (because it is not
> possible to rewrite the file while handling the comments sensibly).

I'm sorry but you don't need to get rid of the comments *at all* to have
the facility to update configuration from SQL.

What you need to do is make the choice that the setup is either human
friendly or SQL friendly. You will lose something that you now have to
be able to use SQL commands to edit the setup. Re-read. Ready now?

So, what you do is have a file per GUC, file name is the GUC name, first
line contains *ONLY* current value, the rest of the file is comments.

Note that it's still possible to hand-edit your setup. It's not even
hard to do. If you prefer a single file without comments, it's even
trivial to come up with a script that prepares the machine friendly
layout.

And it's even possible to offer DBAs a choice here: if postgresql.conf
exists, it's used in the old way and the SQL command only issues errors,
if there's a postgresql.conf.d and no postgresql.conf, the SQL commands
are working fine. Add an option for initdb to choose one layout or the
other.

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

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

Re: [HACKERS] Debugging initdb breakage

[David Fetter]

On Tue, Oct 12, 2010 at 10:05:57PM +0200, Dimitri Fontaine wrote:
> Alvaro Herrera  writes:
> > As a note, I've had luck finding bootstrap-time bugs by manually
> > feeding the bootstrapping commands into bootstrap mode, with a
> > leftover dir from "initdb --noclean".  This has helped a few times
> > that there has been no PANIC but just a FATAL or ERROR, for
> > example.  It's easy to attach GDB to such a backend.
> 
> Nice tip, thanks!

Where should we preserve this, other than the mailing list archives?

Cheers,
David.
-- 
David Fetter  http://fetter.org/
Phone: +1 415 235 3778  AIM: dfetter666  Yahoo!: dfetter
Skype: davidfetter  XMPP: david.fet...@gmail.com
iCal: webcal://www.tripit.com/feed/ical/people/david74/tripit.ics

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] host name support in pg_hba.conf

[Tom Lane]

Peter Eisentraut  writes:
> Hopefully final patch, which addresses the above issues, adds some
> documentation enhancements, and the possibility to quote host names (in
> case someone wants to have a host named "samehost").

A few minor gripes:

> +   If a host name is specified (anything that is not an IP address
> +   or a special key word is processed as a potential host name),
> +   that name is compared with the result of a reverse name
> +   resolution of the client's IP address (e.g., reverse DNS
> +   lookup, if DNS is used).  If there is a match, then a forward
> +   name resolution (e.g., forward DNS lookup) is performed on the
> +   host name to check if it resolves to an IP address that is
> +   equal to the client's IP address.  If both directions match,
> +   then the entry is considered to match.

I think the reason you're getting repeated questions about why the
reverse DNS lookup is needed is that the documentation fails to explain
that.  It'd be helpful if this part of the docs pointed out why the
apparently-extra lookup is necessary.  If I understand correctly, the
point is that we do the reverse lookup only once per connection (when
first finding a name-based pg_hba.conf entry) and that saves us having
to do forward lookup on all the name-based entries that don't match.

Which means BTW that the technique loses if the first name-based entry
matches the connection, and only wins when the match comes at the third
or later name-based entry.  Are we really sure this is a good tradeoff?
Are we sure there shouldn't be a way to turn it off?  I'm a bit
concerned about the fact that the argument seems to be "better
performance" versus "makes it completely useless for some use cases",
as here:
http://archives.postgresql.org/pgsql-hackers/2010-08/msg00726.php

Another smaller point is that it might be helpful if the wording didn't
make it sound like we expect the forward DNS lookup to produce just
one IP address.  Perhaps "If there is a match, then a forward name
resolution (e.g., forward DNS lookup) is performed on the host name to
check if any of the addresses it resolves to are equal to the client's
IP address."

In check_hostname():

> + port->remote_hostname = strdup(remote_hostname);

Seems like this had better be pstrdup(), or at least have an error
check.  Dumping core at the next line is not a real acceptable
substitute for an "out of memory" error.

> + /* Lookup IP from host name and check against original IP */
> + ret = getaddrinfo(port->remote_hostname, NULL, NULL, &gai_result);
> + if (ret != 0)
> + ereport(ERROR,
> + (errcode(ERRCODE_CONFIG_FILE_ERROR),
> +  errmsg("getaddrinfo failed on \"%s\": %s",
> + port->remote_hostname, 
> gai_strerror(ret;

Is ERRCODE_CONFIG_FILE_ERROR really the most appropriate code here?

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] Debugging initdb breakage

[Dimitri Fontaine]

Alvaro Herrera  writes:
> As a note, I've had luck finding bootstrap-time bugs by manually feeding
> the bootstrapping commands into bootstrap mode, with a leftover dir from
> "initdb --noclean".  This has helped a few times that there has been no
> PANIC but just a FATAL or ERROR, for example.  It's easy to attach GDB
> to such a backend.

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

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

Re: [HACKERS] Git cvsserver serious issue

[Magnus Hagander]

On Fri, Oct 8, 2010 at 08:09, Magnus Hagander  wrote:
> On Fri, Oct 8, 2010 at 03:52, Andrew Dunstan  wrote:
>>
>>
>> On 10/07/2010 03:37 PM, Magnus Hagander wrote:
>>>
>>> On Thu, Oct 7, 2010 at 21:31, Andrew Dunstan  wrote:

 On 10/07/2010 10:11 AM, Magnus Hagander wrote:
>><
>> OTOH, this patch seems pretty small and simple to maintain.
>
> True, it is rather small.
>
> Does anybody know if there's an automated way to maintain that on
> freebsd ports, and if so, how that works? I want to be *sure* we can't
> accidentally upgrade git-cvsserver *without* the patch, since that is
> a security issue.
>
 Why not just make a local copy somewhere else and patch and run that?
 It's
 just a Perl script, no?
>>>
>>> Yeah, but then we have to remember to manually patch that one when
>>> somebody *else* finds/fixes a security issue. We have automatic
>>> monitoring on the ports stuff to detect when that happens..
>>
>> There's a simpler solution which I have just tested. Instead of patching,
>> use the Pg driver instead of SQLite. Set the dbname to %m. If the database
>> doesn't exist the cvs checkout will fail. So we just set up databases for
>> the modules we want to export (master and RELn_m_STABLE for the live
>> branches).
>
> A database per branch seems like a horrible idea in general, but if it
> works us around the bug, it seems like a doable idea.. As long as
> we'll never have a branch called "postgres" or "git" (already in use
> on that box).
>
> I'll look into it.

Should be up and working now. master branch is ready, working on
prepping rel9_0_stable.

CVSROOT is :pserver:anonym...@git.postgresql.org:/postgresql.git

module name is master or REL9_0_STABLE. master available now,
rel_9_0_stable sohuld be available in about half an hour.

-- 
 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] Which file does the SELECT?

[Dimitri Fontaine]

David Fetter  writes:

> On Mon, Oct 11, 2010 at 04:14:04PM +0530, Vaibhav Kaushal wrote:
>> Is there something like that in Kdevelop? I dont use CLI editors much.
>
> KDevelop is listed as one of the editors that support ctags.

I've just developed some code for the backend and used cscope (from
Emacs directly) which is excellent.

  http://cscope.sourceforge.net/
  http://kscope.sourceforge.net/
  http://www.ziplink.net/~felaco/cbrowser/

Unfortunately the KDE GUI for it seems unmaintained now.

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

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

Re: [HACKERS] [JDBC] Support for JDBC setQueryTimeout, et al.

[Tom Lane]

David Fetter  writes:
> Let's imagine you have a connection pooler with two clients, A and B.
> A calls setQueryTimeout, then starts a query, which terminates in
> time, but dies before handling it.  B connects to the pool, gets A's
> connection, and finds a statement_timeout that's not the default, even
> though only A's single query was supposed to have that
> statement_timeout.  This is not a situation that can be resolved
> without being able to set a timer *on the server side*.

Actually, that seems like a fine argument why this should *not* be
implemented on the server side... although I would expect a connection
pooler to roll back GUC changes when switching users, so the argument
seems to presume several rather broken implementation decisions in
order to make the scenario possible.

> While I'd *like* to put in a whole infrastructure for setting GUCs on
> a per-statement basis, I don't believe that we need to get out that
> giant sledgehammer for this case, even though it's worth solving.

You'd need to first convince people that SET LOCAL doesn't solve the
problem well enough already.

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] ISN patch that applies cleanly with git apply

[Peter Geoghegan]

I would like to hear what people think of my observations about the
design of contrib/isn. In particular, I'd like Jan Otto to contribute
- he probably has more domain knowledge than I do. I haven't heard
from Jan about the proposed regression test.

In producing this patch, did you work off the listing of all the
628,000 assigned publisher codes that is only available in book form
at a cost of €558? How might I verify the correctness of the new
ISBN_range, preferably without spending €558?:-)

-- 
Regards,
Peter Geoghegan

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

Re: [HACKERS] [JDBC] Support for JDBC setQueryTimeout, et al.

[Magnus Hagander]

On Tue, Oct 12, 2010 at 17:55, David Fetter  wrote:
> On Tue, Oct 12, 2010 at 10:37:00AM -0500, Kevin Grittner wrote:
>> David Fetter  wrote:
>> > Is there something incomplete about the ones I sent, and if so,
>> > what?
>>
>> Well, I'm still curious why it was necessary to modify the server
>> side to implement an interface feature for which everything needed
>> seems to be present on the client side.
>
> Not everything is.
>
> Let's imagine you have a connection pooler with two clients, A and B.
> A calls setQueryTimeout, then starts a query, which terminates in
> time, but dies before handling it.  B connects to the pool, gets A's
> connection, and finds a statement_timeout that's not the default, even
> though only A's single query was supposed to have that
> statement_timeout.  This is not a situation that can be resolved
> without being able to set a timer *on the server side*.

Sure it can. The connection pooler just needs to issue a RESET ALL
statement when it hands over a connection from one client to another.
Isn't that what for example pgbouncer does - at least when configured
per instructions?

Also, doesn't this affect *all* settings, not just timeout, if it
doesn't? Imagine client A executing a SET datestyle for example.

AFAICS, any connection pooler that *doesn't* issue a reset between
handing this around is broken, isn't it?

>> If so, we should probably have an implementation in some other
>> interface to confirm that the server-side support fits.  If not, why
>> touch the server side code at all?
>
> See above.
>
> While I'd *like* to put in a whole infrastructure for setting GUCs on
> a per-statement basis, I don't believe that we need to get out that
> giant sledgehammer for this case, even though it's worth solving.

We don't usually put in fixes for just one out of 105 cases, do we?

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

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

Re: [HACKERS] Bug in information_schema: column names don't match spec

[Peter Eisentraut]

On fre, 2010-10-08 at 11:19 -0400, Tom Lane wrote:
> Was this just a thinko in our transcription, or did they rename the
> view columns somewhere along the line?

I suppose you figured this out already, but in some of my older draft
copies, the column did indeed have a different name.  The other columns
were just copying mistakes, apparently.

> Perhaps more to the point, what shall we do about it?  Just renaming
> the view columns is easy enough, but should we try to do something to
> limit the impact on existing clients?

I wouldn't worry about it unless we get complaints.  Users can change
the view easily enough if they need.


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

Re: [HACKERS] host name support in pg_hba.conf

[Peter Eisentraut]

On tis, 2010-10-12 at 10:11 +0900, KaiGai Kohei wrote:
> Is it really necessary to check reverse dns entries?

This has been extensively discussed in this thread.


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

Re: [HACKERS] host name support in pg_hba.conf

[Peter Eisentraut]

On mån, 2010-10-11 at 21:34 +0300, Peter Eisentraut wrote:
> > * Why getnameinfo() in the fallback loop?
> 
> I checked through my git history; this was actually a leftover from
> some
> debugging code.  I'll remove it.
> 
> > * Slash ('/') after the hostname
> > 
> > At the parse_hba_line(), the parsed token which contains either
> > hostname or cidr address is sliced into two parts on the first '/'
> > character, if exist.
> > Then, even if cidr_slash is not NULL, it shall be ignored when
> > top-half of the token is hostname, not numeric address.
> 
> OK, I'll fix that.

Hopefully final patch, which addresses the above issues, adds some
documentation enhancements, and the possibility to quote host names (in
case someone wants to have a host named "samehost").
diff --git a/doc/src/sgml/client-auth.sgml b/doc/src/sgml/client-auth.sgml
index 5cf2f29..0d515a1 100644
--- a/doc/src/sgml/client-auth.sgml
+++ b/doc/src/sgml/client-auth.sgml
@@ -80,9 +80,9 @@
A record is made
up of a number of fields which are separated by spaces and/or tabs.
Fields can contain white space if the field value is quoted.
-   Quoting one of the keywords in a database or user name field (e.g.,
+   Quoting one of the keywords in a database, user, or address field (e.g.,
all or replication) makes the word lose its special
-   character, and just match a database or user with that name.
+   character, and just match a database, user, or host with that name.
   
 
   
@@ -101,9 +101,9 @@
A record can have one of the seven formats
 
 local  database  user  auth-method  auth-options
-host   database  user  CIDR-address  auth-method  auth-options
-hostssldatabase  user  CIDR-address  auth-method  auth-options
-hostnossl  database  user  CIDR-address  auth-method  auth-options
+host   database  user  address  auth-method  auth-options
+hostssldatabase  user  address  auth-method  auth-options
+hostnossl  database  user  address  auth-method  auth-options
 host   database  user  IP-address  IP-mask  auth-method  auth-options
 hostssldatabase  user  IP-address  IP-mask  auth-method  auth-options
 hostnossl  database  user  IP-address  IP-mask  auth-method  auth-options
@@ -218,13 +218,17 @@ hostnossl  database  user
 
 
 
- CIDR-address
+ address
  
   
Specifies the client machine IP address range that this record
-   matches. This field contains an IP address in standard dotted decimal
-   notation and a CIDR mask length. (IP addresses can only be
-   specified numerically, not as domain or host names.)  The mask
+   matches.  This field can contain either a host name, an IP
+   address range, or one of the special key words mentioned below.
+  
+
+  
+   An IP address is specified in standard dotted decimal
+   notation with a CIDR mask length.  The mask
length indicates the number of high-order bits of the client
IP address that must match.  Bits to the right of this must
be zero in the given IP address.
@@ -233,14 +237,7 @@ hostnossl  database  user
   
 
   
-   Instead of a CIDR-address, you can write
-   samehost to match any of the server's own IP
-   addresses, or samenet to match any address in any
-   subnet that the server is directly connected to.
-  
-
-  
-   Typical examples of a CIDR-address are
+   Typical examples of an IP address range specified this way are
172.20.143.89/32 for a single host, or
172.20.143.0/24 for a small network, or
10.6.0.0/16 for a larger one.
@@ -260,6 +257,35 @@ hostnossl  database  user
   
 
   
+   You can also write
+   samehost to match any of the server's own IP
+   addresses, or samenet to match any address in any
+   subnet that the server is directly connected to.
+  
+
+  
+   If a host name is specified (anything that is not an IP address
+   or a special key word is processed as a potential host name),
+   that name is compared with the result of a reverse name
+   resolution of the client's IP address (e.g., reverse DNS
+   lookup, if DNS is used).  If there is a match, then a forward
+   name resolution (e.g., forward DNS lookup) is performed on the
+   host name to check if it resolves to an IP address that is
+   equal to the client's IP address.  If both directions match,
+   then the entry is considered to match.
+  
+
+  
+   When host names are specified
+   in pg_hba.conf, you should make sure that
+   name resolution is reasonably fast.  It can be of advantage to
+   set up a local name resolution cache such
+   as nscd.  Also, you may wish to enable the
+   configuration parameter log_hostname to see
+   the client's host name instead of the IP address in the log.
+  
+
+  
This field only applies to host,
hostssl, and hostnossl records.
   
@@ -

Re: [HACKERS] [JDBC] Support for JDBC setQueryTimeout, et al.

[Kevin Grittner]

David Fetter  wrote:
 
> Let's imagine you have a connection pooler with two clients,
> A and B.
 
I'm with you so far.
 
> A calls setQueryTimeout, then starts a query, which terminates in
> time, but dies before handling it.
 
Here you lost me.  I don't know what that means.
 
> B connects to the pool, gets A's connection, and finds a
> statement_timeout that's not the default
 
Why?  I would consider the JDBC QueryTimeout property to be
orthogonal to the server's statement_timeout GUC.  Perhaps that's
why we're seeing things so differently.
 
> even though only A's single query was supposed to have that
> statement_timeout.  This is not a situation that can be resolved
> without being able to set a timer *on the server side*.
 
That will be true if we conflate these two things.  I don't think we
should.
 
http://download.oracle.com/javase/6/docs/api/java/sql/Statement.html#setQueryTimeout%28int%29
 
This time limit should apply to the overall time allowed in the
driver for the execute, executeQuery and executeUpdate of the Java
Statement object.  It should not be trying to pick apart individual
SQL statements within the execute request, and it should not affect
any other statement on the connection.
 
I think both patches are making this harder than it needs to be.
 
-Kevin

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

Re: [HACKERS] [JDBC] Support for JDBC setQueryTimeout, et al.

[David Fetter]

On Tue, Oct 12, 2010 at 10:37:00AM -0500, Kevin Grittner wrote:
> David Fetter  wrote:
> > Is there something incomplete about the ones I sent, and if so,
> > what?
>  
> Well, I'm still curious why it was necessary to modify the server
> side to implement an interface feature for which everything needed
> seems to be present on the client side.

Not everything is.

Let's imagine you have a connection pooler with two clients, A and B.
A calls setQueryTimeout, then starts a query, which terminates in
time, but dies before handling it.  B connects to the pool, gets A's
connection, and finds a statement_timeout that's not the default, even
though only A's single query was supposed to have that
statement_timeout.  This is not a situation that can be resolved
without being able to set a timer *on the server side*.

> Is this intended to be useful for other interfaces?

Anybody doing similar functionality, namely a per-statement timeout,
would need this infrastructure, and for the same reason.

> If so, we should probably have an implementation in some other
> interface to confirm that the server-side support fits.  If not, why
> touch the server side code at all?

See above.

While I'd *like* to put in a whole infrastructure for setting GUCs on
a per-statement basis, I don't believe that we need to get out that
giant sledgehammer for this case, even though it's worth solving.

Incidentally, this dovetails neatly with the isolation concerns that
motivated the "true serializability" patch you're working on :)

Cheers,
David.
-- 
David Fetter  http://fetter.org/
Phone: +1 415 235 3778  AIM: dfetter666  Yahoo!: dfetter
Skype: davidfetter  XMPP: david.fet...@gmail.com
iCal: webcal://www.tripit.com/feed/ical/people/david74/tripit.ics

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] [JDBC] Support for JDBC setQueryTimeout, et al.

[Kevin Grittner]

David Fetter  wrote:
 
> Is there something incomplete about the ones I sent, and if so,
> what?
 
Well, I'm still curious why it was necessary to modify the server
side to implement an interface feature for which everything needed
seems to be present on the client side.  Is this intended to be
useful for other interfaces?  If so, we should probably have an
implementation in some other interface to confirm that the
server-side support fits.  If not, why touch the server side code at
all?
 
These are not rhetorical questions.  There may be some reason, but
if so, I'd like to see it stated, rather than trying to infer it
through reverse engineering.
 
-Kevin

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

Re: [HACKERS] Debugging initdb breakage

[Alvaro Herrera]

Excerpts from Dimitri Fontaine's message of dom oct 10 17:38:01 -0300 2010:
> Hi,
> 
> So I'm beginning to work on the extension support for dump and restore,
> and that begins with a new pg_extension catalog. I managed to break
> initdb already, of course, but I'm fighting my way out — no luck with
> gdb, it won't catch the Assert failure and show me a backtrace. I tried
> "set follow-fork-mode child" in gdb, in different systems, to no avail.

As a note, I've had luck finding bootstrap-time bugs by manually feeding
the bootstrapping commands into bootstrap mode, with a leftover dir from
"initdb --noclean".  This has helped a few times that there has been no
PANIC but just a FATAL or ERROR, for example.  It's easy to attach GDB
to such a backend.

-- 
Álvaro Herrera 
The PostgreSQL Company - Command Prompt, Inc.
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] [JDBC] Support for JDBC setQueryTimeout, et al.

[David Fetter]

On Tue, Oct 12, 2010 at 04:04:56AM -0500, Radosław Smogura wrote:
> On Mon, 11 Oct 2010 08:29:16 -0500, "Kevin Grittner"
>  wrote:
> > David Fetter  wrote:
> >  
> >> Please find enclosed a WIP patch from one of my co-workers
> >> intended to support JDBC's setQueryTimeout, along with the patch
> >> for JDBC that uses it.
> >  
> > I agree that it would be very nice to support this JDBC feature, but
> > I'm not clear on why this can't be done with just JDBC changes using
> > the java.util.Timer class and the existing Statement.cancel()
> > method.  Can you explain why the backend needed to be touched?
> >  
> > -Kevin
> 
> I sent such patch fully in Java
> (http://archives.postgresql.org/pgsql-jdbc/2009-11/msg00010.php),
> implementing cancellation with Timer and "cancel query" facility of
> PSQL server,

Would you like to update it?

Is there something incomplete about the ones I sent, and if so, what?

> unfortunately none has revised it, even that setQuertyTimeout todo
> is for long time on dashboard, and it's important for enterprise
> class software.

That sounds a tad buzzword-y.  What exactly is it that *you* see as
important to delivering this feature?  I'm thinking JDBC1 compliance,
but you might have something very different in mind.

Cheers,
David.
-- 
David Fetter  http://fetter.org/
Phone: +1 415 235 3778  AIM: dfetter666  Yahoo!: dfetter
Skype: davidfetter  XMPP: david.fet...@gmail.com
iCal: webcal://www.tripit.com/feed/ical/people/david74/tripit.ics

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] security hook on table creation

[Robert Haas]

On Tue, Oct 12, 2010 at 9:20 AM, KaiGai Kohei  wrote:
> As I introduced before, the idea of two hooks makes obvious where
> we should put the security hooks; it is next to the existing DAC
> checking. It is the best guideline, even if we will touch the code
> around object creation in the future version.
>
> If the creation-hook would be put on the place far from existing
> DAC checks, what provides us a guideline to deploy security hooks?
> It seems to me the idea of only post-creation hook try to lose
> this kind of benefit instead of half dozen of exceptions.
>
> I think MVCC visibility is just an actualization of the matters.
> The point is that we can be released from the task to consider
> where is the right place for security hooks, as long as it will
> be placed next to the existing DAC checks.
> It seems to me its simplicity of design is unignorable benefit.

In either design, you have to decide where to put the post-creation
hook.  In your design, you ALSO need to decide where to put the
pre-creation hook.  Deciding where to put the pre-creation hook may be
simple, but it is not as simple as not having it at all.

A possibly legitimate reason to have a pre-creation hook is to prevent
users from consuming more excessive system resources by repeatedly
performing operations that SE-Linux will end up denying, but only
after they're basically complete.

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

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

Re: [HACKERS] pgxs docdir question

[Devrim GÜNDÜZ]

On Tue, 2010-10-12 at 09:36 -0400, Tom Lane wrote:
> 
> I think docdir is separate from the regular prefix.  Try specifying
> --docdir to configure. 

Thanks, I'll test (not now, but soon).


Regards,
-- 
Devrim GÜNDÜZ
PostgreSQL Danışmanı/Consultant, Red Hat Certified Engineer
PostgreSQL RPM Repository: http://yum.pgrpms.org
Community: devrim~PostgreSQL.org, devrim.gunduz~linux.org.tr
http://www.gunduz.org  Twitter: http://twitter.com/devrimgunduz


signature.asc
Description: This is a digitally signed message part

Re: [HACKERS] pgxs docdir question

[Tom Lane]

Devrim =?ISO-8859-1?Q?G=DCND=DCZ?=  writes:
> On Mon, 2010-10-11 at 13:30 -0400, Tom Lane wrote:
>> What does "pg_config --docdir" report?

> $ /usr/pgsql-9.0/bin/pg_config --docdir
> /usr/share/doc/pgsql

> I was expecting it to be somewhere in /usr/pgsql-9.0, where it is the
> --prefix. Am I wrong?

I think docdir is separate from the regular prefix.  Try specifying
--docdir to configure.

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] Keywords in pg_hba.conf should be field-specific

[Fujii Masao]

On Sat, Oct 9, 2010 at 11:13 AM, Brendan Jurd  wrote:
> I understand that 'replication' is a keyword as far as the database
> name is concerned, but I was surprised to find that it was treated as
> a keyword in the username field also.  I had a look in
> src/backend/libpq/hba.c, and next_token() appears to be completely
> naive about this.  'replication' (along with 'all', 'sameuser',
> 'samegroup' and 'samerole')  is treated as a keyword wherever it
> appears, not just in the field where it is relevant.  next_token()
> appends a newline to the end of the 'replication' username token, and
> that's why the entry doesn't match my connection attempt.
>
> I suspect this is going to trip a lot of people up.  We could just
> document it and tell people that if they want to use 'replication' as
> a username, they'd better quote it in pg_hba.conf.  But I'd prefer to
> actually solve the problem.

Agreed. We should address that.

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] security hook on table creation

[KaiGai Kohei]

(2010/10/12 20:59), Robert Haas wrote:

2010/10/11 KaiGai Kohei:

It enables us to put security hooks independent from MVCC visibility of
the new database object. If we pay attention for visibility of new database
object, it seems to me amount of things to understand and maintain will be
increased, although MVCC visibility is fundamentally unrelated stuff from
viewpoint of the access control.

In the idea of two hooks, the prep-creation hook shall be invoked in same
visibility of existing permission checks, and the post-creation hook shall
be invoked in same visibility of simple_heap_* operations.
I think it enables to reduce amount of things to understand and maintain,
because the scope we should pay attention become small, if we can put
security hooks independent from MVCC visibility.

Perhaps, the problem may be intangible, but I don't think it is fair
enough if we have to pay attention about MVCC visibility of plugin
modules whenever we try to apply a patch around creation hooks.


This may be nothing more than a matter of opinion, but it seems to me
that what you're proposing makes this vastly more complicated for no
particular benefit.  Instead of having one hook that can cover a wide
variety of use cases, you're going to need individual hooks for each
object type plus the ability to pass data between them.


In the broad outline, I also agree with one main security which can
cover most of use cases. However, the only difference is that I'm
saying we should handle prep-creation case as exception of the main
hook.
As I introduced before, the idea of two hooks makes obvious where
we should put the security hooks; it is next to the existing DAC
checking. It is the best guideline, even if we will touch the code
around object creation in the future version.

If the creation-hook would be put on the place far from existing
DAC checks, what provides us a guideline to deploy security hooks?
It seems to me the idea of only post-creation hook try to lose
this kind of benefit instead of half dozen of exceptions.

I think MVCC visibility is just an actualization of the matters.
The point is that we can be released from the task to consider
where is the right place for security hooks, as long as it will
be placed next to the existing DAC checks.
It seems to me its simplicity of design is unignorable benefit.


And the point
of this, apparently, is so that you can avoid using the standard
syscache functions that the entire backend uses for retrieving
information about objects and instead extract it in some other way;
and/or avoid having to deal with the MVCC properties of which the rest
of the backend must be aware.


It just means it is not impossible...
However, it requires the plugin modules need to know everything;
such as what is visible/invisible. It seems to me too closely-
coupled interface.

Thanks,
--
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] Docs for archive_cleanup_command are poor

[Fujii Masao]

On Sat, Oct 9, 2010 at 10:04 AM, Brendan Jurd  wrote:
> Hi folks,
>
> I have just set up HS+SR for the first time, and for the most part,
> the docs were excellent.  The one exception for me was the discussion
> of archive_cleanup_command.  This is a pretty important part of
> constructing a healthy standby server, and IMO the docs don't give it
> the treatment it deserves.
>
> Under "25.2.4. Setting Up a Standby Server", we have:
>
> "You can use archive_cleanup_command to prune the archive of files no
> longer needed by the standby."
>
> ... then a few paragraphs later ...
>
> "If you're using a WAL archive, its size can be minimized using the
> archive_cleanup_command  option to remove files that are no longer
> required by the standby server. Note however, that if you're using the
> archive for backup purposes, you need to retain files needed to
> recover from at least the latest base backup, even if they're no
> longer needed by the standby."
>
> So there are a couple of brief mentions of what
> archive_cleanup_command is for, but nothing about how it works, no
> exampes of how to use it, and no links at all.  Contrast how we deal
> with archive_command, restore_command and primary_conninfo.
>
> I'd like to suggest a few ways we could improve on this:
>
> 1. Remove the former paragraph.  It's stranded out there on its own in
> the middle of some unrelated text, and doesn't say anything of
> substance not also said in the latter paragraph.
>
> 2. Include an example archive_cleanup_command in the recovery.conf
> example snippet.
>
> 3. Link to 26.1 which actually explains how a_c_c works.
>
> 4. Mention, and link to, pg_archivecleanup from both 25.2.4 and 26.1.
> This is the utility that most newcomers to WAL archiving will want to
> use, so it's rather weird of us not to advertise it.
>
> I'm willing to write a patch for this, but I thought I'd raise the
> suggestions on-list first, before getting too invested.  So, please
> comment if you have an opinion on this.

Agreed.

And, ISTM that we should mention that we must not just specify
pg_archivecleanup in archive_cleanup_command when there are multiple
standby servers. This is because, in that case, we must calculate
the oldest restart point in those standbys and delete the archived
WAL files according to that point.

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] security hook on table creation

[Robert Haas]

2010/10/11 KaiGai Kohei :
> It enables us to put security hooks independent from MVCC visibility of
> the new database object. If we pay attention for visibility of new database
> object, it seems to me amount of things to understand and maintain will be
> increased, although MVCC visibility is fundamentally unrelated stuff from
> viewpoint of the access control.
>
> In the idea of two hooks, the prep-creation hook shall be invoked in same
> visibility of existing permission checks, and the post-creation hook shall
> be invoked in same visibility of simple_heap_* operations.
> I think it enables to reduce amount of things to understand and maintain,
> because the scope we should pay attention become small, if we can put
> security hooks independent from MVCC visibility.
>
> Perhaps, the problem may be intangible, but I don't think it is fair
> enough if we have to pay attention about MVCC visibility of plugin
> modules whenever we try to apply a patch around creation hooks.

This may be nothing more than a matter of opinion, but it seems to me
that what you're proposing makes this vastly more complicated for no
particular benefit.  Instead of having one hook that can cover a wide
variety of use cases, you're going to need individual hooks for each
object type plus the ability to pass data between them.  And the point
of this, apparently, is so that you can avoid using the standard
syscache functions that the entire backend uses for retrieving
information about objects and instead extract it in some other way;
and/or avoid having to deal with the MVCC properties of which the rest
of the backend must be aware.  Maybe somebody else has a different
opinion, but I just can't get even a little excited about that.

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

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

Re: [HACKERS] wip: functions median and percentile

[Pavel Stehule]

2010/10/12 Hitoshi Harada :
> 2010/10/12 Pavel Stehule :
>> Hello
>>
>> 2010/10/11 Greg Stark :
>>> On Sun, Oct 10, 2010 at 2:16 PM, Tom Lane  wrote:
 It was pointed out upthread that while median isn't presently
 in the standard, Oracle defines it in terms of percentile_cont(0.5)
 which *is* in the standard.
>>>
>>> Uhmm, then why don't we implement that? We could provide median() as a
>>> short-cut but percentile_cont() doesn't sound much harder to implement
>>> than median() and more general.
>>
>> The problem is in interface. The original patch did it, but I removed
>> it. We cannot to unsure immutability of some parameters now. Can we
>> enhance a AGGREGATE to allow some mark like IMMUTABLE parameter and
>> probably we should to support ANSI syntax:
>>
>> PERCENTILE_CONT ( expression1 )
>> WITHIN GROUP ( ORDER BY expression2 [ ASC | DESC ] )
>>
>> This syntax allows to divide a muttable and immutable parameters.
>
> If this is only a syntax sugar for mutable/immutable parameter, then I
> guess it's time to take it serious to implement in our syntax,
> although I'm not sure if it affects more execution model than
> interface.

I though about it, the question is an interface for PL languages.
There are not problem for C.

Regards

Pavel Stehule


>
> Regards,
>
>
>
> --
> Hitoshi Harada
>

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

Re: [HACKERS] [JDBC] Support for JDBC setQueryTimeout, et al.

[Radosław Smogura]

On Mon, 11 Oct 2010 08:29:16 -0500, "Kevin Grittner"
 wrote:
> David Fetter  wrote:
>  
>> Please find enclosed a WIP patch from one of my co-workers
>> intended to support JDBC's setQueryTimeout, along with the patch
>> for JDBC that uses it.
>  
> I agree that it would be very nice to support this JDBC feature, but
> I'm not clear on why this can't be done with just JDBC changes using
> the java.util.Timer class and the existing Statement.cancel()
> method.  Can you explain why the backend needed to be touched?
>  
> -Kevin

I sent such patch fully in Java
(http://archives.postgresql.org/pgsql-jdbc/2009-11/msg00010.php),
implementing cancellation with Timer and "cancel query" facility of PSQL
server, unfortunately none has revised it, even that setQuertyTimeout todo
is for long time on dashboard, and it's important for enterprise class
software.

--
Radosław Smogura
http://www.softperience.eu

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

Re: [HACKERS] pgxs docdir question

[Devrim GÜNDÜZ]

On Mon, 2010-10-11 at 13:30 -0400, Tom Lane wrote:
> What does "pg_config --docdir" report?

$ /usr/pgsql-9.0/bin/pg_config --docdir
/usr/share/doc/pgsql

I was expecting it to be somewhere in /usr/pgsql-9.0, where it is the
--prefix. Am I wrong?

Regards,
-- 
Devrim GÜNDÜZ
PostgreSQL Danışmanı/Consultant, Red Hat Certified Engineer
PostgreSQL RPM Repository: http://yum.pgrpms.org
Community: devrim~PostgreSQL.org, devrim.gunduz~linux.org.tr
http://www.gunduz.org  Twitter: http://twitter.com/devrimgunduz


signature.asc
Description: This is a digitally signed message part