[HACKERS] Privileges and inheritance

[Peter Eisentraut]

I would like to propose a change in the way privilege checking is done
with inheritance hierarchies.  Currently, selecting from a table that
has children requires explicit privileges on all the children.  This is
inconsistent with all other commands, which treat children as implicitly
part of the parent table.  (Arguably, it exposes an implementation
detail, since you could just as well implement inheritance by keeping
all the children's data for the inherited columns in the parent's heap.)

As inheritance has now found new popularity as a partitioning mechanism,
this exacerbates the annoyance because you have to copy the privilege
sets to possibly dozens or hundreds of subtables in cumbersome ways for
really no good reason.

If you use inheritance for data modeling (the original purpose), you
face another problem.  Either you grant table privileges on all the
child tables, thus giving users access to more information than they
were supposed to have, or you grant column privileges on only those
columns that were inherited, being careful to keep that set updated
whenever table definitions are altered.  (Before 8.4 you couldn't even
do that.)  It's messy.

So let's get rid of that.  Selecting (or in general, operating) on a
table with children only checks the privileges on that table, not the
children.  Is there any use case where the current behavior is useful at
all?  (What I gather from history and the code, I think it was just
forgotten to change this when we switched away from the table* syntax
way back when.)  FWIW, changing this behavior would also be more
SQL-conforming.

We could use a GUC variable to ease the transition, perhaps like
sql_inheritance = no | yes_without_privileges | yes

Comments?



-- 
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] clang's static checker report.

[Grzegorz Jaskiewicz]

new version under:
http://zlew.org/postgresql_static_check/scan-build-2009-10-03-1/

What's strange, is the increase of 48.2 percent in reports, that  
happened about two weeks before (weekend before the previous one).


enjoy.


--
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] FSM search modes

[decibel]

On Oct 1, 2009, at 4:18 PM, Robert Haas wrote:


On Thu, Oct 1, 2009 at 5:08 PM, Tom Lane  wrote:

Robert Haas  writes:

The elephant in the room here is that if the relation is a million
pages of which 1-100,000 and 1,000,000 are in use, no amount of bias
is going to help us truncate the relation unless every tuple on page
1,000,000 gets updated or deleted.


Well, there is no way to move a tuple across pages in a user- 
invisible,
non-blocking fashion, so our ability to do something automatic  
about the
above scenario is limited.  The discussion at the moment is about  
ways
of reducing the probability of getting into that situation in the  
first

place.  That doesn't preclude also providing some more-invasive tools
that people can use when they do get into that situation; but let's
not let I-want-a-magic-pony syndrome prevent us from doing anything
at all.


That's fair enough, but it's our usual practice to consider, before
implementing a feature or code change, what fraction of the people it
will actually help and by how much.  If there's a way that we can
improve the behavior of the system in this area, I am all in favor of
it, but I have pretty modest expectations for how much real-world
benefit will ensue.  I suspect that it's pretty common for large



Speaking of helping other cases...

Something else that's been talked about is biasing FSM searches in  
order to try and keep a table clustered. If it doesn't add a lot of  
overhead, it would be nice to keep that in mind. I don't know where  
something like randomly reseting the search would go in the code, but  
I suspect it wouldn't be very expandable in the future.


But like Tom said, the top goal here is to help deal with bloat, not  
other fanciness.

--
Decibel!, aka Jim C. Nasby, Database Architect  deci...@decibel.org
Give your computer some brain candy! www.distributed.net Team #1828



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

ECPG dynamic cursorname patch revised and split Re: [HACKERS] CommitFest 2009-09, two weeks on

[Boszormenyi Zoltan]

Robert Haas írta:
> On Fri, Oct 2, 2009 at 9:01 PM, Boszormenyi Zoltan  wrote:
>   
>> Hi,
>>
>> Michael Meskes írta:
>> 
>>> It is accepted either way. I was just pointing out that it might be easier 
>>> to
>>> review/commit at least parts of your patches if they can be applied 
>>> seperately.
>>>
>>>   
>> I have split up (and cleaned up a little) the dynamic
>> cursorname patch into smaller logical, easier-to-review
>> pieces. Descriptions below.
>>
>> ...
>> After every patch (except 4) both the core and ecpg
>> "make check" are successful.
>> 
>
> It would've been nice if you'd changed the subject line before posting these.
>   

At 3am, I forgot this. :(
I hope the archive threading doesn't break and
my previous mail will be referenced by this one.

> Also, please update commitfest.postgresql.org appropriately.
>   

I just did, thanks for the reminder.

Best regards,
Zoltán Böszörményi

-- 
Bible has answers for everything. Proof:
"But let your communication be, Yea, yea; Nay, nay: for whatsoever is more
than these cometh of evil." (Matthew 5:37) - basics of digital technology.
"May your kingdom come" - superficial description of plate tectonics

--
Zoltán Böszörményi
Cybertec Schönig & Schönig GmbH
http://www.postgresql.at/


-- 
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] Getting the red out (of the buildfarm)

[Tom Lane]

Peter Eisentraut  writes:
> On Wed, 2009-09-23 at 10:20 -0400, Tom Lane wrote:
>> comet_moth, gothic_moth: these are failing the new plpython_unicode
>> test
>> in locale cs_CZ.ISO8859-2.  Somebody needs to do something about that.
>> If it's left to me I'll probably just remove the test that has
>> multiple
>> results.

> This is, at first glance, not a valid variant result.  It's a genuine
> failure that needs investigation.  I can't reproduce the problem with
> the equivalent locale on Linux, so Zdenek might need to look into it.

Uh, I can reproduce it just fine on Fedora 11, and OS X too.  These
are running python 2.6 and 2.6.1 respectively ... maybe the behavior
is python version dependent?

As far as I can tell, PLyObject_ToDatum is invoking PLyUnicode_Str and
then PyString_AsString, and what it gets back from the latter is
(in C string notation) "\200\0".  Possibly what this means is that
python thinks that that is the correct LATIN2 representation of
\u0080.

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] latest hstore patch

[Andrew Gierth]

> "Bruce" == Bruce Momjian  writes:

 >> There's still the issue of how to get the improved module
 >> definition (new functions etc) into a migrated database.  That's
 >> not specific to hstore in any way though, it would affect any
 >> contrib module that had added stuff in a new release.

 Bruce> Most modules just install functions, which are easily
 Bruce> uninstalled/reinstalled.  A data type like hstore is more
 Bruce> complicated assuming it is the data type that is changing and
 Bruce> not the support functions.

For hstore, the various changes (and their solutions if any) are roughly
these:

1) new functions and operators - these don't present a migration issue
other than that they won't be available in a migrated db unless added,
which can be done after the fact with CREATE FUNCTION / CREATE OPERATOR
as in the install script. (This issue is the same for dump/restore
upgrades if the new version is not installed prior to the restore)

2) new internal C names for some functions - this is addressed in hstore
by defining both the new and old names, so no migration issue there
(running an after the fact CREATE OR REPLACE FUNCTION, as in the install
script, will remove the references to the old names; but even that much
isn't necessary unless there's actually a naming conflict)

3) Change in the representation of the underlying data. This is handled
by having the code recognize the old format and convert it on the fly;
this isn't ideal, but it does work.

4) Change in the SQL-level definition of the data type (specifically,
the new code adds SEND and RECEIVE functions which weren't previously
present). This is a hard one; currently, even for a dump/restore
upgrade, this requires that you run the new version's .sql file before
restoring the dump, otherwise you get the old type definition with
those functions missing, and there's no convenient way to add them
back.  A migration upgrade would have the same issue.

5) Changes to operator classes; the new version adds two new
opclasses, which is easy, but it also adds new operators to two
opclasses defined in the old version, and there's no ALTER or CREATE
OR REPLACE for those.  Again, with a dump/restore upgrade this is
fixable by installing the new version before restoring; if you don't
do that, there's no convenient way to enable access to the new
functionality short of dropping the old opclasses (and all the indexes
that use them) and recreating them.

It's really items (4) and (5) (and similar ones, such as changes to
operator definitions, and other SQL-level objects that don't have an
OR REPLACE option) that currently present an obstacle for all module
authors. (3) is something that's more of a case-by-case problem which
can be tackled within the module itself.

-- 
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] FSM search modes

[Bruce Momjian]

Kevin Grittner wrote:
> Tom Lane  wrote: 
>  
> > (Hm, so we might want to make the probability depend on
> > max_connections?)
>  
> Without doing rigorous math on it, I'd guess that to prevent
> contention among n connections you'd want the probably of resetting
> the sweep to be about 1 / (n * 2).  That would mean you'd advance to
> the nth page about 60.6% of the time without resetting the sweep.  For
> less contention, 1 / (n * 4) would let you get to the nth page about
> 77.9% of the time.
>  
> > Maybe what we want is some bias against inserting in the last half
> > or quarter of the table, or some such rule, rather than necessarily
> > heading for the start of the relation.
>  
> I think it would make sense to just start using this once you get into
> the last half or quarter of the free pages.  If you go with the last
> quarter, then you might want to use a higher probability than I
> suggested above, although that would tend to leave you with contention
> when all the free space was in the last quarter.  I'd be inclined to
> use something like the above probability and start using it at 50%.

Two things that might not have been mentioned:  First, only reset if you
are given a page in the last 1/4 of the table;  that way, if there is no
free space in the last 1/4 of the table, you will not be resetting.  A
second idea is to heavily bias against using the last table page with
data in it; that should help bias toward empty pages on the end of the
table.

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

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

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

Re: [HACKERS] Postgres server goes in recovery mode repeteadly

[daveg]

On Fri, Oct 02, 2009 at 10:41:07AM -0400, Alvaro Herrera wrote:
> daveg escribió:
> 
> > I work with Kunal and have been looking into this. It appears to be the same
> > as the bug described in:
> > 
> >   http://archives.postgresql.org/pgsql-bugs/2009-09/msg00355.php
> > 
> > as I have localized it to a NULL pointer deference in
> > RelationCacheInitializePhase2() as well. Tom speculates in:
> > 
> >   http://archives.postgresql.org/pgsql-bugs/2009-09/msg00372.php
> > 
> > that large numbers of table drops might trigger this. The system in question
> > creates and drops temp tables at a high rate which tends to confirm this. 
> 
> Did you test the patch posted by Tom?

We are testing it since last night in our test environment. If it does not
break anything (unlikely) we will deploy it next week. However, since the
problem is only occasional, only happens every few days on one of 50+ hosts,
it will take some extended time without further segfaults to say anything
confident about the patches effectiveness.

-dg
 
-- 
David Gould   da...@sonic.net  510 536 1443510 282 0869
If simplicity worked, the world would be overrun with insects.

-- 
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] CommitFest 2009-09, two weeks on

[Robert Haas]

On Fri, Oct 2, 2009 at 9:01 PM, Boszormenyi Zoltan  wrote:
> Hi,
>
> Michael Meskes írta:
>> It is accepted either way. I was just pointing out that it might be easier to
>> review/commit at least parts of your patches if they can be applied 
>> seperately.
>>
>
> I have split up (and cleaned up a little) the dynamic
> cursorname patch into smaller logical, easier-to-review
> pieces. Descriptions below.
>
> 1) 1a-unified-optfromin-fetch-ctxdiff.patch
>
> ecpg supports optional FROM/IN in FETCH and
> MOVE statements (mainly because it's required by
> Informix-compatibility). Unify core and ecpg grammar
> as per Tom Lane's suggestion.
>
> 2) 1b-cursor_name-ctxdiff.patch
>
> "name" -> "cursor_name" transition in core grammar
> and ecpg grammar. Currently it does nothing, it's a
> preparation phase. Depends on patch 1.
>
> 3) 1c-remove-var-from-list.patch
>
> Introduce function remove_variable_from_list().
> It is used by the dynamic cursor, SQLDA and DESCRIBE
> patches for different reasons. Applicable separately.
>
> 4) 1d-dynamiccur-ctxdiff.patch
>
> The real point of the whole series in this email.
> Extend "cursor_name" in the ecpg grammar to actually
> accept a character variable. The cursorname-as-variable
> is replaced in the final SQL script with a $0 placeholder.
> Doesn't compile as-is, requires patch 5 to get the
> two shift/reduce conflicts fixed. Depends on patches
> 1, 2 and 3.
>
> 5) 1e-fix-shiftreduce-ctxdiff.patch
>
> De-factorize "BACKWARD opt_from_in cursor_name"
> and "FORWARD opt_from_in cursor_name" out of
> fetch_args and pull them up into FetchStmt in the ecpg
> grammar. Depends on patch 4.
> One line in parse.pl is not clear for me:
>    $replace_line{'fetch_args'} = 'ignore';
> The genarated preproc.y is the same with or without
> this line. But as the previous version had it with
> "fetch_direction", I left it in.
>
> 6) 1f-cursorname-in-varchar-ctxdiff.patch
>
> Allow that varchar can be used as cursorname as well.
> Other character variable types were already supported.
> Depends on patch 4.
>
> 7) 1g-regressiontests-ctxdiff.patch
>
> Introduce cursor.pgc regression test for both native
> and compat mode. Depends on all patches.
>
> 8) 1h-fix-parse.pl-ctxdiff.patch
>
> Now useless patch, in the previous dynamic cursorname
> patch the following scenario occured: the same rule
> had both an "addon" and a "rule" extension. Without
> this fix, the following code was generated in preproc.y:
>    ruleA: 
>       {
>             
>       {
>             
>       }
> With the cleanup I did during this splitup, this scenario
> doesn't happen, but this fix may be considered useful.
> Applicable separately.
>
> After every patch (except 4) both the core and ecpg
> "make check" are successful.

It would've been nice if you'd changed the subject line before posting these.

Also, please update commitfest.postgresql.org appropriately.

...Robert

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

Re: [HACKERS] Unicode UTF-8 table formatting for psql text output

[Brad T. Sliger]

On Friday 02 October 2009 04:21:35 Roger Leigh wrote:
> On Wed, Sep 30, 2009 at 06:50:46PM -0400, Tom Lane wrote:
> > Roger Leigh  writes:
> > >> On Wed, 2009-09-30 at 11:03 -0400, Andrew Dunstan wrote:
> > >>> Thinking about this some more, ISTM a much better way of approaching
> > >>> it would be to provide a flag for psql to turn off the fancy
> > >>> formatting, and have pg_regress use that flag.
> > >
> > > The attached patch implements this feature.  It adds a
> > > --no-pretty-formatting/-G option to psql (naming isn't my fort�,
> > > so feel free to change it!).  This is also documented in the
> > > SGML docs, and help text.  Lastly, this option is used when invoking
> > > psql by pg_regress, which results in a working testsuite in a UTF-8
> > > locale.
> >
> > It would be a good idea to tie this to a psql magic variable (like
> > ON_ERROR_STOP) so that it could conveniently be set in ~/.psqlrc.
> > I'm not actually sure that we need a dedicated command line switch
> > for it, since you could use "-v varname" instead.
>
> I have attached a patch which implements the feature as a pset
> variable.  This also slightly simplifies some of the patch since
> the table style is passed to functions directly in printTableContent
> rather than separately.  The psql option '-P tablestyle=ascii' is
> passed to psql in pg_regress_main.c which means the testsuite doesn't
> fail any more.  The option is documented in the psql docs, and is
> also tab-completed.  Users can just put '\pset tablestyle ascii' in
> their .psqlrc if they want the old format in a UTF-8 locale.
>
> To follow up on the comments about the problems of defaulting to
> UTF-8.  There are just two potential problems with defaulting, both of
> which are problems with the user's mis-configuration of their system
> and (IMHO) not something that postgresql needs to care about.
> 1) The user lacks a font containing the line-drawing characters.
>It's very rare for a fixed-width terminal font to not contain
>these characters, and the patch as provided sticks to the most
>basic range from the VT100 set which are most commonly provided.
> 2) The user's terminal emulator is not configured to accept UTF-8
>input.  If you're using a UTF-8 locale, then this is necessary
>to display non-ASCII characters, and is done automatically by
>almost every terminal emulator out there (on Linux, they default
>to using nl_langinfo(CODESET) unless configured otherwise, which
>is a very simple change if required).  On any current GNU/Linux
>system, you have to go out of your way to break the defaults.
>
> The patch currently switches to UTF-8 automatically /when available/.
> IMO this is the correct behaviour since it will work for all but the
> handful of users who misconfigured their system, and provides an
> immediate benefit.  We spent years making UTF-8 work out of the box on
> Linux and Unix systems, and it seems a trifle unfair to penalise all
> users for the sake of the few who just didn't set up their terminal
> emulator correctly (their setup is already broken, since non-ASCII
> characters returned by queries are /already/ going to be displayed
> incorrectly).
>
>
> Regards,
> Roger

I looked at psql-utf8-table-5.patch.

Lint(1) says there is an extra trailing ',' in src/bin/psql/print.h. in 
'typedef enum printTextRule'.  The addition to 
src/bin/psql/command.c could use a comment, like adjacent code.

'ASCII' and 'UTF8' may need  tags in 
doc/src/sgml/ref/psql-ref.sgml, like adjacent 
code.  I'm not sure someone who hasn't seen this patch in action would 
immediately know what it does from the 
documentation.  `gmake html` works without the patch, but fails with the patch:

openjade:ref/psql-ref.sgml:1692:15:E: document type does not allow 
element "TERM" here; assuming 
missing "VARLISTENTRY" start-tag

After the patch, `\pset format wrapped` produces '\pset: unknown 
option: format'.  I saw this in interactive psql 
and from .psqlrc.  I think this can be fixed by changing the addition to 
src/bin/psql/command.c from an 'if' clause to 
an 'else if' clause.

Otherwise, the patch applied, built and installed.  The `gmake check` 
tests all passed with LANG and/or LC_ALL 
set.  The various tablestyle options seem to work.  The default behavior with 
respect to various LANG and LC_ALL 
values seems reasonable and can be overridden.

Thanks,

--bts

-- 
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] Rejecting weak passwords

[Bruce Momjian]

Tom Lane wrote:
> Magnus Hagander  writes:
> > That said, it would still be good to have something actually *useful*
> > in contrib. A bit more than just comparing userid and password.
> > Perhaps at least being able to set the min length, and the requirement
> > on having >1 "character class"?
> 
> +1.  There's still the issue of not being able to do much with a
> pre-MD5'd password, though.

Agreed.  I am still a little worried that people will think they are
checking for weak passwords when, for MD5, they are not.  I am also
worried that people will unknowingly reduce their security (not use MD5)
to allow weak password checking.

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

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

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

[HACKERS] 8.5 TODO: any info on "Create dump tool for write-ahead logs..." in PITR section (1.4)?

[shakahsha...@gmail.com]

Can anyone elaborate (or point me to some additional info) on the 8.5
TODO item in the "Point-In-Time Recover (PITR) section (1.4)":
  Create dump tool for write-ahead logs for use in determining
transaction id for point-in-time recovery
 This is useful for checking PITR recovery.

I poked around a bit and found some code that "walks" the WAL logs (in
src/backend/access/transam/xlog.c, I think) and I could probably
figure out how to display a WAL log file contents, but I'm hoping
someone can provide some context as to what issue this TODO item is
trying to address (i.e., what output would be useful).

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

Re: [HACKERS] [PATCH] 8.5 TODO: Add comments to output indicating version of pg_dump and of the database server

[Bruce Momjian]

Jim Cox wrote:
> On Fri, Oct 2, 2009 at 6:11 PM, Bruce Momjian  wrote:
> > Tom Lane wrote:
> >> Bruce Momjian  writes:
> >> > Are we sure we don't want a date/time in the ASCII dump? ?It would
> >> > affect database diffs, but I can see it useful too.
> >>
> >> We used to have one, and it was removed by popular demand
> >> (or actually demoted to be printed only in verbose mode, IIRC).
> >> I don't feel a need to revisit that decision.
> >
> > OK, thanks, I had forgotten.
> >
> 
> To Tom's point, the patch only outputs the new info in verbose mode,
> and in that mode a timestamp is indeed already output (it is the
> subsequent line in the ASCII dump, actually).

Thank you.

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

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

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

Re: [HACKERS] [PATCH] 8.5 TODO: Add comments to output indicating version of pg_dump and of the database server

[Jim Cox]

On Fri, Oct 2, 2009 at 6:11 PM, Bruce Momjian  wrote:
> Tom Lane wrote:
>> Bruce Momjian  writes:
>> > Are we sure we don't want a date/time in the ASCII dump?  It would
>> > affect database diffs, but I can see it useful too.
>>
>> We used to have one, and it was removed by popular demand
>> (or actually demoted to be printed only in verbose mode, IIRC).
>> I don't feel a need to revisit that decision.
>
> OK, thanks, I had forgotten.
>

To Tom's point, the patch only outputs the new info in verbose mode,
and in that mode a timestamp is indeed already output (it is the
subsequent line in the ASCII dump, actually).

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

Re: [HACKERS] [PATCH] 8.5 TODO: Add comments to output indicating version of pg_dump and of the database server

[Bruce Momjian]

Tom Lane wrote:
> Bruce Momjian  writes:
> > Are we sure we don't want a date/time in the ASCII dump?  It would
> > affect database diffs, but I can see it useful too.
> 
> We used to have one, and it was removed by popular demand
> (or actually demoted to be printed only in verbose mode, IIRC).
> I don't feel a need to revisit that decision.

OK, thanks, I had forgotten.

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

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

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

Re: [HACKERS] [PATCH] 8.5 TODO: Add comments to output indicating version of pg_dump and of the database server

[Tom Lane]

Bruce Momjian  writes:
> Are we sure we don't want a date/time in the ASCII dump?  It would
> affect database diffs, but I can see it useful too.

We used to have one, and it was removed by popular demand
(or actually demoted to be printed only in verbose mode, IIRC).
I don't feel a need to revisit that decision.

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] [PATCH] 8.5 TODO: Add comments to output indicating version of pg_dump and of the database server

[Bruce Momjian]

Jim Cox wrote:
> On Tue, Sep 29, 2009 at 12:00 AM, Alvaro Herrera
>  wrote:
> > Jim Cox escribi?:
> >
> >> Attached s/b a patch for the 8.5 TODO "Add comments to output indicating 
> >> version
> >> of pg_dump and of the database server" (pg_dump/pg_restore section, 9.2).
> >
> > Hmm, what happens if you do a pg_dump -Fc? ?Is this info saved anywhere
> > in the dump? ?Surely if thi is useful in the text dump, it is useful in
> > the binary format dumps too.
> 
> (forgot to reply all)
> 
> pg_restore's "-l, --list print summarized TOC of the archive" option
> does display the information in custom format dumps, e.g.:
> 
> prompt$ /usr/local/pgsql/bin/pg_restore -l < /tmp/mytest.dump
> ;
> ; Archive created at Tue Sep 29 13:48:37 2009
> ; dbname: mytest
> ; TOC Entries: 9
> ; Compression: -1
> ; Dump Version: 1.11-0
> ; Format: CUSTOM
> ; Integer: 4 bytes
> ; Offset: 8 bytes
> ; Dumped from database version: 8.4.0
> ; Dumped by pg_dump version: 8.4.0

Are we sure we don't want a date/time in the ASCII dump?  It would
affect database diffs, but I can see it useful too.

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

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

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

Re: [HACKERS] latest hstore patch

[Dimitri Fontaine]

"David E. Wheeler"  writes:
> On Oct 2, 2009, at 10:04 AM, Alvaro Herrera wrote:
>> Yes, that's my point too, against David's argument that "surely someone
>> must have solved it".  What we have here is a new problem, so it's not
>> so clear that there's any solution at all (yet).
>
> Yeah, I didn't mean that someone must've solved it for PostgreSQL, but that
> this sort of problem must have been solved before, wherever  binary data
> storage is an issue.

In the extension proposal you can find the idea of an upgrade hook
function called with current and new version of the extension as
arguments. This allows for the extension authors to provide the data
conversion support. We'd in fact want pg_migrator to find any columm
using a datatype offered by the extension and for each of them run:

  UPDATE t SET col = ext_upgrade_function(current_version, new_version, col);

A way to indicate that no ondisk change has been made will be a nice
optimisation, allowing to entirely skip the UPDATE step. Those
information should be easy to get from each extension's metadata (which
can point to functions, like ext_ondisk_change(version, version)) and
from pg_depend (any user column hosting an extension provided datatype
should have a tuple there, right?).

The reactions to this part of the proposal where not very warm, in
particular some where concerned that we still have a table rewrite here,
which pg_migrator tries hard to avoid, AFAIUI. But upgrading ondisk
format without rewriting table content is not something I feel able to
help provide.

Regards,
-- 
dim

PS: the original proposal for the hook let the upgrade function find
which columns to upgrade, on reflexion it's not that friendly...

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

Re: [HACKERS] Hot Standby on git

[Simon Riggs]

On Fri, 2009-10-02 at 18:04 +0300, Heikki Linnakangas wrote:
> Simon Riggs wrote:
> > I will add code to make a shutdown checkpoint be a valid starting place
> > for Hot Standby, as long as there are no in-doubt prepared transactions.
> > That way we know there are no xids still running and no locks, without
> > needing to write a record to say so.
> 
> Ok, I can live with that, and should be dead simple to implement.

First cut changes made, for discussion.

-- 
 Simon Riggs   www.2ndQuadrant.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] Hot Standby 0.2.1

[Simon Riggs]

On Sun, 2009-09-27 at 14:59 +0300, Heikki Linnakangas wrote:
> The locking in smgr_redo_commit and smgr_redo_abort doesn't look right.
> First of all, smgr_redo_abort is not holding XidGenLock and
> ProcArrayLock while modifying ShmemVariableCache->nextXid and
> ShmemVariableCache->latestCompletedXid, respectively, like
> smgr_redo_commit is. Attached patch fixes that.
> 
> But I think there's a little race condition in there anyway, as they
> remove the finished xids from known-assigned-xids list by calling
> ExpireTreeKnownAssignedTransactionIds, and
> ShmemVariableCache->latestCompletedXid is updated only after that. We're
> not holding ProcArrayLock between those two steps, like we do during
> normal transaction commit. I'm not sure what kind of issues that can
> lead to, but it seems like it can lead to broken snapshots if someone
> calls GetSnapshotData() between those steps.

I confess I didn't know what you were talking about when you wrote this
and was expecting you to have a coffee and retract. I realise now you
meant xact_redo_commit() rather than smgr_ and it makes sense at last.

I've just committed about half of your patch exactly, but not all.

I've avoided making the changes to
ShmemVariableCache->latestCompletedXid directly and moved the update to
ExpireTreeKnownAssignedTransactionIds() which already had access to
max_xid while holding ProcArrayLock. Hopefully that resolves your
comment about race condition.

Also, I noticed that you removed the line 
TransactionIdAdvance(ShmemVariableCache->nextXid);
in xact_redo_abort(). That looks like an error to me, since this follows
neither the comment nor how it is coded in xact_redo_commit(). Let me
know if there was some other reason for that line removal and I'll make
the change again.

-- 
 Simon Riggs   www.2ndQuadrant.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] Triggers on columns

[Peter Eisentraut]

On Thu, 2009-10-01 at 10:40 +0900, Itagaki Takahiro wrote:
> > Peter Eisentraut  wrote:
> > > If you want a "pretty" option on pg_get_triggerdef(), you could nowadays
> > > also implement that via a parameter default value instead of a second
> > > function.
> > 
> > OK, I'll rewrite it to use default parameter.
> 
> I tried to remove pg_get_triggerdef_ext() and add a second argument
> with default value to pg_get_triggerdef(), but there is a problem.
> 
> The definition of pg_get_triggerdef will be the following:
> DATA(insert OID = 1662 (
> pg_get_triggerdefPGNSP PGUID 12 1 0 0 f f f t f s 2 1 25 "26 16" 
> _null_ _null_ _null_
> "({CONST :consttype 16 :consttypmod -1 :constlen 1 :constbyval true 
> :constisnull false :location 41 :constvalue 1 [ 0 0 0 0 0 0 0 0 ]})"
> pg_get_triggerdef _null_ _null_ _null_ ));
> 
> The problem is in :constvalue part; It will be
> ":constvalue 1 [ 0 0 0 0 0 0 0 0 ]" on 64bit platform, but
> ":constvalue 1 [ 0 0 0 0 ]" on 32bit platform.
> I think we should not use default values in functions listed on pg_proc.h,
> so the previous patch is better than default value version.

OK, but what you can do is point both variants to the same C function
and check with PG_NARGS() with how many arguments you were called.  That
would save some of the indirections.


-- 
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] latest hstore patch

[Bruce Momjian]

David E. Wheeler wrote:
> On Oct 2, 2009, at 11:14 AM, Bruce Momjian wrote:
> 
> > Well, if it is just changed syntax, we could wack around the system
> > catalogs.  If storage changes, we have to dump/reload that data type.
> 
> Andrew solved this problem for hstore by making the new version able  
> to read the old representation. It will also update to the new  
> representation when you update a value.

Nice job.

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

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

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

Re: [HACKERS] latest hstore patch

[David E. Wheeler]

On Oct 2, 2009, at 11:14 AM, Bruce Momjian wrote:


Well, if it is just changed syntax, we could wack around the system
catalogs.  If storage changes, we have to dump/reload that data type.


Andrew solved this problem for hstore by making the new version able  
to read the old representation. It will also update to the new  
representation when you update a value.


Best,

David

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

Re: [HACKERS] LIMIT 1 == EXISTS optimization?

[Tom Lane]

Richard Rowell  writes:
> I'm no backend guru, so I was hoping someone could explain what the original
> query-plan was doing.  If all you need to know is if a row exists, why loop
> over all 3M rows?  It seems very simplistic to assume the a LIMIT 1 clause
> on the end of all EXISTS subqueries would be a general case optimization...

[ squint... ]  It should be assuming that already.

It looks like your case might have something to do with using or not
using a partial index.  Can you extract a reproducible test case?
And what PG version is this exactly?

regards, tom lane

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

[HACKERS] remove useless set of active snap

[Alvaro Herrera]

This patch removes a useless pushing of an active snapshot on
PortalStart.  Instead of push/get/pop of the active snapshot, without
any intervening use of the active snapshot, we just pass a local
snapshot down to CreateQueryDesc.

-- 
Alvaro Herrerahttp://www.CommandPrompt.com/
The PostgreSQL Company - Command Prompt, Inc.
Index: src/backend/tcop/pquery.c
===
RCS file: /home/alvherre/Code/cvs/pgsql/src/backend/tcop/pquery.c,v
retrieving revision 1.131
diff -c -p -r1.131 pquery.c
*** src/backend/tcop/pquery.c	11 Jun 2009 14:49:02 -	1.131
--- src/backend/tcop/pquery.c	2 Oct 2009 18:11:32 -
*** PortalStart(Portal portal, ParamListInfo
*** 466,471 
--- 466,472 
  	MemoryContext oldContext;
  	QueryDesc  *queryDesc;
  	int			eflags;
+ 	Snapshot	mysnap;
  
  	AssertArg(PortalIsValid(portal));
  	AssertState(portal->status == PORTAL_DEFINED);
*** PortalStart(Portal portal, ParamListInfo
*** 499,509 
  		{
  			case PORTAL_ONE_SELECT:
  
! /* Must set snapshot before starting executor. */
! if (snapshot)
! 	PushActiveSnapshot(snapshot);
! else
! 	PushActiveSnapshot(GetTransactionSnapshot());
  
  /*
   * Create QueryDesc in portal's context; for the moment, set
--- 500,506 
  		{
  			case PORTAL_ONE_SELECT:
  
! mysnap = snapshot ? snapshot : GetTransactionSnapshot();
  
  /*
   * Create QueryDesc in portal's context; for the moment, set
*** PortalStart(Portal portal, ParamListInfo
*** 511,517 
   */
  queryDesc = CreateQueryDesc((PlannedStmt *) linitial(portal->stmts),
  			portal->sourceText,
! 			GetActiveSnapshot(),
  			InvalidSnapshot,
  			None_Receiver,
  			params,
--- 508,514 
   */
  queryDesc = CreateQueryDesc((PlannedStmt *) linitial(portal->stmts),
  			portal->sourceText,
! 			mysnap,
  			InvalidSnapshot,
  			None_Receiver,
  			params,
*** PortalStart(Portal portal, ParamListInfo
*** 556,562 
  portal->portalPos = 0;
  portal->posOverflow = false;
  
- PopActiveSnapshot();
  break;
  
  			case PORTAL_ONE_RETURNING:
--- 553,558 

-- 
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] latest hstore patch

[Bruce Momjian]

David E. Wheeler wrote:
> On Oct 2, 2009, at 9:43 AM, Alvaro Herrera wrote:
> 
> >> Plus lots of stuff on pgFoundry. It's a problem that needs to be
> >> solved. Surely someone, somewhere, has solved this problem, no?
> >
> > Dump & reload?
> 
> Hahahahaha. No, really. "Dump & reload" is a phrase that end users  
> will not put up with for much longer.

Well, if it is just changed syntax, we could wack around the system
catalogs.  If storage changes, we have to dump/reload that data type.

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

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

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

[HACKERS] LIMIT 1 == EXISTS optimization?

[Richard Rowell]

I was just troubleshooting a slow query

SELECT * FROM da_answer a
 WHERE
a.provider_id IN ( SELECT visibility_bypass_providers( 0, 0 ) ) OR  --
ownership
(
  EXISTS (  -- Visibility grant
SELECT v.client_answer_id FROM sp_client_answervisibility v
  JOIN sp_sharing_group_provider_tree t ON v.sharing_group_id =
t.sharing_group_id AND t.provider_id = 0
WHERE
v.client_answer_id = a.answer_id AND v.visible = TRUE
  ) AND NOT EXISTS ( -- Visibility deny
SELECT v.client_answer_id FROM sp_client_answervisibility v
  JOIN sp_sharing_group_provider_tree t ON v.sharing_group_id =
t.sharing_group_id AND t.provider_id = 0
WHERE
v.client_answer_id = a.answer_id AND v.visible = FALSE
  ) AND --ROI goes here
a.covered_by_roi = TRUE
)

The subplan 3 in the explain seemed to be looping through 3 million rows
which explained the slowdown


QUERY
PLAN


 Bitmap Heap Scan on da_answer a  (cost=222.43..946804.85 rows=22309
width=70) (actual time=15.717..5141.001 rows=34810 loops=1)
   Recheck Cond: (question_id = 18)
   Filter: ((hashed SubPlan 1) OR ((alternatives: SubPlan 2 or hashed
SubPlan 3) AND (NOT (alternatives: SubPlan 4 or hashed SubPlan 5)) AND
covered_by_roi))
   ->  Bitmap Index Scan on daanswer_questionid  (cost=0.00..221.26
rows=35695 width=0) (actual time=6.438..6.438 rows=35060 loops=1)
 Index Cond: (question_id = 18)
   SubPlan 1
 ->  Result  (cost=0.00..0.05 rows=1 width=0) (actual time=3.683..4.621
rows=1728 loops=1)
   SubPlan 2
 ->  Merge Join  (cost=9.04..17.43 rows=1 width=0) (never executed)
   Merge Cond: (v.sharing_group_id = t.sharing_group_id)
   ->  Index Scan using
clientanswervisibility_answerid_sharinggroupid_allow on
sp_client_answervisibility v  (cost=0.00..8.38 rows=3 width=4) (never
executed)
 Index Cond: (client_answer_id = $1)
   ->  Sort  (cost=9.04..9.04 rows=4 width=4) (never executed)
 Sort Key: t.sharing_group_id
 ->  Bitmap Heap Scan on sp_sharing_group_provider_tree t
(cost=2.05..9.03 rows=4 width=4) (never executed)
   Recheck Cond: (provider_id = 0)
   ->  Bitmap Index Scan on
sharinggroupprovidertree_providerid  (cost=0.00..2.05 rows=4 width=0) (never
executed)
 Index Cond: (provider_id = 0)
   SubPlan 3
 ->  Nested Loop  (cost=0.00..52203.49 rows=2316644 width=4) (actual
time=0.053..2827.799 rows=3321883 loops=1)
   ->  Index Scan using sharinggroupprovidertree_providerid on
sp_sharing_group_provider_tree t  (cost=0.00..10.03 rows=4 width=4) (actual
time=0.024..0.030 rows=3 loops=1)
 Index Cond: (provider_id = 0)
   ->  Index Scan using spclientanswervisibility_sharinggroupid on
sp_client_answervisibility v  (cost=0.00..13011.17 rows=14877 width=8)
(actual time=0.014..512.286 rows=1107294 loops=3)
 Index Cond: (v.sharing_group_id = t.sharing_group_id)
 Filter: v.visible
   SubPlan 4
 ->  Nested Loop  (cost=0.00..8.19 rows=1 width=0) (never executed)
   ->  Index Scan using
clientanswervisibility_answerid_sharinggroupid_deny on
sp_client_answervisibility v  (cost=0.00..4.13 rows=1 width=4) (never
executed)
 Index Cond: (client_answer_id = $1)
   ->  Index Scan using
sp_sharing_group_provider_tree_sharing_group_id_key on
sp_sharing_group_provider_tree t  (cost=0.00..4.05 rows=1 width=4) (never
executed)
 Index Cond: ((t.sharing_group_id = v.sharing_group_id) AND
(t.provider_id = 0))
   SubPlan 5
 ->  Nested Loop  (cost=2993.74..35065.77 rows=542897 width=4) (actual
time=105.162..105.162 rows=0 loops=1)
   ->  Bitmap Heap Scan on sp_sharing_group_provider_tree t
(cost=2.05..9.03 rows=4 width=4) (actual time=0.037..0.047 rows=3 loops=1)
 Recheck Cond: (provider_id = 0)
 ->  Bitmap Index Scan on
sharinggroupprovidertree_providerid  (cost=0.00..2.05 rows=4 width=0)
(actual time=0.027..0.027 rows=3 loops=1)
   Index Cond: (provider_id = 0)
   ->  Bitmap Heap Scan on sp_client_answervisibility v
(cost=2991.69..8755.47 rows=3486 width=8) (actual time=35.030..35.030 rows=0
loops=3)
 Recheck Cond: ((v.sharing_group_id = t.sharing_group_id)
AND (NOT v.visible))
 ->  Bitmap Index Scan on
clientanswervisibility_answerid_sharinggroupid_deny  (cost=0.00..2991.51
rows=3486 width=0) (actual time=35.027..35.027 rows=0 loops=3)
   Index Cond: (v.sharing_group_id = t.sharing_group_id)
 Total runtime: 5170.291 ms
(42 rows)


So on a whim I tossed a LIMIT 1 into both exists clauses:

SELECT * FROM da_answer a

Re: [HACKERS] CREATE OR REPLACE FUNCTION vs ownership

[Pavel Stehule]

2009/10/2 Stephen Frost :
> * Tom Lane (t...@sss.pgh.pa.us) wrote:
>> My inclination is to think that the right behavior for REPLACE FUNCTION
>> is to keep the old proowner and proacl values, because that's what it
>> always has done and nobody's complained.
>
> +1.

+1
Pavel

>
>        Stephen
>
> -BEGIN PGP SIGNATURE-
> Version: GnuPG v1.4.9 (GNU/Linux)
>
> iEYEARECAAYFAkrGMm4ACgkQrzgMPqB3kiiA5wCgis9FDnbm3wQ2cktKDxOK2hjL
> ZqQAnRHl3rnXTki4WUBcS+iiZRWuzvSU
> =Uq6m
> -END PGP SIGNATURE-
>
>

-- 
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] latest hstore patch

[David E. Wheeler]

On Oct 2, 2009, at 10:04 AM, Alvaro Herrera wrote:


The point is it's *not* solved in the context of using pg_migrator.


Yes, that's my point too, against David's argument that "surely  
someone

must have solved it".  What we have here is a new problem, so it's not
so clear that there's any solution at all (yet).


Yeah, I didn't mean that someone must've solved it for PostgreSQL, but  
that this sort of problem must have been solved before, wherever  
binary data storage is an issue.


Best,

David

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

[HACKERS] Inappropriate failure conditions in foreign_data regression test

[Tom Lane]

By chance I noticed that the foreign_data regression test fails if run
in an installation where "bob" is a live user.  It appears to be
assuming that half a dozen other fairly common names don't belong to
real users, either.  Could we make this a little less fragile?  Maybe
call them no_such_user_N.  Or for that matter do we really need quite
so many tests of the same error condition?

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] latest hstore patch

[Alvaro Herrera]

Tom Lane wrote:
> Alvaro Herrera  writes:
> > David E. Wheeler wrote:
> >> Plus lots of stuff on pgFoundry. It's a problem that needs to be
> >> solved. Surely someone, somewhere, has solved this problem, no?
> 
> > Dump & reload?
> 
> The point is it's *not* solved in the context of using pg_migrator.

Yes, that's my point too, against David's argument that "surely someone
must have solved it".  What we have here is a new problem, so it's not
so clear that there's any solution at all (yet).

-- 
Alvaro Herrerahttp://www.CommandPrompt.com/
The PostgreSQL Company - Command Prompt, Inc.

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

Re: [HACKERS] CREATE OR REPLACE FUNCTION vs ownership

[Stephen Frost]

* Tom Lane (t...@sss.pgh.pa.us) wrote:
> My inclination is to think that the right behavior for REPLACE FUNCTION
> is to keep the old proowner and proacl values, because that's what it
> always has done and nobody's complained. 

+1.

Stephen


signature.asc
Description: Digital signature

Re: [HACKERS] latest hstore patch

[Tom Lane]

Alvaro Herrera  writes:
> David E. Wheeler wrote:
>> Plus lots of stuff on pgFoundry. It's a problem that needs to be
>> solved. Surely someone, somewhere, has solved this problem, no?

> Dump & reload?

The point is it's *not* solved in the context of using pg_migrator.

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] [PATCH] DefaultACLs

[Petr Jelinek]

Petr Jelinek napsal(a):

Robert Haas napsal(a):

I'm going to reiterate what I suggested upthread...  let's let the
default, global default ACL contain the hard-wired privileges, instead
of making them hardwired.  Then your objects will get those privileges
not because they are hard-wired, but because you haven't changed your
global default ACL to not contain them.  


That's somewhat how I implemented it although not just on global level 
but in any single filter, what we now have as defaults (before this 
patch) is used as template for default acls and you can revoke it. You 
just can't revoke anything you granted anywhere in the default acls chain.


Reminds me I forgot to adjust the docs. Attached patch fixes that (no 
other changes).


--
Regards
Petr Jelinek (PJMODOS)



defacl-2009-10-02.diff.gz
Description: Unix tar archive

-- 
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] latest hstore patch

[David E. Wheeler]

On Oct 2, 2009, at 9:43 AM, Alvaro Herrera wrote:


Plus lots of stuff on pgFoundry. It's a problem that needs to be
solved. Surely someone, somewhere, has solved this problem, no?


Dump & reload?


Hahahahaha. No, really. "Dump & reload" is a phrase that end users  
will not put up with for much longer.


Best,

David

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

Re: [HACKERS] latest hstore patch

[Alvaro Herrera]

David E. Wheeler wrote:

> Plus lots of stuff on pgFoundry. It's a problem that needs to be
> solved. Surely someone, somewhere, has solved this problem, no?

Dump & reload?

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

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

Re: [HACKERS] CREATE OR REPLACE FUNCTION vs ownership

[Tom Lane]

I wrote:
> Whichever way you think it should work, there's a bug here that goes
> back several versions, and I rather suspect we may have the same issue
> for other REPLACE-type commands ...

BTW, I looked around for related problems and don't see any.  We only
have CREATE OR REPLACE for functions, rules, and views.  Rules don't
have any permissions or shared dependencies of their own.  CREATE OR
REPLACE VIEW really does work like an ALTER --- it optionally
adds some columns, and then does a REPLACE RULE on the view rule.

I think we do have a documentation problem for CREATE OR REPLACE VIEW
too, in that it ought to mention explicitly that permissions and
non-SELECT rules for the view remain in place.

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] CREATE OR REPLACE FUNCTION vs ownership

[David E. Wheeler]

On Oct 2, 2009, at 8:49 AM, Tom Lane wrote:


The ideal is that backends will start using the new function
implementation on the next call after the REPLACE commits (but any
evaluations already in progress must of course continue with the text
they have).  We have been gradually getting closer to that ideal over
the years, although I think there are still cases where it will take a
little longer --- for instance if a SQL function gets inlined I think
the inlined code will persist for the duration of the query's  
execution.

I don't believe there are still any cases where you actually have to
reconnect to get it to notice the update.

(At least this is true for plpgsql --- not sure if all the other PLs
are equally up to speed.)


Ah, good to know. Perhaps an audit is in order…

Best,

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

Re: [HACKERS] CREATE OR REPLACE FUNCTION vs ownership

[Tom Lane]

"David E. Wheeler"  writes:
> Okay, this convinces me otherwise. But is it not in fact the case that  
> CREATE OR REPLACE FUNCTION doesn't expire the old version of the  
> function in the cache of other processes?

It is not.

> Don't those processes have  
> to reconnect in order to see the new function?

The ideal is that backends will start using the new function
implementation on the next call after the REPLACE commits (but any
evaluations already in progress must of course continue with the text
they have).  We have been gradually getting closer to that ideal over
the years, although I think there are still cases where it will take a
little longer --- for instance if a SQL function gets inlined I think
the inlined code will persist for the duration of the query's execution.
I don't believe there are still any cases where you actually have to
reconnect to get it to notice the update.

(At least this is true for plpgsql --- not sure if all the other PLs
are equally up to speed.)

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] Issues for named/mixed function notation patch

[Jeff Davis]

On Fri, 2009-10-02 at 16:06 +0200, Pavel Stehule wrote:
> see attachment, please

Thank you, marked as "ready for committer". 

Regards,
Jeff Davis


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

Re: [HACKERS] latest hstore patch

[David E. Wheeler]

On Oct 2, 2009, at 8:20 AM, Bruce Momjian wrote:


Most modules just install functions, which are easily
uninstalled/reinstalled.  A data type like hstore is more complicated
assuming it is the data type that is changing and not the support
functions.


Lots of modules install data types. From contrib:

* hstore
* uin
* citext
* cube
* inarray
* ltree

Plus lots of stuff on pgFoundry. It's a problem that needs to be  
solved. Surely someone, somewhere, has solved this problem, no?


Best,

David

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

Re: [HACKERS] CREATE OR REPLACE FUNCTION vs ownership

[Robert Haas]

On Fri, Oct 2, 2009 at 10:25 AM, Caleb Welton  wrote:
> Right - so the subtle point here is that ALTER means something
> different from CREATE OR REPLACE.  "ALTER" means to make a
> modification to something; to change it; to adjust one particular
> property of the object without disturbing the others.  On the other
> hand, "REPLACE" means to get rid of something and replace it with an
> entirely new thing.  I think that is exactly why we have ALTER TABLE
> but CREATE OR REPLACE FUNCTION.
>
> Now, if we want to have an ALTER FUNCTION that replaces the function
> definition and leaves the owner intact - fine!  But that is not what
> REPLACE means.
>
> By this argument CREATE OR REPLACE FUNCTION should be able to change the
> return type of the function; which it can't.

No, because when we REPLACE we (rightly) prohibit a replacement that
is incompatible with the existing uses of the function.

...Robert

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

Re: [HACKERS] CREATE OR REPLACE FUNCTION vs ownership

[David E. Wheeler]

On Oct 2, 2009, at 7:49 AM, Tom Lane wrote:


But in fact CREATE OR REPLACE is *not* meant to be the same as DROP
followed by CREATE.  What it is meant to do is allow you to replace  
the

implementation of the function while existing callers see it as still
being the same function.  Thus we prevent you from changing the name,
arguments, or result type of the function.  If we were to replace the
permissions that would result in a more insidious but still real  
reason
why former callers would suddenly stop working.  In effect,  
permissions

are part of the function's API.


Okay, this convinces me otherwise. But is it not in fact the case that  
CREATE OR REPLACE FUNCTION doesn't expire the old version of the  
function in the cache of other processes? Don't those processes have  
to reconnect in order to see the new function?


Best,

David

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

Re: [HACKERS] latest hstore patch

[Bruce Momjian]

Tom Lane wrote:
> Bruce Momjian  writes:
> > Are there any pg_migrator affects in this patch?  We had discussed this
> > issue in the past with this patch.
> 
> The code is upward compatible with the old on-disk format, so that
> end of things is fine.
> 
> There's still the issue of how to get the improved module definition
> (new functions etc) into a migrated database.  That's not specific
> to hstore in any way though, it would affect any contrib module that
> had added stuff in a new release.

Most modules just install functions, which are easily
uninstalled/reinstalled.  A data type like hstore is more complicated
assuming it is the data type that is changing and not the support
functions.

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

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

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

Re: [HACKERS] latest hstore patch

[Tom Lane]

Bruce Momjian  writes:
> Are there any pg_migrator affects in this patch?  We had discussed this
> issue in the past with this patch.

The code is upward compatible with the old on-disk format, so that
end of things is fine.

There's still the issue of how to get the improved module definition
(new functions etc) into a migrated database.  That's not specific
to hstore in any way though, it would affect any contrib module that
had added stuff in a new release.

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] latest hstore patch

[Bruce Momjian]

Tom Lane wrote:
> Andrew Gierth  writes:
> > Hstore patch incorporating changes as previously discussed.
> > In addition the requested new features of conversions to and from
> > array formats have been added (with docs).
> 
> Applied with some mostly-cosmetic editorialization.

Are there any pg_migrator affects in this patch?  We had discussed this
issue in the past with this patch.

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

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

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

Re: [HACKERS] latest hstore patch

[Bruce Momjian]

Tom Lane wrote:
> > I intentionally avoided hstore_to_array because it would be unclear
> > which one it meant (the 1-d or 2-d result).
> 
> hstore_to_list seems like a pretty horrible name though for something
> that produces an array.  I also note that "array" means "1-D array"
> according to no less an authority than the SQL standard ;-).  I think
> we could live with hstore_to_array and hstore_to_matrix.  Thoughts,
> other ideas?

Off topic, but in normal English usage I thought 'vector' was a 1-D
array, and an array could be any number of dimensions.

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

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

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

Re: [HACKERS] Hot Standby on git

[Heikki Linnakangas]

Simon Riggs wrote:
> I will add code to make a shutdown checkpoint be a valid starting place
> for Hot Standby, as long as there are no in-doubt prepared transactions.
> That way we know there are no xids still running and no locks, without
> needing to write a record to say so.

Ok, I can live with that, and should be dead simple to implement.

-- 
  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] CREATE OR REPLACE FUNCTION vs ownership

[Tom Lane]

KaiGai Kohei  writes:
> Robert Haas wrote:
>> I disagree.  I think David has this one right.  I expect the results
>> of CREATE OR REPLACE to be the same as the result of CREATE would have
>> been had the object not existed.

> If so, it seems to me CREATE OR REPLACE is equivalent to a pair of
> actions: 1) DROP FUNCTION (if exist) and 2) CREATE FUNCTION.

But in fact CREATE OR REPLACE is *not* meant to be the same as DROP
followed by CREATE.  What it is meant to do is allow you to replace the
implementation of the function while existing callers see it as still
being the same function.  Thus we prevent you from changing the name,
arguments, or result type of the function.  If we were to replace the
permissions that would result in a more insidious but still real reason
why former callers would suddenly stop working.  In effect, permissions
are part of the function's API.

Another big reason not to change it is that it's worked like this ever
since we had function permissions at all.  It seems highly likely that
we could introduce silent security breakages into applications that have
been depending on the old behavior.

I think the original reasoning for the behavior may have been that
ownership/permissions are the only properties of the function that
cannot be specified in the CREATE OR REPLACE FUNCTION syntax.  It makes
sense to leave them alone, because that is more likely to be right than
reverting to default.

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] Postgres server goes in recovery mode repeteadly

[Alvaro Herrera]

daveg escribió:

> I work with Kunal and have been looking into this. It appears to be the same
> as the bug described in:
> 
>   http://archives.postgresql.org/pgsql-bugs/2009-09/msg00355.php
> 
> as I have localized it to a NULL pointer deference in
> RelationCacheInitializePhase2() as well. Tom speculates in:
> 
>   http://archives.postgresql.org/pgsql-bugs/2009-09/msg00372.php
> 
> that large numbers of table drops might trigger this. The system in question
> creates and drops temp tables at a high rate which tends to confirm this. 

Did you test the patch posted by Tom?

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

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

Re: [HACKERS] CREATE OR REPLACE FUNCTION vs ownership

[Caleb Welton]

On 10/1/09 9:26 PM, "Robert Haas"  wrote:

2009/10/1 KaiGai Kohei :
> Robert Haas wrote:
>> On Thu, Oct 1, 2009 at 8:52 PM, Euler Taveira de Oliveira
>>  wrote:
>>> David E. Wheeler escreveu:
 On Oct 1, 2009, at 3:42 PM, Tom Lane wrote:

> My inclination is to think that the right behavior for REPLACE FUNCTION
> is to keep the old proowner and proacl values, because that's what it
> always has done and nobody's complained.  But I suppose a case could
> be made that you're completely replacing the function and so you should
> replace its ownership/permissions too.  The CREATE FUNCTION reference
> page fails to specify either way, which is a documentation bug as well.
>
> Comments?
 The latter, I think. If I replace a function, I should be the new owner.
 To me it makes no sense for someone else to own it.

>>> Hmm... Using the same logic, if I add a new column should I be the table
>>> owner? If you're changing the function that is because you have permission.
>>>
>>> IMHO the owner should be preserved. In my mind, REPLACE is for changing the
>>> content and not the properties (name, owner, etc).
>
> If so, it seems to me CREATE OR REPLACE is equivalent to ALTER FUNCTION
> with currently unsupported option. In this case, it is not necessary to
> check CREATE privilege on the namespace because it does not affect to
> its name/schema.

Right - so the subtle point here is that ALTER means something
different from CREATE OR REPLACE.  "ALTER" means to make a
modification to something; to change it; to adjust one particular
property of the object without disturbing the others.  On the other
hand, "REPLACE" means to get rid of something and replace it with an
entirely new thing.  I think that is exactly why we have ALTER TABLE
but CREATE OR REPLACE FUNCTION.

Now, if we want to have an ALTER FUNCTION that replaces the function
definition and leaves the owner intact - fine!  But that is not what
REPLACE means.

By this argument CREATE OR REPLACE FUNCTION should be able to change the
return type of the function; which it can't.

>> I disagree.  I think David has this one right.  I expect the results
>> of CREATE OR REPLACE to be the same as the result of CREATE would have
>> been had the object not existed.
>
> If so, it seems to me CREATE OR REPLACE is equivalent to a pair of
> actions: 1) DROP FUNCTION (if exist) and 2) CREATE FUNCTION.

Except that you don't have to drop and recreate the dependencies, if any.

...Robert

And there are things that you can do with a real drop/create that you cannot
do with create/replace.   In other words I agree with others that this is more
of an ALTER operation.

-Caleb

Re: [HACKERS] CommitFest 2009-09, two weeks on

[Boszormenyi Zoltan]

Michael Meskes írta:
> On Thu, Oct 01, 2009 at 09:05:55PM +0200, Boszormenyi Zoltan wrote:
>   
>> Yes, but technical problems and solutions do. ECPG claims
>> to be ESQL/C compatible, but at places it's only half compatible.
>> 
>
> Where does it claim to be fully compatible?
>   

I didn't say it claims to be fully compatible, but it's
a "hint" that "ecpg -C INFORMIX[_SE]" exists. :-)

>> This comment is misleading and reflects quite a narrow POV.
>> Not only OPEN and DECLARE may be out of scope,
>> but FETCH and CLOSE as well. The reason why ESQL/C
>> allows this construct is that this ultimately allows using
>> embedded SQL in event-driven code in a straightforward way.
>> For this purpose, native ECPG code is not usable currently,
>> or you need programming tricks, like tracking whether the
>> cursor is open and protecting DECLARE and OPEN under
>> some conditional branch to avoid double open, etc. A straight
>> DECLARE, OPEN, FETCH(s) and CLOSE series in
>> the same function is only good for batch programming.
>> 
>
> Examples?
>   

I took my outofscope.pgc example from our "out of
scope" patch and shortened it. Compare the ecpg-native and
compat outputs and the esql output of the same file. The ecpg
outputs are generated with 8.4.1 plus out patches added, the
native output differs only from 8.3.7's ecpg in the amount of
whitespaces emitted between literals. The get_record1()
function can be called from a button-handler, e.g. when pressing
PgDn, or similar... No tricks needed, straightforward code.

Best regards,
Zoltán Böszörményi

-- 
Bible has answers for everything. Proof:
"But let your communication be, Yea, yea; Nay, nay: for whatsoever is more
than these cometh of evil." (Matthew 5:37) - basics of digital technology.
"May your kingdom come" - superficial description of plate tectonics

--
Zoltán Böszörményi
Cybertec Schönig & Schönig GmbH
http://www.postgresql.at/

#include 
#include 
#include 
#include 

exec sql begin declare section;
exec sql include struct.h;
exec sql end declare section;

exec sql whenever sqlerror stop;

static void
get_var1(MYTYPE **myvar0, MYNULLTYPE **mynullvar0)
{
exec sql begin declare section;
MYTYPE  *myvar = malloc(sizeof(MYTYPE));
MYNULLTYPE  *mynullvar = malloc(sizeof(MYNULLTYPE));
exec sql end declare section;

/* Test DECLARE ... SELECT ... INTO with pointers */

exec sql declare mycur cursor for select * INTO :myvar :mynullvar from 
a1;

if (sqlca.sqlcode != 0)
exit(1);

*myvar0 = myvar;
*mynullvar0 = mynullvar;
}

static void
open_cur1(void)
{
exec sql open mycur;

if (sqlca.sqlcode != 0)
exit(1);
}

static void
get_record1(void)
{
exec sql fetch mycur;

if (sqlca.sqlcode != 0 && sqlca.sqlcode != SQLNOTFOUND)
exit(1);
}

static void
close_cur1(void)
{
exec sql close mycur;

if (sqlca.sqlcode != 0)
exit(1);
}

int
main (void)
{
MYTYPE  *myvar;
MYNULLTYPE  *mynullvar;

char msg[128];

ECPGdebug(1, stderr);

exec sql connect to "test";


exec sql create table a1(id serial primary key, t text, d1 numeric, d2 
float8, c character(10));

exec sql insert into a1(id, t, d1, d2, c) values (default, 'a', 1.0, 2, 
'a');
exec sql insert into a1(id, t, d1, d2, c) values (default, null, null, 
null, null);
exec sql insert into a1(id, t, d1, d2, c) values (default, '"a"', -1.0, 
'nan'::float8, 'a');
exec sql insert into a1(id, t, d1, d2, c) values (default, 'b', 2.0, 3, 
'b');

exec sql commit;

/* Test out-of-scope DECLARE/OPEN/FETCH/CLOSE */

get_var1(&myvar, &mynullvar);
open_cur1();

while (1)
{
memset(myvar, 0, sizeof(MYTYPE));
get_record1();
if (sqlca.sqlcode == SQLNOTFOUND)
break;
printf("id=%d%s t='%s'%s d1=%lf%s d2=%lf%s c = '%s'%s\n",
myvar->id, mynullvar->id ? " (NULL)" : "",
myvar->t, mynullvar->t ? " (NULL)" : "",
myvar->d1, mynullvar->d1 ? " (NULL)" : "",
myvar->d2, mynullvar->d2 ? " (NULL)" : "",
myvar->c, mynullvar->c ? " (NULL)" : "");
}

close_cur1();

exec sql drop table a1;

exec sql commit;

exec sql disconnect all;

return (0);
}

struct mytype {
	int	id;
	char	t[64];
	double	d1; /* dec_t */
	double	d2;
	char	c[30];
};
typedef struct mytype MYTYPE;

struct mynulltype {
	int	id;
	int	t;
	int	d1;
	int	d2;
	int	c;
};
typedef struct mynulltype MYNULLTYPE;
/* Processed by ecpg (4.5.0) */
/* These include files are added by the preprocessor */
#include 
#include 
#include 
/* End of automatic include section */

#line 1 "outofscope.ec"

[HACKERS] commented out para in docs

[Andrew Dunstan]

We have this para in the CREATE TABLE docs, commented out, as shown. It 
seems to have been like that for a long time (see 
).


Surely we should either include it or remove it. Having it commented out 
in the docs seems like just noise.


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] FSM search modes

[Kevin Grittner]

Tom Lane  wrote: 
> "Kevin Grittner"  writes:
>> [pages with free space or total pages in relation?]
> 
> It's going to be the latter --- we do not know, and are *not* going
> to invest the cycles to find out, how many pages have a useful
> amount of free space.  Even finding out the relation physical length
> might be more cycles than we want to spend here ...
 
OK, after mulling this over for a while, I suspect we'd do pretty well
with starting to consider resetting the sweep after hitting 50% of the
last known relation size (or whatever best approximation is available
at low cost), and using a reset probability of 1 / (max_connections *
4).  That gives about a 77.8% chance of getting to at least
max_connections before resetting the sweep.  Since it leaves about an
8.2% chance of getting to at least ten times max_connections pages
before resetting the sweep, I'm inclined to think we'd want to start
that at 50% of the relation rather than waiting until we get to the
last quarter.  As one more data point to consider, if someone has 2000
connections configured (which I've seen mentioned in many posts), you
would get to 50,000 pages past the point where you start using this
technique one time out of 500.
 
-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] CommitFest 2009-09, two weeks on

[Michael Meskes]

On Thu, Oct 01, 2009 at 09:05:55PM +0200, Boszormenyi Zoltan wrote:
> Yes, but technical problems and solutions do. ECPG claims
> to be ESQL/C compatible, but at places it's only half compatible.

Where does it claim to be fully compatible?

> This comment is misleading and reflects quite a narrow POV.
> Not only OPEN and DECLARE may be out of scope,
> but FETCH and CLOSE as well. The reason why ESQL/C
> allows this construct is that this ultimately allows using
> embedded SQL in event-driven code in a straightforward way.
> For this purpose, native ECPG code is not usable currently,
> or you need programming tricks, like tracking whether the
> cursor is open and protecting DECLARE and OPEN under
> some conditional branch to avoid double open, etc. A straight
> DECLARE, OPEN, FETCH(s) and CLOSE series in
> the same function is only good for batch programming.

Examples?

Michael

-- 
Michael Meskes
Michael at Fam-Meskes dot De, Michael at Meskes dot (De|Com|Net|Org)
Michael at BorussiaFan dot De, Meskes at (Debian|Postgresql) dot Org
ICQ: 179140304, AIM/Yahoo/Skype: michaelmeskes, Jabber: mes...@jabber.org
Go VfL Borussia! Go SF 49ers! Use Debian GNU/Linux! Use PostgreSQL!

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

Re: [HACKERS] Hot Standby on git

[Simon Riggs]

On Fri, 2009-10-02 at 13:52 +0300, Heikki Linnakangas wrote:
> Simon Riggs wrote:
> > I'd rather just skip this for now. It's a minor case anyway and there's
> > nothing stopping writing their own RunningXactData records with a
> > function, if it is needed. I can add a function for that.
> 
> That won't help. There's no way to have it in a right place wrt. the
> shutdown checkpoint if it's triggered by a user-callable function.

I notice that you avoid saying "yes, I agree we should remove the two
checks".

I will add code to make a shutdown checkpoint be a valid starting place
for Hot Standby, as long as there are no in-doubt prepared transactions.
That way we know there are no xids still running and no locks, without
needing to write a record to say so.

-- 
 Simon Riggs   www.2ndQuadrant.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] TODO item: Allow more complex user/database default GUC settings

[Bernd Helmle]

--On 1. Oktober 2009 17:22:06 -0400 Alvaro Herrera 
 wrote:



- ALTER ROLE ... IN DATABASE is missing some documentation. If you
want, i can work on this.


Please.


Here's a patch for this. I've kept it separately, so it's easier for you to 
merge it into

your version.

--
Thanks

Bernd

alter_role_docs.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] Using results from INSERT ... RETURNING

[Marko Tiikkaja]

Robert Haas wrote:

Now the point here is that I eventually want to be able to write
something like this:

with foo as (insert into project (name) values ('Writeable CTEs')
returning id) select * from foo;

...but how does this get me any closer?  It seems to me that the plan
for THAT statement has to be a CTE scan over top of BOTH of the
inserts, but here I have two insert nodes that comprise two separate
plans.  The DML node, as presently implemented, supports a list of
plans, but they all have to be of the same type, so it's really only
useful for handling append, and as previously discussed, it's not
clear that the proposed handling is any better than what we already
have.


I don't think you should be able to do this.  I'm not too familiar with
rules, but in your example the rule doesn't modify the output of the
INSERT .. RETURNING so I think it shouldn't do that here either.  How I
see it is that in your example the INSERT INTO shadow would be added to
the top level instead of the CTE and the plan would look something like
this:


 CTE Scan on foo  (cost=0.01..0.03 rows=1 width=4)
   CTE foo
 ->  Insert  (cost=0.00..0.01 rows=1 width=0)
   ->  Result  (cost=0.00..0.01 rows=1 width=0)

 Insert  (cost=0.00..0.01 rows=1 width=0)
   ->  Result  (cost=0.00..0.01 rows=1 width=0)

so you would get the RETURNING output from the CTE and the INSERT to the
shadow table would be executed separately.  I'm not saying that we don't
want to provide the means to do this, but writeable CTEs alone aren't
meant to handle 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] Unicode UTF-8 table formatting for psql text output

[Roger Leigh]

On Wed, Sep 30, 2009 at 06:50:46PM -0400, Tom Lane wrote:
> Roger Leigh  writes:
> >> On Wed, 2009-09-30 at 11:03 -0400, Andrew Dunstan wrote:
> >>> Thinking about this some more, ISTM a much better way of approaching 
> >>> it would be to provide a flag for psql to turn off the fancy 
> >>> formatting, and have pg_regress use that flag.
> 
> > The attached patch implements this feature.  It adds a
> > --no-pretty-formatting/-G option to psql (naming isn't my fort�,
> > so feel free to change it!).  This is also documented in the
> > SGML docs, and help text.  Lastly, this option is used when invoking
> > psql by pg_regress, which results in a working testsuite in a UTF-8
> > locale.
> 
> It would be a good idea to tie this to a psql magic variable (like
> ON_ERROR_STOP) so that it could conveniently be set in ~/.psqlrc.
> I'm not actually sure that we need a dedicated command line switch
> for it, since you could use "-v varname" instead.

I have attached a patch which implements the feature as a pset
variable.  This also slightly simplifies some of the patch since
the table style is passed to functions directly in printTableContent
rather than separately.  The psql option '-P tablestyle=ascii' is
passed to psql in pg_regress_main.c which means the testsuite doesn't
fail any more.  The option is documented in the psql docs, and is
also tab-completed.  Users can just put '\pset tablestyle ascii' in
their .psqlrc if they want the old format in a UTF-8 locale.

To follow up on the comments about the problems of defaulting to
UTF-8.  There are just two potential problems with defaulting, both of
which are problems with the user's mis-configuration of their system
and (IMHO) not something that postgresql needs to care about.
1) The user lacks a font containing the line-drawing characters.
   It's very rare for a fixed-width terminal font to not contain
   these characters, and the patch as provided sticks to the most
   basic range from the VT100 set which are most commonly provided.
2) The user's terminal emulator is not configured to accept UTF-8
   input.  If you're using a UTF-8 locale, then this is necessary
   to display non-ASCII characters, and is done automatically by
   almost every terminal emulator out there (on Linux, they default
   to using nl_langinfo(CODESET) unless configured otherwise, which
   is a very simple change if required).  On any current GNU/Linux
   system, you have to go out of your way to break the defaults.

The patch currently switches to UTF-8 automatically /when available/.
IMO this is the correct behaviour since it will work for all but the
handful of users who misconfigured their system, and provides an
immediate benefit.  We spent years making UTF-8 work out of the box on
Linux and Unix systems, and it seems a trifle unfair to penalise all
users for the sake of the few who just didn't set up their terminal
emulator correctly (their setup is already broken, since non-ASCII
characters returned by queries are /already/ going to be displayed
incorrectly).


Regards,
Roger

-- 
  .''`.  Roger Leigh
 : :' :  Debian GNU/Linux http://people.debian.org/~rleigh/
 `. `'   Printing on GNU/Linux?   http://gutenprint.sourceforge.net/
   `-GPG Public Key: 0x25BFB848   Please GPG sign your mail.
diff --git a/doc/src/sgml/ref/psql-ref.sgml b/doc/src/sgml/ref/psql-ref.sgml
index 85e9375..ad297f8 100644
--- a/doc/src/sgml/ref/psql-ref.sgml
+++ b/doc/src/sgml/ref/psql-ref.sgml
@@ -1689,6 +1689,28 @@ lo_import 152801
   
   
 
+  tablestyle
+  
+  
+  Sets the style of text table output to one
+  of ascii, or utf8.
+  Unique abbreviations are allowed.  (That would mean one
+  letter is enough.)  utf8 will be selected
+  by default if supported by your locale,
+  otherwise ascii will be used.
+  
+
+  
+  UTF8 use Unicode box drawing characters.
+  
+
+  
+  ASCII use plain ASCII characters.
+  
+
+  
+  
+
   
   columns
   
diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c
index da57eb4..e9b6fe0 100644
--- a/src/bin/psql/command.c
+++ b/src/bin/psql/command.c
@@ -1772,6 +1772,24 @@ do_pset(const char *param, const char *value, printQueryOpt *popt, bool quiet)
 			printf(_("Output format is %s.\n"), _align2string(popt->topt.format));
 	}
 
+	if (strcmp(param, "tablestyle") == 0)
+	{
+		if (!value)
+			;
+		else if (pg_strncasecmp("ascii", value, vallen) == 0)
+			popt->topt.table_style = &asciiformat;
+		else if (pg_strncasecmp("utf8", value, vallen) == 0)
+			popt->topt.table_style = &utf8format;
+		else
+		{
+			psql_error("\\pset: allowed table styles are ascii, utf8\n");
+			return false;
+		}
+
+		if (!quiet)
+			printf(_("Table style is %s.\n"), popt->topt.table_style->name);
+	}
+
 	/* set border style/width */
 	else if (strcmp(param, "border") == 0)
 	{
diff --git a/src/bi

Re: [HACKERS] Hot Standby on git

[Heikki Linnakangas]

Simon Riggs wrote:
> I'd rather just skip this for now. It's a minor case anyway and there's
> nothing stopping writing their own RunningXactData records with a
> function, if it is needed. I can add a function for that.

That won't help. There's no way to have it in a right place wrt. the
shutdown checkpoint if it's triggered by a user-callable function.

-- 
  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] Hot Standby on git

[Simon Riggs]

On Fri, 2009-10-02 at 11:26 +0300, Heikki Linnakangas wrote:
> Simon Riggs wrote:
> > On Fri, 2009-10-02 at 10:43 +0300, Heikki Linnakangas wrote:
> > 
> >> It seems dangerous to write a WAL record after the shutdown checkpoint.
> >> It will be overwritten by subsequent startup, which is a recipe for 
> >> trouble.
> > 
> > I've said its a corner case and not worth spending time on. I'm putting
> > it in at your request. If it's not correct before and not correct after,
> > where exactly do you want it?
> 
> I don't know. Perhaps it should go between the REDO pointer of the
> shutdown checkpoint and the checkpoint record itself. Or maybe the
> information should be included in the checkpoint record itself.

I've implemented this but it requires us to remove two checks - one at
shutdown and one at startup on a shutdown checkpoint. I'm not happy
doing that and would like to put them back.

I'd rather just skip this for now. It's a minor case anyway and there's
nothing stopping writing their own RunningXactData records with a
function, if it is needed. I can add a function for that.

-- 
 Simon Riggs   www.2ndQuadrant.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] Using results from INSERT ... RETURNING

[Marko Tiikkaja]

Robert Haas wrote:

Thanks.  I read through this patch some more tonight and I guess I am
a bit confused about what it accomplishes.  AIUI, the point here is to
lay the groundwork for a future patch to allow writeable CTEs, and I
guess I'm not understanding how it's going to do that.


The purpose of this patch is only to move INSERT, UPDATE and DELETE into
plan nodes because for writeable CTEs, we can't use the currently
existing way of handling those operations.  My previous approach was to
only add a special-case node for RETURNING and use the top-level
INSERT/UPDATE/DELETE handling like now, but that would've lead to
copying a lot of code, as Tom pointed out in
http://archives.postgresql.org/pgsql-hackers/2009-07/msg01217.php .  In
that same message, Tom suggested the current approach which to me seems
like the best way to tackle this.  This patch alone doesn't accomplish
anything, but supporting writeable CTEs will be a lot easier as seen in
the git repo David pointed you to.

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] Issues for named/mixed function notation patch

[Pavel Stehule]

2009/10/2 Jeff Davis :
> On Thu, 2009-10-01 at 17:56 +1000, Brendan Jurd wrote:
>> I've had a look through the documentation and cleaned up a few things.
>
> Thanks, that is helpful. I have included that along with some other
> comment updates in the attached patch.
>
> Paval,
>
> In ProcedureCreate(), you match the argument names to see if anything
> changed (in which case you raise an error). The code for that looks more
> complex than I expected because it keeps track of the two argument lists
> using different array indexes (i and j). Is there a reason it you can't
> just iterate through with something like:
>
>  if (p_oldargmodes[i] == PROARGMODE_OUT ||
>      p_oldargmodes[i] == PROARGMODE_TABLE)
>    continue;
>
>  if (strcmp(p_oldargnames[i], p_names[i]) != 0)
>    elog(ERROR, ...

I testing visible interface from outside.

from outside the following functions are same:

foo1(in a1, in a2, in a3, out a1, out a2, out a3)
foo2(in a1, out a1, in a2, out a2, in a3, out a3)

so the used test is immune to this change.

>
> I'm oversimplifying, of course, but I don't understand why you need both
> i and j. Also, there is some unnecessarily verbose code like:
>
>  if (p_modes == NULL || (p_modes != NULL ...
>

when p_modes is null,then all arguments are input. So input parameter
is when p_modes is null (all parameters are input) or is "in",
"inout", "variadic".

> In func_get_detail(), there is:
>
>  /* shouldn't happen, FuncnameGetCandidates messed up */
>  if (best_candidate->ndargs > pform->pronargdefaults)
>    elog(ERROR, "not enough default arguments");
>

best_candidate->ndargs ~ use n default values, it have to be less or
equal than declared defaults in pgproc.

> Why is it only an error if ndargs is greater? Shouldn't they be equal?
>

ndargs == pronargdefaults is correct - it means, use all declared
defaults. But, raise exception when you would to use more defaults
than is declared.

>  /*
>   * Actually only first nargs field of best_candidate->args is valid,
>   * Now, we have to refresh last ndargs items.
>   */
>
> Can you explain the above comment?
>

this is not good formulation. It means, so in this moment we processed
nargs fields, and we have to process others ndargs fields. But i named
or mixed notation, the processed fields should not be first nargs
fields (like positional notation). There should be a gap, that are
filled by defaults. Gaps are identified via bitmap created on cycle
above. In this cycle is processed positional parameters (with
increasing i) and named parameters. Because positional parameters have
to be front of named parameters, then we don't need increase i when
process named p.,

> Please review Brendan and my patches and combine them with your patch.
>

yes, I'll go on this evening, thank you.

Pavel

> Regards,
>        Jeff Davis
>

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

Re: [HACKERS] "make install" now tries to build the documentation

[Peter Eisentraut]

On Thu, 2009-10-01 at 15:17 -0400, Alvaro Herrera wrote:
> Peter Eisentraut wrote:
> 
> > I'm not exactly sure what the goal is, however.  You built the
> > documentation at some point.  Then it gets updated when necessary.  You
> > can delete the documentation by running make -C doc maintainer-clean.
> > Then it's gone forever and never reappears unless you explicitly build
> > it again.
> 
> I think this bit is missing something; the html-stamp file is created on
> the builddir, but maintainer-clean is trying to delete it from the
> source dir.

Yeah, I missed something there, apparently.  I don't use vpath builds so
much, so if you find more things like that, please clean them up or tell
me.


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

Re: [HACKERS] [PATCH] DefaultACLs

[Petr Jelinek]

Robert Haas napsal(a):

On Thu, Oct 1, 2009 at 1:37 PM, Tom Lane  wrote:
  

Petr Jelinek  writes:


because it seems like merging privileges seems to be acceptable for most
(although I am not sure I like it, but I don't have better solution for
managing conflicts), I changed the patch to do just that.
  

It's not clear to me whether we have consensus on this approach.
Last chance for objections, anyone?

The main argument I can see against doing it this way is that it doesn't
provide a means for overriding the hard-wired public grants for object
types that have such (principally functions).  I think that a reasonable
way to address that issue would be for a follow-on patch that allows
changing the hard-wired default privileges for object types.  It might
well be that no one cares enough for it to matter, though.  I think that
in most simple cases what's needed is a way to add privileges, not
subtract them --- and we're already agreed that this mechanism is only
meant to simplify simple cases.



I'm going to reiterate what I suggested upthread...  let's let the
default, global default ACL contain the hard-wired privileges, instead
of making them hardwired.  Then your objects will get those privileges
not because they are hard-wired, but because you haven't changed your
global default ACL to not contain them.
  


That's somewhat how I implemented it although not just on global level 
but in any single filter, what we now have as defaults (before this 
patch) is used as template for default acls and you can revoke it. You 
just can't revoke anything you granted anywhere in the default acls chain.


--
Regards
Petr Jelinek (PJMODOS)

Re: [HACKERS] Hot Standby on git

[Simon Riggs]

On Fri, 2009-10-02 at 11:26 +0300, Heikki Linnakangas wrote:
> Simon Riggs wrote:
> > On Fri, 2009-10-02 at 10:43 +0300, Heikki Linnakangas wrote:
> > 
> >> It seems dangerous to write a WAL record after the shutdown checkpoint.
> >> It will be overwritten by subsequent startup, which is a recipe for 
> >> trouble.
> > 
> > I've said its a corner case and not worth spending time on. I'm putting
> > it in at your request. If it's not correct before and not correct after,
> > where exactly do you want it?
> 
> I don't know. Perhaps it should go between the REDO pointer of the
> shutdown checkpoint and the checkpoint record itself. 

That would seem minimally invasive approach and would appear to work for
both cases. Will do.

-- 
 Simon Riggs   www.2ndQuadrant.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] Issues for named/mixed function notation patch

[Jeff Davis]

On Thu, 2009-10-01 at 17:56 +1000, Brendan Jurd wrote:
> I've had a look through the documentation and cleaned up a few things.

Thanks, that is helpful. I have included that along with some other
comment updates in the attached patch.

Paval,

In ProcedureCreate(), you match the argument names to see if anything
changed (in which case you raise an error). The code for that looks more
complex than I expected because it keeps track of the two argument lists
using different array indexes (i and j). Is there a reason it you can't
just iterate through with something like:

  if (p_oldargmodes[i] == PROARGMODE_OUT ||
  p_oldargmodes[i] == PROARGMODE_TABLE)
continue;

  if (strcmp(p_oldargnames[i], p_names[i]) != 0)
elog(ERROR, ...

I'm oversimplifying, of course, but I don't understand why you need both
i and j. Also, there is some unnecessarily verbose code like:

  if (p_modes == NULL || (p_modes != NULL ...

In func_get_detail(), there is:

  /* shouldn't happen, FuncnameGetCandidates messed up */
  if (best_candidate->ndargs > pform->pronargdefaults)
elog(ERROR, "not enough default arguments");

Why is it only an error if ndargs is greater? Shouldn't they be equal?

  /*
  
   * Actually only first nargs field of best_candidate->args is valid,
   * Now, we have to refresh last ndargs items.
   */

Can you explain the above comment?

Please review Brendan and my patches and combine them with your patch.

Regards,
Jeff Davis
diff --git a/doc/src/sgml/syntax.sgml b/doc/src/sgml/syntax.sgml
index 50c4128..542646d 100644
--- a/doc/src/sgml/syntax.sgml
+++ b/doc/src/sgml/syntax.sgml
@@ -1505,6 +1505,10 @@ sqrt(2)
 The list of built-in functions is in .
 Other functions can be added by the user.

+
+   
+ See  for more details.
+   
   
 
   
diff --git a/src/backend/catalog/namespace.c b/src/backend/catalog/namespace.c
index 9e8ccfa..1c06885 100644
--- a/src/backend/catalog/namespace.c
+++ b/src/backend/catalog/namespace.c
@@ -651,21 +651,19 @@ FuncnameGetCandidates(List *names, int nargs, List *argnames,
 		int			effective_nargs;
 		int			pathpos = 0;
 		bool		variadic;
-		bool		use_defaults = false;		/* be compiler quiet */
+		bool		use_defaults = false;		/* make compiler quiet */
 		Oid			va_elem_type;
 		FuncCandidateList newResult;
 		int		*proargidxs = NULL;
 
-		/* Try to attach names, when mixed or named notation is used. */
+		/* Try to attach names when mixed or named notation is used. */
 		if (argnames != NIL)
 		{
 			/*
-			 * Mixed or named notation 
+			 * Mixed or named notation
 			 *
-			 * We would to disable an call of variadic function with named
-			 * or mixed notation, because it could be messy for users. We
-			 * would to allow only unique arg names, and this is useles for
-			 * variadic functions.
+			 * Variadic functions can't be called using named or mixed
+			 * notation.
 			 */
 			if (OidIsValid(procform->provariadic))
 continue;
@@ -760,9 +758,12 @@ FuncnameGetCandidates(List *names, int nargs, List *argnames,
 		newResult->nargs = effective_nargs;
 
 		/*
-		 * Wait with apply proargidxs on args. Detection ambigouos needs
-		 * consistent args (based on proargs). Store proargidxs for later
-		 * use.
+		 * Save proargidxs in newResult. It's needed later to adjust
+		 * the argument types to be the types corresponding to the
+		 * named arguments (if any), and also to assign positions to
+		 * any NamedArgExpr arguments after the best candidate is
+		 * determined. The former could be done here, but we leave
+		 * both for the caller to do.
 		 */
 		newResult->proargidxs = proargidxs; 
 		memcpy(newResult->args, procform->proargtypes.values,
@@ -980,8 +981,9 @@ VerifyCandidateNamedNotation(HeapTuple proctup, int pronargs, int nargs, List *a
 	Assert(p_argnames != NULL);
 
 	/* 
-	 * A number less or equal nargs means explicit arguments,
-	*/
+	 * pronargs equal to nargs means explicit arguments (no defaults)
+	 */
+
 	*proargidxs = palloc(nargs * sizeof(int));
 	for (i = 0; i < pronargs; i++)
 		argfilling[i] = false;
@@ -1004,7 +1006,7 @@ VerifyCandidateNamedNotation(HeapTuple proctup, int pronargs, int nargs, List *a
 	continue;
 if (p_argnames[i] && strcmp(p_argnames[i], argname) == 0)
 {
-	/*  protect us against duplicated entries from bad written  mixed notation */
+	/*  protect us against duplicated entries from badly written  mixed notation */
 	if (argfilling[pp])
 		return false;
 
@@ -1035,9 +1037,13 @@ VerifyCandidateNamedNotation(HeapTuple proctup, int pronargs, int nargs, List *a
 		ap++;
 	}
 
+	/*
+	 * This function is only called for named and mixed notation, and
+	 * the last argument must be named in either case.
+	 */
 	Assert(notation == CALL_NOTATION_NAMED);
 
-	/* Check for default arguments ? */
+	/* Check for default arguments */
 	if (nargs < pronargs)
 	{
 		int fir

Re: [HACKERS] Hot Standby on git

[Heikki Linnakangas]

Simon Riggs wrote:
> On Fri, 2009-10-02 at 10:43 +0300, Heikki Linnakangas wrote:
> 
>> It seems dangerous to write a WAL record after the shutdown checkpoint.
>> It will be overwritten by subsequent startup, which is a recipe for trouble.
> 
> I've said its a corner case and not worth spending time on. I'm putting
> it in at your request. If it's not correct before and not correct after,
> where exactly do you want it?

I don't know. Perhaps it should go between the REDO pointer of the
shutdown checkpoint and the checkpoint record itself. Or maybe the
information should be included in the checkpoint record itself.

-- 
  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] Hot Standby on git

[Simon Riggs]

On Fri, 2009-10-02 at 10:43 +0300, Heikki Linnakangas wrote:

> It seems dangerous to write a WAL record after the shutdown checkpoint.
> It will be overwritten by subsequent startup, which is a recipe for trouble.

I've said its a corner case and not worth spending time on. I'm putting
it in at your request. If it's not correct before and not correct after,
where exactly do you want it?

-- 
 Simon Riggs   www.2ndQuadrant.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] Hot Standby on git

[Simon Riggs]

On Fri, 2009-10-02 at 10:32 +0300, Heikki Linnakangas wrote:
> Simon Riggs wrote:
> > I will docuemnt the recommendation to set max_standby_delay = 0 if
> > performing an archive recovery (and explain why).
> 
> Hmm, not sure if that's such a good piece of advice either. It will mean
> waiting for queries forever, which probably isn't what you want if
> you're performing archive recovery. Or maybe it is? Maybe -1? I guess it
> depends on the situation...

That assumes that the purpose of the archive recovery is more important
than running queries. As you say, it would mean always waiting. But the
beauty is that you *can* run queries to determine when to stop, so
having them cancelled defeats that purpose.

-- 
 Simon Riggs   www.2ndQuadrant.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] Hot Standby on git

[Heikki Linnakangas]

Simon Riggs wrote:
> On Fri, 2009-10-02 at 10:04 +0300, Heikki Linnakangas wrote:
>> Simon Riggs wrote:
>>> @@ -7061,6 +7061,15 @@ ShutdownXLOG(int code, Datum arg)
>>> else
>>> {
>>> /*
>>> +* Take a snapshot of running transactions and write this to WAL.
>>> +* This allows us to reconstruct the state of running transactions
>>> +* during archive recovery, if required. We do this even if we are
>>> +* not archiving, to allow a cold physical backup of the server to
>>> +* be useful as a read only standby.
>>> +*/
>>> +   GetRunningTransactionData();
>>> +
>>> +   /*
>>>  * If archiving is enabled, rotate the last XLOG file so that all 
>>> the
>>>  * remaining records are archived (postmaster wakes up the archiver
>>>  * process one more time at the end of shutdown). The checkpoint
>>>
>> I don't think this will do any good where it's placed. The checkpoint
>> that follows will have its redo-pointer beyond the running-xacts record,
>> so WAL replay will never see it.
> 
> Perhaps we need two entries then to cover multiple use cases?
> 
> The placement of this was specifically chosen so that it is the last
> entry before the log switch, so that the runningxact record would be
> archived. 
> 
> Yes, we also need one after the shutdown checkpoint to cover the case
> where the whole data directory is copied after shutdown. The comments
> matched the latter case but the position addressed the first case, so it
> looks like I was confused as to which case I was addressing.
> 
> Have updated code to do both. See what you think. Thanks.

It seems dangerous to write a WAL record after the shutdown checkpoint.
It will be overwritten by subsequent startup, which is a recipe for trouble.

-- 
  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] Hot Standby on git

[Simon Riggs]

On Fri, 2009-10-02 at 10:04 +0300, Heikki Linnakangas wrote:
> Simon Riggs wrote:
> > @@ -7061,6 +7061,15 @@ ShutdownXLOG(int code, Datum arg)
> > else
> > {
> > /*
> > +* Take a snapshot of running transactions and write this to WAL.
> > +* This allows us to reconstruct the state of running transactions
> > +* during archive recovery, if required. We do this even if we are
> > +* not archiving, to allow a cold physical backup of the server to
> > +* be useful as a read only standby.
> > +*/
> > +   GetRunningTransactionData();
> > +
> > +   /*
> >  * If archiving is enabled, rotate the last XLOG file so that all 
> > the
> >  * remaining records are archived (postmaster wakes up the archiver
> >  * process one more time at the end of shutdown). The checkpoint
> > 
> 
> I don't think this will do any good where it's placed. The checkpoint
> that follows will have its redo-pointer beyond the running-xacts record,
> so WAL replay will never see it.

Perhaps we need two entries then to cover multiple use cases?

The placement of this was specifically chosen so that it is the last
entry before the log switch, so that the runningxact record would be
archived. 

Yes, we also need one after the shutdown checkpoint to cover the case
where the whole data directory is copied after shutdown. The comments
matched the latter case but the position addressed the first case, so it
looks like I was confused as to which case I was addressing.

Have updated code to do both. See what you think. Thanks.

-- 
 Simon Riggs   www.2ndQuadrant.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] Hot Standby on git

[Heikki Linnakangas]

Simon Riggs wrote:
> I will docuemnt the recommendation to set max_standby_delay = 0 if
> performing an archive recovery (and explain why).

Hmm, not sure if that's such a good piece of advice either. It will mean
waiting for queries forever, which probably isn't what you want if
you're performing archive recovery. Or maybe it is? Maybe -1? I guess it
depends on the situation...

-- 
  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] Hot Standby on git

[Heikki Linnakangas]

Simon Riggs wrote:
> @@ -7061,6 +7061,15 @@ ShutdownXLOG(int code, Datum arg)
> else
> {
> /*
> +* Take a snapshot of running transactions and write this to WAL.
> +* This allows us to reconstruct the state of running transactions
> +* during archive recovery, if required. We do this even if we are
> +* not archiving, to allow a cold physical backup of the server to
> +* be useful as a read only standby.
> +*/
> +   GetRunningTransactionData();
> +
> +   /*
>  * If archiving is enabled, rotate the last XLOG file so that all the
>  * remaining records are archived (postmaster wakes up the archiver
>  * process one more time at the end of shutdown). The checkpoint
> 

I don't think this will do any good where it's placed. The checkpoint
that follows will have its redo-pointer beyond the running-xacts record,
so WAL replay will never see it.

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

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